From 23417a431227474348f36286e61707436eaf87ea Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Wed, 13 Mar 2024 16:18:16 +0100 Subject: [PATCH 1/3] ipc4/helper: ipc4_get_comp_drv: Change parameter type to uint32_t All calls to the ipc4_get_comp_drv function pass the uint32_t value as a parameter, so the type of functions parameter was changed. Signed-off-by: Adrian Warecki --- src/include/sof/ipc/topology.h | 2 +- src/ipc/ipc4/helper.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/include/sof/ipc/topology.h b/src/include/sof/ipc/topology.h index 825898a09811..5f851005f0eb 100644 --- a/src/include/sof/ipc/topology.h +++ b/src/include/sof/ipc/topology.h @@ -47,7 +47,7 @@ typedef uint32_t ipc_comp; #define ipc_from_pipe_connect(x) ((struct ipc4_module_bind_unbind *)x) struct ipc_comp_dev; -const struct comp_driver *ipc4_get_comp_drv(int module_id); +const struct comp_driver *ipc4_get_comp_drv(uint32_t module_id); struct comp_dev *ipc4_get_comp_dev(uint32_t comp_id); int ipc4_add_comp_dev(struct comp_dev *dev); const struct comp_driver *ipc4_get_drv(const uint8_t *uuid); diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index 666df9bdea8f..4fa3821d9f6e 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -945,12 +945,12 @@ static const struct comp_driver *ipc4_library_get_drv(int module_id) } #endif -const struct comp_driver *ipc4_get_comp_drv(int module_id) +const struct comp_driver *ipc4_get_comp_drv(uint32_t module_id) { struct sof_man_fw_desc *desc = NULL; const struct comp_driver *drv; struct sof_man_module *mod; - int entry_index; + uint32_t entry_index; #if CONFIG_LIBRARY return ipc4_library_get_drv(module_id); From f38cba13d18f4929288c369e48baf8a449285b4d Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Wed, 28 Feb 2024 17:07:19 +0100 Subject: [PATCH 2/3] lib_manager: Add lib_manager_get_module_manifest function Added the lib_manager_get_module_manifest function that returns manifest of selected module based on its id. It was performed in many places and moving it into function simplified the code and increased its readability. Signed-off-by: Adrian Warecki --- src/audio/module_adapter/module/modules.c | 15 ++++--- src/include/sof/lib_manager.h | 9 ++++ src/ipc/ipc4/helper.c | 28 +++++++++---- src/library_manager/lib_manager.c | 50 +++++++++++++++-------- src/library_manager/llext_manager.c | 5 +-- 5 files changed, 69 insertions(+), 38 deletions(-) diff --git a/src/audio/module_adapter/module/modules.c b/src/audio/module_adapter/module/modules.c index 1e5ef912f0f4..c995fc5c07b7 100644 --- a/src/audio/module_adapter/module/modules.c +++ b/src/audio/module_adapter/module/modules.c @@ -62,21 +62,20 @@ static int modules_new(struct processing_module *mod, const void *buildinfo, uint32_t log_handle = (uint32_t) dev->drv->tctx; /* Connect loadable module interfaces with module adapter entity. */ /* Check if native Zephyr lib is loaded */ - struct sof_man_fw_desc *desc = lib_manager_get_library_module_desc(module_id); void *system_agent; - if (!desc) { - comp_err(dev, "modules_init(): Failed to load manifest"); - return -ENOMEM; - } - const struct sof_module_api_build_info *mod_buildinfo; if (buildinfo) { mod_buildinfo = buildinfo; } else { - struct sof_man_module *module_entry = - (struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(0)); + const struct sof_man_module *const module_entry = + lib_manager_get_module_manifest(module_id); + + if (!module_entry) { + comp_err(dev, "modules_new(): Failed to load manifest"); + return -ENODATA; + } mod_buildinfo = (struct sof_module_api_build_info *) diff --git a/src/include/sof/lib_manager.h b/src/include/sof/lib_manager.h index a396c1daf282..34b85224073f 100644 --- a/src/include/sof/lib_manager.h +++ b/src/include/sof/lib_manager.h @@ -117,6 +117,15 @@ static inline struct lib_manager_mod_ctx *lib_manager_get_mod_ctx(int module_id) return _ext_lib->desc[lib_id]; } + +/* + * \brief Get module manifest for given module id + * + * param[in] module_id - used to get library manifest + * + * Gets library manifest descriptor using module_id to locate it + */ +const struct sof_man_module *lib_manager_get_module_manifest(const uint32_t module_id); #endif /* diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index 4fa3821d9f6e..1a3884a9d774 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -947,9 +947,9 @@ static const struct comp_driver *ipc4_library_get_drv(int module_id) const struct comp_driver *ipc4_get_comp_drv(uint32_t module_id) { - struct sof_man_fw_desc *desc = NULL; + const struct sof_man_fw_desc *desc = NULL; const struct comp_driver *drv; - struct sof_man_module *mod; + const struct sof_man_module *mod; uint32_t entry_index; #if CONFIG_LIBRARY @@ -957,10 +957,10 @@ const struct comp_driver *ipc4_get_comp_drv(uint32_t module_id) #endif #ifdef RIMAGE_MANIFEST - desc = (struct sof_man_fw_desc *)IMR_BOOT_LDR_MANIFEST_BASE; + desc = (const struct sof_man_fw_desc *)IMR_BOOT_LDR_MANIFEST_BASE; #else - /* Non-rimage platforms have no component facility yet. This - * needs to move to the platform layer. + /* Non-rimage platforms have no component facility yet. + * This needs to move to the platform layer. */ return NULL; #endif @@ -973,13 +973,26 @@ const struct comp_driver *ipc4_get_comp_drv(uint32_t module_id) entry_index = 1; else entry_index = module_id; + + if (entry_index >= desc->header.num_module_entries) { + tr_err(&comp_tr, "Error: entry index %d out of bounds.", entry_index); + return NULL; + } + + mod = (const struct sof_man_module *)((const char *)desc + + SOF_MAN_MODULE_OFFSET(entry_index)); } else { /* Library index greater than 0 possible only when LIBRARY_MANAGER * support enabled. */ #if CONFIG_LIBRARY_MANAGER - desc = lib_manager_get_library_module_desc(module_id); - entry_index = LIB_MANAGER_GET_MODULE_INDEX(module_id); + mod = lib_manager_get_module_manifest(module_id); + + if (!mod) { + tr_err(&comp_tr, "Error: Couldn't find loadable module with id %d.", + module_id); + return NULL; + } #else tr_err(&comp_tr, "Error: lib index:%d, while loadable libraries are not supported!!!", lib_idx); @@ -987,7 +1000,6 @@ const struct comp_driver *ipc4_get_comp_drv(uint32_t module_id) #endif } /* Check already registered components */ - mod = (struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(entry_index)); drv = ipc4_get_drv(mod->uuid); #if CONFIG_LIBRARY_MANAGER diff --git a/src/library_manager/lib_manager.c b/src/library_manager/lib_manager.c index e01fb154634f..e240c98e8f21 100644 --- a/src/library_manager/lib_manager.c +++ b/src/library_manager/lib_manager.c @@ -212,9 +212,9 @@ static int lib_manager_unload_module(const struct sof_man_module *const mod) /* There are modules marked as lib_code. This is code shared between several modules inside * the library. Load all lib_code modules with first none lib_code module load. */ -static int lib_manager_load_libcode_modules(const uint32_t module_id, - const struct sof_man_fw_desc *const desc) +static int lib_manager_load_libcode_modules(const uint32_t module_id) { + const struct sof_man_fw_desc *const desc = lib_manager_get_library_module_desc(module_id); struct ext_library *const ext_lib = ext_lib_get(); const struct sof_man_module *module_entry = (struct sof_man_module *) ((char *)desc + SOF_MAN_MODULE_OFFSET(0)); @@ -250,12 +250,12 @@ static int lib_manager_load_libcode_modules(const uint32_t module_id, /* There are modules marked as lib_code. This is code shared between several modules inside * the library. Unload all lib_code modules with last none lib_code module unload. */ -static int lib_manager_unload_libcode_modules(const uint32_t module_id, - const struct sof_man_fw_desc *const desc) +static int lib_manager_unload_libcode_modules(const uint32_t module_id) { - struct ext_library *const ext_lib = ext_lib_get(); + const struct sof_man_fw_desc *const desc = lib_manager_get_library_module_desc(module_id); const struct sof_man_module *module_entry = (struct sof_man_module *) ((char *)desc + SOF_MAN_MODULE_OFFSET(0)); + struct ext_library *const ext_lib = ext_lib_get(); int ret, idx; if (--ext_lib->mods_exec_load_cnt > 0) @@ -342,25 +342,21 @@ uintptr_t lib_manager_allocate_module(struct processing_module *proc, const struct comp_ipc_config *ipc_config, const void *ipc_specific_config, const void **buildinfo) { - struct sof_man_fw_desc *desc; const struct sof_man_module *mod; const struct ipc4_base_module_cfg *base_cfg = ipc_specific_config; int ret; uint32_t module_id = IPC4_MOD_ID(ipc_config->id); - uint32_t entry_index = LIB_MANAGER_GET_MODULE_INDEX(module_id); tr_dbg(&lib_manager_tr, "lib_manager_allocate_module(): mod_id: %#x", ipc_config->id); - desc = lib_manager_get_library_module_desc(module_id); - if (!desc) { + mod = lib_manager_get_module_manifest(module_id); + if (!mod) { tr_err(&lib_manager_tr, "lib_manager_allocate_module(): failed to get module descriptor"); return 0; } - mod = (struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(entry_index)); - if (module_is_llext(mod)) return llext_manager_allocate_module(proc, ipc_config, ipc_specific_config, buildinfo); @@ -370,7 +366,7 @@ uintptr_t lib_manager_allocate_module(struct processing_module *proc, return 0; #ifdef CONFIG_LIBCODE_MODULE_SUPPORT - ret = lib_manager_load_libcode_modules(module_id, desc); + ret = lib_manager_load_libcode_modules(module_id); if (ret < 0) goto err; #endif /* CONFIG_LIBCODE_MODULE_SUPPORT */ @@ -381,7 +377,7 @@ uintptr_t lib_manager_allocate_module(struct processing_module *proc, tr_err(&lib_manager_tr, "lib_manager_allocate_module(): module allocation failed: %d", ret); #ifdef CONFIG_LIBCODE_MODULE_SUPPORT - lib_manager_unload_libcode_modules(module_id, desc); + lib_manager_unload_libcode_modules(module_id); #endif /* CONFIG_LIBCODE_MODULE_SUPPORT */ goto err; } @@ -394,16 +390,13 @@ uintptr_t lib_manager_allocate_module(struct processing_module *proc, int lib_manager_free_module(const uint32_t component_id) { - struct sof_man_fw_desc *desc; const struct sof_man_module *mod; const uint32_t module_id = IPC4_MOD_ID(component_id); - uint32_t entry_index = LIB_MANAGER_GET_MODULE_INDEX(module_id); int ret; tr_dbg(&lib_manager_tr, "lib_manager_free_module(): mod_id: %#x", component_id); - desc = lib_manager_get_library_module_desc(module_id); - mod = (struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(entry_index)); + mod = lib_manager_get_module_manifest(module_id); if (module_is_llext(mod)) return llext_manager_free_module(component_id); @@ -413,7 +406,7 @@ int lib_manager_free_module(const uint32_t component_id) return ret; #ifdef CONFIG_LIBCODE_MODULE_SUPPORT - ret = lib_manager_unload_libcode_modules(module_id, desc); + ret = lib_manager_unload_libcode_modules(module_id); if (ret < 0) return ret; #endif /* CONFIG_LIBCODE_MODULE_SUPPORT */ @@ -480,6 +473,27 @@ static void lib_manager_update_sof_ctx(void *base_addr, uint32_t lib_id) /* TODO: maybe need to call dcache_writeback here? */ } +const struct sof_man_module *lib_manager_get_module_manifest(const uint32_t module_id) +{ + const uint32_t entry_index = LIB_MANAGER_GET_MODULE_INDEX(module_id); + const struct lib_manager_mod_ctx *const ctx = lib_manager_get_mod_ctx(module_id); + const struct sof_man_fw_desc *desc; + + if (!ctx || !ctx->base_addr) + return NULL; + + desc = (const struct sof_man_fw_desc *)((const char *)ctx->base_addr + + SOF_MAN_ELF_TEXT_OFFSET); + + if (entry_index >= desc->header.num_module_entries) { + tr_err(&lib_manager_tr, "Entry index %d out of bounds.", entry_index); + return NULL; + } + + return (const struct sof_man_module *)((const char *)desc + + SOF_MAN_MODULE_OFFSET(entry_index)); +} + #if CONFIG_INTEL_MODULES static void lib_manager_prepare_module_adapter(struct comp_driver *drv, const struct sof_uuid *uuid) { diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index 2f0afb6ddbe5..4e84d3b6ff71 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -272,16 +272,13 @@ uintptr_t llext_manager_allocate_module(struct processing_module *proc, int llext_manager_free_module(const uint32_t component_id) { - struct sof_man_fw_desc *desc; const struct sof_man_module *mod; const uint32_t module_id = IPC4_MOD_ID(component_id); - uint32_t entry_index = LIB_MANAGER_GET_MODULE_INDEX(module_id); int ret; tr_dbg(&lib_manager_tr, "llext_manager_free_module(): mod_id: %#x", component_id); - desc = lib_manager_get_library_module_desc(module_id); - mod = (struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(entry_index)); + mod = lib_manager_get_module_manifest(module_id); ret = llext_manager_unload_module(module_id, mod); if (ret < 0) From cec215f455fba32447effc9f9c58be08bd65f713 Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Tue, 5 Mar 2024 16:45:34 +0100 Subject: [PATCH 3/3] lib_manager: Rename lib_manager_get_library_module_desc function The lib_manager_get_library_module_desc function was renamed to lib_manager_get_library_manifest, which better describes what it do. Signed-off-by: Adrian Warecki --- src/include/sof/lib_manager.h | 2 +- src/library_manager/lib_manager.c | 6 +++--- src/library_manager/llext_manager.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/include/sof/lib_manager.h b/src/include/sof/lib_manager.h index 34b85224073f..80c0160015a9 100644 --- a/src/include/sof/lib_manager.h +++ b/src/include/sof/lib_manager.h @@ -151,7 +151,7 @@ int lib_manager_register_module(const uint32_t component_id); * * Gets firmware manifest descriptor using module_id to locate it */ -struct sof_man_fw_desc *lib_manager_get_library_module_desc(int module_id); +const struct sof_man_fw_desc *lib_manager_get_library_manifest(int module_id); struct processing_module; /* diff --git a/src/library_manager/lib_manager.c b/src/library_manager/lib_manager.c index e240c98e8f21..8441d2a7fbe7 100644 --- a/src/library_manager/lib_manager.c +++ b/src/library_manager/lib_manager.c @@ -214,7 +214,7 @@ static int lib_manager_unload_module(const struct sof_man_module *const mod) */ static int lib_manager_load_libcode_modules(const uint32_t module_id) { - const struct sof_man_fw_desc *const desc = lib_manager_get_library_module_desc(module_id); + const struct sof_man_fw_desc *const desc = lib_manager_get_library_manifest(module_id); struct ext_library *const ext_lib = ext_lib_get(); const struct sof_man_module *module_entry = (struct sof_man_module *) ((char *)desc + SOF_MAN_MODULE_OFFSET(0)); @@ -252,7 +252,7 @@ static int lib_manager_load_libcode_modules(const uint32_t module_id) */ static int lib_manager_unload_libcode_modules(const uint32_t module_id) { - const struct sof_man_fw_desc *const desc = lib_manager_get_library_module_desc(module_id); + const struct sof_man_fw_desc *const desc = lib_manager_get_library_manifest(module_id); const struct sof_man_module *module_entry = (struct sof_man_module *) ((char *)desc + SOF_MAN_MODULE_OFFSET(0)); struct ext_library *const ext_lib = ext_lib_get(); @@ -450,7 +450,7 @@ void lib_manager_init(void) sof->ext_library = &loader_ext_lib; } -struct sof_man_fw_desc *lib_manager_get_library_module_desc(int module_id) +const struct sof_man_fw_desc *lib_manager_get_library_manifest(int module_id) { struct lib_manager_mod_ctx *ctx = lib_manager_get_mod_ctx(module_id); uint8_t *buffptr = ctx ? ctx->base_addr : NULL; diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index 4e84d3b6ff71..e3d73ae98a12 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -242,7 +242,7 @@ uintptr_t llext_manager_allocate_module(struct processing_module *proc, tr_dbg(&lib_manager_tr, "llext_manager_allocate_module(): mod_id: %#x", ipc_config->id); - desc = lib_manager_get_library_module_desc(module_id); + desc = (struct sof_man_fw_desc *)lib_manager_get_library_manifest(module_id); if (!ctx || !desc) { tr_err(&lib_manager_tr, "llext_manager_allocate_module(): failed to get module descriptor");