From 59cbea36c85bbd0a958bbf03b46fe2521cdb6502 Mon Sep 17 00:00:00 2001 From: Luca Burelli Date: Mon, 10 Feb 2025 11:00:40 +0100 Subject: [PATCH 1/3] llext_manager: refactor: directly pass llext_loader llext_manager_link() is only interested in the 'llext_loader' part of the 'llext_buf_loader' structure. This patch refactors the code to pass a pointer to the llext_loader directly, making the code more readable. In preparation for the new inspection API, this patch also changes the checks for the buildinfo and mod_manifest pointers to be more explicit. No functional change is intended. Signed-off-by: Luca Burelli --- src/library_manager/llext_manager.c | 38 +++++++++++++++-------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index f6c88c9d1514..f3a13928430e 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -238,7 +238,7 @@ static bool llext_manager_section_detached(const elf_shdr_t *shdr) return shdr->sh_addr < SOF_MODULE_DRAM_LINK_END; } -static int llext_manager_link(struct llext_buf_loader *ebl, const char *name, +static int llext_manager_link(struct llext_loader *ldr, const char *name, struct lib_manager_module *mctx, const void **buildinfo, const struct sof_man_module_manifest **mod_manifest) { @@ -268,13 +268,13 @@ static int llext_manager_link(struct llext_buf_loader *ebl, const char *name, .section_detached = llext_manager_section_detached, }; - ret = llext_load(&ebl->loader, name, llext, &ldr_parm); + ret = llext_load(ldr, name, llext, &ldr_parm); if (ret) return ret; } - mctx->segment[LIB_MANAGER_TEXT].addr = ebl->loader.sects[LLEXT_MEM_TEXT].sh_addr; - mctx->segment[LIB_MANAGER_TEXT].size = ebl->loader.sects[LLEXT_MEM_TEXT].sh_size; + mctx->segment[LIB_MANAGER_TEXT].addr = ldr->sects[LLEXT_MEM_TEXT].sh_addr; + mctx->segment[LIB_MANAGER_TEXT].size = ldr->sects[LLEXT_MEM_TEXT].sh_size; tr_dbg(&lib_manager_tr, ".text: start: %#lx size %#x", mctx->segment[LIB_MANAGER_TEXT].addr, @@ -282,8 +282,8 @@ static int llext_manager_link(struct llext_buf_loader *ebl, const char *name, /* All read-only data sections */ mctx->segment[LIB_MANAGER_RODATA].addr = - ebl->loader.sects[LLEXT_MEM_RODATA].sh_addr; - mctx->segment[LIB_MANAGER_RODATA].size = ebl->loader.sects[LLEXT_MEM_RODATA].sh_size; + ldr->sects[LLEXT_MEM_RODATA].sh_addr; + mctx->segment[LIB_MANAGER_RODATA].size = ldr->sects[LLEXT_MEM_RODATA].sh_size; tr_dbg(&lib_manager_tr, ".rodata: start: %#lx size %#x", mctx->segment[LIB_MANAGER_RODATA].addr, @@ -291,31 +291,33 @@ static int llext_manager_link(struct llext_buf_loader *ebl, const char *name, /* All writable data sections */ mctx->segment[LIB_MANAGER_DATA].addr = - ebl->loader.sects[LLEXT_MEM_DATA].sh_addr; - mctx->segment[LIB_MANAGER_DATA].size = ebl->loader.sects[LLEXT_MEM_DATA].sh_size; + ldr->sects[LLEXT_MEM_DATA].sh_addr; + mctx->segment[LIB_MANAGER_DATA].size = ldr->sects[LLEXT_MEM_DATA].sh_size; tr_dbg(&lib_manager_tr, ".data: start: %#lx size %#x", mctx->segment[LIB_MANAGER_DATA].addr, mctx->segment[LIB_MANAGER_DATA].size); - mctx->segment[LIB_MANAGER_BSS].addr = ebl->loader.sects[LLEXT_MEM_BSS].sh_addr; - mctx->segment[LIB_MANAGER_BSS].size = ebl->loader.sects[LLEXT_MEM_BSS].sh_size; + mctx->segment[LIB_MANAGER_BSS].addr = ldr->sects[LLEXT_MEM_BSS].sh_addr; + mctx->segment[LIB_MANAGER_BSS].size = ldr->sects[LLEXT_MEM_BSS].sh_size; tr_dbg(&lib_manager_tr, ".bss: start: %#lx size %#x", mctx->segment[LIB_MANAGER_BSS].addr, mctx->segment[LIB_MANAGER_BSS].size); - ssize_t binfo_o = llext_find_section(&ebl->loader, ".mod_buildinfo"); + *buildinfo = NULL; + ssize_t binfo_o = llext_find_section(ldr, ".mod_buildinfo"); if (binfo_o >= 0) - *buildinfo = llext_peek(&ebl->loader, binfo_o); + *buildinfo = llext_peek(ldr, binfo_o); - ssize_t mod_o = llext_find_section(&ebl->loader, ".module"); + *mod_manifest = NULL; + ssize_t mod_o = llext_find_section(ldr, ".module"); if (mod_o >= 0) - *mod_manifest = llext_peek(&ebl->loader, mod_o); + *mod_manifest = llext_peek(ldr, mod_o); - return binfo_o >= 0 && mod_o >= 0 ? 0 : -EPROTO; + return *buildinfo && *mod_manifest ? 0 : -EPROTO; } /* Count "module files" in the library, allocate and initialize memory for their descriptors */ @@ -427,8 +429,8 @@ static int llext_manager_link_single(uint32_t module_id, const struct sof_man_fw mod_size = ALIGN_UP(mod_array[i].segment[LIB_MANAGER_TEXT].file_offset - mod_offset, PAGE_SZ); - uintptr_t dram_base = (uintptr_t)desc - SOF_MAN_ELF_TEXT_OFFSET; - struct llext_buf_loader ebl = LLEXT_BUF_LOADER((uint8_t *)dram_base + mod_offset, mod_size); + uint8_t *dram_base = (uint8_t *)desc - SOF_MAN_ELF_TEXT_OFFSET; + struct llext_buf_loader ebl = LLEXT_BUF_LOADER(dram_base + mod_offset, mod_size); /* * LLEXT linking is only needed once for all the "drivers" in the @@ -436,7 +438,7 @@ static int llext_manager_link_single(uint32_t module_id, const struct sof_man_fw * dependencies, sets up sections and retrieves buildinfo and * mod_manifest */ - ret = llext_manager_link(&ebl, mod_array[entry_index - inst_idx].name, mctx, + ret = llext_manager_link(&ebl.loader, mod_array[entry_index - inst_idx].name, mctx, buildinfo, mod_manifest); if (ret < 0) { tr_err(&lib_manager_tr, "linking failed: %d", ret); From 68bcdc5646bef9a93fa7861b17e9537ec9f00a5b Mon Sep 17 00:00:00 2001 From: Luca Burelli Date: Fri, 21 Feb 2025 09:29:47 +0100 Subject: [PATCH 2/3] llext_manager: persist llext_buf_loader For inspection API to work, it is imperative to keep the LLEXT loader object available. This patch adds a pointer to the full loader object in the lib_manager_module structure. The loader object is allocated once and never freed. Signed-off-by: Luca Burelli --- src/include/sof/lib_manager.h | 2 ++ src/library_manager/llext_manager.c | 22 ++++++++++++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/include/sof/lib_manager.h b/src/include/sof/lib_manager.h index 23364d83ba7e..ffe3491d777d 100644 --- a/src/include/sof/lib_manager.h +++ b/src/include/sof/lib_manager.h @@ -97,12 +97,14 @@ struct lib_manager_segment_desc { }; struct llext; +struct llext_buf_loader; struct lib_manager_module { unsigned int start_idx; /* Index of the first driver from this module in * the library-global driver list */ const struct sof_man_module_manifest *mod_manifest; struct llext *llext; /* Zephyr loadable extension context */ + struct llext_buf_loader *ebl; /* Zephyr loadable extension buffer loader */ bool mapped; struct lib_manager_segment_desc segment[LIB_MANAGER_N_SEGMENTS]; }; diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index f3a13928430e..4aa549f39158 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -238,11 +238,12 @@ static bool llext_manager_section_detached(const elf_shdr_t *shdr) return shdr->sh_addr < SOF_MODULE_DRAM_LINK_END; } -static int llext_manager_link(struct llext_loader *ldr, const char *name, +static int llext_manager_link(const char *name, struct lib_manager_module *mctx, const void **buildinfo, const struct sof_man_module_manifest **mod_manifest) { struct llext **llext = &mctx->llext; + struct llext_loader *ldr = &mctx->ebl->loader; int ret; if (*llext && !mctx->mapped) { @@ -352,6 +353,7 @@ static int llext_manager_mod_init(struct lib_manager_mod_ctx *ctx, offs = mod_array[i].segment[LIB_MANAGER_TEXT].file_offset; ctx->mod[n_mod].mapped = false; ctx->mod[n_mod].llext = NULL; + ctx->mod[n_mod].ebl = NULL; ctx->mod[n_mod++].start_idx = i; } @@ -429,8 +431,20 @@ static int llext_manager_link_single(uint32_t module_id, const struct sof_man_fw mod_size = ALIGN_UP(mod_array[i].segment[LIB_MANAGER_TEXT].file_offset - mod_offset, PAGE_SZ); - uint8_t *dram_base = (uint8_t *)desc - SOF_MAN_ELF_TEXT_OFFSET; - struct llext_buf_loader ebl = LLEXT_BUF_LOADER(dram_base + mod_offset, mod_size); + if (!mctx->ebl) { + /* allocate once, never freed */ + mctx->ebl = rmalloc(SOF_MEM_ZONE_RUNTIME_SHARED, 0, SOF_MEM_CAPS_RAM, + sizeof(struct llext_buf_loader)); + if (!mctx->ebl) { + tr_err(&lib_manager_tr, "loader alloc failed"); + return 0; + } + + uint8_t *dram_base = (uint8_t *)desc - SOF_MAN_ELF_TEXT_OFFSET; + + *mctx->ebl = (struct llext_buf_loader)LLEXT_BUF_LOADER(dram_base + mod_offset, + mod_size); + } /* * LLEXT linking is only needed once for all the "drivers" in the @@ -438,7 +452,7 @@ static int llext_manager_link_single(uint32_t module_id, const struct sof_man_fw * dependencies, sets up sections and retrieves buildinfo and * mod_manifest */ - ret = llext_manager_link(&ebl.loader, mod_array[entry_index - inst_idx].name, mctx, + ret = llext_manager_link(mod_array[entry_index - inst_idx].name, mctx, buildinfo, mod_manifest); if (ret < 0) { tr_err(&lib_manager_tr, "linking failed: %d", ret); From c449256a863284d5660a020ade38460d1a4e5559 Mon Sep 17 00:00:00 2001 From: Luca Burelli Date: Fri, 21 Feb 2025 11:48:34 +0100 Subject: [PATCH 3/3] llext_manager: convert to use new LLEXT inspection API This patch converts the llext_manager to use the new LLEXT inspection API. The new API allows to get information about sections and regions without the need to access the internal structures of the LLEXT loader, decoupling SOF and LLEXT code and making it easier to maintain. NOTE: Once loaded the first time, the extensions are never unloaded, so the inspection data is also never freed. If this behavior needs to be modified to allow extensions to be fully removed from memory, the inspection data in the loader must be freed as well by calling the llext_free_inspection_data() function before the final llext_unload(). Signed-off-by: Luca Burelli --- src/library_manager/llext_manager.c | 115 ++++++++++++++++++---------- 1 file changed, 73 insertions(+), 42 deletions(-) diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index 4aa549f39158..5c4547bd0ca7 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -70,40 +71,49 @@ static int llext_manager_align_unmap(void __sparse_cache *vma, size_t size) return sys_mm_drv_unmap_region(aligned_vma, ALIGN_UP(pre_pad_size + size, PAGE_SZ)); } -static int llext_manager_load_data_from_storage(const struct llext *ext, +/* + * Map the memory range covered by 'vma' and 'size' as writable, copy all + * sections that belong to the specified 'region' and are contained in the + * memory range, then remap the same area according to the 'flags' parameter. + */ +static int llext_manager_load_data_from_storage(const struct llext_loader *ldr, + const struct llext *ext, + enum llext_mem region, void __sparse_cache *vma, - const uint8_t *load_base, size_t size, uint32_t flags) { unsigned int i; + const void *region_addr; int ret = llext_manager_align_map(vma, size, SYS_MM_MEM_PERM_RW); - const elf_shdr_t *shdr; if (ret < 0) { tr_err(&lib_manager_tr, "cannot map %u of %p", size, (__sparse_force void *)vma); return ret; } - size_t init_offset = 0; + llext_get_region_info(ldr, ext, region, NULL, ®ion_addr, NULL); /* Need to copy sections within regions individually, offsets may differ */ - for (i = 0, shdr = llext_section_headers(ext); i < llext_section_count(ext); i++, shdr++) { + for (i = 0; i < llext_section_count(ext); i++) { + const elf_shdr_t *shdr; + enum llext_mem s_region = LLEXT_MEM_COUNT; + size_t s_offset = 0; + + llext_get_section_info(ldr, ext, i, &shdr, &s_region, &s_offset); + + /* skip sections not in the requested region */ + if (s_region != region) + continue; + + /* skip detached sections (will be outside requested VMA area) */ if ((uintptr_t)shdr->sh_addr < (uintptr_t)vma || (uintptr_t)shdr->sh_addr >= (uintptr_t)vma + size) continue; - if (!init_offset) - init_offset = shdr->sh_offset; - - /* found a section within the region */ - size_t offset = shdr->sh_offset - init_offset; - - if (shdr->sh_type != SHT_NOBITS) { - ret = memcpy_s((__sparse_force void *)shdr->sh_addr, size - offset, - load_base + offset, shdr->sh_size); - if (ret < 0) - return ret; - } + ret = memcpy_s((__sparse_force void *)shdr->sh_addr, size - s_offset, + (const uint8_t *)region_addr + s_offset, shdr->sh_size); + if (ret < 0) + return ret; } /* @@ -165,23 +175,29 @@ static int llext_manager_load_module(struct lib_manager_module *mctx) } } + const struct llext_loader *ldr = &mctx->ebl->loader; const struct llext *ext = mctx->llext; /* Copy Code */ - ret = llext_manager_load_data_from_storage(ext, va_base_text, ext->mem[LLEXT_MEM_TEXT], - text_size, SYS_MM_MEM_PERM_EXEC); + ret = llext_manager_load_data_from_storage(ldr, ext, LLEXT_MEM_TEXT, + va_base_text, text_size, SYS_MM_MEM_PERM_EXEC); if (ret < 0) return ret; /* Copy read-only data */ - ret = llext_manager_load_data_from_storage(ext, va_base_rodata, ext->mem[LLEXT_MEM_RODATA], - rodata_size, 0); + ret = llext_manager_load_data_from_storage(ldr, ext, LLEXT_MEM_RODATA, + va_base_rodata, rodata_size, 0); if (ret < 0) goto e_text; /* Copy writable data */ - ret = llext_manager_load_data_from_storage(ext, va_base_data, ext->mem[LLEXT_MEM_DATA], - data_size, SYS_MM_MEM_PERM_RW); + /* + * NOTE: va_base_data and data_size refer to an address range that + * spans over the BSS area as well, so the mapping will cover + * both, but only LLEXT_MEM_DATA sections will be copied. + */ + ret = llext_manager_load_data_from_storage(ldr, ext, LLEXT_MEM_DATA, + va_base_data, data_size, SYS_MM_MEM_PERM_RW); if (ret < 0) goto e_rodata; @@ -244,6 +260,7 @@ static int llext_manager_link(const char *name, { struct llext **llext = &mctx->llext; struct llext_loader *ldr = &mctx->ebl->loader; + const elf_shdr_t *hdr; int ret; if (*llext && !mctx->mapped) { @@ -267,6 +284,7 @@ static int llext_manager_link(const char *name, .relocate_local = !*llext, .pre_located = true, .section_detached = llext_manager_section_detached, + .keep_section_info = true, }; ret = llext_load(ldr, name, llext, &ldr_parm); @@ -274,49 +292,55 @@ static int llext_manager_link(const char *name, return ret; } - mctx->segment[LIB_MANAGER_TEXT].addr = ldr->sects[LLEXT_MEM_TEXT].sh_addr; - mctx->segment[LIB_MANAGER_TEXT].size = ldr->sects[LLEXT_MEM_TEXT].sh_size; + /* All code sections */ + llext_get_region_info(ldr, *llext, LLEXT_MEM_TEXT, &hdr, NULL, NULL); + mctx->segment[LIB_MANAGER_TEXT].addr = hdr->sh_addr; + mctx->segment[LIB_MANAGER_TEXT].size = hdr->sh_size; tr_dbg(&lib_manager_tr, ".text: start: %#lx size %#x", mctx->segment[LIB_MANAGER_TEXT].addr, mctx->segment[LIB_MANAGER_TEXT].size); /* All read-only data sections */ - mctx->segment[LIB_MANAGER_RODATA].addr = - ldr->sects[LLEXT_MEM_RODATA].sh_addr; - mctx->segment[LIB_MANAGER_RODATA].size = ldr->sects[LLEXT_MEM_RODATA].sh_size; + llext_get_region_info(ldr, *llext, LLEXT_MEM_RODATA, &hdr, NULL, NULL); + mctx->segment[LIB_MANAGER_RODATA].addr = hdr->sh_addr; + mctx->segment[LIB_MANAGER_RODATA].size = hdr->sh_size; tr_dbg(&lib_manager_tr, ".rodata: start: %#lx size %#x", mctx->segment[LIB_MANAGER_RODATA].addr, mctx->segment[LIB_MANAGER_RODATA].size); /* All writable data sections */ - mctx->segment[LIB_MANAGER_DATA].addr = - ldr->sects[LLEXT_MEM_DATA].sh_addr; - mctx->segment[LIB_MANAGER_DATA].size = ldr->sects[LLEXT_MEM_DATA].sh_size; + llext_get_region_info(ldr, *llext, LLEXT_MEM_DATA, &hdr, NULL, NULL); + mctx->segment[LIB_MANAGER_DATA].addr = hdr->sh_addr; + mctx->segment[LIB_MANAGER_DATA].size = hdr->sh_size; tr_dbg(&lib_manager_tr, ".data: start: %#lx size %#x", mctx->segment[LIB_MANAGER_DATA].addr, mctx->segment[LIB_MANAGER_DATA].size); - mctx->segment[LIB_MANAGER_BSS].addr = ldr->sects[LLEXT_MEM_BSS].sh_addr; - mctx->segment[LIB_MANAGER_BSS].size = ldr->sects[LLEXT_MEM_BSS].sh_size; + /* Writable uninitialized data section */ + llext_get_region_info(ldr, *llext, LLEXT_MEM_BSS, &hdr, NULL, NULL); + mctx->segment[LIB_MANAGER_BSS].addr = hdr->sh_addr; + mctx->segment[LIB_MANAGER_BSS].size = hdr->sh_size; tr_dbg(&lib_manager_tr, ".bss: start: %#lx size %#x", mctx->segment[LIB_MANAGER_BSS].addr, mctx->segment[LIB_MANAGER_BSS].size); *buildinfo = NULL; - ssize_t binfo_o = llext_find_section(ldr, ".mod_buildinfo"); - - if (binfo_o >= 0) - *buildinfo = llext_peek(ldr, binfo_o); + ret = llext_section_shndx(ldr, *llext, ".mod_buildinfo"); + if (ret >= 0) { + llext_get_section_info(ldr, *llext, ret, &hdr, NULL, NULL); + *buildinfo = llext_peek(ldr, hdr->sh_offset); + } *mod_manifest = NULL; - ssize_t mod_o = llext_find_section(ldr, ".module"); - - if (mod_o >= 0) - *mod_manifest = llext_peek(ldr, mod_o); + ret = llext_section_shndx(ldr, *llext, ".module"); + if (ret >= 0) { + llext_get_section_info(ldr, *llext, ret, &hdr, NULL, NULL); + *mod_manifest = llext_peek(ldr, hdr->sh_offset); + } return *buildinfo && *mod_manifest ? 0 : -EPROTO; } @@ -618,7 +642,14 @@ int llext_manager_free_module(const uint32_t component_id) /* Protected by IPC serialization */ if (mctx->llext->use_count > 1) { - /* llext_unload() will return a positive number */ + /* + * At least 2 users: llext_unload() will never actually free + * the extension but only reduce the refcount and return its + * new value (must be a positive number). + * NOTE: if this is modified to allow extension unload, the + * inspection data in the loader must be freed as well by + * calling the llext_free_inspection_data() function. + */ int ret = llext_unload(&mctx->llext); if (ret <= 0) {