-
Notifications
You must be signed in to change notification settings - Fork 8.2k
LLEXT dependencies: add support for linking between LLEXT objects #77473
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
Changes from all commits
7deb037
d1d3c67
d596509
b8ec7e3
9a1f217
b2114ea
c110441
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,6 +62,9 @@ enum llext_mem { | |
| struct llext_loader; | ||
| /** @endcond */ | ||
|
|
||
| /* Maximim number of dependency LLEXTs */ | ||
| #define LLEXT_MAX_DEPENDENCIES 8 | ||
|
|
||
| /** | ||
| * @brief Structure describing a linkable loadable extension | ||
| * | ||
|
|
@@ -111,6 +114,9 @@ struct llext { | |
|
|
||
| /** Extension use counter, prevents unloading while in use */ | ||
| unsigned int use_count; | ||
|
|
||
| /** Array of extensions, whose symbols this extension accesses */ | ||
| struct llext *dependency[LLEXT_MAX_DEPENDENCIES]; | ||
|
||
| }; | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,4 +31,4 @@ int start(void) | |
|
|
||
| return 0; | ||
| } | ||
| LL_EXTENSION_SYMBOL(start); | ||
| EXPORT_SYMBOL(start); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,4 +23,4 @@ int start(void) | |
|
|
||
| return 0; | ||
| } | ||
| LL_EXTENSION_SYMBOL(start); | ||
| EXPORT_SYMBOL(start); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,4 +62,4 @@ int start(void) | |
|
|
||
| return 0; | ||
| } | ||
| LL_EXTENSION_SYMBOL(start); | ||
| EXPORT_SYMBOL(start); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,4 +61,4 @@ int start(void) | |
|
|
||
| return 0; | ||
| } | ||
| LL_EXTENSION_SYMBOL(start); | ||
| EXPORT_SYMBOL(start); | ||
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,6 +52,86 @@ static size_t llext_file_offset(struct llext_loader *ldr, size_t offset) | |
| return offset; | ||
| } | ||
|
|
||
| /* | ||
| * We increment use-count every time a new dependent is added, and have to | ||
| * decrement it again, when one is removed. Ideally we should be able to add | ||
| * arbitrary numbers of dependencies, but using lists for this doesn't work, | ||
| * because multiple extensions can have common dependencies. Dynamically | ||
| * allocating dependency entries would be too wasteful. In this initial | ||
| * implementation we use an array of dependencies, if at some point we run out | ||
| * of array entries, we'll implement re-allocation. | ||
| * We add dependencies incrementally as we discover them, but we only ever | ||
| * expect them to be removed all at once, when their user is removed. So the | ||
| * dependency array is always "dense" - it cannot have NULL entries between | ||
| * valid ones. | ||
| */ | ||
| static int llext_dependency_add(struct llext *ext, struct llext *dependency) | ||
| { | ||
| unsigned int i; | ||
|
|
||
| for (i = 0; i < ARRAY_SIZE(ext->dependency); i++) { | ||
| if (ext->dependency[i] == dependency) { | ||
| return 0; | ||
| } | ||
|
|
||
| if (!ext->dependency[i]) { | ||
| ext->dependency[i] = dependency; | ||
| dependency->use_count++; | ||
|
|
||
| return 0; | ||
| } | ||
| } | ||
|
|
||
| return -ENOENT; | ||
| } | ||
|
|
||
| void llext_dependency_remove_all(struct llext *ext) | ||
| { | ||
| unsigned int i; | ||
|
|
||
| for (i = 0; i < ARRAY_SIZE(ext->dependency) && ext->dependency[i]; i++) { | ||
| /* | ||
| * The use-count of dependencies is tightly bound to dependent's | ||
| * life cycle, so it shouldn't underrun. | ||
| */ | ||
| ext->dependency[i]->use_count--; | ||
| __ASSERT(ext->dependency[i]->use_count, "LLEXT dependency use-count underrun!"); | ||
| /* No need to NULL-ify the pointer - ext is freed after this */ | ||
| } | ||
| } | ||
|
|
||
| struct llext_extension_sym { | ||
| struct llext *ext; | ||
pillo79 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| const char *sym; | ||
| const void *addr; | ||
| }; | ||
|
|
||
| static int llext_find_extension_sym_iterate(struct llext *ext, void *arg) | ||
| { | ||
| struct llext_extension_sym *se = arg; | ||
| const void *addr = llext_find_sym(&ext->exp_tab, se->sym); | ||
|
|
||
| if (addr) { | ||
| se->addr = addr; | ||
| se->ext = ext; | ||
pillo79 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return 1; | ||
| } | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| static const void *llext_find_extension_sym(const char *sym_name, struct llext **ext) | ||
| { | ||
| struct llext_extension_sym se = {.sym = sym_name}; | ||
|
|
||
| llext_iterate(llext_find_extension_sym_iterate, &se); | ||
| if (ext) { | ||
| *ext = se.ext; | ||
| } | ||
|
|
||
| return se.addr; | ||
| } | ||
|
|
||
| static void llext_link_plt(struct llext_loader *ldr, struct llext *ext, | ||
| elf_shdr_t *shdr, bool do_local, elf_shdr_t *tgt) | ||
| { | ||
|
|
@@ -141,13 +221,25 @@ static void llext_link_plt(struct llext_loader *ldr, struct llext *ext, | |
|
|
||
| switch (stb) { | ||
| case STB_GLOBAL: | ||
| /* First try the global symbol table */ | ||
| link_addr = llext_find_sym(NULL, | ||
| SYM_NAME_OR_SLID(name, sym_tbl.st_value)); | ||
|
|
||
| if (!link_addr) { | ||
| /* Next try internal tables */ | ||
| link_addr = llext_find_sym(&ext->sym_tab, name); | ||
| } | ||
|
|
||
| if (!link_addr) { | ||
| /* Finally try any loaded tables */ | ||
| struct llext *dep; | ||
|
|
||
| link_addr = llext_find_extension_sym(name, &dep); | ||
| if (link_addr) { | ||
| llext_dependency_add(ext, dep); | ||
| } | ||
| } | ||
|
|
||
| if (!link_addr) { | ||
| LOG_WRN("PLT: cannot find idx %u name %s", j, name); | ||
| continue; | ||
|
|
@@ -297,6 +389,16 @@ int llext_link(struct llext_loader *ldr, struct llext *ext, bool do_local) | |
| link_addr = (uintptr_t)llext_find_sym(NULL, | ||
| SYM_NAME_OR_SLID(name, sym.st_value)); | ||
|
|
||
| if (link_addr == 0) { | ||
| /* Try loaded tables */ | ||
| struct llext *dep; | ||
|
|
||
| link_addr = (uintptr_t)llext_find_extension_sym(name, &dep); | ||
| if (link_addr) { | ||
| llext_dependency_add(ext, dep); | ||
| } | ||
| } | ||
|
|
||
|
Comment on lines
+392
to
+401
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very minor nit - if you ever have to refactor anything: this bit of code is added in the last commit, which only deals with dependency tracking. Similar to what happens for Xtensa, it could be split between the commit that introduces the actual lookup and the one adding tracking. |
||
| if (link_addr == 0) { | ||
| LOG_ERR("Undefined symbol with no entry in " | ||
| "symbol table %s, offset %zd, link section %d", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,4 +22,4 @@ void test_entry(void) | |
| /* unused */ | ||
| } | ||
|
|
||
| LL_EXTENSION_SYMBOL(number); | ||
| EXPORT_SYMBOL(number); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,4 +50,4 @@ void test_entry(void) | |
|
|
||
| run_id += 1; | ||
| } | ||
| LL_EXTENSION_SYMBOL(test_entry); | ||
| EXPORT_SYMBOL(test_entry); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,4 +45,4 @@ void test_entry(void) | |
|
|
||
| run_id += 1; | ||
| } | ||
| LL_EXTENSION_SYMBOL(test_entry); | ||
| EXPORT_SYMBOL(test_entry); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a Kconfig, not a blocker but would be good to see a follow up for