From 991dc8e1fd287e7f0866f279c019161098f76a77 Mon Sep 17 00:00:00 2001 From: Liam Girdwood Date: Sun, 1 Nov 2026 19:52:12 +0000 Subject: [PATCH 1/4] zephyr: vpage: fix out-of-bounds element access on free num_elems_in_use is the number of allocation elements in use, so the last element in use is at index num_elems_in_use - 1. When freeing, vpages_free_and_unmap() copied velems[num_elems_in_use] over the freed element and then cleared it. That is one past the last element in use, and past the end of the array once all VPAGE_MAX_ALLOCS elements are in use. Any free other than of the most recent allocation therefore dropped the tracking of the last allocation, which could then never be freed. Decrement the counter first, then move the last element. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Liam Girdwood --- zephyr/lib/vpage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zephyr/lib/vpage.c b/zephyr/lib/vpage.c index 8a7e54261006..089d3707de87 100644 --- a/zephyr/lib/vpage.c +++ b/zephyr/lib/vpage.c @@ -232,11 +232,11 @@ static int vpages_free_and_unmap(uintptr_t *ptr) } /* move the last element over the released one, clear the last element */ + vpage_ctx.num_elems_in_use--; if (vpage_ctx.num_elems_in_use != elem_idx) vpage_ctx.velems[elem_idx] = vpage_ctx.velems[vpage_ctx.num_elems_in_use]; vpage_ctx.velems[vpage_ctx.num_elems_in_use].pages = 0; vpage_ctx.velems[vpage_ctx.num_elems_in_use].vpage = 0; - vpage_ctx.num_elems_in_use--; /* success update the free pages */ vpage_ctx.free_pages += pages; From 3ec9a8101dd57651404ad2a6b8dbc468bda5b7d6 Mon Sep 17 00:00:00 2001 From: Liam Girdwood Date: Sun, 1 Nov 2026 19:53:38 +0000 Subject: [PATCH 2/4] zephyr: vpage: add an API to reserve virtual pages without mapping The LLEXT library manager needs virtual address space for loadable modules, but it maps the module sections itself, with different permissions for code, read-only and writable data. Using the shared virtual page allocator for that avoids a second, private virtual address range for modules. Split allocation into reserving a range of virtual pages and mapping physical pages to it, and export: - vpage_reserve() to reserve virtual pages without mapping them, - vpage_reserve_at() to reserve a range at a given address, needed to restore modules after the DSP has been powered down, because the allocator starts out empty on every boot, - vpage_release() to release a reserved range, - vpage_get_region() to get the region to map reserved pages in. vpage_alloc() and vpage_free() keep working as before. Co-developed-by: Jyri Sarha Signed-off-by: Jyri Sarha Signed-off-by: Liam Girdwood Co-Authored-By: Claude Opus 5.5 --- zephyr/include/sof/lib/vpage.h | 46 +++++ zephyr/lib/vpage.c | 344 ++++++++++++++++++++++++++------- 2 files changed, 320 insertions(+), 70 deletions(-) diff --git a/zephyr/include/sof/lib/vpage.h b/zephyr/include/sof/lib/vpage.h index ef66ce0ae533..045d8b69fb71 100644 --- a/zephyr/include/sof/lib/vpage.h +++ b/zephyr/include/sof/lib/vpage.h @@ -11,6 +11,7 @@ #define __SOF_LIB_VPAGE_H__ #include +#include #include #ifdef __cplusplus @@ -41,6 +42,51 @@ void *vpage_alloc(unsigned int pages); */ void vpage_free(void *ptr); +/** + * @brief Reserve contiguous virtual pages. + * + * Reserves a specified number of contiguous virtual memory pages without + * mapping any physical pages to them. For users that need to map the pages + * themselves, e.g. with different permissions for parts of the range. + * + * @param[in] pages Number of pages (typically 4KB per page) to reserve. + * @return Pointer to the reserved virtual memory region, or NULL on failure. + */ +void *vpage_reserve(unsigned int pages); + +/** + * @brief Reserve contiguous virtual pages at a given address. + * + * Like vpage_reserve() but for a range at a fixed address, e.g. for users that + * had the range reserved before the DSP was powered down and are restored at + * the same address. + * + * @param[in] ptr Page aligned address of the first page to reserve. + * @param[in] pages Number of pages (typically 4KB per page) to reserve. + * @return 0 on success or a negative error code if any of the pages is already + * in use or outside of the virtual page region. + */ +int vpage_reserve_at(void *ptr, unsigned int pages); + +/** + * @brief Release reserved virtual pages. + * + * Releases virtual memory pages, reserved with vpage_reserve() or + * vpage_reserve_at(). The caller has to unmap any pages it has mapped first. + * + * @param[in] ptr Pointer to the first reserved page. + */ +void vpage_release(void *ptr); + +/** + * @brief Get the virtual memory region managed by the allocator. + * + * Users of vpage_reserve() need it to map pages in the reserved range. + * + * @return The region, or NULL if no virtual page region could be created. + */ +const struct sys_mm_drv_region *vpage_get_region(void); + /** @}*/ #ifdef __cplusplus diff --git a/zephyr/lib/vpage.c b/zephyr/lib/vpage.c index 089d3707de87..fed4a8cd254e 100644 --- a/zephyr/lib/vpage.c +++ b/zephyr/lib/vpage.c @@ -65,16 +65,104 @@ struct vpage_context { static struct vpage_context vpage_ctx; /** - * @brief Allocate and map virtual memory pages + * @brief Find the allocation element of a range of virtual memory pages * - * Allocates memory pages from the virtual page allocator. - * Maps physical memory pages to the virtual region as needed. + * @param ptr Pointer to the first page of the range. + * @param elem_idx Pointer to store the index of the allocation element. + * @retval number of pages in the range or 0 if not found. + */ +static unsigned int vpages_find(void *ptr, unsigned int *elem_idx) +{ + unsigned int alloc_idx, i; + + /* check for valid ptr which must be page aligned */ + CHECKIF(!IS_ALIGNED(ptr, CONFIG_MM_DRV_PAGE_SIZE)) { + LOG_ERR("error: invalid non aligned page pointer %p", ptr); + return 0; + } + + alloc_idx = (POINTER_TO_UINT(ptr) - POINTER_TO_UINT(vpage_ctx.virtual_region->addr)) / + CONFIG_MM_DRV_PAGE_SIZE; + + /* find the allocation element */ + for (i = 0; i < vpage_ctx.num_elems_in_use; i++) { + if (vpage_ctx.velems[i].vpage == alloc_idx) { + LOG_DBG("found allocation element %d pages %u vpage %u for ptr %p", + i, vpage_ctx.velems[i].pages, vpage_ctx.velems[i].vpage, ptr); + *elem_idx = i; + return vpage_ctx.velems[i].pages; + } + } + + LOG_ERR("error: invalid page pointer %p not found", ptr); + return 0; +} + +/** + * @brief Account for a reserved range of virtual memory pages * - * @param pages Number of pages to allocate. - * @param ptr Pointer to store the address of allocated pages. + * @param vaddr Pointer to the first page of the range. + * @param pages Number of pages in the range. + */ +static void vpages_add_elem(void *vaddr, unsigned int pages) +{ + vpage_ctx.free_pages -= pages; + + /* Elements are acquired densely, just use the next one */ + vpage_ctx.velems[vpage_ctx.num_elems_in_use].pages = pages; + vpage_ctx.velems[vpage_ctx.num_elems_in_use].vpage = + (POINTER_TO_UINT(vaddr) - + POINTER_TO_UINT(vpage_ctx.virtual_region->addr)) / + CONFIG_MM_DRV_PAGE_SIZE; + vpage_ctx.num_elems_in_use++; +} + +/** + * @brief Release a range of virtual memory pages + * + * Returns the virtual pages to the allocator. Doesn't touch any mappings. + * + * @param ptr Pointer to the first page of the range. + * @param elem_idx Index of the allocation element of the range. + * @param pages Number of pages in the range. * @retval 0 if successful. */ -__cold static int vpages_alloc_and_map(unsigned int pages, void **ptr) +static int vpages_release_elem(void *ptr, unsigned int elem_idx, unsigned int pages) +{ + int ret; + + /* free virtual blocks */ + ret = sys_mem_blocks_free_contiguous(&vpage_ctx.vpage_blocks, ptr, pages); + if (ret < 0) { + LOG_ERR("error: failed to free %u continuous virtual page blocks at %p, error %d", + pages, ptr, ret); + return ret; + } + + /* move the last element over the released one, clear the last element */ + vpage_ctx.num_elems_in_use--; + if (vpage_ctx.num_elems_in_use != elem_idx) + vpage_ctx.velems[elem_idx] = vpage_ctx.velems[vpage_ctx.num_elems_in_use]; + vpage_ctx.velems[vpage_ctx.num_elems_in_use].pages = 0; + vpage_ctx.velems[vpage_ctx.num_elems_in_use].vpage = 0; + + /* success update the free pages */ + vpage_ctx.free_pages += pages; + + return 0; +} + +/** + * @brief Reserve virtual memory pages + * + * Reserves memory pages from the virtual page allocator without mapping + * them. + * + * @param pages Number of pages to reserve. + * @param ptr Pointer to store the address of reserved pages. + * @retval 0 if successful. + */ +__cold static int vpages_reserve(unsigned int pages, void **ptr) { void *vaddr; int ret; @@ -115,29 +203,102 @@ __cold static int vpages_alloc_and_map(unsigned int pages, void **ptr) return ret; } - /* map the virtual blocks in virtual region to free physical blocks */ - ret = sys_mm_drv_map_region_safe(vpage_ctx.virtual_region, vaddr, - 0, pages * CONFIG_MM_DRV_PAGE_SIZE, SYS_MM_MEM_PERM_RW); + vpages_add_elem(vaddr, pages); + + /* return the virtual address */ + *ptr = vaddr; + + return 0; +} + +/** + * @brief Reserve a specific range of virtual memory pages + * + * Reserves memory pages at a given address, e.g. for users that had them + * reserved before the DSP was powered down and are restored at the same + * address. + * + * @param ptr Pointer to the first page to reserve. + * @param pages Number of pages to reserve. + * @retval 0 if successful. + */ +__cold static int vpages_reserve_at(void *ptr, unsigned int pages) +{ + uintptr_t start, end; + int ret; + + assert_can_be_cold(); + + /* the virtual region is missing if init found no space for it */ + if (!vpage_ctx.virtual_region) + return -ENODEV; + + if (!pages) + return 0; + + CHECKIF(!IS_ALIGNED(ptr, CONFIG_MM_DRV_PAGE_SIZE)) { + LOG_ERR("error: invalid non aligned page pointer %p", ptr); + return -EINVAL; + } + + start = POINTER_TO_UINT(vpage_ctx.virtual_region->addr); + end = start + vpage_ctx.virtual_region->size; + if (POINTER_TO_UINT(ptr) < start || + pages > (end - POINTER_TO_UINT(ptr)) / CONFIG_MM_DRV_PAGE_SIZE) { + LOG_ERR("error: %u pages at %p outside of the virtual region", pages, ptr); + return -EINVAL; + } + + /* check for allocation elements */ + if (vpage_ctx.num_elems_in_use >= VPAGE_MAX_ALLOCS) { + LOG_ERR("error: max allocation elements reached"); + return -ENOMEM; + } + + /* fails if any of the pages is already in use */ + ret = sys_mem_blocks_get(&vpage_ctx.vpage_blocks, ptr, pages); if (ret < 0) { - LOG_ERR("error: failed to map virtual region %p to physical region %p, error %d", - vaddr, vpage_ctx.virtual_region->addr, ret); - sys_mem_blocks_free_contiguous(&vpage_ctx.vpage_blocks, vaddr, pages); + LOG_ERR("error: failed to reserve %u virtual pages at %p, error %d", + pages, ptr, ret); return ret; } - /* success update the free pages */ - vpage_ctx.free_pages -= pages; + vpages_add_elem(ptr, pages); - /* Elements are acquired densely, just use the next one */ - vpage_ctx.velems[vpage_ctx.num_elems_in_use].pages = pages; - vpage_ctx.velems[vpage_ctx.num_elems_in_use].vpage = - (POINTER_TO_UINT(vaddr) - - POINTER_TO_UINT(vpage_ctx.virtual_region->addr)) / - CONFIG_MM_DRV_PAGE_SIZE; - vpage_ctx.num_elems_in_use++; + return 0; +} - /* return the virtual address */ - *ptr = vaddr; +/** + * @brief Allocate and map virtual memory pages + * + * Allocates memory pages from the virtual page allocator. + * Maps physical memory pages to the virtual region as needed. + * + * @param pages Number of pages to allocate. + * @param ptr Pointer to store the address of allocated pages. + * @retval 0 if successful. + */ +__cold static int vpages_alloc_and_map(unsigned int pages, void **ptr) +{ + int ret; + + assert_can_be_cold(); + + ret = vpages_reserve(pages, ptr); + if (ret < 0 || !*ptr) + return ret; + + /* map the virtual blocks in virtual region to free physical blocks */ + ret = sys_mm_drv_map_region_safe(vpage_ctx.virtual_region, *ptr, + 0, pages * CONFIG_MM_DRV_PAGE_SIZE, SYS_MM_MEM_PERM_RW); + if (ret < 0) { + LOG_ERR("error: failed to map virtual region %p to physical region %p, error %d", + *ptr, vpage_ctx.virtual_region->addr, ret); + /* the element just added is the last one */ + vpages_release_elem(*ptr, vpage_ctx.num_elems_in_use - 1, pages); + *ptr = NULL; + return ret; + } return 0; } @@ -180,40 +341,15 @@ __cold void *vpage_alloc(unsigned int pages) */ static int vpages_free_and_unmap(uintptr_t *ptr) { - unsigned int alloc_idx, elem_idx; - unsigned int pages = 0; + unsigned int elem_idx, pages; int ret; if (!vpage_ctx.virtual_region) return -ENODEV; - /* check for valid ptr which must be page aligned */ - CHECKIF(!IS_ALIGNED(ptr, CONFIG_MM_DRV_PAGE_SIZE)) { - LOG_ERR("error: invalid non aligned page pointer %p", ptr); + pages = vpages_find(ptr, &elem_idx); + if (!pages) return -EINVAL; - } - - alloc_idx = (POINTER_TO_UINT(ptr) - POINTER_TO_UINT(vpage_ctx.virtual_region->addr)) / - CONFIG_MM_DRV_PAGE_SIZE; - - /* find the allocation element */ - for (elem_idx = 0; elem_idx < VPAGE_MAX_ALLOCS; elem_idx++) { - if (vpage_ctx.velems[elem_idx].pages > 0 && - vpage_ctx.velems[elem_idx].vpage == alloc_idx) { - pages = vpage_ctx.velems[elem_idx].pages; - - LOG_DBG("found allocation element %d pages %u vpage %u for ptr %p", - elem_idx, vpage_ctx.velems[elem_idx].pages, - vpage_ctx.velems[elem_idx].vpage, ptr); - break; - } - } - - /* check we found allocation element */ - CHECKIF(!pages) { - LOG_ERR("error: invalid page pointer %p not found", ptr); - return -EINVAL; - } /* unmap the pages from virtual region */ ret = sys_mm_drv_unmap_region((void *)ptr, pages * CONFIG_MM_DRV_PAGE_SIZE); @@ -223,25 +359,7 @@ static int vpages_free_and_unmap(uintptr_t *ptr) return ret; } - /* free physical blocks */ - ret = sys_mem_blocks_free_contiguous(&vpage_ctx.vpage_blocks, ptr, pages); - if (ret < 0) { - LOG_ERR("error: failed to free %u continuous virtual page blocks at %p, error %d", - pages, ptr, ret); - return ret; - } - - /* move the last element over the released one, clear the last element */ - vpage_ctx.num_elems_in_use--; - if (vpage_ctx.num_elems_in_use != elem_idx) - vpage_ctx.velems[elem_idx] = vpage_ctx.velems[vpage_ctx.num_elems_in_use]; - vpage_ctx.velems[vpage_ctx.num_elems_in_use].pages = 0; - vpage_ctx.velems[vpage_ctx.num_elems_in_use].vpage = 0; - - /* success update the free pages */ - vpage_ctx.free_pages += pages; - - return ret; + return vpages_release_elem(ptr, elem_idx, pages); } /** @@ -263,6 +381,92 @@ void vpage_free(void *ptr) vpage_ctx.total_pages); } +/** + * @brief Reserve virtual pages + * Reserves virtual memory pages without mapping them. + * + * @param pages Number of pages (usually 4kB large) to reserve. + * @retval NULL on reservation failure. + */ +__cold void *vpage_reserve(unsigned int pages) +{ + void *ptr = NULL; + int ret; + + assert_can_be_cold(); + + k_mutex_lock(&vpage_ctx.lock, K_FOREVER); + ret = vpages_reserve(pages, &ptr); + k_mutex_unlock(&vpage_ctx.lock); + if (ret < 0) + LOG_ERR("vpage_reserve failed %d for %u pages, total %u free %u", + ret, pages, vpage_ctx.total_pages, vpage_ctx.free_pages); + else + LOG_INF("vpage_reserve ptr %p pages %u free %u/%u", ptr, pages, + vpage_ctx.free_pages, vpage_ctx.total_pages); + return ptr; +} + +/** + * @brief Reserve virtual pages at a given address + * Reserves virtual memory pages at a given address without mapping them. + * + * @param ptr Pointer to the first page to reserve. + * @param pages Number of pages (usually 4kB large) to reserve. + * @retval 0 if successful. + */ +__cold int vpage_reserve_at(void *ptr, unsigned int pages) +{ + int ret; + + assert_can_be_cold(); + + k_mutex_lock(&vpage_ctx.lock, K_FOREVER); + ret = vpages_reserve_at(ptr, pages); + k_mutex_unlock(&vpage_ctx.lock); + if (ret < 0) + LOG_ERR("vpage_reserve_at failed %d for %u pages at %p, total %u free %u", + ret, pages, ptr, vpage_ctx.total_pages, vpage_ctx.free_pages); + else + LOG_INF("vpage_reserve_at ptr %p pages %u free %u/%u", ptr, pages, + vpage_ctx.free_pages, vpage_ctx.total_pages); + return ret; +} + +/** + * @brief Release reserved virtual pages + * Releases virtual memory pages, reserved with vpage_reserve() or + * vpage_reserve_at(). Any mappings have to be removed by the caller. + * + * @param ptr Pointer to the first reserved page. + */ +void vpage_release(void *ptr) +{ + unsigned int elem_idx, pages; + int ret = -ENODEV; + + k_mutex_lock(&vpage_ctx.lock, K_FOREVER); + if (vpage_ctx.virtual_region) { + pages = vpages_find(ptr, &elem_idx); + ret = pages ? vpages_release_elem(ptr, elem_idx, pages) : -EINVAL; + } + k_mutex_unlock(&vpage_ctx.lock); + + if (!ret) + LOG_INF("vptr %p release free/total pages %u/%u", ptr, vpage_ctx.free_pages, + vpage_ctx.total_pages); +} + +/** + * @brief Get the virtual memory region of the allocator + * + * @retval the region or NULL if there is no virtual page region. + */ +const struct sys_mm_drv_region *vpage_get_region(void) +{ + return vpage_ctx.virtual_region; +} + /** * @brief Initialize virtual page allocator * From 84ae7ca3d7d7c05ac39e5b5585a0dd8ac0b8a023 Mon Sep 17 00:00:00 2001 From: Liam Girdwood Date: Wed, 4 Nov 2026 13:02:24 +0000 Subject: [PATCH 3/4] zephyr: test: add vpage reservation and free order tests The existing vpage boot test frees allocations in an order, that never moves an allocation element, so it didn't catch the out-of-bounds element access in vpage_free(). Add tests for: - freeing an allocation, that isn't the last one: the remaining allocations must still be tracked and freed. This fails without commit "zephyr: vpage: fix out-of-bounds element access on free". - vpage_reserve() and vpage_release(): reserved pages are within the region and aren't mapped, a reserved range or a part of it can't be reserved again, and it can be after it has been released. - vpage_reserve_at() with ranges outside of the region or unaligned. Signed-off-by: Liam Girdwood Co-Authored-By: Claude Opus 5.5 Signed-off-by: Liam Girdwood --- zephyr/test/vpage.c | 102 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/zephyr/test/vpage.c b/zephyr/test/vpage.c index 038e299eba70..1fdee121b5ac 100644 --- a/zephyr/test/vpage.c +++ b/zephyr/test/vpage.c @@ -10,7 +10,9 @@ #include #include +#include #include +#include #include LOG_MODULE_DECLARE(sof_boot_test, CONFIG_SOF_LOG_LEVEL); @@ -34,3 +36,103 @@ ZTEST(sof_boot, vpage) vpage_free(p1); } + +#define PAGE_SZ CONFIG_MM_DRV_PAGE_SIZE + +/* Check that a range is inside the virtual page region */ +static void vpage_check_in_region(void *ptr, unsigned int pages) +{ + const struct sys_mm_drv_region *region = vpage_get_region(); + uintptr_t start = POINTER_TO_UINT(region->addr); + + zassert_true(IS_ALIGNED(ptr, PAGE_SZ), "%p not page aligned", ptr); + zassert_true(POINTER_TO_UINT(ptr) >= start && + POINTER_TO_UINT(ptr) + pages * PAGE_SZ <= start + region->size, + "%p outside of the region", ptr); +} + +/* + * Freeing an allocation, that isn't the last one, moves the last allocation + * element into its place. Free the first of three allocations, allocate + * again and check that the third allocation is still tracked, i.e. that its + * pages are really freed by vpage_free(). + */ +ZTEST(sof_boot, vpage_free_order) +{ + void *a = vpage_alloc(1); + void *b = vpage_alloc(1); + void *c = vpage_alloc(1); + void *d; + + zassert_not_null(a); + zassert_not_null(b); + zassert_not_null(c); + + vpage_free(a); + + d = vpage_alloc(1); + zassert_not_null(d); + + vpage_free(c); + + /* only succeeds if the pages of c have been freed */ + zassert_ok(vpage_reserve_at(c, 1), "pages at %p not freed", c); + vpage_release(c); + + vpage_free(b); + vpage_free(d); +} + +/* Allocated pages are mapped, reserved pages aren't */ +ZTEST(sof_boot, vpage_reserve) +{ + uintptr_t phys; + void *p; + + zassert_not_null(vpage_get_region()); + zassert_is_null(vpage_reserve(0)); + + p = vpage_alloc(1); + zassert_not_null(p); + vpage_check_in_region(p, 1); + zassert_ok(sys_mm_drv_page_phys_get(p, &phys), "allocated page %p not mapped", p); + vpage_free(p); + + p = vpage_reserve(2); + zassert_not_null(p); + vpage_check_in_region(p, 2); + zassert_not_ok(sys_mm_drv_page_phys_get(p, &phys), "reserved page %p mapped", p); + zassert_not_ok(sys_mm_drv_page_phys_get((uint8_t *)p + PAGE_SZ, &phys), + "reserved page %p mapped", (uint8_t *)p + PAGE_SZ); + + /* neither the whole range nor a part of it can be reserved again */ + zassert_not_ok(vpage_reserve_at(p, 2)); + zassert_not_ok(vpage_reserve_at((uint8_t *)p + PAGE_SZ, 1)); + + vpage_release(p); + + /* the whole range is free again after release */ + zassert_ok(vpage_reserve_at(p, 2)); + vpage_release(p); +} + +/* vpage_reserve_at() only accepts page aligned ranges within the region */ +ZTEST(sof_boot, vpage_reserve_at_invalid) +{ + const struct sys_mm_drv_region *region = vpage_get_region(); + uint8_t *start, *end; + + zassert_not_null(region); + + start = region->addr; + end = start + region->size; + + zassert_ok(vpage_reserve_at(start, 0)); + zassert_equal(vpage_reserve_at(start - PAGE_SZ, 1), -EINVAL); + zassert_equal(vpage_reserve_at(end, 1), -EINVAL); + zassert_equal(vpage_reserve_at(end - PAGE_SZ, 2), -EINVAL); + + /* CHECKIF() asserts or skips the check in some configurations */ + if (!IS_ENABLED(CONFIG_ASSERT_ON_ERRORS) && !IS_ENABLED(CONFIG_NO_RUNTIME_CHECKS)) + zassert_equal(vpage_reserve_at(start + 1, 1), -EINVAL); +} From 43fbae6236e7199c9ddb013c854687e9d551ecc3 Mon Sep 17 00:00:00 2001 From: Liam Girdwood Date: Sun, 1 Nov 2026 19:55:09 +0000 Subject: [PATCH 4/4] library_manager: llext: place relocatable modules in virtual pages LLEXT modules are linked at build time to run at fixed addresses in a dedicated virtual memory region. Each module gets the addresses after the previous one, so all modules have to be built together, and the region has to be large enough for all modules even if only a few of them are used. Relocatable (ET_REL) modules don't need fixed addresses: they still carry all their relocations, so the build-time addresses are only placeholders. When such a module is linked for the first time, reserve virtual pages for it from the shared virtual page allocator and move all its sections, that are copied to SRAM, by the same offset into that range before calling llext_load(). The offset is a multiple of the largest section alignment, at least a page, so moving by it keeps the relative placement and alignment of all sections. References, that the linker has already resolved, stay valid, and LLEXT relocates the rest. Detached sections stay in DRAM and aren't moved. The ELF header and section header table are checked before use, as llext_load() only validates them after the sections have been moved. As before, a module is linked only once. The addresses stay assigned to the module until the firmware is rebooted, so re-instantiating a module only maps and copies it again. After the DSP has been powered down, the modules are restored from DRAM with the addresses they were linked for, so reserve those pages again, because the allocator starts out empty on every boot. Without that the allocator hands them out again, e.g. for a DP module heap. llext_load() returns the use count of an extension, if it has already been loaded, so only release the pages on a negative return value. Shared object (ET_DYN) modules can only run at their build-time addresses, they keep using the dedicated region, and so do relocatable modules if there is no virtual page region. Co-developed-by: Jyri Sarha Signed-off-by: Jyri Sarha Signed-off-by: Liam Girdwood Co-Authored-By: Claude Opus 5.5 --- src/include/sof/lib_manager.h | 1 + src/library_manager/llext_manager.c | 161 +++++++++++++++++++++-- src/library_manager/llext_manager_dram.c | 18 ++- zephyr/CMakeLists.txt | 6 +- 4 files changed, 174 insertions(+), 12 deletions(-) diff --git a/src/include/sof/lib_manager.h b/src/include/sof/lib_manager.h index 70ca5f97102c..9c05ac340835 100644 --- a/src/include/sof/lib_manager.h +++ b/src/include/sof/lib_manager.h @@ -119,6 +119,7 @@ struct lib_manager_module { bool domain_dp; struct lib_manager_segment_desc segment[LIB_MANAGER_N_SEGMENTS]; struct lib_manager_segment_desc writable_merged; + struct lib_manager_segment_desc vma; /* Reserved virtual pages for ET_REL modules */ }; struct lib_manager_mod_ctx { diff --git a/src/library_manager/llext_manager.c b/src/library_manager/llext_manager.c index 267955040cce..76dc737c8d79 100644 --- a/src/library_manager/llext_manager.c +++ b/src/library_manager/llext_manager.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -299,20 +300,29 @@ __cold static int llext_manager_load_module(struct lib_manager_module *mctx) struct llext_loader *ldr = &mctx->ebl->loader; const struct llext *ext = mctx->llext; - /* find dedicated virtual memory zone */ - const struct sys_mm_drv_region *virtual_memory_regions = sys_mm_drv_query_memory_regions(); const struct sys_mm_drv_region *virtual_region; - if (!virtual_memory_regions) - return -EFAULT; + if (mctx->vma.size) { + /* relocated to pages, reserved from the virtual page allocator */ + virtual_region = vpage_get_region(); + if (!virtual_region) + return -EFAULT; + } else { + /* find dedicated virtual memory zone */ + const struct sys_mm_drv_region *virtual_memory_regions = + sys_mm_drv_query_memory_regions(); - SYS_MM_DRV_MEMORY_REGION_FOREACH(virtual_memory_regions, virtual_region) { - if (virtual_region->attr == VIRTUAL_REGION_LLEXT_LIBRARIES_ATTR) - break; - } + if (!virtual_memory_regions) + return -EFAULT; + + SYS_MM_DRV_MEMORY_REGION_FOREACH(virtual_memory_regions, virtual_region) { + if (virtual_region->attr == VIRTUAL_REGION_LLEXT_LIBRARIES_ATTR) + break; + } - if (!virtual_region->size) - return -EFAULT; + if (!virtual_region->size) + return -EFAULT; + } /* Copy Code */ ret = llext_manager_load_data_from_storage(virtual_region, ldr, ext, LLEXT_MEM_TEXT, @@ -434,6 +444,120 @@ static bool llext_manager_section_detached(const elf_shdr_t *shdr) return shdr->sh_addr < SOF_MODULE_DRAM_LINK_END; } +/* + * Relocatable (ET_REL) modules are linked at placeholder addresses. Reserve + * virtual pages for all sections, that will be copied to SRAM, and move them + * there, keeping their relative placement, so that llext_load() relocates the + * module to run at those addresses. Detached sections stay in DRAM and keep + * their addresses. Shared object (ET_DYN) modules can only run at addresses, + * fixed at build time, they stay in the dedicated virtual memory region. + */ +__cold static int llext_manager_place(struct lib_manager_module *mctx) +{ + static const uint8_t elf_magic[] = {0x7f, 'E', 'L', 'F'}; + uint8_t *elf = (uint8_t *)mctx->ebl->buf; + size_t len = mctx->ebl->len; + const elf_ehdr_t *ehdr = (const elf_ehdr_t *)elf; + uintptr_t start = UINTPTR_MAX, end = 0, base; + size_t align = PAGE_SZ, size; + unsigned int i, shnum; + elf_shdr_t *shdr; + void *vma; + + assert_can_be_cold(); + + /* llext_load() validates the image, only check what is used here */ + if (len < sizeof(*ehdr) || memcmp(ehdr->e_ident, elf_magic, sizeof(elf_magic))) + return -ENOEXEC; + + /* + * Fall back to the addresses from the build for shared object modules + * and without a virtual page region. Build-time addresses will go away, + * all addresses will be assigned when modules are linked. + */ + if (ehdr->e_type != ET_REL) { + tr_warn(&lib_manager_tr, "not a relocatable module, using build-time addresses"); + return 0; + } + + if (!vpage_get_region()) { + tr_warn(&lib_manager_tr, "no virtual page region, using build-time addresses"); + return 0; + } + + /* + * The section header table must be within the image and must not + * overlap the ELF header: section addresses are rewritten below. Read + * the number of sections only once for the same reason. + */ + shnum = ehdr->e_shnum; + if (ehdr->e_shentsize != sizeof(*shdr) || ehdr->e_shoff < sizeof(*ehdr) || + ehdr->e_shoff > len || shnum > (len - ehdr->e_shoff) / sizeof(*shdr) || + !IS_ALIGNED(elf + ehdr->e_shoff, sizeof(uint32_t))) { + tr_err(&lib_manager_tr, "invalid section header table"); + return -ENOEXEC; + } + + shdr = (elf_shdr_t *)(elf + ehdr->e_shoff); + + /* Find the address range and the largest alignment of SRAM sections */ + for (i = 0; i < shnum; i++) { + if (!(shdr[i].sh_flags & SHF_ALLOC) || !shdr[i].sh_size || + llext_manager_section_detached(shdr + i)) + continue; + + if (shdr[i].sh_addr + shdr[i].sh_size < shdr[i].sh_addr || + !is_power_of_two(shdr[i].sh_addralign ? : 1)) { + tr_err(&lib_manager_tr, "invalid section %u", i); + return -ENOEXEC; + } + + start = MIN(start, shdr[i].sh_addr); + end = MAX(end, shdr[i].sh_addr + shdr[i].sh_size); + align = MAX(align, shdr[i].sh_addralign); + } + + /* Nothing to copy to SRAM, nothing to move */ + if (!end) + return 0; + + /* + * Moving by a multiple of the largest section alignment, at least a + * page, preserves the alignment of all sections. If that alignment is + * larger than a page, reserve extra pages to find an aligned start. + */ + start = ALIGN_DOWN(start, align); + size = ALIGN_UP(end - start, PAGE_SZ) + align - PAGE_SZ; + + vma = vpage_reserve(size / PAGE_SZ); + if (!vma) { + tr_err(&lib_manager_tr, "cannot reserve %#zx bytes of virtual memory", size); + return -ENOMEM; + } + + base = ALIGN_UP((uintptr_t)vma, align); + + /* Move all SRAM sections by the same offset, llext_load() relocates them */ + for (i = 0; i < shnum; i++) { + if (!(shdr[i].sh_flags & SHF_ALLOC) || !shdr[i].sh_size || + llext_manager_section_detached(shdr + i)) + continue; + + shdr[i].sh_addr += base - start; + } + + /* The ELF image is kept in DRAM over DSP power cycles */ + sys_cache_data_flush_range(shdr, shnum * sizeof(*shdr)); + + /* Released only if linking fails, restored with the module after D3 */ + mctx->vma.addr = (uintptr_t)vma; + mctx->vma.size = size; + + tr_dbg(&lib_manager_tr, "placed %#lx-%#lx at %#lx", start, end, base); + + return 0; +} + __cold static int llext_manager_link(const char *name, struct lib_manager_module *mctx, const void **buildinfo, const struct sof_man_module_manifest **mod_manifest) @@ -468,8 +592,23 @@ __cold static int llext_manager_link(const char *name, .section_detached = llext_manager_section_detached, .keep_section_info = true, }; + bool placed = false; + + if (!*llext) { + ret = llext_manager_place(mctx); + if (ret < 0) + return ret; + placed = mctx->vma.size; + } + + /* a positive value is the use count of an already loaded extension */ ret = llext_load(ldr, name, llext, &ldr_parm); + if (ret < 0 && placed) { + vpage_release((void *)mctx->vma.addr); + mctx->vma.addr = 0; + mctx->vma.size = 0; + } if (ret) return ret; } @@ -583,6 +722,8 @@ __cold static int llext_manager_mod_init(struct lib_manager_mod_ctx *ctx, ctx->mod[n_mod].llext = NULL; ctx->mod[n_mod].ebl = NULL; ctx->mod[n_mod].n_dependent = 0; + ctx->mod[n_mod].vma.addr = 0; + ctx->mod[n_mod].vma.size = 0; ctx->mod[n_mod].start_idx = i; if (n_mod) ctx->mod[n_mod - 1].n_mod = i - ctx->mod[n_mod - 1].start_idx; diff --git a/src/library_manager/llext_manager_dram.c b/src/library_manager/llext_manager_dram.c index 2f6cff2b3501..f014bb9000ae 100644 --- a/src/library_manager/llext_manager_dram.c +++ b/src/library_manager/llext_manager_dram.c @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -232,6 +233,18 @@ int llext_manager_restore_from_dram(void) /* Not instantiated - nothing to restore */ continue; + /* + * The virtual page allocator starts out empty on every + * boot, reserve the pages the module is linked at again + */ + if (mod[k].vma.size && + vpage_reserve_at((void *)mod[k].vma.addr, + mod[k].vma.size / CONFIG_MM_DRV_PAGE_SIZE) < 0) { + tr_err(&lib_manager_tr, "cannot reclaim %#zx bytes at %#lx", + mod[k].vma.size, mod[k].vma.addr); + goto nomem; + } + /* Loaders are supplied by the caller */ struct llext_buf_loader *bldr = rmalloc(SOF_MEM_FLAG_KERNEL | SOF_MEM_FLAG_COHERENT, sizeof(*bldr)); @@ -329,8 +342,11 @@ int llext_manager_restore_from_dram(void) continue; for (k = 0; k < ctx->n_mod; k++) { - if (mod[k].llext) + if (mod[k].llext) { llext_unload(&mod[k].llext); + if (mod[k].vma.size) + vpage_release((void *)mod[k].vma.addr); + } if (mod[k].ebl) rfree(mod[k].ebl); diff --git a/zephyr/CMakeLists.txt b/zephyr/CMakeLists.txt index 47dcaca6cf4c..d179ad32d4db 100644 --- a/zephyr/CMakeLists.txt +++ b/zephyr/CMakeLists.txt @@ -290,8 +290,12 @@ if (CONFIG_SOC_SERIES_INTEL_ADSP_ACE) ${SOF_PLATFORM_PATH}/novalake/lib/clk.c ) + # the LLEXT library manager reserves virtual pages for loadable modules + if(CONFIG_SOF_VREGIONS OR CONFIG_LIBRARY_MANAGER) + zephyr_library_sources(lib/vpage.c) + endif() + zephyr_library_sources_ifdef(CONFIG_SOF_VREGIONS - lib/vpage.c lib/vregion.c )