From 6f280ddc330d0ed3808a871d4e5ea16e6d2b94bc Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Thu, 13 Feb 2025 16:16:42 +0100 Subject: [PATCH 1/6] lib_manager: Simplyfy module instance allocate functions Extend the lib_manager_get_instance_bss_address function to also return the size of the bss section. This simplify the lib_manager_allocate_module_instance and lib_manager_free_module_instance functions. Signed-off-by: Adrian Warecki --- src/library_manager/lib_manager.c | 58 ++++++++++++++----------------- 1 file changed, 26 insertions(+), 32 deletions(-) diff --git a/src/library_manager/lib_manager.c b/src/library_manager/lib_manager.c index c5fd1afda6fc..b7ecf8078c84 100644 --- a/src/library_manager/lib_manager.c +++ b/src/library_manager/lib_manager.c @@ -280,31 +280,27 @@ static int lib_manager_unload_libcode_modules(const uint32_t module_id) } #endif /* CONFIG_LIBCODE_MODULE_SUPPORT */ -static void __sparse_cache *lib_manager_get_instance_bss_address(uint32_t module_id, - uint32_t instance_id, - const struct sof_man_module *mod) +static void lib_manager_get_instance_bss_address(uint32_t instance_id, + const struct sof_man_module *mod, + void __sparse_cache **va_addr, size_t *size) { - uint32_t instance_bss_size = - mod->segment[SOF_MAN_SEGMENT_BSS].flags.r.length / mod->instance_max_count; - uint32_t inst_offset = instance_bss_size * PAGE_SZ * instance_id; - void __sparse_cache *va_base = - (void __sparse_cache *)(mod->segment[SOF_MAN_SEGMENT_BSS].v_base_addr + - inst_offset); - - tr_dbg(&lib_manager_tr, "instance_bss_size: %#x, pointer: %p", - instance_bss_size, (__sparse_force void *)va_base); - - return va_base; + *size = mod->segment[SOF_MAN_SEGMENT_BSS].flags.r.length / mod->instance_max_count * + PAGE_SZ; + size_t inst_offset = *size * instance_id; + *va_addr = (void __sparse_cache *)(mod->segment[SOF_MAN_SEGMENT_BSS].v_base_addr + + inst_offset); + + tr_dbg(&lib_manager_tr, "instance_bss_size: %#zx, pointer: %p", *size, + (__sparse_force void *)*va_addr); } -static int lib_manager_allocate_module_instance(uint32_t module_id, uint32_t instance_id, - uint32_t is_pages, const struct sof_man_module *mod) +static int lib_manager_allocate_module_instance(uint32_t instance_id, uint32_t is_pages, + const struct sof_man_module *mod) { - uint32_t bss_size = - (mod->segment[SOF_MAN_SEGMENT_BSS].flags.r.length / mod->instance_max_count) - * PAGE_SZ; - void __sparse_cache *va_base = lib_manager_get_instance_bss_address(module_id, - instance_id, mod); + size_t bss_size; + void __sparse_cache *va_base; + + lib_manager_get_instance_bss_address(instance_id, mod, &va_base, &bss_size); if ((is_pages * PAGE_SZ) > bss_size) { tr_err(&lib_manager_tr, "invalid is_pages: %u, required: %u", @@ -324,14 +320,12 @@ static int lib_manager_allocate_module_instance(uint32_t module_id, uint32_t ins return 0; } -static int lib_manager_free_module_instance(uint32_t module_id, uint32_t instance_id, - const struct sof_man_module *mod) +static int lib_manager_free_module_instance(uint32_t instance_id, const struct sof_man_module *mod) { - uint32_t bss_size = - (mod->segment[SOF_MAN_SEGMENT_BSS].flags.r.length / mod->instance_max_count) - * PAGE_SZ; - void __sparse_cache *va_base = lib_manager_get_instance_bss_address(module_id, - instance_id, mod); + size_t bss_size; + void __sparse_cache *va_base; + + lib_manager_get_instance_bss_address(instance_id, mod, &va_base, &bss_size); /* * Unmap bss memory. */ @@ -343,8 +337,8 @@ uintptr_t lib_manager_allocate_module(const struct comp_ipc_config *ipc_config, { const struct sof_man_module *mod; const struct ipc4_base_module_cfg *base_cfg = ipc_specific_config; + const uint32_t module_id = IPC4_MOD_ID(ipc_config->id); int ret; - uint32_t module_id = IPC4_MOD_ID(ipc_config->id); tr_dbg(&lib_manager_tr, "mod_id: %#x", ipc_config->id); @@ -367,8 +361,8 @@ uintptr_t lib_manager_allocate_module(const struct comp_ipc_config *ipc_config, goto err; #endif /* CONFIG_LIBCODE_MODULE_SUPPORT */ - ret = lib_manager_allocate_module_instance(module_id, IPC4_INST_ID(ipc_config->id), - base_cfg->is_pages, mod); + ret = lib_manager_allocate_module_instance(IPC4_INST_ID(ipc_config->id), base_cfg->is_pages, + mod); if (ret < 0) { tr_err(&lib_manager_tr, "module allocation failed: %d", ret); #ifdef CONFIG_LIBCODE_MODULE_SUPPORT @@ -406,7 +400,7 @@ int lib_manager_free_module(const uint32_t component_id) return ret; #endif /* CONFIG_LIBCODE_MODULE_SUPPORT */ - ret = lib_manager_free_module_instance(module_id, IPC4_INST_ID(component_id), mod); + ret = lib_manager_free_module_instance(IPC4_INST_ID(component_id), mod); if (ret < 0) { tr_err(&lib_manager_tr, "free module instance failed: %d", ret); return ret; From 0b9209bcac7aff49a59bb241a5c037b837516b88 Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Thu, 13 Feb 2025 16:21:19 +0100 Subject: [PATCH 2/6] lib_manager: Remove unnecessary ops.prepare assign Remove redundant value assignment to ops.prepare in the lib_manager_register_module function. This value is already set by the lib_manager_prepare_module_adapter function. Signed-off-by: Adrian Warecki --- src/library_manager/lib_manager.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/library_manager/lib_manager.c b/src/library_manager/lib_manager.c index b7ecf8078c84..8ae46f21d282 100644 --- a/src/library_manager/lib_manager.c +++ b/src/library_manager/lib_manager.c @@ -649,7 +649,6 @@ int lib_manager_register_module(const uint32_t component_id) build_info->api_version_number.full == IADK_MODULE_API_CURRENT_VERSION) { /* Use module_adapter functions */ drv->ops.create = module_adapter_new; - drv->ops.prepare = module_adapter_prepare; } else { /* Check if module is NOT native */ if (build_info->format != SOF_MODULE_API_BUILD_INFO_FORMAT || From 8ae7b5852786e04ce284ff08eb933139da136a30 Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Thu, 13 Feb 2025 16:22:19 +0100 Subject: [PATCH 3/6] lib_manager: Fix memory leak in lib_manager_register_module Free allocated driver structure when an unsupported loadable module api version is detected. Signed-off-by: Adrian Warecki --- src/library_manager/lib_manager.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/library_manager/lib_manager.c b/src/library_manager/lib_manager.c index 8ae46f21d282..0411f6721f2a 100644 --- a/src/library_manager/lib_manager.c +++ b/src/library_manager/lib_manager.c @@ -654,7 +654,8 @@ int lib_manager_register_module(const uint32_t component_id) if (build_info->format != SOF_MODULE_API_BUILD_INFO_FORMAT || build_info->api_version_number.full != SOF_MODULE_API_CURRENT_VERSION) { tr_err(&lib_manager_tr, "Unsupported module API version"); - return -ENOEXEC; + ret = -ENOEXEC; + goto cleanup; } } } From 690c9fb2ca8adebcb375b95dc94e3dbb61f1b426 Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Wed, 12 Feb 2025 12:00:23 +0100 Subject: [PATCH 4/6] lib_manager: Add module id verification Add verification of the module id passed as a parameter to the lib_manager_get_mod_ctx function. Add error handling to the lib_manager_free_module function. Signed-off-by: Adrian Warecki --- src/include/sof/lib_manager.h | 2 +- src/library_manager/lib_manager.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/include/sof/lib_manager.h b/src/include/sof/lib_manager.h index 201be8282748..cc0b73e0d7e7 100644 --- a/src/include/sof/lib_manager.h +++ b/src/include/sof/lib_manager.h @@ -145,7 +145,7 @@ static inline struct lib_manager_mod_ctx *lib_manager_get_mod_ctx(int module_id) uint32_t lib_id = LIB_MANAGER_GET_LIB_ID(module_id); struct ext_library *_ext_lib = ext_lib_get(); - if (!_ext_lib) + if (!_ext_lib || lib_id >= LIB_MANAGER_MAX_LIBS) return NULL; return _ext_lib->desc[lib_id]; diff --git a/src/library_manager/lib_manager.c b/src/library_manager/lib_manager.c index 0411f6721f2a..10bf60ed6aa6 100644 --- a/src/library_manager/lib_manager.c +++ b/src/library_manager/lib_manager.c @@ -386,6 +386,10 @@ int lib_manager_free_module(const uint32_t component_id) tr_dbg(&lib_manager_tr, "mod_id: %#x", component_id); mod = lib_manager_get_module_manifest(module_id); + if (!mod) { + tr_err(&lib_manager_tr, "failed to get module descriptor"); + return -EINVAL; + } if (module_is_llext(mod)) return llext_manager_free_module(component_id); From ee524e34e72e0f872745b6739e77133b9dff2219 Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Thu, 13 Feb 2025 16:30:55 +0100 Subject: [PATCH 5/6] module_adapter: Fix memory leak in module_adapter_new Add missing memory deallocation in the error handling code of the module_adapter_new function. This memory can be allocated by the module_adapter_init_data function. Signed-off-by: Adrian Warecki --- src/audio/module_adapter/module_adapter.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/audio/module_adapter/module_adapter.c b/src/audio/module_adapter/module_adapter.c index 1209532bcd76..bb0c79144d00 100644 --- a/src/audio/module_adapter/module_adapter.c +++ b/src/audio/module_adapter/module_adapter.c @@ -126,6 +126,10 @@ struct comp_dev *module_adapter_new(const struct comp_driver *drv, comp_dbg(dev, "module_adapter_new() done"); return dev; err: +#if CONFIG_IPC_MAJOR_4 + if (mod) + rfree(mod->priv.cfg.input_pins); +#endif rfree(mod); rfree(dev); return NULL; From 0b811e6c47b3e1362a30f815b02ffefeb55fbe7d Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Thu, 13 Feb 2025 17:17:54 +0100 Subject: [PATCH 6/6] modules: Remove redundant call to lib_manager_free_module Remove unnecessary call lib_manager_free_module from the modules_free function. Module release is already handled by lib_manager. Signed-off-by: Adrian Warecki --- src/audio/module_adapter/module/modules.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/audio/module_adapter/module/modules.c b/src/audio/module_adapter/module/modules.c index 68050089f037..b58f2f47054c 100644 --- a/src/audio/module_adapter/module/modules.c +++ b/src/audio/module_adapter/module/modules.c @@ -145,11 +145,6 @@ static int modules_free(struct processing_module *mod) if (ret) comp_err(dev, "modules_free(): iadk_wrapper_free failed with error: %d", ret); - /* Free module resources allocated in L2 memory. */ - ret = lib_manager_free_module(dev->ipc_config.id); - if (ret < 0) - comp_err(dev, "modules_free(), lib_manager_free_module() failed!"); - return ret; }