cmake: New target which generates a sort of development kit for llext #69831
Conversation
zephyrbot
added
area: Samples
edersondisouza
force-pushed
the
llext-edk
branch
2 times, most recently
from
48953b0
to
e7999ba
Compare
edersondisouza
force-pushed
the
llext-edk
branch
3 times, most recently
from
5ec17b3
to
3d32e9c
Compare
There was a problem hiding this comment.
my main concern still lies with the comment here: #69831 (comment)
I need some more info / investigation on this.
How much has this been tested / verified ?
Especially, has the loadable extension been tested with floating point instructions ?
I need a bit more detailed info on exactly what / why this happens before approving something which just removes ABI flags.
| set(LLEXT_EDK_INSTALL_DIR $ENV{LLEXT_EDK_INSTALL_DIR}) | ||
| include($ENV{LLEXT_EDK_INSTALL_DIR}/cmake.cflags) |
There was a problem hiding this comment.
Two observations.
First you create a local scoped variable with value from the environment setting, but then the line below uses the environment value in the include, like: include($ENV{LLEXT_EDK_INSTALL_DIR} ...
Why not use the local scoped variable ?
Second, while environment variables are useful, then leftover variables can be annoying as well as making it harder to just do a quick test.
So please also support / provide example on just passing the variable directly on CMake invocation, so this is also supported -DLLEXT_EDK_INSTALL_DIR=<path> and takes precedence over env setting.
| parser.add_argument("-u", "--userspace-only", action="store_true", | ||
| help="Only generate the userpace path of wrappers") |
There was a problem hiding this comment.
I understand your reasoning, but that doesn't really remove the concern.
Note, I will not block PR on this.
@dcpleung please take a look.
| list(APPEND LLEXT_EDK_REMOVE_FLAGS | ||
| --sysroot=.* | ||
| -fmacro-prefix-map=.* | ||
| ) | ||
|
|
||
| list(APPEND LLEXT_EDK_APPEND_FLAGS |
There was a problem hiding this comment.
I understand your line of thought, but the toolchain infrastructure, and thus the cmake/compiler/gcc/* is for setting up the toolchain for building Zephyr. Something which is done in the current CMake invocation.
Where those flags are for exporting when building the extension, which is something quite different.
There is no obvious place for an alternative location atm.
I did consider if llext should have its own CMake module, but that collides a bit with the fact that a llext-edk target in now created.
Accepted to keep for now, but could you file an enhancement issue so that we don't forget a later cleanup, as well as providing a bit of history as to why this location was accepted. So that we can reference in future PRs.
Besides the LLEXT_CFLAGS, which have all that is needed to compile, generate more granular ones, LLEXT_INCLUDE_CFLAGS, LLEXT_ALL_INCLUDE_CFLAGS, LLEXT_GENERATED_INCLUDE_CFLAGS and LLEXT_BASE_CFLAGS. These are done for convenience, as they can help on different setups, such as unit testing. Signed-off-by: Ederson de Souza <ederson.desouza@intel.com>
This way they can be reused by the LLEXT EDK. Signed-off-by: Ederson de Souza <ederson.desouza@intel.com>
Some flags are common between in tree extensions and out of tree supported by the EDK. Instead of duplicating those flags, the EDK reuses the llext ones. However, as the EDK has its own needs, two new lists, `LLEXT_EDK_APPEND_FLAGS` and `LLEXT_EDK_REMOVE_FLAGS` are defined to allow EDK to append or remove flags as needed. Signed-off-by: Ederson de Souza <ederson.desouza@intel.com>
Instead of hardcoding `autoconf.h` imacro, get the list of imacros from the llext flags. As those come in the form of absolute paths, they also need to be massaged to point from the EDK directory without revealing host complete paths. Also, the EDK now keeps the imacros on a different flag, `LLEXT_GENERATED_IMACROS_CFLAGS`, to keep it similar to other generated includes. Signed-off-by: Ederson de Souza <ederson.desouza@intel.com>
This test uses pytest to generate an EDK from a simple Zephyr application, and uses this EDK to build a simple extension, to ensure that EDK generation is sane. Signed-off-by: Ederson de Souza <ederson.desouza@intel.com>
Shows a simple application which loads extensions and some simple extensions. While everything is inside Zephyr tree, everything can actually be build from different directories (even machines), as long as the EDK is generated from the application and used by the extensions. More information is available at sample's README. This sample is build only for twister, as it requires a few steps to be properly run, namely build the EDK, install it somewhere, build the extensions using the EDK and finally build the application with the extensions. Signed-off-by: Ederson de Souza <ederson.desouza@intel.com>
edersondisouza
dismissed stale reviews from cfriedt, kartben, teburd, and pillo79
via
e5c0593
|
v8:
|
edersondisouza
requested review from
tejlmand,
pillo79,
kartben,
teburd and
cfriedt
There was a problem hiding this comment.
Approved.
Still concerned on the removal, but at least the code is better commented and there is a followup issue, #72832.
Currently the best that can be done, while also allowing the feature to be merged and get more user feedback.
Loadable extensions need access to Zephyr (and Zephyr application) includes and some CFLAGS to be properly built. This RFC adds a new target,
llext-edk, which generates a tar file with those includes and flags that can be loaded from make and make files.This tar file can then be used by extension developers to build extensions that can be loaded by the Zephyr application - it becomes some sort of - for a better name, suggestions welcome =D - Extension Development Kit or EDK.
The main point of the EDK is to provide an interface for extension developers, which may not have access to the Zephyr application source code, so their build is entirely out of tree. The expected usage flow would be like:
west build -t llext-edk;The final form of the extension or how it is loaded by the application is beyond the scope of the EDK - it's a "contract" between application and extension developers. That said, I can see how in the future some features for this could be included in Zephyr and the EDK.
Last patch in this series is an example of an application and a few extensions. The README contains the instructions for building everything, and should give a good feeling of how everything fits together. Ideally, this sample would live in several repositories: one for the application, and one for each extension, to show how the EDK acts as the bridge to build the extensions. But to ease review (and because having several repositories is too much), it's another sample.
As with any RFC, comments/critics/suggestions are welcome =D
Note 1: One of the patches is actually taken from #69540 - and it's here because it's needed for the sample to work. Once that PR is in, I'll rebase this one.
Note 2: The sample uses
EXPORT_SYMBOLto export some Zephyr APIs to the extensions (like semaphores and threads). However,checkpatch.plcomplains that the exports should be close to the symbols being exported. I didn't go changing several places in Zephyr just for this, to avoid distraction during review, but I can update the PR to do that. Not sure how Zephyr will handle this: will every API be exported at some point?