[FEATURE] Move rimage toml config files to SOF repository #7270

kv2019i · 2023-03-14T10:34:44Z

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

lgirdwood · 2023-03-14T12:51:04Z

@mwasko @marcinszkudlinski fyi - obvious effort reduction here to keep toml manifest configs alongside FW code.

marc-hb · 2023-03-14T15:35:24Z

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.

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:

rimage: add ipc4 extended module config introduction sof-docs#451

kv2019i · 2023-03-14T15:59:52Z

@marc-hb wrote:

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

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 · 2023-03-14T17:32:54Z

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" .ri rimage file, so they seem fairly independent from each other. So why stick to obsolete and crashing rimage "technology" for the extended manifest?

EDIT: C re-implementation of elfutils submitted in

New functions to read elf files and parse modules (SP3) rimage#155

pjdobrowolski · 2023-03-15T14:53:27Z

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 · 2023-03-15T17:21:59Z

@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 · 2023-03-17T11:04:12Z

After taking a look at thesofproject/sof-docs#451 I wonder why we're even building the extended manifest using C code in 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 · 2023-03-17T19:00:59Z

@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:

No support for file inclusion. We have a separate toml file for each platform. If a module is used by multiple platforms, we have an identical description of the module's capabilities/configuration duplicated in multiple files. A solution could be to separate module descriptions into separate files and include them in platform files.
Lack of support for labels. At this moment, some fields of the module's conﬁguration is represented as hexadecimal bit masks that are completely unreadable by humans. Textual value labels should be introduced to make the configuration more readable.
Enabling/disabling building of a given module requires correcting toml files.

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 · 2023-03-17T20:01:59Z

@lgirdwood I think @marc-hb was referring to the extended manifest created from toml files.

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/

My idea to solve these problems is to move the information from the toml files to the source code of the modules...

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 issue button in the top-right corner of every comment.

pjdobrowolski · 2023-03-20T06:24:31Z

Moving rimage from one programing language to another is simple in theory. We had that discussion already last year and this year and it is not a "weekend" job.

softwarecki · 2023-03-20T09:54:22Z

I created an issue as @marc-hb asks #7304

This patch copies rimage/config to sof/config, and use toml files in sof/config for signing. Firmware manifest configuration files should stay with SOF instead of rimage tool. Deletion of rimage/config will be done later. Link: thesofproject#7270 Signed-off-by: Chao Song <chao.song@linux.intel.com>

lgirdwood · 2023-03-21T21:39:29Z

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.

@softwarecki Ack - can we have an ELF section which can be looked up and the meta data can be parsed. i.e. the metadata is an array of objects all with a type and size header (allows it to be easily extended when required).

kv2019i · 2023-03-22T10:41:28Z

Note on tjhe Zephyr dependency we realized yesterday with @nashif . Zephyr depends on rimage and the static platform bits of toml files, to create bootable flash images. If we move the toml files, this creates an awkward dependency from Zephyr to SOF repo (i.e. to run low-level driver unit tests on hardware targets, Zephyr needs to depend on the high-level app code repository).

One option is to duplicate the static bits (these don't change after all) of platform description in Zephyr repository (i.e. have subsets of the tomls files there), but ideally we should be able to share this configuration data and host the platform-specific bits e.g. in Zephyr (where the low-level code and drivers are anyways) from the audio application bits (audio modules, their properties and performance descriptions which are inherently linked to code in sof/src/audio).

marc-hb · 2023-03-22T14:34:28Z

Note on tjhe Zephyr dependency we realized yesterday with @nashif .

I think you mean this revert

Revert "xtensa: remove ELF section address rewriting" zephyrproject-rtos/zephyr#56099

These longer term ideas should probably be discussed in a new issue to keep this shorter-term PR "on topic".

Thanks @softwarecki for trying :-)

The deleted "install" target copied only the rimage executable and left the config files behind. This gave the wrong impression that the executable is useful without config files. "Installing" also gave the wrong impression that rimage versions are somewhat stable and relatively independent of SOF versions: they're not. In fact there is no such thing as an rimage "version": everyone should always use the exact rimage _git commit_ from the west manifest or git submodule. There are no rimage "releases" and no semantic versioning or anything like it, rimage is effectively part of the SOF source and build and run directly from its build directory by practically every SOF developer and SOF CI thanks to sof/src/arch/xtensa/CMakeLists.txt#ExternalProject_Add(rimage_ep ... sof/scripts/xtensa-build-zephyr.py#def build_rimage() sof/zephyr/CMakeLists.txt#set(RIMAGE_CONFIG_PATH ... etc. Providing an "install" target greatly increases the chances of different people and CIs using different rimage versions which is the last thing we want considering the many significant rimage changes happening right now, examples: - zephyrproject-rtos/zephyr#56099 - zephyrproject-rtos/zephyr#54700 - thesofproject/sof#7270 - thesofproject/sof#7304 - thesofproject/sof#7320 - thesofproject#155 While it's useful for multiple files (e.g.: config files), a CMake target was always overkill to copy a single file. Someone or some script who really wants to copy the rimage binary to some other place that the build directory for some (discouraged) reason _can still do so_ with a much more basic, simpler and more transparent file copy command. Finally, the default "bin" DESTINATION required root access which is otherwise totally unncessary to build SOF. Signed-off-by: Marc Herbert <marc.herbert@intel.com>

The deleted "install" target copied only the rimage executable and left the config files behind. This gave the wrong impression that the executable is useful without config files. "Installing" also gave the wrong impression that rimage versions are somewhat stable and relatively independent of SOF versions: they're not. In fact there is no such thing as an rimage "version": everyone should always use the exact rimage _git commit_ from the west manifest or git submodule. There are no rimage "releases" and no semantic versioning or anything like it, rimage is effectively part of the SOF source and build and run directly from its build directory by practically every SOF developer and SOF CI thanks to sof/src/arch/xtensa/CMakeLists.txt#ExternalProject_Add(rimage_ep ... sof/scripts/xtensa-build-zephyr.py#def build_rimage() sof/zephyr/CMakeLists.txt#set(RIMAGE_CONFIG_PATH ... etc. Providing an "install" target greatly increases the chances of different people and CIs using different rimage versions which is the last thing we want considering the many significant rimage changes happening right now, examples: - zephyrproject-rtos/zephyr#56099 - zephyrproject-rtos/zephyr#54700 - thesofproject/sof#7270 - thesofproject/sof#7304 - thesofproject/sof#7320 - #155 While it's useful for multiple files (e.g.: config files), a CMake target was always overkill to copy a single file. Someone or some script who really wants to copy the rimage binary to some other place that the build directory for some (discouraged) reason _can still do so_ with a much more basic, simpler and more transparent file copy command. Finally, the default "bin" DESTINATION required root access which is otherwise totally unncessary to build SOF. Signed-off-by: Marc Herbert <marc.herbert@intel.com>

lgirdwood · 2023-05-11T20:12:41Z

Moved to v2.7 to give time to align with Zephyr

kv2019i · 2023-06-20T12:22:44Z

This probably needs some rimage work to allow rimage to process toml input from multiple separate files. This then allows to have base platform descriptions in rimage repository (or in Zephyr upstream), and move the algo-specific toml sections to SOF git.

mengdonglin · 2023-08-16T09:30:09Z

This probably needs some rimage work to allow rimage to process toml input from multiple separate files. This then allows to have base platform descriptions in rimage repository (or in Zephyr upstream), and move the algo-specific toml sections to SOF git.

@kv2019i @lgirdwood @alex-cri , @aiChaoSONG will move the algo-specific toml sections to SOF git next month, good to move to v2.8?

kv2019i · 2023-08-16T18:11:54Z

Ack, let's move to v2.8, I don't think we have bandwidth for this.

aiChaoSONG · 2023-08-17T02:36:29Z

@kv2019i Do we want a simple toml move(this is quick) or we also want to split platform config and algorithm config?

kv2019i · 2023-08-17T08:20:29Z

@aiChaoSONG I think the consensus now is we want to split the platform and algorithm configs, and that is likely to need more work (support of combinting multilple toml files with some inclusion mechanism, and deciding where to put the platform files). I agree the simple move was already covered by your original PR and could be quicker. But if we now do the split implementation, we need more runway for that.

lgirdwood · 2023-08-21T15:31:43Z

@kv2019i ok, so v2.8 we need to do the topology and tooling split too.

marc-hb · 2023-09-07T17:34:09Z

Or much simpler; single repo for rimage and SOF:

[FEATURE] remove SOF build time dependency to rimage headers #8178

This patch copies zephyr RTOS base SOF image toml files from rimage repo to SOF repo. Link: thesofproject#7270 Signed-off-by: Chao Song <chao.song@linux.intel.com>

This patch splits toml configs to platform related configs and module related configs. Link: thesofproject#7270 Signed-off-by: Chao Song <chao.song@linux.intel.com>

This patch copies zephyr RTOS base SOF image toml files from rimage repo to SOF repo. Files are copied from rimage main branch, commit ID: 7bc2958 Config: Add DCblock to TGL, TGL-H, MTL, and LNL Link: thesofproject#7270 Signed-off-by: Chao Song <chao.song@linux.intel.com>

This patch splits toml configs to platform related configs and module related configs. Link: thesofproject#7270 Signed-off-by: Chao Song <chao.song@linux.intel.com>

kv2019i · 2023-09-25T17:36:01Z

Since my comment on August, discussion has spread out even more, so marking this as "unclear" for the moment.

kv2019i · 2023-10-11T18:19:22Z

Implemented with #8297 , thanks @marc-hb !

Fix intel_adsp_generic.rst following the merge of the rimage git repository back into the sof git repository: - thesofproject/sof#7270 Signed-off-by: Marc Herbert <marc.herbert@intel.com>

marc-hb · 2023-12-08T08:13:20Z

Small zephyr doc update submitted, please review:

boards: xtensa: docs: rimage is no more a separate git repository zephyrproject-rtos/zephyr#66307

Fix intel_adsp_generic.rst following the merge of the rimage git repository back into the sof git repository: - thesofproject/sof#7270 Signed-off-by: Marc Herbert <marc.herbert@intel.com>

Fix intel_adsp_generic.rst following the merge of the rimage git repository back into the sof git repository: - thesofproject/sof#7270 (cherry picked from commit 22f7516) Original-Signed-off-by: Marc Herbert <marc.herbert@intel.com> GitOrigin-RevId: 22f7516 Change-Id: I79792b8c093f3d95dcbba6365f3edff40e49273c Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/5109850 Tested-by: Fabio Baltieri <fabiobaltieri@google.com> Reviewed-by: Fabio Baltieri <fabiobaltieri@google.com> Commit-Queue: Fabio Baltieri <fabiobaltieri@google.com> Tested-by: ChromeOS Prod (Robot) <chromeos-ci-prod@chromeos-bot.iam.gserviceaccount.com>

Fix intel_adsp_generic.rst following the merge of the rimage git repository back into the sof git repository: - thesofproject/sof#7270 Signed-off-by: Marc Herbert <marc.herbert@intel.com>

kv2019i added enhancement New feature or request IPC4 Issues observed with IPC4 (same IPC as Windows) labels Mar 14, 2023

kv2019i added this to the v2.6 milestone Mar 14, 2023

aiChaoSONG mentioned this issue Mar 15, 2023

Copy rimage toml config files to SOF repository #7280

Closed

marc-hb mentioned this issue Mar 21, 2023

[FEATURE] Move extended manifest description from toml to elf section #7304

Closed

marc-hb mentioned this issue Apr 18, 2023

cmake: remove "install" target thesofproject/rimage#159

Merged

lgirdwood modified the milestones: v2.6, v2.7 May 11, 2023

marc-hb mentioned this issue May 23, 2023

rimage not fetched with west update zephyrproject-rtos/zephyr#58202

Closed

lgirdwood assigned kv2019i Jul 4, 2023

kv2019i modified the milestones: v2.7, v2.8 Aug 16, 2023

aiChaoSONG mentioned this issue Sep 5, 2023

[RFC] rimage: Add support for multiple toml file thesofproject/rimage#176

Closed

marc-hb mentioned this issue Sep 22, 2023

copy zephyr based sof image toml files from rimage to sof and split to platform and module toml #8243

Closed

aiChaoSONG pushed a commit to aiChaoSONG/sof that referenced this issue Sep 22, 2023

sof: config: split toml configs

92df3af

This patch splits toml configs to platform related configs and module related configs. Link: thesofproject#7270 Signed-off-by: Chao Song <chao.song@linux.intel.com>

aiChaoSONG pushed a commit to aiChaoSONG/sof that referenced this issue Sep 22, 2023

sof: config: split toml configs

64a5170

This patch splits toml configs to platform related configs and module related configs. Link: thesofproject#7270 Signed-off-by: Chao Song <chao.song@linux.intel.com>

kv2019i added the unclear Unclear enhancement request label Sep 25, 2023

marc-hb mentioned this issue Oct 9, 2023

Transfer rimage git history to subdirectory tools/rimage/ in the sof git repo #8297

Merged

kv2019i closed this as completed Oct 11, 2023

marc-hb mentioned this issue Dec 8, 2023

boards: xtensa: docs: rimage is no more a separate git repository zephyrproject-rtos/zephyr#66307

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEATURE] Move rimage toml config files to SOF repository #7270

[FEATURE] Move rimage toml config files to SOF repository #7270

kv2019i commented Mar 14, 2023 •

edited by marc-hb

Loading

lgirdwood commented Mar 14, 2023

marc-hb commented Mar 14, 2023 •

edited

Loading

kv2019i commented Mar 14, 2023

marc-hb commented Mar 14, 2023 •

edited

Loading

pjdobrowolski commented Mar 15, 2023

aborisovich commented Mar 15, 2023 •

edited

Loading

lgirdwood commented Mar 17, 2023

softwarecki commented Mar 17, 2023

marc-hb commented Mar 17, 2023

pjdobrowolski commented Mar 20, 2023

softwarecki commented Mar 20, 2023 •

edited

Loading

lgirdwood commented Mar 21, 2023 •

edited

Loading

kv2019i commented Mar 22, 2023

marc-hb commented Mar 22, 2023 •

edited

Loading

lgirdwood commented May 11, 2023

kv2019i commented Jun 20, 2023

mengdonglin commented Aug 16, 2023

kv2019i commented Aug 16, 2023

aiChaoSONG commented Aug 17, 2023

kv2019i commented Aug 17, 2023

lgirdwood commented Aug 21, 2023

marc-hb commented Sep 7, 2023

kv2019i commented Sep 25, 2023

kv2019i commented Oct 11, 2023

marc-hb commented Dec 8, 2023

[FEATURE] Move rimage toml config files to SOF repository #7270

[FEATURE] Move rimage toml config files to SOF repository #7270

Comments

kv2019i commented Mar 14, 2023 • edited by marc-hb Loading

lgirdwood commented Mar 14, 2023

marc-hb commented Mar 14, 2023 • edited Loading

kv2019i commented Mar 14, 2023

marc-hb commented Mar 14, 2023 • edited Loading

pjdobrowolski commented Mar 15, 2023

aborisovich commented Mar 15, 2023 • edited Loading

lgirdwood commented Mar 17, 2023

softwarecki commented Mar 17, 2023

marc-hb commented Mar 17, 2023

pjdobrowolski commented Mar 20, 2023

softwarecki commented Mar 20, 2023 • edited Loading

lgirdwood commented Mar 21, 2023 • edited Loading

kv2019i commented Mar 22, 2023

marc-hb commented Mar 22, 2023 • edited Loading

lgirdwood commented May 11, 2023

kv2019i commented Jun 20, 2023

mengdonglin commented Aug 16, 2023

kv2019i commented Aug 16, 2023

aiChaoSONG commented Aug 17, 2023

kv2019i commented Aug 17, 2023

lgirdwood commented Aug 21, 2023

marc-hb commented Sep 7, 2023

kv2019i commented Sep 25, 2023

kv2019i commented Oct 11, 2023

marc-hb commented Dec 8, 2023

kv2019i commented Mar 14, 2023 •

edited by marc-hb

Loading

marc-hb commented Mar 14, 2023 •

edited

Loading

marc-hb commented Mar 14, 2023 •

edited

Loading

aborisovich commented Mar 15, 2023 •

edited

Loading

softwarecki commented Mar 20, 2023 •

edited

Loading

lgirdwood commented Mar 21, 2023 •

edited

Loading

marc-hb commented Mar 22, 2023 •

edited

Loading