Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

platform: imx93_a55: handle linker additions at SOF level #9372

Conversation

LaurentiuM1234
Copy link
Contributor

SOF requires some additions to the Zephyr linker script. This should be handled inside SOF instead of Zephyr.

Since 8f33d9dc8498 ("soc: imx93: remove custom linker script") removes the custom linker script from Zephyr, this means the custom SOF linker sections will have to be handled on SOF side.

This also includes a Zephyr hash bump to pull/77228/head to pull in the following commits:

8f33d9dc8498 soc: imx93: remove custom linker script

Depends on: zephyrproject-rtos/zephyr#77228

@LaurentiuM1234 LaurentiuM1234 marked this pull request as ready for review August 19, 2024 11:13
@marc-hb
Copy link
Collaborator

marc-hb commented Aug 20, 2024

OK, this difference in https://github.com/thesofproject/sof/actions/runs/10452157029/job/28940502308?pr=9372
was very unusual and interesting:

Downloads$ diff -u -b ?in/build-sof-staging/sof-info/imx8/zephyr_version.h
--- lin/build-sof-staging/sof-info/imx8/zephyr_version.h	2024-08-19 11:12:34
+++ win/build-sof-staging/sof-info/imx8/zephyr_version.h	2024-08-19 11:14:22
@@ -19,7 +19,7 @@
 #define KERNEL_VERSION_EXTENDED_STRING  "3.7.99+0"
 #define KERNEL_VERSION_TWEAK_STRING     "3.7.99+0"

-#define BUILD_VERSION v3.7.0-1237-g8f33d9dc8498
+#define BUILD_VERSION v3.7.0-1237-g03b829ce71cd


 #endif /* _KERNEL_VERSION_H_ */

After a fortunately short time, I found the root cause in zephyrproject-rtos/zephyr#77228

@LaurentiuM1234 LaurentiuM1234 force-pushed the fix/imx93_linker_removal branch from 8f33d9d to 03b829c

A GitHub race condition between @LaurentiuM1234 and... @LaurentiuM1234 !

I have debugged countless build reproducibility issues but I had never seen any like this. @LaurentiuM1234 , please be nicer on my heart, it's not getting any younger :-D

@lgirdwood
Copy link
Member

@teburd can you comment here ?

@LaurentiuM1234
Copy link
Contributor Author

LaurentiuM1234 commented Aug 21, 2024

@teburd can you comment here ?

Comments on this change would be very much appreciated. Especially regarding the idea of using linker script snippets on SOF-side to handle any required additions.

@lgirdwood
Copy link
Member

@teburd can you comment here ?

Comments on this change would be very much appreciated. Especially regarding the idea of using linker script snippets on SOF-side to handle any required additions.

My thinking is that all things linker should really come from Zephyr and we should transition towards that goal, but I dont know yet if this is missing from Zephyr or how much effort would be needed.

@LaurentiuM1234
Copy link
Contributor Author

@teburd can you comment here ?

Comments on this change would be very much appreciated. Especially regarding the idea of using linker script snippets on SOF-side to handle any required additions.

My thinking is that all things linker should really come from Zephyr and we should transition towards that goal, but I dont know yet if this is missing from Zephyr or how much effort would be needed.

mm, I would think the Zephyr linker stuff should be relatively generic and allow it to be used by as many applications as possible. Adding application-specific linker snippets on Zephyr side is not that big of a deal since you'd encase these in ifdef blocks but it just makes the SOC scripts (or the SOC CMakeLists.txt files) messier I believe?

I think Xtensa is a bit special in this regard since there's no architecture generic linker script so each vendor has its own linker script anyways. Instead, ARM and ARM64 have a "generic", architecture linker script also used as the SOC linker script.

Anyways, since we do have the zephyr_linker_sources cmake commands I would think we should keep the Zephyr linker scripts "clean" and handle all the additions required by SOF here.

@lgirdwood
Copy link
Member

@nashif who can give advise on Zephyr linker here ? Can we support application linker overlays for certain application sections ?

@LaurentiuM1234
Copy link
Contributor Author

any other take on this? if not, I'd prefer handling the SOF-related linker additions in SOF if reasonable to do so/ if Zephyr infrastructure allows us to do so.

@marc-hb marc-hb removed their request for review August 26, 2024 22:21
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

@LaurentiuM1234 we can merge after the Zephyr part is done.

@LaurentiuM1234
Copy link
Contributor Author

LaurentiuM1234 commented Sep 12, 2024

Zephyr dependency merged, updated hash to to that commit's. Dropping DNM tag.

!! Note that this commit modifies west.yml !!

@LaurentiuM1234 LaurentiuM1234 changed the title [DNM] platform: imx93_a55: handle linker additions at SOF level platform: imx93_a55: handle linker additions at SOF level Sep 12, 2024
@LaurentiuM1234 LaurentiuM1234 force-pushed the fix/imx93_linker_removal branch from 81e7f6b to 54ad8d1 Compare September 12, 2024 11:03
@lgirdwood
Copy link
Member

Zephyr dependency merged, updated hash to to that commit's. Dropping DNM tag.

!! Note that this commit modifies west.yml !!

Its best practice for bisect-ability to upstream the west update 1st. Can you do 2 patches, one for west 1st and 2nd patch for linker ?

@LaurentiuM1234
Copy link
Contributor Author

Zephyr dependency merged, updated hash to to that commit's. Dropping DNM tag.
!! Note that this commit modifies west.yml !!

Its best practice for bisect-ability to upstream the west update 1st. Can you do 2 patches, one for west 1st and 2nd patch for linker ?

You mean have them in different PRs? If so, the west.yml change will break the arm64 CI job w/o the other patch. If that's alright then I'm fine with it.

@kv2019i
Copy link
Collaborator

kv2019i commented Sep 16, 2024

@LaurentiuM1234 @lgirdwood A combine patch is ok (and even required) if Zephyr side has a change that breaks SOF build. With two separate commits, the SOF bisect is broken, so better to have the west.yml update in the same commit (and maintain bisect-capability on SOF side).

@LaurentiuM1234 LaurentiuM1234 force-pushed the fix/imx93_linker_removal branch from 54ad8d1 to 6702131 Compare September 16, 2024 12:07
@LaurentiuM1234
Copy link
Contributor Author

Rebased

@lgirdwood
Copy link
Member

SOFCI TEST

@lgirdwood
Copy link
Member

@LaurentiuM1234 linker error:

[390/393] Building C object zephyr/CMakeFiles/zephyr_final.dir/isr_tables.c.obj
[391/393] Linking CXX executable zephyr/zephyr.elf
Generating files from /srv/home/jenkins/workspace/sof_config_build/build-lnl/zephyr/zephyr.elf for board: intel_adsp
[392/393] Extracting .mod(ule) files for rimage
[393/393] west sign --if-tool-available --tool rimage ...
FAILED: zephyr/zephyr.ri /srv/home/jenkins/workspace/sof_config_build/build-lnl/zephyr/zephyr.ri 
cd /srv/home/jenkins/workspace/sof_config_build/build-lnl/zephyr/soc/soc/ace20_lnl/common && west sign --if-tool-available --tool rimage --build-dir /srv/home/jenkins/workspace/sof_config_build/build-lnl
Traceback (most recent call last):
  File "/srv/home/jenkins/.local/bin/west", line 8, in <module>
    sys.exit(main())
  File "/srv/home/jenkins/.local/lib/python3.8/site-packages/west/app/main.py", line 866, in main
    app.run(argv or sys.argv[1:])
  File "/srv/home/jenkins/.local/lib/python3.8/site-packages/west/app/main.py", line 111, in run
    self.run_command(argv)
  File "/srv/home/jenkins/.local/lib/python3.8/site-packages/west/app/main.py", line 385, in run_command
    self.run_extension(args.command, argv)
  File "/srv/home/jenkins/.local/lib/python3.8/site-packages/west/app/main.py", line 436, in run_extension
    self.cmd = self.extensions[name].factory()
  File "/srv/home/jenkins/.local/lib/python3.8/site-packages/west/commands.py", line 538, in __call__
    mod = _commands_module_from_file(self.py_file)
  File "/srv/home/jenkins/.local/lib/python3.8/site-packages/west/commands.py", line 693, in _commands_module_from_file
    spec.loader.exec_module(mod)
  File "<frozen importlib._bootstrap_external>", line 848, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/srv/home/jenkins/workspace/sof_config_build/zephyr/scripts/west_commands/sign.py", line 22, in <module>
    from runners.core import BuildConfiguration
  File "/srv/home/jenkins/workspace/sof_config_build/zephyr/scripts/west_commands/runners/__init__.py", line 8, in <module>
    from runners.core import ZephyrBinaryRunner, MissingProgram
  File "/srv/home/jenkins/workspace/sof_config_build/zephyr/scripts/west_commands/runners/core.py", line 392, in <module>
    class ZephyrBinaryRunner(abc.ABC):
  File "/srv/home/jenkins/workspace/sof_config_build/zephyr/scripts/west_commands/runners/core.py", line 780, in ZephyrBinaryRunner
    def get_rtt_address(self) -> int | None:
TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'
ninja: build stopped: subcommand failed.
FATAL ERROR: command exited with status 1: /srv/home/jenkins/.local/bin/cmake --build /srv/home/jenkins/workspace/sof_config_build/build-lnl
Traceback (most recent call last):
  File "./sof/scripts/xtensa-build-zephyr.py", line 1226, in <module>
    main()
  File "./sof/scripts/xtensa-build-zephyr.py", line 1220, in main
    build_platforms()
  File "./sof/scripts/xtensa-build-zephyr.py", line 884, in build_platforms
    raise cpe
  File "./sof/scripts/xtensa-build-zephyr.py", line 878, in build_platforms
    execute_command(build_cmd, cwd=west_top, env=platf_build_environ, sof_log_env=True)
  File "./sof/scripts/xtensa-build-zephyr.py", line 397, in execute_command
    return subprocess.run(*run_args, **run_kwargs)
  File "/usr/lib/python3.8/subprocess.py", line 516, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['west', 'build', '--build-dir', 'build-lnl', '--board', 'intel_adsp/ace20_lnl', '/srv/home/jenkins/workspace/sof_config_build/sof/app', '-p', 'always', '--', '-DEXTRA_CONF_FILE=/srv/home/jenkins/workspace/sof_config_build/sof/app/debug_overlay.conf;/srv/home/jenkins/workspace/sof_config_build/sof/app/llext_relocatable.conf']' returned non-zero exit status 1.

SOF requires some additions to the Zephyr linker script.
This should be handled inside SOF instead of Zephyr.

Since 3f2790b89ca5 ("soc: imx93: remove custom linker script")
removes the custom linker script from Zephyr, this means
the custom SOF linker sections will have to be handled on SOF
side.

This also includes a Zephyr hash bump to 3f2790b89ca5,
which brings the following (potentially) relevant patches:

dc5f1bfb3f2e west: fix for Python prior to 3.10
3f2790b89ca5 soc: imx9: remove custom linker script
d389c95935c5 soc: intel_adsp: ace: Configurable SRAM
retention mode and cleanup
be041b14fe51 Intel: ADSP: move HPSRAM mask into assembly
b3b8360f3993 west: runners: Add `west rtt` command with pyocd
implementation

Signed-off-by: Laurentiu Mihalcea <[email protected]>
@LaurentiuM1234 LaurentiuM1234 force-pushed the fix/imx93_linker_removal branch from 6702131 to 3b12451 Compare September 17, 2024 15:43
@LaurentiuM1234
Copy link
Contributor Author

@LaurentiuM1234 linker error:

[390/393] Building C object zephyr/CMakeFiles/zephyr_final.dir/isr_tables.c.obj
[391/393] Linking CXX executable zephyr/zephyr.elf
Generating files from /srv/home/jenkins/workspace/sof_config_build/build-lnl/zephyr/zephyr.elf for board: intel_adsp
[392/393] Extracting .mod(ule) files for rimage
[393/393] west sign --if-tool-available --tool rimage ...
FAILED: zephyr/zephyr.ri /srv/home/jenkins/workspace/sof_config_build/build-lnl/zephyr/zephyr.ri 
cd /srv/home/jenkins/workspace/sof_config_build/build-lnl/zephyr/soc/soc/ace20_lnl/common && west sign --if-tool-available --tool rimage --build-dir /srv/home/jenkins/workspace/sof_config_build/build-lnl
Traceback (most recent call last):
  File "/srv/home/jenkins/.local/bin/west", line 8, in <module>
    sys.exit(main())
  File "/srv/home/jenkins/.local/lib/python3.8/site-packages/west/app/main.py", line 866, in main
    app.run(argv or sys.argv[1:])
  File "/srv/home/jenkins/.local/lib/python3.8/site-packages/west/app/main.py", line 111, in run
    self.run_command(argv)
  File "/srv/home/jenkins/.local/lib/python3.8/site-packages/west/app/main.py", line 385, in run_command
    self.run_extension(args.command, argv)
  File "/srv/home/jenkins/.local/lib/python3.8/site-packages/west/app/main.py", line 436, in run_extension
    self.cmd = self.extensions[name].factory()
  File "/srv/home/jenkins/.local/lib/python3.8/site-packages/west/commands.py", line 538, in __call__
    mod = _commands_module_from_file(self.py_file)
  File "/srv/home/jenkins/.local/lib/python3.8/site-packages/west/commands.py", line 693, in _commands_module_from_file
    spec.loader.exec_module(mod)
  File "<frozen importlib._bootstrap_external>", line 848, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/srv/home/jenkins/workspace/sof_config_build/zephyr/scripts/west_commands/sign.py", line 22, in <module>
    from runners.core import BuildConfiguration
  File "/srv/home/jenkins/workspace/sof_config_build/zephyr/scripts/west_commands/runners/__init__.py", line 8, in <module>
    from runners.core import ZephyrBinaryRunner, MissingProgram
  File "/srv/home/jenkins/workspace/sof_config_build/zephyr/scripts/west_commands/runners/core.py", line 392, in <module>
    class ZephyrBinaryRunner(abc.ABC):
  File "/srv/home/jenkins/workspace/sof_config_build/zephyr/scripts/west_commands/runners/core.py", line 780, in ZephyrBinaryRunner
    def get_rtt_address(self) -> int | None:
TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'
ninja: build stopped: subcommand failed.
FATAL ERROR: command exited with status 1: /srv/home/jenkins/.local/bin/cmake --build /srv/home/jenkins/workspace/sof_config_build/build-lnl
Traceback (most recent call last):
  File "./sof/scripts/xtensa-build-zephyr.py", line 1226, in <module>
    main()
  File "./sof/scripts/xtensa-build-zephyr.py", line 1220, in main
    build_platforms()
  File "./sof/scripts/xtensa-build-zephyr.py", line 884, in build_platforms
    raise cpe
  File "./sof/scripts/xtensa-build-zephyr.py", line 878, in build_platforms
    execute_command(build_cmd, cwd=west_top, env=platf_build_environ, sof_log_env=True)
  File "./sof/scripts/xtensa-build-zephyr.py", line 397, in execute_command
    return subprocess.run(*run_args, **run_kwargs)
  File "/usr/lib/python3.8/subprocess.py", line 516, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['west', 'build', '--build-dir', 'build-lnl', '--board', 'intel_adsp/ace20_lnl', '/srv/home/jenkins/workspace/sof_config_build/sof/app', '-p', 'always', '--', '-DEXTRA_CONF_FILE=/srv/home/jenkins/workspace/sof_config_build/sof/app/debug_overlay.conf;/srv/home/jenkins/workspace/sof_config_build/sof/app/llext_relocatable.conf']' returned non-zero exit status 1.

Changed the Zephyr hash bump to dc5f1bfb3f2e ("west: fix for Python prior to 3.10"). Hopefully, that'll take care of the internal CI build failures.

@kv2019i
Copy link
Collaborator

kv2019i commented Sep 18, 2024

@LaurentiuM1234 Ah, now you hit this problem zephyrproject-rtos/zephyr#78479 (revert) and zephyrproject-rtos/zephyr#78612 (fix)

@LaurentiuM1234
Copy link
Contributor Author

Moving to draft. Submitted #9491 which removes the need to use the extra linker sections.

@LaurentiuM1234 LaurentiuM1234 marked this pull request as draft September 18, 2024 19:25
@LaurentiuM1234 LaurentiuM1234 deleted the fix/imx93_linker_removal branch September 23, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants