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

aiChaoSONG · 2023-09-22T03:05:00Z

This patch copies Intel IPC4 toml files from rimage to sof repo, and split toml files
to platform related toml and module related toml.
Also update build scripts.

cc:

marc-hb

In the relevant commit message, please add the exact rimage commit you copied the .toml files from.
Rename the copied files in a separate commit.

This will make it easy to compare and follow the git history of the copied files across multiple repos.

PS: it is possible to merge the git histories of several repos but that would a bit overkill for so few files.

scripts/xtensa-build-zephyr.py

marc-hb · 2023-09-22T04:26:57Z

Looks like we have a problem with IPC3:

https://github.com/thesofproject/sof/actions/runs/6269606713/job/17026293267?pr=8243

FileNotFoundError: [Errno 2] No such file or directory:
 '/zep_workspace/sof/config/platform/imx8.toml'

Too bad you just removed the -i IPC4 option?

marc-hb · 2023-09-22T04:29:02Z

The "ninja not found" failures are worrying but unrelated to this PR, I've seen them elsewhere. Re-running the job in the Github interface usually gets rid of them.

EDIT, filed new:

[BUG] [Windows CI] CMake was unable to find a build program corresponding to "Ninja" #8250

aiChaoSONG · 2023-09-22T05:21:30Z

@marc-hb Didn't expect the failures on non-intel platforms, I need to add fallback for non-intel platforms to still read toml file from rimage.
@kv2019i Or should I also copy imx toml files to sof? can we have someone from IMX help to comment here?

marc-hb · 2023-09-22T05:39:08Z

Or should I also copy imx toml files to sof?

Yes I was wondering what makes this move IPC4-specific so far - if anything? Couldn't it be for everything Zephyr? Would be much simpler.

BTW there is a LOT of context here and so far this PR links to none. @aiChaoSONG please add a link to at least #7270 and maybe to #8178 as well

cc: @dbaluta, @andyross

aiChaoSONG · 2023-09-22T05:40:52Z

@marc-hb Ok, I am thinking(and working) to move toml for zephyr base sof, instead of only moving toml for intel IPC4.

marc-hb

Rather than:

config/
├── module
│   └── lnl.toml
└── platform
    └── lnl.toml

... could we have this instead?

config/
└── lnl
    ├── module.toml
    └── platform.toml

Then the script can concatenate config/$platf/*.toml without any hardcoded name. More flexible and future proof.

Also, git log -- config/lnl/ is easier and more logical.

Or if needed

config/
└── lnl
    ├── 10-module.toml
    └── 20-platform.toml

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 <[email protected]>

The default build config in mainline SOF for CAVS2.5 platforms is IPC4, there is no need to keep cavs suffix for image toml files. Signed-off-by: Chao Song <[email protected]>

This patch splits toml configs to platform related configs and module related configs. Link: thesofproject#7270 Signed-off-by: Chao Song <[email protected]>

As image toml files are copied to sof repo now, use them for image signing. Signed-off-by: Chao Song <[email protected]>

kv2019i · 2023-09-22T08:18:44Z

@marc-hb @aiChaoSONG I'm wondering why not move everyone over. If conceptually rimage is made just a tool with configuration data elsewhere, then all platforms (supported in mailine) move. Limiting to Zephyr doesn't make too much sense as the toml contents is just the same for Zephyr and non-Zephyr.

@thesofproject/amd @thesofproject/mediatek @thesofproject/nxp inputs? This probably doesn't affect you much as long as builds continue to work as before.

aiChaoSONG · 2023-09-22T08:49:40Z

@kv2019i We have deprecated XTOS and IPC3 on sof mainline, so I only move those with zephyr RTOS. As we won't touch xtensa-build-all.sh anymore, can we count on AMD/NXP/MTK to move their own toml and do script update? The main problem for me is that we don't have their xtensa toolchain and could never build their firmware successfully, let along test signing correctness.

kv2019i · 2023-09-22T09:16:34Z

@aiChaoSONG Aa right. We do have "gcc-build-default-platforms" that covers all platforms and if I checked right, only place where the toml path is defined is work/sof/src/arch/xtensa/CMakeLists.txt (in two make rules plus one comment). So if you change that, that should work for all XTOS builds.

But ack. This gets iffier if we want to use a different layout for the toml files in SOF repo. E.g. for IMX targets that support both XTOS and Zephyr still, this gets really complicated.

So maybe in this PR:

copy all Zephyr platforms to SOF
keep IPC3 only targets intact
once ready, remove the IPC4 algorithm sections from rimage repository
- removing Intel platform definitions is potentially a problem as that would break Zephyr unit tests now (FYI @nashif ), so this can be done only once Zephyr is ready for removal
- toml files used by both Zephyr/XTOS (like IMX) cannot be removed as that would break XTOS IMX builds

FYI @nashif - I think in Zephyr, the platform toml file should be generated from DTS data at build time, but that has to be done first, before we can remove any of the current toml data from the repository.

aiChaoSONG · 2023-09-22T09:28:15Z

@kv2019i After looking at cmake files, firmware using xtos is signed with cmake steps. As there is only platform related toml contents for XTOS, I could file another PR(or maybe add more commits in this PR), which copies xtos based sof toml files to sof, and update below cmake, it will be quite smooth transition.

Talking about toml files shared between Zephyr and XTOS, they are imx8.toml, imx8m.toml and imx8x.toml, they are still shared after copying the rest of toml files (because only intel platform tomls have those module related configuration).

-c "${PROJECT_SOURCE_DIR}/rimage/config/${fw_name}.toml" will be changed to -c "${PROJECT_SOURCE_DIR}/config/${fw_name}/10-platform.toml"

		run_rimage
		COMMAND ${PROJECT_BINARY_DIR}/rimage_ep/build/rimage
			-o sof-${fw_name}.ri
			-c "${PROJECT_SOURCE_DIR}/rimage/config/${fw_name}.toml"
			-s ${MEU_OFFSET}
			-k ${RIMAGE_PRIVATE_KEY}
			-i ${RIMAGE_IMR_TYPE}
			-f ${SOF_MAJOR}.${SOF_MINOR}.${SOF_MICRO}
			-b ${SOF_BUILD}
			-e
			sof-${fw_name}

I suggest the copying of the rest of xtos toml files a new PR, because I cannot test it, we can count on amd/mtk/nxp to test it while not block our progress.

lgirdwood · 2023-09-22T14:18:28Z

config/lnl/10-platform.toml

@@ -0,0 +1,57 @@
+version = [3, 0]


Ehm, not following why we have a 10 prefix in our filenames. I want to #include these via the pre-processor.

lgirdwood · 2023-09-22T14:19:32Z

scripts/xtensa-build-zephyr.py

@@ -585,8 +582,32 @@ def build_rimage():
 			rimage_build_cmd.append("-v")
 	execute_command(rimage_build_cmd, cwd=west_top)

-
-def rimage_options(platform_dict):
+def concat_rimage_configs(platform_name, dest_dir):


nak, lets use the pre-processor. We also need to add Kconfig detail into the toml too.

lgirdwood · 2023-09-22T14:34:14Z

@aiChaoSONG we are going to pre-process our toml files in teh same way we pre-process linker scripts. I would split this PR into 2 PRs.
PR 1 ) just moves the existing tomls files.
PR 2 ) adds pre-processing support to toml. i.e.

/* Intel platform.toml.in */
#include <zephyr/meteorlake/memory-defs.h>
// define all memory regions using macros from C header

Then this would be included by the product toml.in

/* meteorlake toml.in */
#include <platform.toml>
#include <cse.toml>
#if CONFIG_VOLUME 
    #include <volume.toml>
#endif
// and so on

marc-hb · 2023-09-25T04:23:23Z

The main problem for me is that we don't have their xtensa toolchain and could never build their firmware successfully,

Our docker container has most toolchains, this is what CI uses everyday:

https://github.com/thesofproject/sof/actions/runs/6271755640/job/17031962306?pr=8243

I would split this PR into 2 PRs.

+1, please only one breakage at a time.

marc-hb · 2023-09-25T04:56:48Z

removing Intel platform definitions is potentially a problem as that would break Zephyr unit tests now (FYI @nashif ), so this can be done only once Zephyr is ready for removal

Yes this will break that. BTW this is why rimage was moved out of SOF: to make it possible to run Zephyr tests on hardware _independently from sof.git.

With hindsight we know that extracting rimage.git from sof.git was a mistake and we'd like to go back and put everything in a single sof.git repo like it was. But this is not happening right now. So this is where we are right now: moving ALL .toml files out of rimage will break Zephyr tests big time.

I think this PR is losing sight of the main objective. The main and most important objective is: support multiple .toml files. So this generic feature must be the first PR, before splitting and moving files around. Once the ability and main feature is merged in the main branch(es) and available for everyone, then it will become easier in a SECOND step for every developer, test suite and CI to try and see what breaks when .toml files get sliced and diced.

Now jumping the gun to this second requirement: the location of .toml files. #7270 considered an interesting possibility: moving only the modules / IPC4 specific .toml to sof.git. Why? Simply because locating modules .toml files out of rimage.git is the only thing actually required! This is the only actual problem to solve. Having the platform .toml files in rimage.git never caused any real problem. Then #7270 assumed that moving ALL .toml files would be "an easier solution". But now we now this assumption was wrong: it's actually much more complex to move all platform .toml files - and it's NOT a requirement. Moving everything just "sounds cool" like extracting rimage.git sounded cool but it's just a lot of extra work and breakages with zero value.

marc-hb

I've been at last spending some time reading, thinking about the issues here.

==> tl;dr: do NOT implement any multi-toml logic in xtensa-build-zephyr.py. It's just the wrong place.

"All problems in computer science can be solved by another level of indirection"

"...except for the problem of too many layers of indirection/obfuscation".

xtensa-build-zephyr.py is a very good demonstration of the latter.

Every time there are too many layers of indirection, debugging and fixing issues becomes a nightmare because every layer obscures everything below it.

The number of build indirection layers involved when using xtensa-build-zephyr.py is insane: https://docs.zephyrproject.org/latest/develop/west/sign.html#rimage
This page does not even list xtensa-build-zephyr.py, Github Actions and Docker!

Debugging build issues is always like "peeling an onion": trying to reproduce with fewer and fewer layers until you can finally see where the problem is. I know because I'm one of the very rare persons fixing the build (just last week: zephyrproject-rtos/zephyr#63003, #8250, #8253, ...)

The more logic there is a layer of indirection, the harder it is to peel. For that reason, everyone who has ever worked on xtensa-build-zephyr.py always agreed it should be kept as simple and thin as possible.

As a matter of fact I just spent a number of weeks moving rimage-related logic OUT of xtensa-build-zephyr.py:

[FEATURE] pass vendor specific FW signing options to west sign #6917

Ideally, configuring rimage with multiple configuration files belongs to rimage (or maybe even better: in https://github.com/thesofproject/tomlc99) So I don't understand why you abandoned thesofproject/rimage#179.

In 179 @lgirdwood requested C pre-processing of .toml files. Then you closed 179 and replaced it with this PR which still does NOT use the C pre-processor...? I'm not following. Instead you moved file concatenation and rimage-related logic to xtensa-build-zephyr.py = as far away from rimage as possible :-(

If multi-toml can't be done directly in rimage for whatever reason, then the second best choice is a place that is much closer to rimage: either Zephyr's CMake or west sign.

BTW using the C pre-processor feels overly complex and somewhat overkill to me but if we go that route then you definitely want that to be either part of or below the build system The build system knows about the C pre-processor, Kconfig, toolchains etc. unlike xtensa-build-zephyr.py which is quite a distance away from it.

Last but not least: some developers don't even use xtensa-build-zephyr.py. I've seen internal wiki pages documenting how to use west directly and avoid xtensa-build-zephyr.py. One less layer of indirection.

aiChaoSONG · 2023-09-25T12:27:55Z

@marc-hb I suppose this is better to be done with CMake if we choose to use GNU preprocessor, because it is much close to C codes and kconfig, which unfortunately is not my expertise yet.

@RanderWang Chrome asked Rander if we could make the values in toml file more readable, I don't think this could be easily done with preprocessor. Maybe Intergrate some python scripts as a CMake custom command would help, I don't know.

@marc-hb If you have any good idea, and familiar with CMake, maybe you can take over this task?

lgirdwood · 2023-09-25T15:21:51Z

@aiChaoSONG pls work with @lyakh on pre-processor for toml modules

marc-hb · 2023-10-05T23:27:30Z

I'm making very good progress moving the rimage git repo back into sof:

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

btian1 · 2023-10-09T13:26:12Z

config/imx8.toml

@@ -0,0 +1,20 @@
+version = [1, 0]  # use simple file write


imx platform also support zephyr build, right?

kv2019i · 2023-10-13T09:33:33Z

Converted back to draft. Please mark again for review when/if you update this after the recent rimage change #8178. If we go for a different PR (to introduce preprocessor for toml), then this can be closed.

aiChaoSONG · 2023-10-31T02:01:06Z

close, rimage movement is done by Marc, I think Guennadi will do toml preprocess. this is not the right direction to fix.

toml: modularise TOML configuration #8490

aiChaoSONG requested review from marc-hb, lgirdwood, plbossart, mmaka1, lbetlej, dbaluta and kv2019i as code owners September 22, 2023 03:05

marc-hb requested changes Sep 22, 2023

View reviewed changes

scripts/xtensa-build-zephyr.py Outdated Show resolved Hide resolved

scripts/xtensa-build-zephyr.py Outdated Show resolved Hide resolved

marc-hb reviewed Sep 22, 2023

View reviewed changes

aiChaoSONG force-pushed the sof_toml branch from 72c1797 to ad9b25f Compare September 22, 2023 06:55

aiChaoSONG requested a review from marc-hb September 22, 2023 07:17

Chao Song added 4 commits September 22, 2023 15:49

sof: config: rename image toml file name for TGL/TGL-H

b4a83a0

The default build config in mainline SOF for CAVS2.5 platforms is IPC4, there is no need to keep cavs suffix for image toml files. Signed-off-by: Chao Song <[email protected]>

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 <[email protected]>

xtensa-build-zephyr: sign firmware with toml from sof repo

7526922

As image toml files are copied to sof repo now, use them for image signing. Signed-off-by: Chao Song <[email protected]>

aiChaoSONG force-pushed the sof_toml branch from ad9b25f to 7526922 Compare September 22, 2023 08:08

aiChaoSONG mentioned this pull request Sep 22, 2023

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

Closed

aiChaoSONG changed the title ~~copy IPC4 toml files from rimage to sof and split to platform and module toml~~ copy zephyr based sof image toml files from rimage to sof and split to platform and module toml Sep 22, 2023

lgirdwood requested changes Sep 22, 2023

View reviewed changes

RanderWang mentioned this pull request Sep 25, 2023

ipc4: add google rtc AEC support for mtl thesofproject/rimage#180

Merged

marc-hb requested changes Sep 25, 2023

View reviewed changes

kv2019i mentioned this pull request Sep 25, 2023

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

Closed

btian1 reviewed Oct 9, 2023

View reviewed changes

config/imx8.toml Outdated

@@ -0,0 +1,20 @@

version = [1, 0] # use simple file write

Copy link

Contributor

btian1 Oct 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imx platform also support zephyr build, right?

marc-hb mentioned this pull request Oct 9, 2023

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

Merged

kv2019i marked this pull request as draft October 13, 2023 09:32

This was referenced Oct 27, 2023

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

Closed

rimage: mtl: fix key slot setup based on imr type #8405

Closed

aiChaoSONG closed this Oct 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

aiChaoSONG commented Sep 22, 2023 •

edited by marc-hb

Loading

marc-hb left a comment

marc-hb commented Sep 22, 2023 •

edited

Loading

marc-hb commented Sep 22, 2023 •

edited

Loading

aiChaoSONG commented Sep 22, 2023 •

edited

Loading

marc-hb commented Sep 22, 2023

aiChaoSONG commented Sep 22, 2023

marc-hb left a comment •

edited

Loading

kv2019i commented Sep 22, 2023

aiChaoSONG commented Sep 22, 2023

kv2019i commented Sep 22, 2023

aiChaoSONG commented Sep 22, 2023 •

edited

Loading

lgirdwood Sep 22, 2023

lgirdwood Sep 22, 2023

lgirdwood commented Sep 22, 2023

marc-hb commented Sep 25, 2023 •

edited

Loading

marc-hb commented Sep 25, 2023 •

edited

Loading

marc-hb left a comment •

edited

Loading

aiChaoSONG commented Sep 25, 2023 •

edited

Loading

lgirdwood commented Sep 25, 2023

marc-hb commented Oct 5, 2023

btian1 Oct 9, 2023

kv2019i commented Oct 13, 2023 •

edited by marc-hb

Loading

aiChaoSONG commented Oct 31, 2023 •

edited by marc-hb

Loading

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

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

Conversation

aiChaoSONG commented Sep 22, 2023 • edited by marc-hb Loading

marc-hb left a comment

Choose a reason for hiding this comment

marc-hb commented Sep 22, 2023 • edited Loading

marc-hb commented Sep 22, 2023 • edited Loading

aiChaoSONG commented Sep 22, 2023 • edited Loading

marc-hb commented Sep 22, 2023

aiChaoSONG commented Sep 22, 2023

marc-hb left a comment • edited Loading

Choose a reason for hiding this comment

kv2019i commented Sep 22, 2023

aiChaoSONG commented Sep 22, 2023

kv2019i commented Sep 22, 2023

aiChaoSONG commented Sep 22, 2023 • edited Loading

lgirdwood Sep 22, 2023

Choose a reason for hiding this comment

lgirdwood Sep 22, 2023

Choose a reason for hiding this comment

lgirdwood commented Sep 22, 2023

marc-hb commented Sep 25, 2023 • edited Loading

marc-hb commented Sep 25, 2023 • edited Loading

marc-hb left a comment • edited Loading

Choose a reason for hiding this comment

aiChaoSONG commented Sep 25, 2023 • edited Loading

lgirdwood commented Sep 25, 2023

marc-hb commented Oct 5, 2023

btian1 Oct 9, 2023

Choose a reason for hiding this comment

kv2019i commented Oct 13, 2023 • edited by marc-hb Loading

aiChaoSONG commented Oct 31, 2023 • edited by marc-hb Loading

aiChaoSONG commented Sep 22, 2023 •

edited by marc-hb

Loading

marc-hb commented Sep 22, 2023 •

edited

Loading

marc-hb commented Sep 22, 2023 •

edited

Loading

aiChaoSONG commented Sep 22, 2023 •

edited

Loading

marc-hb left a comment •

edited

Loading

aiChaoSONG commented Sep 22, 2023 •

edited

Loading

marc-hb commented Sep 25, 2023 •

edited

Loading

marc-hb commented Sep 25, 2023 •

edited

Loading

marc-hb left a comment •

edited

Loading

aiChaoSONG commented Sep 25, 2023 •

edited

Loading

kv2019i commented Oct 13, 2023 •

edited by marc-hb

Loading

aiChaoSONG commented Oct 31, 2023 •

edited by marc-hb

Loading