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: add build commands and tests for relocatable ELFs #71279

Merged
merged 5 commits into from
Apr 11, 2024

Conversation

pillo79
Copy link
Collaborator

@pillo79 pillo79 commented Apr 9, 2024

This PR introduces build support in the llext subsystem for relocatable objects, and a Twister test for validating this PR and #70793, which added llext support for them. It is intended to replace #70991 by providing the same result on the new CMake infrastructure merged in #67997.

It does so by:

  • introducing the LLEXT_BINARY_TYPE Kconfig, to select the build type among object files, relocatable files, or shared libraries, effectively making CMake code for llext almost arch-independent. The automatic defaults are for the tested combinations, but this gives developers more freedom to experiment.
  • relaxing add_llext_target rules to support multiple source files for the binary types that include a link step;
  • adding build support for the relocatable case;
  • incorporating the last 2 commits of [DNM] LLEXT: Relocatable test #70991 from @lyakh adding support for zero-based addresses and a relocatable test case.

pillo79 and others added 5 commits April 9, 2024 16:14
Add a new Kconfig option to select the binary object type for the llext
subsystem. This will allow to fully decouple the architecture type from
the kind of binary object that is expected by the loader.

The defaults have been chosen to match the current behavior of the ARM
and Xtensa architectures, but developers can now more easily experiment
with other object types.

Signed-off-by: Luca Burelli <[email protected]>
This change allows the `add_llext_target` function to accept multiple
source files when building an ELF shared library. The ELF object
target type is still limited to a single source file, since there is no
linking step in that case.

Also fixes a minor typo in another llext function documentation.

Signed-off-by: Luca Burelli <[email protected]>
This commit adds support for building relocatable (partially linked)
ELF files as the binary object type for the llext subsystem.

Signed-off-by: Luca Burelli <[email protected]>
Currently LLEXT on Xtensa supports relocatable extensions, linked for
a specific address range, while relocation itself takes place in a
temporary buffer. For this section addresses have to be set correctly
by the linker for their target locations.

This commit adds support for relocatable extensions, built without
using specific memory addresses and run at the same addresses, where
they are loaded.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
LLEXT on Xtensa now supports both shared and relocatable (partially
linked) extensions. This commit adds a copy of the LLEXT test for the
relocatable case.

Signed-off-by: Luca Burelli <[email protected]>
Signed-off-by: Guennadi Liakhovetski <[email protected]>
@pillo79 pillo79 changed the title llext: add build commands and tests for the relocatable case llext: add build commands and tests for relocatable ELFs Apr 9, 2024
Copy link
Collaborator

@lyakh lyakh left a comment

Choose a reason for hiding this comment

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

Thanks! Tested locally too.

Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

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

You're removing message(FATAL_ERROR "add_llext_target: unsupported architecture"). So isn't that enough to remove the LOADER_BUILD_ONLY test limitation?

@pillo79
Copy link
Collaborator Author

pillo79 commented Apr 10, 2024

You're removing message(FATAL_ERROR "add_llext_target: unsupported architecture"). So isn't that enough to remove the LOADER_BUILD_ONLY test limitation?

Good idea! I have tried removing the barriers but I get a weird error on x86, native and a53 arches:

In file included from zephyr/include/zephyr/toolchain.h:50,
                 from zephyr/include/zephyr/sys/__assert.h:11,
                 from zephyr/include/zephyr/sys/iterable_sections.h:10,
                 from zephyr/include/zephyr/llext/symbol.h:10,
                 from zephyr/tests/subsys/llext/simple/src/relative_jump_ext.c:15:
zephyr/include/zephyr/toolchain/gcc.h:573:2: error: #error processor architecture not supported
  573 | #error processor architecture not supported
      |  ^~~~~

(and all other llext tests). The compiler is missing autoconf.h; this is due to the fact that on these arches, the build.ninja rules for llext code have no FLAGS = line at all, even though the CMake code is obviously the same as the arches we are testing. This is next level for me as well... 🤷‍♂️

@pillo79
Copy link
Collaborator Author

pillo79 commented Apr 10, 2024

on these arches, the build.ninja rules for llext code have no FLAGS = line at all, even though the CMake code is obviously the same as the arches we are testing. This is next level for me as well... 🤷‍♂️

blah blah daemons blah blah

Turns out the reason was way clearer - arches where LLEXT is not tested yet do not define an LLEXT_REMOVE_FLAGS variable, so the code turns it into an empty regexp, which promptly results in $<FILTER:...> erasing the whole list of Zephyr flags. 🤦‍♂️

I have this fixed locally, however since the PR is already approved I will open a separate one with this cleanup/refactor as soon as this lands.

@marc-hb
Copy link
Collaborator

marc-hb commented Apr 10, 2024

Turns out the reason was way clearer

Nice! Well done.

since the PR is already approved...

Agreed.

Note dropping LOADER_BUILD_ONLY is the ultimate goal BUT if you have it to keep it for some more time and gradually increase the list of architectures that actually run tests instead then it would be great progress too. A first, 64bits arch passing the tests would be an especially big milestone!

@MaureenHelm MaureenHelm merged commit ce01d96 into zephyrproject-rtos:main Apr 11, 2024
38 checks passed
@pillo79 pillo79 deleted the llext-relocatable-test branch April 22, 2024 15:28
@henrikbrixandersen henrikbrixandersen added the area: llext Linkable Loadable Extensions label Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Build System area: llext Linkable Loadable Extensions area: Xtensa Xtensa Architecture
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants