For linux msm/flashmap support#210
Conversation
|
@andersson Could you please also add an integration dry-run test with a minimal zip file to the PR? |
Failure to allocate erase command results in an error about program commands and program commands allocations doesn't provide an error message. Improve this, and use ux_err() and a return instead, as the return value is handled by the callee. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
It's not obvious that list_add() appends elements to the list, and upcoming patches needs a few new list operations. Improve the implementation by renaming list_add() to list_append() and introduce list_prepend() for cases where we need to inject entries in the beginning of the queue. Conditionally defined container_of() to allow using list.h without including qdl.h. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
A global list for each Firehose operation type, such as the program (and erase) actions prevents us from building up work queues that are a mix of different operations. One use case where this need appears is in the flashing of images across multiple storage media, where <configure>, <program>, and <patch> operations needs to be executed for each memory type in order. Replace the access of the global "program" list with a list passed from the client context. Move the loop over the actions to firehose.c in order to allow this to be extended to the other type of firehose operations as well. The "program" struct serves as the embryo for the "firehose operations" tagged struct. In doing this the "is_erase" can be removed, as the type of the object will handle this. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
While not being part of the typical flash flow, a unified model for parsing read-XML files into the same firehose operations queue keeps the code unified. Drop the global "read_ops" and move the read operations to the client-provided list. The read operation parameters are merged into the firehose operations struct to provide a unified operations object. Using a tagged struct, instead of a tagged union avoids bloat formed by the indirection of the union, at a limited overhead (we don't have a lot of operations). Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Continue the effort of unifying the firehose operations under a single queue, by moving "patch" operations to the shared, client-provided, list. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The "read" and "program" operations may have deferred GPT lookup when partitions are read or written by name. The fat structure allow us to take a single pass over the operations list and resolve those deferred entries, so replace the two implementations with a single one. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Rather than having the firehose implementation figure out if and what should be marked as bootable, turn this into a firehose operation that the client can queue. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
With "program", "erase", "patch", and "read" now moved to the shared firehose operations list, we could queue operations that work across multiple memory types, if we just move the "configure" operation to the list as well. Introduce a "configure" operation and allow this to carry the storage-type flag. The storage_type in the "qdl" struct can now change as we progress through the list, so this is updated as we come across the "configure" operation. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
15c7b3c to
3099723
Compare
Added both a dry-run test of flashmap.json and flashmap.zip, and then two specific unit-tests for json and flashmap parsing to ensure those are parsed as expected. |
3099723 to
4d720e9
Compare
With the introduction of "configure" operations, it's now possible to
have a series of "program" and "patch" operations for one storage media
followed by a selection ("configure") of another storage media and more
"program" and "patch" operations.
The way patch entries are presented in the UX, with the total presented
only at the end does not adequately represent what happened.
Instead of counting the total number of patches, move this logic to the
handling of a "configure" event and count the number of patches until
the next "configure", then once we apply that last seen patch print out
the summary.
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
4d720e9 to
a992f67
Compare
|
Dropped the two unit tests from the PR for now, while I'm trying to make them run on Windows and get past the checkpatch warnings. The two commits with tests can be found at https://github.com/quic-bjorande/qdl/commits/for-linux-msm/flashmap-support-test/ @igoropaniuk I would love some input on the latter. |
|
@andersson I've tried to troubleshoot the issue you mentioned and my assumption why flashmap_zip test likely fails on Windows is that The previous json test passes because it never touches the filesystem - no On the Windows job in |
|
btw, can we consider using an existing unit test framework rather than building our own test infrastructure (with our own macro for asserts, fixtures etc)? For example, Example of usage of cmocka was in #207 |
igoropaniuk
left a comment
There was a problem hiding this comment.
LGTM! just some minor comments
Today there are a few different places through the code where files are opened for reading, either in a oneshot fashion or for streaming (e.g. during programming). In order to allow these operations to read files packaged into compressed zip files, and to avoid duplication of the load operation, introduce the "qdl_file" file reading abstraction for these use cases. It's not yet known how VIP is handled in these archives and to reduce the impact of the change this time, the scope is limited to reading the files used during normal flashing - this can obviously be considered for extension later. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
When distributing builds the user/developer is forced to determine which programmer, program, patch files should be passed to QDL, to which storage device each set should be programmed. This isn't user friendly and it's prone of various forms of mistakes. A solution to this problem can be found in the build artifacts of a Qualcomm Windows-releases where a "flashmap" file describes the very aspects we're looking for. Implement support for parsing this json blob, populate the Sahara images list based on the "programmer" information, parse the "program" and "patch" files for each memory type and populate the firehose operations queue with "configure", "program", and "patch" operations. Introduce a new "flash" subcommand to trigger this new implementation: qdl flash flashmap.json In certain cases the user might want to only flash the subset of images pertaining to a particular storage media - such as recovering their spinor content. The user can through a suffix operation select which storage media to flash, e.g.: qdl flash flashmap.json::spinor,ufs The json parser is derived from the implementation previously in use in the pd-mapper userspace service, to avoid adding additional library dependencies. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The flashmap and related files form a package, suitable for distribution and one-step installation, but having to unzip the archive in order to flash/install its content is not convenient for the user. Extend the qdl_file helper code to also provide access to files within a zip file, then extend the flashmap code to attempt to decode the package file as such. Pass the optional zip object around to the various places where we might read files from the package. With this we can now "flash" the installer.zip directly: qdl flash installer.zip The selective flashing mechanism is carried into the zip handling, allowing the user to flash specific memories only, such as: qdl flash installer.zip::spinor Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The new "flash" command is used to install installer packages; zip files containing programmer, image, program, patch and a flashmap.json files. The flashmap.json file describes to QDL which programmer should be used, as well as which program and patch files should be flashed to each storage device in the system. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
The flashmap support saves the user from having to specify programmer, program and patch XML files, it allows reading the flashmap.json either among the flat files, or as part of a zip-container of all the parts. Add dry-run tests for the two cases, by introducing a flashmap.json and zipping up all the files, then run qdl on the two versions. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
a992f67 to
bf825ff
Compare
When distributing builds it's cumbersome and error prone to rely on the user to run QDL on the right programmer, program, and patch files, for each memory type. In Qualcomm's Windows builds this is managed by packaging all the relevant files into a zip archive and include a "flashmap" definition, which tells the flash tools how to process the files across the different memories.
In order to implement this mechanism in QDL the various global lists of firehose operations are merged to a single list, where the client can "mix" configure, program, patch, read, etc operations. Support for parsing the current version of the flashmap json file is introduced, followed by support for getting all these files directly out of a zip file.
The end result is the ability to completely flash something like Hamoa CRD by issuing:
qdl flash installer.zip
Worth pointing out is that the current version of the flashmap json format does not support specifying multiple programmers, so recent devices such as Nord are not yet supported.