From 24369d2535ed761da7f1265ffa760e7ebadda9be Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 28 May 2024 10:22:19 +0200 Subject: [PATCH 1/6] llext: (cosmetic) group headers Re-group headers more logically in eq_iir.c and mixin_mixout.c. Signed-off-by: Guennadi Liakhovetski --- src/audio/eq_iir/eq_iir.c | 3 +-- src/audio/mixin_mixout/mixin_mixout.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/audio/eq_iir/eq_iir.c b/src/audio/eq_iir/eq_iir.c index f60f38ffd4b5..6c298e4d59ff 100644 --- a/src/audio/eq_iir/eq_iir.c +++ b/src/audio/eq_iir/eq_iir.c @@ -262,9 +262,8 @@ SOF_MODULE_INIT(eq_iir, sys_comp_module_eq_iir_interface_init); /* modular: llext dynamic link */ #include -#include - #include +#include #define UUID_EQIIR 0xE6, 0xC0, 0x50, 0x51, 0xF9, 0x27, 0xC8, 0x4E, \ 0x83, 0x51, 0xC7, 0x05, 0xB6, 0x42, 0xD1, 0x2F diff --git a/src/audio/mixin_mixout/mixin_mixout.c b/src/audio/mixin_mixout/mixin_mixout.c index a7bd1c92fbc2..e89a5a15185b 100644 --- a/src/audio/mixin_mixout/mixin_mixout.c +++ b/src/audio/mixin_mixout/mixin_mixout.c @@ -969,9 +969,8 @@ SOF_MODULE_INIT(mixout, sys_comp_module_mixout_interface_init); /* modular: llext dynamic link */ #include -#include - #include +#include #define UUID_MIXIN 0xB2, 0x6E, 0x65, 0x39, 0x71, 0x3B, 0x49, 0x40, \ 0x8D, 0x3F, 0xF9, 0x2C, 0xD5, 0xC4, 0x3C, 0x09 From b327542f5771b970e6331e59900498c6847bf538 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 28 May 2024 10:58:55 +0200 Subject: [PATCH 2/6] llext: update memory access flags when mapping When dynamically mapping memory, we need to update access flags according to the type of the mapping. Signed-off-by: Guennadi Liakhovetski --- src/library_manager/llext_manager.c | 42 ++++++++++++++++++----------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index 23c35eb3d15d..bb98a041b079 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -51,6 +51,15 @@ extern struct tr_ctx lib_manager_tr; #define PAGE_SZ CONFIG_MM_DRV_PAGE_SIZE +static int llext_manager_update_flags(void __sparse_cache *vma, size_t size, uint32_t flags) +{ + size_t pre_pad_size = (uintptr_t)vma & (PAGE_SZ - 1); + void *aligned_vma = (__sparse_force uint8_t *)vma - pre_pad_size; + + return sys_mm_drv_update_region_flags(aligned_vma, + ALIGN_UP(pre_pad_size + size, PAGE_SZ), flags); +} + static int llext_manager_align_map(void __sparse_cache *vma, size_t size, uint32_t flags) { size_t pre_pad_size = (uintptr_t)vma & (PAGE_SZ - 1); @@ -71,15 +80,28 @@ static int llext_manager_align_unmap(void __sparse_cache *vma, size_t size) static int llext_manager_load_data_from_storage(void __sparse_cache *vma, void *s_addr, size_t size, uint32_t flags) { - int ret = llext_manager_align_map(vma, size, flags); + int ret = llext_manager_align_map(vma, size, SYS_MM_MEM_PERM_RW); if (ret < 0) { tr_err(&lib_manager_tr, "cannot map %u of %p", size, (__sparse_force void *)vma); return ret; } - /* TODO: Change attributes for memory to FLAGS */ - return memcpy_s((__sparse_force void *)vma, size, s_addr, size); + ret = memcpy_s((__sparse_force void *)vma, size, s_addr, size); + if (ret < 0) + return ret; + + /* + * We don't know what flags we're changing to, maybe the buffer will be + * executable or read-only. Need to write back caches now + */ + dcache_writeback_region(vma, size); + + ret = llext_manager_update_flags(vma, size, flags); + if (!ret && (flags & SYS_MM_MEM_PERM_EXEC)) + icache_invalidate_region(vma, size); + + return ret; } static int llext_manager_load_module(uint32_t module_id, const struct sof_man_module *mod) @@ -103,23 +125,11 @@ static int llext_manager_load_module(uint32_t module_id, const struct sof_man_mo if (ret < 0) return ret; - /* .text contains instructions and it also often contains local data */ - dcache_writeback_region(va_base_text, st_text_size); - icache_invalidate_region(va_base_text, st_text_size); - /* Copy RODATA */ ret = llext_manager_load_data_from_storage(va_base_rodata, src_rodata, st_rodata_size, SYS_MM_MEM_PERM_RW); if (ret < 0) - goto e_text; - - /* Some data can be accessed as uncached, in fact that's the default */ - dcache_writeback_region(va_base_rodata, st_rodata_size); - - return 0; - -e_text: - llext_manager_align_unmap(va_base_text, st_text_size); + llext_manager_align_unmap(va_base_text, st_text_size); return ret; } From ce7bf6596df441c11988fcd9aaf227fd8b4f4045 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 28 May 2024 12:43:45 +0200 Subject: [PATCH 3/6] llext: use the first module for ELF information When an LLEXT module contains multiple Module Adapter instances, their manifests are stored in an array in the .module section. Those array entries contain per-instance information like module entry points, names, UUIDs, but ELF information is common for all instances. Store it in the first array entry to avoid confusion. Signed-off-by: Guennadi Liakhovetski --- src/library_manager/llext_manager.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index bb98a041b079..a5871b9c912f 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -251,7 +251,7 @@ uintptr_t llext_manager_allocate_module(struct processing_module *proc, const void *ipc_specific_config) { struct sof_man_fw_desc *desc; - struct sof_man_module *mod, *mod_array; + struct sof_man_module *mod_array; int ret; uint32_t module_id = IPC4_MOD_ID(ipc_config->id); uint32_t entry_index = LIB_MANAGER_GET_MODULE_INDEX(module_id); @@ -270,7 +270,6 @@ uintptr_t llext_manager_allocate_module(struct processing_module *proc, } mod_array = (struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(0)); - mod = mod_array + entry_index; /* LLEXT linking is only needed once for all the modules in the library */ ret = llext_manager_link(desc, mod_array, module_id, &proc->priv, (const void **)&buildinfo, @@ -291,11 +290,11 @@ uintptr_t llext_manager_allocate_module(struct processing_module *proc, ctx->mod_manifest = mod_manifest; /* Map .text and the rest as .data */ - ret = llext_manager_load_module(module_id, mod); + ret = llext_manager_load_module(module_id, mod_array); if (ret < 0) return 0; - ret = llext_manager_allocate_module_bss(module_id, mod); + ret = llext_manager_allocate_module_bss(module_id, mod_array); if (ret < 0) { tr_err(&lib_manager_tr, "llext_manager_allocate_module(): module allocation failed: %d", From 81ee1b61d514b37d45ba1df8e5f50b38e5fc2af7 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 5 Jun 2024 10:53:00 +0200 Subject: [PATCH 4/6] llext: fix error checking llext_find_section() returns a negative error code when it cannot find the requested section, not 0. Fix error checking. Reported-by: Luca Burelli Signed-off-by: Guennadi Liakhovetski --- src/library_manager/llext_manager.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index a5871b9c912f..67150f5561c9 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -235,12 +235,12 @@ static int llext_manager_link(struct sof_man_fw_desc *desc, struct sof_man_modul ssize_t binfo_o = llext_find_section(&ebl.loader, ".mod_buildinfo"); - if (binfo_o) + if (binfo_o >= 0) *buildinfo = llext_peek(&ebl.loader, binfo_o); ssize_t mod_o = llext_find_section(&ebl.loader, ".module"); - if (mod_o) + if (mod_o >= 0) *mod_manifest = llext_peek(&ebl.loader, mod_o); return 0; From 09d33aaf83a9743fd2f59f2149f339d64c9930f1 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 28 May 2024 13:55:11 +0200 Subject: [PATCH 5/6] llext: split read-only and writable data properly We map memory pages for loaded modules dynamically and we're able to set memory flags for write access or for code execution. This commit takes advantage of the recently added section grouping and maps the three module parts separately: executable code, read-only data and writable data, including zero-initialised .bss. This also cleans up references, pointing into module storage in IMR instead of the mapped addresses. Signed-off-by: Guennadi Liakhovetski --- src/include/sof/lib_manager.h | 16 +- src/library_manager/llext_manager.c | 227 +++++++++++++++------------- 2 files changed, 140 insertions(+), 103 deletions(-) diff --git a/src/include/sof/lib_manager.h b/src/include/sof/lib_manager.h index adf482f83f25..4429efdd938b 100644 --- a/src/include/sof/lib_manager.h +++ b/src/include/sof/lib_manager.h @@ -83,10 +83,24 @@ struct ipc_lib_msg { struct sof_man_module_manifest; +enum { + LIB_MANAGER_TEXT, + LIB_MANAGER_DATA, + LIB_MANAGER_RODATA, + LIB_MANAGER_BSS, + LIB_MANAGER_N_SEGMENTS, +}; + +struct lib_manager_segment_desc { + uintptr_t addr; + size_t size; + size_t file_offset; +}; + struct lib_manager_mod_ctx { void *base_addr; const struct sof_man_module_manifest *mod_manifest; - size_t segment_size[3]; + struct lib_manager_segment_desc segment[LIB_MANAGER_N_SEGMENTS]; }; struct ext_library { diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index 67150f5561c9..1d9b434d7d4e 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -108,82 +108,105 @@ static int llext_manager_load_module(uint32_t module_id, const struct sof_man_mo { struct lib_manager_mod_ctx *ctx = lib_manager_get_mod_ctx(module_id); uint8_t *load_base = (uint8_t *)ctx->base_addr; + + /* Executable code (.text) */ void __sparse_cache *va_base_text = (void __sparse_cache *) - mod->segment[SOF_MAN_SEGMENT_TEXT].v_base_addr; - void *src_txt = (void *)(load_base + mod->segment[SOF_MAN_SEGMENT_TEXT].file_offset); - size_t st_text_size = ctx->segment_size[SOF_MAN_SEGMENT_TEXT]; + ctx->segment[LIB_MANAGER_TEXT].addr; + void *src_txt = (void *)(load_base + ctx->segment[LIB_MANAGER_TEXT].file_offset); + size_t text_size = ctx->segment[LIB_MANAGER_TEXT].size; + + /* Read-only data (.rodata and others) */ void __sparse_cache *va_base_rodata = (void __sparse_cache *) - mod->segment[SOF_MAN_SEGMENT_RODATA].v_base_addr; + ctx->segment[LIB_MANAGER_RODATA].addr; void *src_rodata = (void *)(load_base + - mod->segment[SOF_MAN_SEGMENT_RODATA].file_offset); - size_t st_rodata_size = ctx->segment_size[SOF_MAN_SEGMENT_RODATA]; + ctx->segment[LIB_MANAGER_RODATA].file_offset); + size_t rodata_size = ctx->segment[LIB_MANAGER_RODATA].size; + + /* Writable data (.data, .bss and others) */ + void __sparse_cache *va_base_data = (void __sparse_cache *) + ctx->segment[LIB_MANAGER_DATA].addr; + void *src_data = (void *)(load_base + + ctx->segment[LIB_MANAGER_DATA].file_offset); + size_t data_size = ctx->segment[LIB_MANAGER_DATA].size; + + /* .bss, should be within writable data above */ + void __sparse_cache *bss_addr = (void __sparse_cache *) + ctx->segment[LIB_MANAGER_BSS].addr; + size_t bss_size = ctx->segment[LIB_MANAGER_BSS].size; int ret; + /* Check, that .bss is within .data */ + if (bss_size && + ((uintptr_t)bss_addr + bss_size < (uintptr_t)va_base_data || + (uintptr_t)bss_addr >= (uintptr_t)va_base_data + data_size)) { + tr_err(&lib_manager_tr, ".bss %#x @ %p isn't within writable data %#x @ %p!", + bss_size, bss_addr, data_size, (void *)va_base_data); + return -EPROTO; + } + /* Copy Code */ - ret = llext_manager_load_data_from_storage(va_base_text, src_txt, st_text_size, - SYS_MM_MEM_PERM_RW | SYS_MM_MEM_PERM_EXEC); + ret = llext_manager_load_data_from_storage(va_base_text, src_txt, text_size, + SYS_MM_MEM_PERM_EXEC); if (ret < 0) return ret; - /* Copy RODATA */ + /* Copy read-only data */ ret = llext_manager_load_data_from_storage(va_base_rodata, src_rodata, - st_rodata_size, SYS_MM_MEM_PERM_RW); + rodata_size, 0); if (ret < 0) - llext_manager_align_unmap(va_base_text, st_text_size); + goto e_text; - return ret; -} + /* Copy writable data */ + ret = llext_manager_load_data_from_storage(va_base_data, src_data, + data_size, SYS_MM_MEM_PERM_RW); + if (ret < 0) + goto e_rodata; -static int llext_manager_unload_module(uint32_t module_id, const struct sof_man_module *mod) -{ - struct lib_manager_mod_ctx *ctx = lib_manager_get_mod_ctx(module_id); - void __sparse_cache *va_base_text = (void __sparse_cache *) - mod->segment[SOF_MAN_SEGMENT_TEXT].v_base_addr; - size_t st_text_size = ctx->segment_size[SOF_MAN_SEGMENT_TEXT]; - void __sparse_cache *va_base_rodata = (void __sparse_cache *) - mod->segment[SOF_MAN_SEGMENT_RODATA].v_base_addr; - size_t st_rodata_size = ctx->segment_size[SOF_MAN_SEGMENT_RODATA]; - int ret; + memset((__sparse_force void *)ctx->segment[LIB_MANAGER_BSS].addr, 0, + ctx->segment[LIB_MANAGER_BSS].size); - ret = llext_manager_align_unmap(va_base_text, st_text_size); - if (ret < 0) - return ret; + return 0; - return llext_manager_align_unmap(va_base_rodata, st_rodata_size); -} +e_rodata: + llext_manager_align_unmap(va_base_rodata, rodata_size); +e_text: + llext_manager_align_unmap(va_base_text, text_size); -static void __sparse_cache *llext_manager_get_bss_address(uint32_t module_id, - const struct sof_man_module *mod) -{ - return (void __sparse_cache *)mod->segment[SOF_MAN_SEGMENT_BSS].v_base_addr; + return ret; } -static int llext_manager_allocate_module_bss(uint32_t module_id, - const struct sof_man_module *mod) +static int llext_manager_unload_module(uint32_t module_id, const struct sof_man_module *mod) { - /* FIXME: just map .bss together with .data and simply memset(.bss, 0) */ struct lib_manager_mod_ctx *ctx = lib_manager_get_mod_ctx(module_id); - size_t bss_size = ctx->segment_size[SOF_MAN_SEGMENT_BSS]; - void __sparse_cache *va_base = llext_manager_get_bss_address(module_id, mod); + /* Executable code (.text) */ + void __sparse_cache *va_base_text = (void __sparse_cache *) + ctx->segment[LIB_MANAGER_TEXT].addr; + size_t text_size = ctx->segment[LIB_MANAGER_TEXT].size; - /* Map bss memory and clear it. */ - if (llext_manager_align_map(va_base, bss_size, SYS_MM_MEM_PERM_RW) < 0) - return -ENOMEM; + /* Read-only data (.rodata, etc.) */ + void __sparse_cache *va_base_rodata = (void __sparse_cache *) + ctx->segment[LIB_MANAGER_RODATA].addr; + size_t rodata_size = ctx->segment[LIB_MANAGER_RODATA].size; - memset((__sparse_force void *)va_base, 0, bss_size); + /* Writable data (.data, .bss, etc.) */ + void __sparse_cache *va_base_data = (void __sparse_cache *) + ctx->segment[LIB_MANAGER_DATA].addr; + size_t data_size = ctx->segment[LIB_MANAGER_DATA].size; + int err = 0, ret; - return 0; -} + ret = llext_manager_align_unmap(va_base_text, text_size); + if (ret < 0) + err = ret; -static int llext_manager_free_module_bss(uint32_t module_id, - const struct sof_man_module *mod) -{ - struct lib_manager_mod_ctx *ctx = lib_manager_get_mod_ctx(module_id); - size_t bss_size = ctx->segment_size[SOF_MAN_SEGMENT_BSS]; - void __sparse_cache *va_base = llext_manager_get_bss_address(module_id, mod); + ret = llext_manager_align_unmap(va_base_data, data_size); + if (ret < 0 && !err) + err = ret; + + ret = llext_manager_align_unmap(va_base_rodata, rodata_size); + if (ret < 0 && !err) + err = ret; - /* Unmap bss memory. */ - return llext_manager_align_unmap(va_base, bss_size); + return err; } static int llext_manager_link(struct sof_man_fw_desc *desc, struct sof_man_module *mod, @@ -191,47 +214,59 @@ static int llext_manager_link(struct sof_man_fw_desc *desc, struct sof_man_modul const struct sof_man_module_manifest **mod_manifest) { size_t mod_size = desc->header.preload_page_count * PAGE_SZ - FILE_TEXT_OFFSET_V1_8; - struct llext_buf_loader ebl = LLEXT_BUF_LOADER((uint8_t *)desc - - SOF_MAN_ELF_TEXT_OFFSET + FILE_TEXT_OFFSET_V1_8, - mod_size); + uintptr_t imr_base = (uintptr_t)desc - SOF_MAN_ELF_TEXT_OFFSET; + struct llext_buf_loader ebl = LLEXT_BUF_LOADER((uint8_t *)imr_base + FILE_TEXT_OFFSET_V1_8, + mod_size); struct lib_manager_mod_ctx *ctx = lib_manager_get_mod_ctx(module_id); /* Identify if this is the first time loading this module */ - struct llext_load_param ldr_parm = {!ctx->segment_size[SOF_MAN_SEGMENT_TEXT]}; + struct llext_load_param ldr_parm = { + .relocate_local = !ctx->segment[LIB_MANAGER_TEXT].size, + .pre_located = true, + }; int ret = llext_load(&ebl.loader, mod->name, &md->llext, &ldr_parm); if (ret) return ret; - mod->segment[SOF_MAN_SEGMENT_TEXT].v_base_addr = ebl.loader.sects[LLEXT_MEM_TEXT].sh_addr; - mod->segment[SOF_MAN_SEGMENT_TEXT].file_offset = - (uintptr_t)md->llext->mem[LLEXT_MEM_TEXT] - - (uintptr_t)desc + SOF_MAN_ELF_TEXT_OFFSET; - ctx->segment_size[SOF_MAN_SEGMENT_TEXT] = ebl.loader.sects[LLEXT_MEM_TEXT].sh_size; + ctx->segment[LIB_MANAGER_TEXT].addr = ebl.loader.sects[LLEXT_MEM_TEXT].sh_addr; + ctx->segment[LIB_MANAGER_TEXT].file_offset = + (uintptr_t)md->llext->mem[LLEXT_MEM_TEXT] - imr_base; + ctx->segment[LIB_MANAGER_TEXT].size = ebl.loader.sects[LLEXT_MEM_TEXT].sh_size; - tr_dbg(&lib_manager_tr, ".text: start: %#x size %#x offset %#x", - mod->segment[SOF_MAN_SEGMENT_TEXT].v_base_addr, - ctx->segment_size[SOF_MAN_SEGMENT_TEXT], - mod->segment[SOF_MAN_SEGMENT_TEXT].file_offset); + tr_dbg(&lib_manager_tr, ".text: start: %#lx size %#x offset %#x", + ctx->segment[LIB_MANAGER_TEXT].addr, + ctx->segment[LIB_MANAGER_TEXT].size, + ctx->segment[LIB_MANAGER_TEXT].file_offset); /* This contains all other sections, except .text, it might contain .bss too */ - mod->segment[SOF_MAN_SEGMENT_RODATA].v_base_addr = + ctx->segment[LIB_MANAGER_RODATA].addr = ebl.loader.sects[LLEXT_MEM_RODATA].sh_addr; - mod->segment[SOF_MAN_SEGMENT_RODATA].file_offset = - (uintptr_t)md->llext->mem[LLEXT_MEM_RODATA] - - (uintptr_t)desc + SOF_MAN_ELF_TEXT_OFFSET; - ctx->segment_size[SOF_MAN_SEGMENT_RODATA] = ebl.loader.prog_data_size; + ctx->segment[LIB_MANAGER_RODATA].file_offset = + (uintptr_t)md->llext->mem[LLEXT_MEM_RODATA] - imr_base; + ctx->segment[LIB_MANAGER_RODATA].size = ebl.loader.sects[LLEXT_MEM_RODATA].sh_size; + + tr_dbg(&lib_manager_tr, ".rodata: start: %#lx size %#x offset %#x", + ctx->segment[LIB_MANAGER_RODATA].addr, + ctx->segment[LIB_MANAGER_RODATA].size, + ctx->segment[LIB_MANAGER_RODATA].file_offset); - tr_dbg(&lib_manager_tr, ".data: start: %#x size %#x offset %#x", - mod->segment[SOF_MAN_SEGMENT_RODATA].v_base_addr, - ctx->segment_size[SOF_MAN_SEGMENT_RODATA], - mod->segment[SOF_MAN_SEGMENT_RODATA].file_offset); + ctx->segment[LIB_MANAGER_DATA].addr = + ebl.loader.sects[LLEXT_MEM_DATA].sh_addr; + ctx->segment[LIB_MANAGER_DATA].file_offset = + (uintptr_t)md->llext->mem[LLEXT_MEM_DATA] - imr_base; + ctx->segment[LIB_MANAGER_DATA].size = ebl.loader.sects[LLEXT_MEM_DATA].sh_size; - mod->segment[SOF_MAN_SEGMENT_BSS].v_base_addr = ebl.loader.sects[LLEXT_MEM_BSS].sh_addr; - ctx->segment_size[SOF_MAN_SEGMENT_BSS] = ebl.loader.sects[LLEXT_MEM_BSS].sh_size; + tr_dbg(&lib_manager_tr, ".data: start: %#lx size %#x offset %#x", + ctx->segment[LIB_MANAGER_DATA].addr, + ctx->segment[LIB_MANAGER_DATA].size, + ctx->segment[LIB_MANAGER_DATA].file_offset); - tr_dbg(&lib_manager_tr, ".bss: start: %#x size %#x", - mod->segment[SOF_MAN_SEGMENT_BSS].v_base_addr, - ctx->segment_size[SOF_MAN_SEGMENT_BSS]); + ctx->segment[LIB_MANAGER_BSS].addr = ebl.loader.sects[LLEXT_MEM_BSS].sh_addr; + ctx->segment[LIB_MANAGER_BSS].size = ebl.loader.sects[LLEXT_MEM_BSS].sh_size; + + tr_dbg(&lib_manager_tr, ".bss: start: %#lx size %#x", + ctx->segment[LIB_MANAGER_BSS].addr, + ctx->segment[LIB_MANAGER_BSS].size); ssize_t binfo_o = llext_find_section(&ebl.loader, ".mod_buildinfo"); @@ -243,7 +278,7 @@ static int llext_manager_link(struct sof_man_fw_desc *desc, struct sof_man_modul if (mod_o >= 0) *mod_manifest = llext_peek(&ebl.loader, mod_o); - return 0; + return binfo_o >= 0 && mod_o >= 0 ? 0 : -EPROTO; } uintptr_t llext_manager_allocate_module(struct processing_module *proc, @@ -286,21 +321,20 @@ uintptr_t llext_manager_allocate_module(struct processing_module *proc, return -ENOEXEC; } - /* ctx->mod_manifest points to the array of module manifests */ - ctx->mod_manifest = mod_manifest; - - /* Map .text and the rest as .data */ + /* Map executable code and data */ ret = llext_manager_load_module(module_id, mod_array); if (ret < 0) return 0; - ret = llext_manager_allocate_module_bss(module_id, mod_array); - if (ret < 0) { - tr_err(&lib_manager_tr, - "llext_manager_allocate_module(): module allocation failed: %d", - ret); - return 0; - } + /* Manifest is in read-only data */ + uintptr_t imr_rodata = (uintptr_t)ctx->base_addr + + ctx->segment[LIB_MANAGER_RODATA].file_offset; + uintptr_t va_rodata_base = ctx->segment[LIB_MANAGER_RODATA].addr; + size_t offset = (uintptr_t)mod_manifest - imr_rodata; + + /* ctx->mod_manifest points to an array of module manifests */ + ctx->mod_manifest = (const struct sof_man_module_manifest *)(va_rodata_base + + offset); } return ctx->mod_manifest[entry_index].module.entry_point; @@ -312,21 +346,10 @@ int llext_manager_free_module(const uint32_t component_id) const uint32_t module_id = IPC4_MOD_ID(component_id); const unsigned int base_module_id = LIB_MANAGER_GET_LIB_ID(module_id) << LIB_MANAGER_LIB_ID_SHIFT; - int ret; tr_dbg(&lib_manager_tr, "llext_manager_free_module(): mod_id: %#x", component_id); mod = lib_manager_get_module_manifest(base_module_id); - ret = llext_manager_unload_module(base_module_id, mod); - if (ret < 0) - return ret; - - ret = llext_manager_free_module_bss(base_module_id, mod); - if (ret < 0) { - tr_err(&lib_manager_tr, - "llext_manager_free_module(): free module bss failed: %d", ret); - return ret; - } - return 0; + return llext_manager_unload_module(base_module_id, mod); } From eaf4a13343a3dec23c3a0b43d7b960f0209602cb Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Mon, 3 Jun 2024 17:05:44 +0200 Subject: [PATCH 6/6] llext: add support for .bss directly adjacent to .data Zephyr places .bss into a separate section element, still if it's immediately adjacent to writable data we can merge and allocate them together. Signed-off-by: Guennadi Liakhovetski --- src/library_manager/llext_manager.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index 1d9b434d7d4e..e4341678ba91 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -137,11 +137,22 @@ static int llext_manager_load_module(uint32_t module_id, const struct sof_man_mo /* Check, that .bss is within .data */ if (bss_size && - ((uintptr_t)bss_addr + bss_size < (uintptr_t)va_base_data || + ((uintptr_t)bss_addr + bss_size <= (uintptr_t)va_base_data || (uintptr_t)bss_addr >= (uintptr_t)va_base_data + data_size)) { - tr_err(&lib_manager_tr, ".bss %#x @ %p isn't within writable data %#x @ %p!", - bss_size, bss_addr, data_size, (void *)va_base_data); - return -EPROTO; + if ((uintptr_t)bss_addr + bss_size == (uintptr_t)va_base_data && + !((uintptr_t)bss_addr & (PAGE_SZ - 1))) { + /* .bss directly in front of writable data and properly aligned, prepend */ + va_base_data = bss_addr; + data_size += bss_size; + } else if ((uintptr_t)bss_addr == (uintptr_t)va_base_data + data_size) { + /* .bss directly behind writable data, append */ + data_size += bss_size; + } else { + tr_err(&lib_manager_tr, ".bss %#x @%p isn't within writable data %#x @%p!", + bss_size, (__sparse_force void *)bss_addr, + data_size, (__sparse_force void *)va_base_data); + return -EPROTO; + } } /* Copy Code */ @@ -162,8 +173,7 @@ static int llext_manager_load_module(uint32_t module_id, const struct sof_man_mo if (ret < 0) goto e_rodata; - memset((__sparse_force void *)ctx->segment[LIB_MANAGER_BSS].addr, 0, - ctx->segment[LIB_MANAGER_BSS].size); + memset((__sparse_force void *)bss_addr, 0, bss_size); return 0;