linux-msm / qdl

BSD 3-Clause "New" or "Revised" License
196 stars 76 forks source link

Implement sparse image flashing (yet another) #6

Open blenk92 opened 4 years ago

blenk92 commented 4 years ago

Hi,

unfortunately I didn't see https://github.com/andersson/qdl/pull/5 until I was almost done implementing sparse image flashing as well. However, I thought as I anyway already invested the time I might just open another PR so you can choose what ever solution you prefer.

As far as I can see https://github.com/andersson/qdl/pull/5 does not use the API provided in (include/sparse/sparse.h) which I did. So the only thing that needed to be done was handling input provided by the callback function sparse_file_callback().

abozhinov444 commented 4 years ago

Hi, are there any specific dependencies? Because i cannot build it. ` make

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o firehose.o firehose.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o qdl.o qdl.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o sahara.o sahara.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o util.o util.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o patch.o patch.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o program.o program.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o ufs.o ufs.c

make -C libsparse

make[1]: Entering directory '/home/c_abozhi/projects/qdl2/libsparse'

g++ -O2 -Wall -I include -c -o backed_block.o backed_block.cpp

g++ -O2 -Wall -I include -c -o output_file.o output_file.cpp

g++ -O2 -Wall -I include -c -o sparse.o sparse.cpp

g++ -O2 -Wall -I include -c -o sparse_crc32.o sparse_crc32.cpp

g++ -O2 -Wall -I include -c -o sparse_err.o sparse_err.cpp

g++ -O2 -Wall -I include -c -o sparse_read.o sparse_read.cpp

g++ -O2 -Wall -I include -c -o stringprintf.o stringprintf.cpp

ar rcs libsparse.a backed_block.o output_file.o sparse.o sparse_crc32.o sparse_err.o sparse_read.o stringprintf.o

make[1]: Leaving directory '/home/c_abozhi/projects/qdl2/libsparse'

g++ -o qdl firehose.o qdl.o sahara.o util.o patch.o program.o ufs.o libsparse/libsparse.a libsparse/libsparse.a xml2-config --libs -ludev

/usr/bin/ld: libsparse/libsparse.a(output_file.o): undefined reference to symbol 'gzerror' //lib/x86_64-linux-gnu/libz.so.1: error adding symbols: DSO missing from command line

collect2: error: ld returned 1 exit status

Makefile:11: recipe for target 'qdl' failed

make: *** [qdl] Error 1 `

blenk92 commented 4 years ago

Hi, are there any specific dependencies? Because i cannot build it. ` make

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o firehose.o firehose.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o qdl.o qdl.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o sahara.o sahara.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o util.o util.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o patch.o patch.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o program.o program.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o ufs.o ufs.c

make -C libsparse

make[1]: Entering directory '/home/c_abozhi/projects/qdl2/libsparse'

g++ -O2 -Wall -I include -c -o backed_block.o backed_block.cpp

g++ -O2 -Wall -I include -c -o output_file.o output_file.cpp

g++ -O2 -Wall -I include -c -o sparse.o sparse.cpp

g++ -O2 -Wall -I include -c -o sparse_crc32.o sparse_crc32.cpp

g++ -O2 -Wall -I include -c -o sparse_err.o sparse_err.cpp

g++ -O2 -Wall -I include -c -o sparse_read.o sparse_read.cpp

g++ -O2 -Wall -I include -c -o stringprintf.o stringprintf.cpp

ar rcs libsparse.a backed_block.o output_file.o sparse.o sparse_crc32.o sparse_err.o sparse_read.o stringprintf.o

make[1]: Leaving directory '/home/c_abozhi/projects/qdl2/libsparse'

g++ -o qdl firehose.o qdl.o sahara.o util.o patch.o program.o ufs.o libsparse/libsparse.a libsparse/libsparse.a xml2-config --libs -ludev

/usr/bin/ld: libsparse/libsparse.a(output_file.o): undefined reference to symbol 'gzerror' //lib/x86_64-linux-gnu/libz.so.1: error adding symbols: DSO missing from command line

collect2: error: ld returned 1 exit status

Makefile:11: recipe for target 'qdl' failed

make: *** [qdl] Error 1 `

My bad.. seems like I didn't specfiy -lz. I updated that... For some reason works for me without...

Thanks for the hint ;-)

Could you try again?

abozhinov444 commented 4 years ago

Now it works. I have tried to flash some images, but after first sparse image it exits. How is it verified? [PROGRAM] flashed "abootbak" successfully LOG: start 790528, num 16384 LOG: Finished sector address 806912 LOG: start 806912, num 28216 LOG: Finished sector address 835128 LOG: start 835128, num 0 LOG: Finished sector address 835128 qdl: failed to read: Connection timed out [PROGRAM] failed [PROGRAM] flashed "boot" successfully at 417kB/s

I am trying to flash boot, system, systemrw and user images. They work with my solution. Now i am debugging. Will report back, again.

blenk92 commented 4 years ago

Now it works. I have tried to flash some images, but after first sparse image it exits. How is it verified? [PROGRAM] flashed "abootbak" successfully LOG: start 790528, num 16384 LOG: Finished sector address 806912 LOG: start 806912, num 28216 LOG: Finished sector address 835128 LOG: start 835128, num 0 LOG: Finished sector address 835128 qdl: failed to read: Connection timed out [PROGRAM] failed [PROGRAM] flashed "boot" successfully at 417kB/s

I am trying to flash boot, system, systemrw and user images. They work with my solution. Now i am debugging. Will report back, again.

hmm do you have any kind of additional info you can give me? e.g. xmls, or at best also the images? (I flashed some devices and it never failed)

But as I said, I'm fine with your solution as well. Seems to also work for me... I'm not sure if it is then worth spending too much time to debug this solution...

abozhinov444 commented 4 years ago

Sorry, but i could not share xml and images, because of "Company Policies" .

Why you call firehose_program_sparse_callback at line 544, again? It brokes things on my device.

blenk92 commented 4 years ago

Sorry, but i could not share xml and images, because of "Company Policies" .

Why you call firehose_program_sparse_callback at line 544, again? It brokes things on my device.

Well if there is still data in the buffer this also needs to be flashed. Flashing is always performed if a don't care block is reached or the buffer is full. Maybe the buffer is already empty ? That should be checked for sure... Could you retry?

abozhinov444 commented 4 years ago

I will retry tomorrow. But what i understand when i read the libsparse code is that sparse_file_callback() process all the file at one pass with don't care chunks. Did you remember where was bug in my code before? This function should write "skip" chunks.

blenk92 commented 4 years ago

I will retry tomorrow. But what i understand when i read the libsparse code is that sparse_file_callback() process all the file at one pass with don't care chunks. Did you remember where was bug in my code before? This function should write "skip" chunks.

Not sure, if this is what you mean.. But I'd argue that it depends on the use-case if "don't care chunks" have to be written.. Actually, I would even argue if chunks are "don't care", then the image really shouldn't care what kind of data there is (which means for me there I don't have to overwrite existing data). Am I missing something here?

abozhinov444 commented 4 years ago

I mean that all "don't care chunk" are written during sparse_file_callback() and last call of firehose_program_sparse_callback() is not needed. Function that writes it is named callback_file_skip(). Also all data should be overwritten, i didn't mean anything else.

blenk92 commented 4 years ago

I mean that all "don't care chunk" are written during sparse_file_callback() and last call of firehose_program_sparse_callback() is not needed. Function that writes it is named callback_file_skip(). Also all data should be overwritten, i didn't mean anything else.

I just checked your implementation, and from what i saw you are doing the exact same thing, not write the don't care chunks (as they obviously do not contain data). From what I see the don't care chunks are not written during the flash (i expect the data agrument of firehose_program_sparse_callback() to be NULL in that case).

Also I figured out why it's failing, increasing the timeout fixed it for me. Maybe you also want to retry... The last call is indeed needed because the buffer may still contain data that hasn't been flashed yet. That, however, depends on the sparse image itself.

abozhinov444 commented 4 years ago

No, i am writing, i just modified source to pass zero filled buffer. So in my callback i do not process NULL case.

For the second paragraph, i think it is better just to send data on 1 megabyte chunk like in my code. Because sparse callback returns a lot 4096kb buffers and when they are send rapidly fast as Host PC is times faster device programmer fails. This with timeouts is workaround for me, because if image is bigger then usual, writing will fail again.

blenk92 commented 4 years ago

we have got this patch now in production for more than 6 months. Images have increased in size quite some and we haven't seen any issues (with quite some flashes of different images per day), so I don't think that the timeout thing is an actual issue. From my point this is certainly well-tested.

jwinarske commented 3 years ago

@andersson what are your thoughts on merging this? Happy to help