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.

Sign up for GitHub

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

Jump to bottom

LLEXT: persistent dependency #9804

Merged
merged 11 commits into from
Feb 18, 2025
Merged

LLEXT: persistent dependency #9804

merged 11 commits into from
Feb 18, 2025
+391 −116

Conversation

lyakh
Copy link
Collaborator

@lyakh lyakh commented Jan 30, 2025

this should replace #9399

@lyakh lyakh force-pushed the depend-persist branch 6 times, most recently from 698ac69 to 497d5b7 Compare February 5, 2025 16:21
@lyakh
Copy link
Collaborator Author

lyakh commented Feb 6, 2025

SOFCI TEST

@lyakh
Copy link
Collaborator Author

lyakh commented Feb 7, 2025

CI is good now. The latest test re-run was to test a Zephyr logging fix from yesterday, and it does indeed (again) fix logging during module unloading. This testing version of the PR uses MTL to test the new approach to dependencies, so MTL results are most important, but others obviously shouldn't regress either. MTL use of LLEXT including dependencies can be seen in https://sof-ci.01.org/sofpr/PR9804/build10554/devicetest/index.html?model=MTLP_SDW_AIOC&testcase=check-playback-all-formats - the very first test there plays audio to the "Speaker" PCM, whose pipeline contains eq_fir, which uses the FIR library module. QuickBuild failure is expected - it doesn't support loading the "openmodules" library yet. Otherwise the results are:
TGL / cAVS 2.5: green + untested "nocodec" https://sof-ci.01.org/sofpr/PR9804/build10555/devicetest/index.html
MTL: green + untested HDA https://sof-ci.01.org/sofpr/PR9804/build10554/devicetest/index.html
LNL: green + untested "nocodec" https://sof-ci.01.org/sofpr/PR9804/build10553/devicetest/index.html

@lyakh lyakh mentioned this pull request Feb 7, 2025
Merged
@lyakh
Copy link
Collaborator Author

lyakh commented Feb 7, 2025

splitting this: fast_get() fixes go to #9817 , some simple commits go to #9818

@lyakh
Copy link
Collaborator Author

lyakh commented Feb 7, 2025

Updated the Zephyr PR, let's re-test

@lyakh
Copy link
Collaborator Author

lyakh commented Feb 7, 2025

SOFCI TEST

@pillo79 pillo79 mentioned this pull request Feb 10, 2025
Closed
@lyakh lyakh mentioned this pull request Feb 11, 2025
Closed
@lgirdwood
Copy link
Member

splitting this: fast_get() fixes go to #9817 , some simple commits go to #9818

@lyakh both now merged, do you need to rebase this ?

@lyakh lyakh mentioned this pull request Feb 12, 2025
Closed
@lyakh
Copy link
Collaborator Author

lyakh commented Feb 13, 2025

Need to keep last CI results in https://sof-ci.01.org/sofpr/PR9804/build10714/devicetest/index.html before rebasing.

@pillo79 pillo79 mentioned this pull request Feb 13, 2025
Open
@lyakh
Copy link
Collaborator Author

lyakh commented Feb 13, 2025

SOFCI TEST

@lyakh lyakh force-pushed the depend-persist branch 2 times, most recently from 8a15be7 to c208dae Compare February 14, 2025 15:22
@lyakh
Copy link
Collaborator Author

lyakh commented Feb 16, 2025

SOFCI TEST

@lyakh
Copy link
Collaborator Author

lyakh commented Feb 16, 2025

CI: all alsabat tests failed for MTL HDA in https://sof-ci.01.org/sofpr/PR9804/build10787/devicetest/index.html, interestingly they've also failed in an unrelated PR https://sof-ci.01.org/sofpr/PR9829/build10805/devicetest/index.html but the error seems to be different.

@lyakh
Copy link
Collaborator Author

lyakh commented Feb 17, 2025

CI: all alsabat tests failed for MTL HDA both in https://sof-ci.01.org/sofpr/PR9804/build10787/devicetest/index.html and in https://sof-ci.01.org/sofpr/PR9804/build10815/devicetest/index.html and the errors seem similar

WARNING: Found low peak 0.73 Hz, very close to DC

They've also failed in an unrelated PR https://sof-ci.01.org/sofpr/PR9829/build10805/devicetest/index.html With a similar error.

The pause-release failure in https://sof-ci.01.org/sofpr/PR9804/build10815/devicetest/index.html?model=MTLP_SDW_AIOC&testcase=multiple-pause-resume-50 is less clear: there's a non-fatal IPC timeout there, seemingly caused by an underrun , accompanied by a long sequence of

[ 1280.311771] <wrn> dai_comp: dai_common_copy: comp:2 0x30003 nothing to copy, src_frames: 0, sink_frames: 96
[ 1280.312766] <wrn> dai_comp: dai_common_copy: comp:2 0x30003 nothing to copy, src_frames: 0, sink_frames: 96
[ 1280.313766] <wrn> dai_comp: dai_common_copy: comp:2 0x30003 nothing to copy, src_frames: 0, sink_frames: 96
[ 1280.314766] <wrn> dai_comp: dai_common_copy: comp:2 0x30003 nothing to copy, src_frames: 0, sink_frames: 96
[ 1280.315766] <wrn> dai_comp: dai_common_copy: comp:2 0x30003 nothing to copy, src_frames: 0, sink_frames: 96
[ 1280.316766] <wrn> dai_comp: dai_common_copy: comp:2 0x30003 nothing to copy, src_frames: 0, sink_frames: 96
[ 1280.317766] <wrn> dai_comp: dai_common_copy: comp:2 0x30003 nothing to copy, src_frames: 0, sink_frames: 96

in the firmware log. Such messages appear in other instances of these tests too, but in much smaller numbers. The https://sof-ci.01.org/sofpr/PR9804/build10787/devicetest/index.html test doesn't have them.

@lyakh
Copy link
Collaborator Author

lyakh commented Feb 17, 2025

CI:

I'll now drop the last 3 commits to make this "mergeable"

lyakh added 2 commits February 17, 2025 12:45
@lyakh
Update Zephyr
df43333 
Update Zephyr to fetch commits:

037cb87276bb ("llext: export spinlock debugging symbols to extensions")
a2a587ae20b4 ("llext: fix symbol exporting for ET_REL")

and the new LLEXT inspection API.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@lyakh
llext: add auxiliary library type
75c21e1 
Add a dedicated type for auxiliary LLEXT objects, not implementing
the Module Adapter API.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
lyakh added 9 commits February 17, 2025 12:45
@lyakh
llext: move the llext pointer to struct lib_manager_module
f1763d4 
The LLEXT context is currently bound to the Module Adapter API which
shouldn't be the case because we also need generic LLEXT-based
loadable SOF objects. This patch moves the context to struct
lib_manager_module which is safe, because the library manager is
always needed when dynamically loading modules.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@lyakh
llext: extract linking of a single module into a function
068d632 
Extract localisation and linking of a single LLEXT module into a
separate function, it will be re-used for auxiliary modules.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@lyakh
llext: call llext_unload() internally
d76aada 
Currently lib_manager_module_free() first checks, if an LLEXT module
is being freed. If so, it first calls llext_unload() and then - if
the use-count is reported as zero, calls llext_manager_free_module().
Simplify this process by just calling llext_manager_free_module() and
letting it handle llext_unload() internally.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@lyakh
Revert "llext: remove LIB_MANAGER_BSS"
40e6862 
This reverts commit cf31c9c. We do
now need to store BSS information for auxiliary LLEXT management.
When instantiating such a module for the second time, we don't have
access to Zephyr LLEXT information any more, so we need to use our
cached information for .bss allocation.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@lyakh
rimage: auxiliary modules don't have TOML configuration
cc42fc0 
Auxiliary modules, implemented as LLEXT and used to provide
functionality for other modules, don't have TOML configuration, make
rimage skip that step for them.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@lyakh
llext: simplify llext_manager_load_module() prototype
ffacce8 
llext_manager_load_module()'s ebl argument is only used for .bss
alignment. Calculate it automatically to eliminate ebl, because it
isn't available during following loads. e.g. when reloading
dependencies.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@lyakh
llext: keep minimal Zephyr context when unloading modules
294f155 
Currently when module's last instance is destroyed, it's freed and
its Zephyr context is destroyed too. At the same time the module is
kept in DRAM in a linked and relocated state, which means, that next
time when we have to use it, we need to tell Zephyr to instantiate it
while skipping the linking step. Additionally this makes handling
dependencies inconvenient: they are created during the linking step
by Zephyr and therefore are lost when the module is released. Then,
as described above, they're not recreated when the linking step is
skipped during a subsequent load.

To fix this problem this commit avoids destroying module's Zephyr
context when freeing. This costs around 200 bytes but makes handling
of dependencies possible.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@lyakh
llext: add dependencies
1c67251 
Add support for auxiliary modules, exporting symbols to other
modules. In such cases Zephyr LLEXT API generates a dependency list,
available while the dependent module is loaded. SOF now preserves the
minimum module context even while it isn't used, that includes
dependency lists, so on a repeated load they're still available.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@lyakh
fir: enable building FIR support code as an LLEXT object
b8f3c9b 
FIR support code is used by eq-fir and by tdfb. When both of them are
built as LLEXT modules, FIR supporting functions can be dynamically
loaded too.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@lyakh lyakh marked this pull request as ready for review February 17, 2025 11:47
@lyakh
Copy link
Collaborator Author

lyakh commented Feb 17, 2025

This version is identical to the previous one with just platform enabling dropped. We cannot yet enable openmodules, because the "Internal Intel CI" doesn't support them yet, but we can merge this in expectation of that support being added.

kv2019i
kv2019i approved these changes Feb 18, 2025
View reviewed changes
west.yml
@@ -43,7 +43,7 @@ manifest:

- name: zephyr
repo-path: zephyr
revision: aaa119d75729410ae70fc8ba188634c23214f804
revision: fe29c40a9366b5ffdcdd2eac26023ce4502413b1
Copy link
Collaborator

Choose a reason for hiding this comment

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

we usually have "west.yml: update Zephyr" as the git commit

src/library_manager/llext_manager.c

ret = llext_manager_load_module(dep_ctx[i]);
if (ret < 0) {
llext_depend_unlink(dep_ctx, i - 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This name fooled me a bit but it seems this is correct, this unrolls all the dependencies from n=i...0 ... maybe llext_depend_unlink_rollback() ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kv2019i sure, can rename as a follow-up, in fact it should even be llext_manager_depend_unlink_rollback()...

@lgirdwood lgirdwood merged commit 5661b3f into thesofproject:main Feb 18, 2025
46 of 51 checks passed
@lyakh lyakh deleted the depend-persist branch February 18, 2025 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lgirdwood lgirdwood lgirdwood approved these changes

@kv2019i kv2019i kv2019i approved these changes

@iuliana-prodan iuliana-prodan Awaiting requested review from iuliana-prodan iuliana-prodan is a code owner

@dbaluta dbaluta Awaiting requested review from dbaluta dbaluta is a code owner

@abonislawski abonislawski Awaiting requested review from abonislawski abonislawski is a code owner

@singalsu singalsu Awaiting requested review from singalsu singalsu is a code owner

@plbossart plbossart Awaiting requested review from plbossart plbossart is a code owner

@mmaka1 mmaka1 Awaiting requested review from mmaka1 mmaka1 is a code owner

@lbetlej lbetlej Awaiting requested review from lbetlej lbetlej is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lyakh @lgirdwood @kv2019i