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

llext: fix RISC-V inspection tests #85989

Closed

Conversation

pillo79
Copy link
Collaborator

@pillo79 pillo79 commented Feb 19, 2025

RISC-V uses ".s"-prefixed sections for variables less than 8 bytes. The hardcoded sections used in the inspect test were thus not matching the compiled binary, causing the test to fail.

Fix this by using the correct section names for RISC-V.

RISC-V uses ".s"-prefixed sections for variables less than 8 bytes. The
hardcoded sections used in the inspect test were thus not matching the
compiled binary, causing the test to fail.

Fix this by using the correct section names for RISC-V.

Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
@pillo79 pillo79 marked this pull request as ready for review February 19, 2025 09:58
@zephyrbot zephyrbot added the area: llext Linkable Loadable Extensions label Feb 19, 2025
@zephyrbot zephyrbot requested review from lyakh and teburd February 19, 2025 09:58
@WorldofJARcraft
Copy link
Contributor

WorldofJARcraft commented Feb 19, 2025

Actually, I think it is better to disable the use of .sbss/.sdata: In case an llext uses both .sbss and .bss, there are two non-skipped NOBITS sections. llext_load cannot handle this currently.
Here is a PR that makes the necessary change: #85977

@pillo79 pillo79 added this to the v4.1.0 milestone Feb 19, 2025
@pillo79 pillo79 added the bug The issue is a bug, or the PR is fixing a bug label Feb 19, 2025
@pillo79
Copy link
Collaborator Author

pillo79 commented Feb 19, 2025

Actually, I think it is better to disable the use of .sbss/.sdata: In case an llext uses both .sbss and .bss, there are two non-skipped NOBITS sections. llext_load cannot handle this currently. Here is a PR that makes the necessary change: #85977

@WorldofJARcraft thanks for pointing that out. Maybe we could fix that instead (if they appear close together, it's an easy fix) and keep the sections? Have no experience with RISC-V, does that layout provide a significant speed benefit?

@WorldofJARcraft
Copy link
Contributor

The compiler moves small global symbols such as scalar integers (with a size below a configurable threshold) into the .srodata/.sdata/.sbss sections. All other (larger) globals go into .rodata/.data/.bss, respectively.
The advantage of the layout is the following: RISC-V designates a register the global pointer. This register can be set do the beginning of .sbss. After that, loads and stores for globals in .sbss can be done relative to the global pointer register. This saves 1 instruction over a HI20/LO12 relocation pair.
Zephyr itself has a configuration option RISCV_GP that facilitates this use for the zephyr binary.
Implementing this mechanism for llext is non-trivial - it would require merging of the .sbss/.sdata sections for all loaded llexts and zephyr itself.

@WorldofJARcraft
Copy link
Contributor

One more thing: the compiler does not generate GP-relative accesses itself - this is implemented as a linker relaxation, which needs to be explicitly enabled (--relax-gp).
So there is no risk of llexts suddenly trying to use this mechanism.

@pillo79
Copy link
Collaborator Author

pillo79 commented Feb 19, 2025

I see, thank you very much for the explanations. No need to go that far then, so I will close this.
I think your PR should be marked as a bugfix however!

(Also, for posterity, looked into multiple NOBITS sections and it's doable but non-trivial for shared objects).

@pillo79 pillo79 closed this Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: llext Linkable Loadable Extensions bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants