From 20f2be40111c8a01c84f258fe14d11346315aabc Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Mon, 25 Nov 2024 15:32:33 +0200 Subject: [PATCH 1/7] llext: simplify offset calculation when copying sections Use section offset from the section header in llext_manager_load_data_from_storage() to simplify calculations and eliminate a function parameter. Signed-off-by: Guennadi Liakhovetski --- src/library_manager/llext_manager.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index a3bc9b09b2a9..b9739f868d59 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -80,10 +80,8 @@ static int llext_manager_align_unmap(void __sparse_cache *vma, size_t size) static int llext_manager_load_data_from_storage(const struct llext *ext, void __sparse_cache *vma, const uint8_t *load_base, - size_t region_offset, size_t size, uint32_t flags) { - const uint8_t *s_addr = load_base + region_offset; unsigned int i; int ret = llext_manager_align_map(vma, size, SYS_MM_MEM_PERM_RW); const elf_shdr_t *shdr; @@ -93,17 +91,22 @@ static int llext_manager_load_data_from_storage(const struct llext *ext, return ret; } + size_t init_offset = 0; + /* 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++) { if ((uintptr_t)shdr->sh_addr < (uintptr_t)vma || (uintptr_t)shdr->sh_addr >= (uintptr_t)vma + size) continue; - size_t offset = shdr->sh_offset + FILE_TEXT_OFFSET_V1_8 - region_offset; + if (!init_offset) + init_offset = shdr->sh_offset; /* found a section within the region */ + size_t offset = shdr->sh_offset - init_offset; + ret = memcpy_s((__sparse_force void *)shdr->sh_addr, size - offset, - s_addr + offset, shdr->sh_size); + load_base + offset, shdr->sh_size); if (ret < 0) return ret; } @@ -125,7 +128,6 @@ static int llext_manager_load_module(const struct llext *ext, const struct llext uint32_t module_id, const struct sof_man_module *mod) { struct lib_manager_mod_ctx *ctx = lib_manager_get_mod_ctx(module_id); - const uint8_t *load_base = (const uint8_t *)ctx->base_addr; /* Executable code (.text) */ void __sparse_cache *va_base_text = (void __sparse_cache *) @@ -170,22 +172,19 @@ static int llext_manager_load_module(const struct llext *ext, const struct llext } /* Copy Code */ - ret = llext_manager_load_data_from_storage(ext, va_base_text, load_base, - ctx->segment[LIB_MANAGER_TEXT].file_offset, + ret = llext_manager_load_data_from_storage(ext, va_base_text, ext->mem[LLEXT_MEM_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, load_base, - ctx->segment[LIB_MANAGER_RODATA].file_offset, + ret = llext_manager_load_data_from_storage(ext, va_base_rodata, ext->mem[LLEXT_MEM_RODATA], rodata_size, 0); if (ret < 0) goto e_text; /* Copy writable data */ - ret = llext_manager_load_data_from_storage(ext, va_base_data, load_base, - ctx->segment[LIB_MANAGER_DATA].file_offset, + ret = llext_manager_load_data_from_storage(ext, va_base_data, ext->mem[LLEXT_MEM_DATA], data_size, SYS_MM_MEM_PERM_RW); if (ret < 0) goto e_rodata; From 5cbea2b8b9695b04c10c3d99b90d5ae9a1111e58 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Mon, 25 Nov 2024 15:44:46 +0200 Subject: [PATCH 2/7] llext: remove an unused field file_offset in struct lib_manager_segment_desc is unused, remove it. Signed-off-by: Guennadi Liakhovetski --- src/include/sof/lib_manager.h | 1 - src/library_manager/llext_manager.c | 21 ++++++--------------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/src/include/sof/lib_manager.h b/src/include/sof/lib_manager.h index 4429efdd938b..592207a8c650 100644 --- a/src/include/sof/lib_manager.h +++ b/src/include/sof/lib_manager.h @@ -94,7 +94,6 @@ enum { struct lib_manager_segment_desc { uintptr_t addr; size_t size; - size_t file_offset; }; struct lib_manager_mod_ctx { diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index b9739f868d59..93269df47fd0 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -258,38 +258,29 @@ static int llext_manager_link(struct llext_buf_loader *ebl, uintptr_t dram_base, return ret; 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] - dram_base; ctx->segment[LIB_MANAGER_TEXT].size = ebl->loader.sects[LLEXT_MEM_TEXT].sh_size; - tr_dbg(&lib_manager_tr, ".text: start: %#lx size %#x offset %#x", + tr_dbg(&lib_manager_tr, ".text: start: %#lx size %#x", ctx->segment[LIB_MANAGER_TEXT].addr, - ctx->segment[LIB_MANAGER_TEXT].size, - ctx->segment[LIB_MANAGER_TEXT].file_offset); + ctx->segment[LIB_MANAGER_TEXT].size); /* All read-only data sections */ ctx->segment[LIB_MANAGER_RODATA].addr = ebl->loader.sects[LLEXT_MEM_RODATA].sh_addr; - ctx->segment[LIB_MANAGER_RODATA].file_offset = - (uintptr_t)md->llext->mem[LLEXT_MEM_RODATA] - dram_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", + tr_dbg(&lib_manager_tr, ".rodata: start: %#lx size %#x", ctx->segment[LIB_MANAGER_RODATA].addr, - ctx->segment[LIB_MANAGER_RODATA].size, - ctx->segment[LIB_MANAGER_RODATA].file_offset); + ctx->segment[LIB_MANAGER_RODATA].size); /* All writable data sections */ 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] - dram_base; ctx->segment[LIB_MANAGER_DATA].size = ebl->loader.sects[LLEXT_MEM_DATA].sh_size; - tr_dbg(&lib_manager_tr, ".data: start: %#lx size %#x offset %#x", + tr_dbg(&lib_manager_tr, ".data: start: %#lx size %#x", ctx->segment[LIB_MANAGER_DATA].addr, - ctx->segment[LIB_MANAGER_DATA].size, - ctx->segment[LIB_MANAGER_DATA].file_offset); + ctx->segment[LIB_MANAGER_DATA].size); 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; From 1caf36ce3b4b8bd1838aa989f013be361774f0e1 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Mon, 25 Nov 2024 15:52:29 +0200 Subject: [PATCH 3/7] llext: remove LIB_MANAGER_BSS We don't need to store .bss address and size until the module is freed since .bss is allocated together with .data and we only need its size when initialising it, not when freeing module memory. Remove it to save memory. Signed-off-by: Guennadi Liakhovetski --- src/include/sof/lib_manager.h | 1 - src/library_manager/llext_manager.c | 11 ++--------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/include/sof/lib_manager.h b/src/include/sof/lib_manager.h index 592207a8c650..eb7bc21856d2 100644 --- a/src/include/sof/lib_manager.h +++ b/src/include/sof/lib_manager.h @@ -87,7 +87,6 @@ enum { LIB_MANAGER_TEXT, LIB_MANAGER_DATA, LIB_MANAGER_RODATA, - LIB_MANAGER_BSS, LIB_MANAGER_N_SEGMENTS, }; diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index 93269df47fd0..387545fa11ae 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -146,8 +146,8 @@ static int llext_manager_load_module(const struct llext *ext, const struct llext /* .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; + ebl->loader.sects[LLEXT_MEM_BSS].sh_addr; + size_t bss_size = ebl->loader.sects[LLEXT_MEM_BSS].sh_size; int ret; /* Check, that .bss is within .data */ @@ -282,13 +282,6 @@ static int llext_manager_link(struct llext_buf_loader *ebl, uintptr_t dram_base, ctx->segment[LIB_MANAGER_DATA].addr, ctx->segment[LIB_MANAGER_DATA].size); - 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"); if (binfo_o >= 0) From 3065164048e82dd4ec824e10f30561b15cc8f42a Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Mon, 25 Nov 2024 17:42:19 +0200 Subject: [PATCH 4/7] rimage: (cosmetic) remove redundant NULL-checks free(NULL) is allowed, remove redundant checks. Signed-off-by: Guennadi Liakhovetski --- tools/rimage/src/adsp_config.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/tools/rimage/src/adsp_config.c b/tools/rimage/src/adsp_config.c index b38fdc2544e9..ed5162bae4d9 100644 --- a/tools/rimage/src/adsp_config.c +++ b/tools/rimage/src/adsp_config.c @@ -2359,23 +2359,12 @@ void adsp_free(struct adsp *adsp) if (!adsp) return; - if (adsp->man_v1_5) - free(adsp->man_v1_5); - - if (adsp->man_v1_5_sue) - free(adsp->man_v1_5_sue); - - if (adsp->man_v1_8) - free(adsp->man_v1_8); - - if (adsp->man_v2_5) - free(adsp->man_v2_5); - - if (adsp->modules) - free(adsp->modules); - - if (adsp->name) - free((char *)adsp->name); + free(adsp->man_v1_5); + free(adsp->man_v1_5_sue); + free(adsp->man_v1_8); + free(adsp->man_v2_5); + free(adsp->modules); + free((void *)adsp->name); free(adsp); } From b7674d22ed2edc350905db193ff0dcaba2b1788a Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 26 Nov 2024 13:25:13 +0200 Subject: [PATCH 5/7] llext: (cosmetic) simplify 3 functions Remove unused arguments from llext_manager_load_module(), llext_manager_unload_module() and llext_manager_link(). Signed-off-by: Guennadi Liakhovetski --- src/library_manager/llext_manager.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index 387545fa11ae..d97aa3dad298 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -125,7 +125,7 @@ static int llext_manager_load_data_from_storage(const struct llext *ext, } static int llext_manager_load_module(const struct llext *ext, const struct llext_buf_loader *ebl, - uint32_t module_id, const struct sof_man_module *mod) + uint32_t module_id) { struct lib_manager_mod_ctx *ctx = lib_manager_get_mod_ctx(module_id); @@ -201,7 +201,7 @@ static int llext_manager_load_module(const struct llext *ext, const struct llext return ret; } -static int llext_manager_unload_module(uint32_t module_id, const struct sof_man_module *mod) +static int llext_manager_unload_module(uint32_t module_id) { struct lib_manager_mod_ctx *ctx = lib_manager_get_mod_ctx(module_id); /* Executable code (.text) */ @@ -240,7 +240,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, uintptr_t dram_base, +static int llext_manager_link(struct llext_buf_loader *ebl, const char *name, uint32_t module_id, struct module_data *md, const void **buildinfo, const struct sof_man_module_manifest **mod_manifest) @@ -325,7 +325,7 @@ uintptr_t llext_manager_allocate_module(struct processing_module *proc, mod_array = (struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(0)); /* LLEXT linking is only needed once for all the modules in the library */ - ret = llext_manager_link(&ebl, dram_base, mod_array[0].name, module_id, md, + ret = llext_manager_link(&ebl, mod_array[0].name, module_id, md, (const void **)&buildinfo, &mod_manifest); if (ret < 0) { tr_err(&lib_manager_tr, "linking failed: %d", ret); @@ -342,7 +342,7 @@ uintptr_t llext_manager_allocate_module(struct processing_module *proc, } /* Map executable code and data */ - ret = llext_manager_load_module(md->llext, &ebl, module_id, mod_array); + ret = llext_manager_load_module(md->llext, &ebl, module_id); if (ret < 0) return 0; @@ -355,16 +355,13 @@ uintptr_t llext_manager_allocate_module(struct processing_module *proc, int llext_manager_free_module(const uint32_t component_id) { - const struct sof_man_module *mod; 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; tr_dbg(&lib_manager_tr, "llext_manager_free_module(): mod_id: %#x", component_id); - mod = lib_manager_get_module_manifest(base_module_id); - - return llext_manager_unload_module(base_module_id, mod); + return llext_manager_unload_module(base_module_id); } bool comp_is_llext(struct comp_dev *comp) From bade3d9e2aa1f9a6231e17e4d7fe7ec36c448618 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 26 Nov 2024 15:59:22 +0200 Subject: [PATCH 6/7] llext: (cosmetic) remove function names from logging Zephyr logging prints function names, no need to include them explicitly. Signed-off-by: Guennadi Liakhovetski --- src/library_manager/llext_manager.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index d97aa3dad298..77930e2829c4 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -313,12 +313,10 @@ uintptr_t llext_manager_allocate_module(struct processing_module *proc, mod_size); struct module_data *md = &proc->priv; - tr_dbg(&lib_manager_tr, "llext_manager_allocate_module(): mod_id: %#x", - ipc_config->id); + tr_dbg(&lib_manager_tr, "mod_id: %#x", ipc_config->id); if (!ctx || !desc) { - tr_err(&lib_manager_tr, - "llext_manager_allocate_module(): failed to get module descriptor"); + tr_err(&lib_manager_tr, "failed to get module descriptor"); return 0; } @@ -336,8 +334,7 @@ uintptr_t llext_manager_allocate_module(struct processing_module *proc, /* First instance: check that the module is native */ if (buildinfo->format != SOF_MODULE_API_BUILD_INFO_FORMAT || buildinfo->api_version_number.full != SOF_MODULE_API_CURRENT_VERSION) { - tr_err(&lib_manager_tr, - "llext_manager_allocate_module(): Unsupported module API version"); + tr_err(&lib_manager_tr, "Unsupported module API version"); return -ENOEXEC; } @@ -359,7 +356,7 @@ int llext_manager_free_module(const uint32_t component_id) const unsigned int base_module_id = LIB_MANAGER_GET_LIB_ID(module_id) << LIB_MANAGER_LIB_ID_SHIFT; - tr_dbg(&lib_manager_tr, "llext_manager_free_module(): mod_id: %#x", component_id); + tr_dbg(&lib_manager_tr, "mod_id: %#x", component_id); return llext_manager_unload_module(base_module_id); } From 12153e7918f8c7d303eb8f0ec1a5674a66077919 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 26 Nov 2024 17:55:17 +0200 Subject: [PATCH 7/7] llext: fix an error code llext_manager_allocate_module() should return 0 when failing, not an error code. Signed-off-by: Guennadi Liakhovetski --- src/library_manager/llext_manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index 77930e2829c4..1644fb36a369 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -335,7 +335,7 @@ uintptr_t llext_manager_allocate_module(struct processing_module *proc, if (buildinfo->format != SOF_MODULE_API_BUILD_INFO_FORMAT || buildinfo->api_version_number.full != SOF_MODULE_API_CURRENT_VERSION) { tr_err(&lib_manager_tr, "Unsupported module API version"); - return -ENOEXEC; + return 0; } /* Map executable code and data */