From a745984d3e8f2353b8334b39ac6ea318114ec103 Mon Sep 17 00:00:00 2001 From: Adrian Warecki Date: Wed, 15 Nov 2023 12:51:33 +0100 Subject: [PATCH] library_manager: Update module load flow The previous code have several issues: * Did not support empty data segment. * It assumed that certain types of segments would be at fixed indexes, without taking into account their flags/types. * Incorrect handling of mapping error. If the virtual address cannot be mapped because is already in use, the error handler will unmap this address. * If there is an error loading one of the modules marked as lib_code, previously loaded modules are not unloaded. This commit fixes the above issues. Signed-off-by: Adrian Warecki --- src/library_manager/lib_manager.c | 207 +++++++++++++++++------------- 1 file changed, 118 insertions(+), 89 deletions(-) diff --git a/src/library_manager/lib_manager.c b/src/library_manager/lib_manager.c index b8f3644cf648..01cd4cc53da9 100644 --- a/src/library_manager/lib_manager.c +++ b/src/library_manager/lib_manager.c @@ -54,7 +54,7 @@ static int lib_manager_load_data_from_storage(void __sparse_cache *vma, void *s_ uint32_t size, uint32_t flags) { int ret = sys_mm_drv_map_region((__sparse_force void *)vma, POINTER_TO_UINT(NULL), - size, flags); + size, SYS_MM_MEM_PERM_RW); if (ret < 0) return ret; @@ -68,114 +68,133 @@ static int lib_manager_load_data_from_storage(void __sparse_cache *vma, void *s_ return 0; } -static int lib_manager_load_module(uint32_t module_id, struct sof_man_module *mod, - struct sof_man_fw_desc *desc) +static int lib_manager_load_module(const uint32_t module_id, + const struct sof_man_module *const mod) { - struct ext_library *ext_lib = ext_lib_get(); - uint32_t lib_id = LIB_MANAGER_GET_LIB_ID(module_id); - size_t load_offset = (size_t)((void *)ext_lib->desc[lib_id]); - void __sparse_cache *va_base_text = (void __sparse_cache *) - mod->segment[SOF_MAN_SEGMENT_TEXT].v_base_addr; - void *src_txt = (void *)(mod->segment[SOF_MAN_SEGMENT_TEXT].file_offset + load_offset); - size_t st_text_size = mod->segment[SOF_MAN_SEGMENT_TEXT].flags.r.length; - void __sparse_cache *va_base_rodata = (void __sparse_cache *) - mod->segment[SOF_MAN_SEGMENT_RODATA].v_base_addr; - void *src_rodata = - (void *)(mod->segment[SOF_MAN_SEGMENT_RODATA].file_offset + load_offset); - size_t st_rodata_size = mod->segment[SOF_MAN_SEGMENT_RODATA].flags.r.length; + const struct ext_library *const ext_lib = ext_lib_get(); + const uint32_t lib_id = LIB_MANAGER_GET_LIB_ID(module_id); + const uintptr_t load_offset = POINTER_TO_UINT(ext_lib->desc[lib_id]); + void *src, *va_base; + size_t size; + uint32_t flags; + int ret, idx; + + for (idx = 0; idx < ARRAY_SIZE(mod->segment); ++idx) { + if (!mod->segment[idx].flags.r.load) + continue; + + flags = 0; + + if (mod->segment[idx].flags.r.code) + flags = SYS_MM_MEM_PERM_EXEC; + else if (!mod->segment[idx].flags.r.readonly) + flags = SYS_MM_MEM_PERM_RW; + + src = UINT_TO_POINTER(mod->segment[idx].file_offset + load_offset); + va_base = UINT_TO_POINTER(mod->segment[idx].v_base_addr); + size = mod->segment[idx].flags.r.length * PAGE_SZ; + ret = lib_manager_load_data_from_storage(va_base, src, size, flags); + if (ret < 0) + goto err; + } + + return 0; + +err: + for (--idx; idx >= 0; --idx) { + if (!mod->segment[idx].flags.r.load) + continue; + + va_base = UINT_TO_POINTER(mod->segment[idx].v_base_addr); + size = mod->segment[idx].flags.r.length * PAGE_SZ; + sys_mm_drv_unmap_region(va_base, size); + } + + return ret; +} + +static int lib_manager_unload_module(const struct sof_man_module *const mod) +{ + void *va_base; + size_t size; + uint32_t idx; int ret; - st_text_size = st_text_size * PAGE_SZ; - st_rodata_size = st_rodata_size * PAGE_SZ; + for (idx = 0; idx < ARRAY_SIZE(mod->segment); ++idx) { + if (!mod->segment[idx].flags.r.load) + continue; - /* Copy Code */ - ret = lib_manager_load_data_from_storage(va_base_text, src_txt, st_text_size, - SYS_MM_MEM_PERM_RW | SYS_MM_MEM_PERM_EXEC); - if (ret < 0) - goto err; + va_base = UINT_TO_POINTER(mod->segment[idx].v_base_addr); + size = mod->segment[idx].flags.r.length * PAGE_SZ; + ret = sys_mm_drv_unmap_region(va_base, size); + if (ret < 0) + return ret; + } - /* Copy RODATA */ - ret = lib_manager_load_data_from_storage(va_base_rodata, src_rodata, - st_rodata_size, SYS_MM_MEM_PERM_RW); - if (ret < 0) - goto err; + return 0; +} - /* 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. - */ - if (!mod->type.lib_code) - ext_lib->mods_exec_load_cnt++; - - if (ext_lib->mods_exec_load_cnt == 1) { - struct sof_man_module *module_entry = - (struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(0)); - for (size_t idx = 0; idx < desc->header.num_module_entries; - ++idx, ++module_entry) { - if (module_entry->type.lib_code) { - ret = lib_manager_load_module(lib_id << LIB_MANAGER_LIB_ID_SHIFT | - idx, mod, desc); - if (ret < 0) - goto err; - } +/* 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_code_modules(const uint32_t module_id, + const struct sof_man_fw_desc *const desc) +{ + 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)); + const uint32_t lib_id = LIB_MANAGER_GET_LIB_ID(module_id); + int ret, idx; + + if (++ext_lib->mods_exec_load_cnt > 1) + return 0; + + for (idx = 0; idx < desc->header.num_module_entries; ++idx, ++module_entry) { + if (module_entry->type.lib_code) { + ret = lib_manager_load_module(lib_id << LIB_MANAGER_LIB_ID_SHIFT | idx, + module_entry); + if (ret < 0) + goto err; } } return 0; err: - sys_mm_drv_unmap_region((__sparse_force void *)va_base_text, st_text_size); - sys_mm_drv_unmap_region((__sparse_force void *)va_base_rodata, st_rodata_size); + for (--idx, --module_entry; idx >= 0; --idx, --module_entry) { + if (module_entry->type.lib_code) { + ret = lib_manager_unload_module(module_entry); + if (ret < 0) + goto err; + } + } return ret; } -static int lib_manager_unload_module(uint32_t module_id, struct sof_man_module *mod, - struct sof_man_fw_desc *desc) +/* 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_code_modules(const uint32_t module_id, + const struct sof_man_fw_desc *const desc) { - struct ext_library *ext_lib = ext_lib_get(); - uint32_t lib_id = LIB_MANAGER_GET_LIB_ID(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 = mod->segment[SOF_MAN_SEGMENT_TEXT].flags.r.length; - void __sparse_cache *va_base_rodata = (void __sparse_cache *) - mod->segment[SOF_MAN_SEGMENT_RODATA].v_base_addr; - size_t st_rodata_size = mod->segment[SOF_MAN_SEGMENT_RODATA].flags.r.length; - int ret; - - st_text_size = st_text_size * PAGE_SZ; - st_rodata_size = st_rodata_size * PAGE_SZ; - - ret = sys_mm_drv_unmap_region((__sparse_force void *)va_base_text, st_text_size); - if (ret < 0) - return ret; - - ret = sys_mm_drv_unmap_region((__sparse_force void *)va_base_rodata, st_rodata_size); - if (ret < 0) - return ret; + 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)); + int ret, idx; - /* 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. - */ - if (mod->type.lib_code) - return ret; + if (--ext_lib->mods_exec_load_cnt > 0) + return 0; - if (!mod->type.lib_code && ext_lib->mods_exec_load_cnt > 0) - ext_lib->mods_exec_load_cnt--; - - if (ext_lib->mods_exec_load_cnt == 0) { - struct sof_man_module *module_entry = - (struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(0)); - for (size_t idx = 0; idx < desc->header.num_module_entries; - ++idx, ++module_entry) { - if (module_entry->type.lib_code) { - ret = - lib_manager_unload_module(lib_id << LIB_MANAGER_LIB_ID_SHIFT | - idx, mod, desc); - } + for (idx = 0; idx < desc->header.num_module_entries; ++idx, ++module_entry) { + if (module_entry->type.lib_code) { + ret = lib_manager_unload_module(module_entry); + if (ret < 0) + return ret; } } - return ret; + return 0; } static void __sparse_cache *lib_manager_get_instance_bss_address(uint32_t module_id, @@ -261,10 +280,16 @@ uint32_t lib_manager_allocate_module(const struct comp_driver *drv, mod = (struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(entry_index)); - ret = lib_manager_load_module(module_id, mod, desc); + ret = lib_manager_load_module(module_id, mod); if (ret < 0) return 0; + ret = lib_manager_load_code_modules(module_id, desc); + if (ret < 0) { + lib_manager_unload_module(mod); + return ret; + } + ret = lib_manager_allocate_module_instance(module_id, IPC4_INST_ID(ipc_config->id), base_cfg->is_pages, mod); if (ret < 0) { @@ -289,7 +314,11 @@ int lib_manager_free_module(const struct comp_driver *drv, desc = lib_manager_get_library_module_desc(module_id); mod = (struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(entry_index)); - ret = lib_manager_unload_module(module_id, mod, desc); + ret = lib_manager_unload_module(mod); + if (ret < 0) + return ret; + + ret = lib_manager_unload_code_modules(module_id, desc); if (ret < 0) return ret;