Transfer rimage git history to subdirectory tools/rimage/ in the sof git repo #8297
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CMake is to be used instead of autotools. So remove the autotools files. This revert commit f6bf4b0 and commit 0d33e87 Signed-off-by: Daniel Leung <[email protected]>
This adds the necessary bits to allow building rimage with CMake with similar configure options as the just removed autotools files. Also update README.md for new build instructions. Signed-off-by: Daniel Leung <[email protected]>
This reflects the changes on SOF project commit b3c1679. Signed-off-by: Daniel Leung <[email protected]>
Since both fw.h and manifest.h are being used in the main SOF repo and also here in the standalone rimage repo, to prevent them getting out of sync, the SOF main repo is going to use the ones coming from the rimage repo. So there is no need for the option anymore. Signed-off-by: Daniel Leung <[email protected]>
Code formatting should be consistent and function arguments should be aligned to opening parenthesis. Signed-off-by: Karol Trzcinski <[email protected]>
Unused arguments shouldn't be passed to function because it makes code messy. Signed-off-by: Karol Trzcinski <[email protected]>
This changes the LICENSE file to match the LICENSE file from the main SOF repo, except they entries for the kconfig scripts which are not here in this repo. Signed-off-by: Daniel Leung <[email protected]>
Fixes following error when compiling code:
...
src/elf.c: In function ‘elf_find_section’:
src/elf.c:469:20: error: initialization discards ‘const’ qualifier
from pointer target type [-Werror=discarded-qualifiers]
469 | Elf32_Ehdr *hdr = &module->hdr;
| ^
cc1: all warnings being treated as errors
...
Signed-off-by: Andrei Emeltchenko <[email protected]>
Fixes issue ".init_array" is not copied to ri image making it zero filled. This causes Zephyr crashing trying to execute functions from this section. Signed-off-by: Andrei Emeltchenko <[email protected]>
Enable simple build test for rimage. Signed-off-by: Pan Xiuli <[email protected]>
In many functions there is need to read user section content and check firmware version or write section content to some manifest or dictionary. Previously to do it, in each function was loop to search for proper section in proper module, then allocate buffer, read content and check for possible error between each step what is quite overwhelming. After change there will be one function responsible for this task. Signed-off-by: Karol Trzcinski <[email protected]>
Extended manifest is a place to store metadata about firmware, known during compilation time - for example firmware version or used compiler. Given information are read on host side before firmware startup. This part of output binary is not signed. Signed-off-by: Karol Trzcinski <[email protected]>
This name is used in different part of source code so should refer to single place instead by copied. Signed-off-by: Karol Trzcinski <[email protected]>
Such a solution improve code hermentization and flexibility. Also such a solution is more relevant because of focus on particular module during section reading, so there won't be possibility to implicity scan section 'A' from first module and section 'B' from second one. Signed-off-by: Karol Trzcinski <[email protected]>
elf_read_section() returns section size when everything is ok, so ret value should be reset to 0 after this function call Signed-off-by: Karol Trzcinski <[email protected]>
Try to build extended manifest only when `e` flag will be set, to prevent error and warning printing during creating optional part of formware image. Signed-off-by: Karol Trzcinski <[email protected]>
ret variable is always set in elf_read_section(), to initial value is not important. Signed-off-by: Karol Trzcinski <[email protected]>
This part of binary should contain valid firmware version. Signed-off-by: Karol Trzcinski <[email protected]>
Sometimes they will be different and this will make developer aware of the differences Signed-off-by: Adrian Bonislawski <[email protected]>
some sections may use different lma than vma address, in such case rimage needs to use lma Signed-off-by: Adrian Bonislawski <[email protected]>
Generator is part of rimage tool and there is any reference to them in kernel source code, so it should be moved out of kernel folder. Signed-off-by: Karol Trzcinski <[email protected]>
Because there is possibility to reference headers from this repo from another one, there is a need to put every header into rimage namespace. Otherwise reference to headers from duplicated namespace (like kernel) from external repo may be ambiguous. Update guard ifdef/define/endif sequence value, to make it more unique Signed-off-by: Karol Trzcinski <[email protected]>
Path added to include should end at 'include' as usual, then it is easy to see where specific headers comes from during reading source code. It is especially important for projects composed from a few parts, like firmware, driver, build and debug tools in this project. Moreover changed "" to <> in include directives because there is specified full path to file from project include root and to be aligned with code style from firmware repository, Signed-off-by: Karol Trzcinski <[email protected]>
Unlike hardcoded target_compile_options(-g) can be overriden from the command line, from the parent ExternalProject_Add(CMAKE_ARGS) (tested) or from a CMake GUI (not tested). Default to Debug because the time spent running rimage is negligible compared to building the image to sign and because the time saved learning CMake when it crashes can be huge for some people. BTW -O2 -g is rarely ever useful. Signed-off-by: Marc Herbert <[email protected]>
This makes the write_firmware NULL pointer added by commit dd77445 and corresponding crash more user-friendly. Before: Program received signal SIGSEGV, Segmentation fault. After: rimage: src/rimage.c:197: main: Assertion `image.adsp->write_firmware' failed. Signed-off-by: Marc Herbert <[email protected]>
This feature is not used by SOF anyway and only causes FW load failures, we can set it to 0xffff so it will work for ADL+ platforms. Signed-off-by: Janusz Jankowski <[email protected]>
TOML aims to be a minimal configuration file format that's easy to read due to obvious semantics. TOML is designed to map unambiguously to a hash table. Signed-off-by: Karol Trzcinski <[email protected]>
This part should be moved to separate function, to allow easily add another way of machine description. Signed-off-by: Karol Trzcinski <[email protected]>
Exported function will be used in configuration parser, to assign correct function version depending on manifest versiony Signed-off-by: Karol Trzcinski <[email protected]>
This allow to add new platform with old version of rimage tool. Default values are easily to define for any fields, so fields duplication in config files should be reduced. Signed-off-by: Karol Trzcinski <[email protected]>
marc-hb
requested review from
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
marc-hb
requested review from
nashif,
andyross,
cujomalainey,
dcpleung,
aiChaoSONG,
softwarecki,
a team and
iuliana-prodan
kv2019i
approved these changes
Oct 10, 2023
|
Merged with a direct push, closing the PR. |
|
Note you can easily import/cherry-pick code changes from the old standalone rimage repo into SOF: cd sof
git remote add old-location rimage
git fetch old-location
git cherry-pick HEAD..old-location/mainThis works because I imported the whole rimage history and because file renames are very easy for git to find in this particular case. |
This was referenced Oct 11, 2023
|
glad to see im not the only one who likes to preserve history :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DO NOT "REBASE and MERGE" this PR! See below.
This PR merges rimage back into the sof git repo, see #8178, #7270 + others why.
The Github interface has always struggled with large PRs. It also shows the oldest commits first :-( But some of it can still be used: go to the "Files Changed" tab, click on "Changes from all commits" then scroll all the way down past all the original rimage commits. Then review only the last 8 commits, one by one.
Every commit can build Zephyr, XTOS and user space tools. It is possible to
git checkoutback and forth across this transfer without any submodule error and git bisectability is preserved. Each commit was tested one by one in test PR #8286DO NOT "REBASE and MERGE" this PR!
This PR includes the entire rimage history (about 300 commits) thanks to
git merge --allow-unrelated-histories. Trying to rebase merge commits fails at best and results in some non-sensical git history at worse. So, do not "rebase and merge" this PR, it must be merged as is. If that's not possible, a maintainer with WRITE permission must manually push it like this:cc: