Repository navigation
[FEATURE] Move rimage toml config files to SOF repository #7270
Description
Is your feature request related to a problem? Please describe.
The toml files in sof/rimage/config/*.toml have recently started covered properties of SOF modules that are no longer static properties of a platform, but rather are describiting attributes that can change when code is changed in SOF main.
This highly problematic if a code to optimize a module's performance is merged to SOF main, a matching update is needed to rimage toml files (for all affected platforms), plus a separate sof/west.yml update to bring that in to SOF builds (with Zephyr where west is used).
Describe the solution you'd like
Rimage repository should only host the tool, not the actual platform-specific content.
Describe alternatives you've considered
Moving only IPC4 specific data is one option, but due to design of rimage, this seems rather complex and in the end the easier solution would be to move all of toml config files.
Additional context
- Recent rimage pull request history -> a sharp uptick in activity with IPC4 platforms:
https://github.com/thesofproject/rimage/pulls?q=is%3Apr+is%3Aclosed+sort%3Aupdated-desc - [BUG] sof_ri_info.py fails on Intel MTL+ binaries #8073
Activity
lgirdwood commentedon Mar 14, 2023
@mwasko @marcinszkudlinski fyi - obvious effort reduction here to keep toml manifest configs alongside FW code.
marc-hb commentedon Mar 14, 2023
Wait: the static properties still exist AFAIK. So shouldn't rimage be able to read from more than one .toml file / from both places? Would simply concatenating the files in the order given be enough? This would mean moving fewer files / less content and not breaking backwards compatibility, probably easier git bisect etc.
PS: I think zephyrproject-rtos/zephyr#54700 should go first. Working on it. EDIT: done.
EDIT: this seems highly relevant:
kv2019i commentedon Mar 14, 2023
@marc-hb wrote:
But why should rimage (the tool) repo host even the static details? I agree we could split, but I'm not sure if there is any benefit to hosting in multiple repos. These are essentially the platforms we maintain in SOF repository. I.e. shouldn't rimage repo be more like a repo for gcc/clang in that it provides a tool that takes X in and spews Y out, but the input data X is hosted in other repos.
marc-hb commentedon Mar 14, 2023
After taking a look at thesofproject/sof-docs#451 I wonder why we're even building the extended manifest using C code in 2023?
https://github.com/thesofproject/rimage/issues?q=crash
AFAIK the extended manifest is merely concatenated to the "main"
.ririmage file, so they seem fairly independent from each other. So why stick to obsolete and crashingrimage"technology" for the extended manifest?EDIT: C re-implementation of elfutils submitted in
pjdobrowolski commentedon Mar 15, 2023
I was just about closing my issue with crashing. Last changes in rimage improved significantly stability of that tool. @marc-hb I know that @aborisovich is huge advocate changing language of that tool but currently @softwarecki invest huge amount of work to make that tool a bit more bugfree. Soooo If you want to discuss future of that tool Andrey is right person ;)
aborisovich commentedon Mar 15, 2023
@pjdobrowolski not exactly, I'm not a person responsible for such strategical decisions.
I had started recently a substitute project in C++, but @mmaka1 convinced me that developing such tool in this languages is not the right way to proceed. We also should consider possible development in Python.
After @softwarecki fixes the tool stopped crashing on Windows and looks much more stable.
lgirdwood commentedon Mar 17, 2023
@marc-hb lets keep on topic. Verification of hashing/signing is faster in C and hence that's why rimage is in C today.
softwarecki commentedon Mar 17, 2023
@lgirdwood I think @marc-hb was referring to the extended manifest created from toml files. It isn't signed, unlike the module manifest. So it can be generated by another tool and inserted at the beginning of the file.
For the toml files, I see the following problems with them:
My idea to solve these problems is to move the information from the toml files to the source code of the modules. While leaving in them only some basic description of the platform and its address space.
We can prepare a structure with the information necessary to generate the extended manifest and put it in a separate section of the elf file. It would be read by rimage (or any other tool in the future). In this way, we already provide some informations now. This is the module manifest (struct sof_man_module_manifest) in the ".module" section and the extended manifest (duplicated name, this is different information than in toml files), (struct ext_man_header) section ".fw_metadata". Module manifest for all modules we can easily move to source code, because its reading is already implemented in rimage. The ext_man_header structure was designed to be extensible, so it could be used to pass the module's pin configuration, etc. to rimage. The advantage of this approach is that rimage (or separate tool) would build all manifests using only a compilation artifacts. Adding/removing a module would not involve updating multiple toml files for different platforms.
When creating a loadable library, we eliminate the risk of taking a toml file that does not match the compiled module. The module metadata would be included in the module file.
marc-hb commentedon Mar 17, 2023
I was a bit vague but yes: the firmware manifest and the extended manifest are two very different problems and they seem to have a very simple and very clear line between them. The former has a limited scope and it is already implemented fully AFAIK whereas the extended manifest seems to be just getting started and currently "exploding" = why this PR was filed.
I haven't seen signing performance being an issue which means existing libraries in any other, high level and popular language could be re-used to avoid parsing, pushing bits and managing memory "by hand".
https://alexgaynor.net/2019/aug/12/introduction-to-memory-unsafety-for-vps-of-engineering/
These longer term ideas should probably be discussed in a new issue to keep this shorter-term PR "on topic". There is a convenient
... -> reference in new issuebutton in the top-right corner of every comment.21 remaining items
marc-hb commentedon Sep 7, 2023
Or much simpler; single repo for rimage and SOF:
sof: config: copy zephyr based sof image toml files to SOF repo
sof: config: split toml configs
sof: config: copy zephyr based sof image toml files to SOF repo
sof: config: split toml configs
kv2019i commentedon Sep 25, 2023
Since my comment on August, discussion has spread out even more, so marking this as "unclear" for the moment.
kv2019i commentedon Oct 11, 2023
Implemented with #8297 , thanks @marc-hb !
boards: xtensa: docs: rimage is no more a separate git repository
marc-hb commentedon Dec 8, 2023
Small zephyr doc update submitted, please review:
boards: xtensa: docs: rimage is no more a separate git repository
boards: xtensa: docs: rimage is no more a separate git repository
boards: xtensa: docs: rimage is no more a separate git repository