From bbaed5831bb9e1e6caa624bb15d95e65155fffb2 Mon Sep 17 00:00:00 2001 From: Liam Girdwood Date: Wed, 21 Oct 2026 17:40:16 +0100 Subject: [PATCH 1/4] vpage: fix element tracking when freeing a non-last allocation vpages_free_and_unmap() moved velems[num_elems_in_use], which is one past the last in-use element, over the released slot. Freeing a non-last allocation therefore left the last element outside the in-use count, so the next allocation overwrote its tracking entry and that allocation could never be freed, leaking its virtual and physical pages. Decrement the count first so the real last element is moved. Add a boot test that frees allocations out of order and checks that the pages can be reused. Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Liam Girdwood --- zephyr/lib/vpage.c | 7 +++++-- zephyr/test/vpage.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/zephyr/lib/vpage.c b/zephyr/lib/vpage.c index ce0da7b5ac97..3eb358501b54 100644 --- a/zephyr/lib/vpage.c +++ b/zephyr/lib/vpage.c @@ -219,12 +219,15 @@ static int vpages_free_and_unmap(uintptr_t *ptr) return ret; } - /* move the last element over the released one, clear the last element */ + /* + * Keep the elements dense: move the last in-use element (num_elems_in_use - 1) + * over the released one and clear the last slot. + */ + 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; diff --git a/zephyr/test/vpage.c b/zephyr/test/vpage.c index 038e299eba70..98ab174dcf73 100644 --- a/zephyr/test/vpage.c +++ b/zephyr/test/vpage.c @@ -34,3 +34,36 @@ ZTEST(sof_boot, vpage) vpage_free(p1); } + +/* Freeing a non-last allocation must not orphan the last tracked element */ +ZTEST(sof_boot, vpage_free_out_of_order) +{ + void *a = vpage_alloc(1); + void *b = vpage_alloc(1); + void *c, *d; + + zassert_not_null(a); + zassert_not_null(b); + + /* the check below needs a and b to be neighbours, first fit normally does this */ + if ((uint8_t *)b != (uint8_t *)a + CONFIG_MM_DRV_PAGE_SIZE) { + vpage_free(a); + vpage_free(b); + ztest_test_skip(); + return; + } + + vpage_free(a); + + /* takes a's page; must not overwrite b's tracking element */ + c = vpage_alloc(1); + zassert_equal(c, a); + + vpage_free(b); + vpage_free(c); + + /* if b's element was lost its page is leaked and the 2 page range can't be reused */ + d = vpage_alloc(2); + zassert_equal(d, a, "pages leaked after out of order free"); + vpage_free(d); +} From ad48a4b9ed1a2516d2ffc2fa13903e17542de267 Mon Sep 17 00:00:00 2001 From: Liam Girdwood Date: Wed, 21 Oct 2026 17:40:16 +0100 Subject: [PATCH 2/4] vpage: release pages mapped by a failed multi-page map The mm driver maps page by page and leaves the pages already mapped, and their physical pages, in place when a later page fails, e.g. when physical L2 is exhausted. vpage then freed the virtual blocks without unmapping them, leaking physical L2. Unmap any page of the failed range that is still mapped before freeing the virtual blocks. Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Liam Girdwood --- zephyr/lib/vpage.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/zephyr/lib/vpage.c b/zephyr/lib/vpage.c index 3eb358501b54..e4ec57ff020d 100644 --- a/zephyr/lib/vpage.c +++ b/zephyr/lib/vpage.c @@ -114,6 +114,19 @@ static int vpages_alloc_and_map(unsigned int pages, void **ptr) 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); + + /* + * The driver maps page by page and leaves already mapped pages (and + * their physical pages) in place on failure, e.g. when physical L2 is + * exhausted. Release them so they do not leak. + */ + for (unsigned int i = 0; i < pages; i++) { + void *va = (uint8_t *)vaddr + i * CONFIG_MM_DRV_PAGE_SIZE; + + if (!sys_mm_drv_page_phys_get(va, NULL)) + sys_mm_drv_unmap_page(va); + } + sys_mem_blocks_free_contiguous(&vpage_ctx.vpage_blocks, vaddr, pages); return ret; } From 7a369efbae6172c5378d69aeb1ac90ddff8f0c77 Mon Sep 17 00:00:00 2001 From: Liam Girdwood Date: Wed, 21 Oct 2026 17:40:28 +0100 Subject: [PATCH 3/4] vpage: size the vpage region without underflow, support small L2 vpage_init() computed the region size as an unsigned subtraction without checking that the image, virtual heap and library regions leave any L2 for vpages. On NVL-S (2MB L2) this underflows into a ~4GB region whose end wraps the 32-bit address space, so sys_mm_drv_map_region_safe() rejects every mapping with -EINVAL and every vpage_alloc() fails. The virtual heap and library regions are only virtual address space backed on demand, so they need not fit in L2 together with the vpages. When they fit, keep the existing layout. Otherwise place the vpage region after the virtual heap, end it at the library base address (or the end of virtual memory), and cap it by the physical L2 left after the image. If no page is left, log an error and leave vpages disabled; vpage_alloc() and vpage_free() then fail with -ENODEV. Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Liam Girdwood --- zephyr/lib/vpage.c | 63 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 53 insertions(+), 10 deletions(-) diff --git a/zephyr/lib/vpage.c b/zephyr/lib/vpage.c index e4ec57ff020d..b740f34afbb3 100644 --- a/zephyr/lib/vpage.c +++ b/zephyr/lib/vpage.c @@ -82,6 +82,10 @@ static int vpages_alloc_and_map(unsigned int pages, void **ptr) if (!ptr) return -EINVAL; + /* the virtual region is missing if init found no space for it */ + if (!vpage_ctx.virtual_region) + return -ENODEV; + if (!pages) { *ptr = NULL; return 0; @@ -188,6 +192,9 @@ static int vpages_free_and_unmap(uintptr_t *ptr) unsigned int pages = 0; 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); @@ -285,19 +292,55 @@ static int vpage_init(void) int ret; /* create the virtual memory region and add it to the system */ - size_t remaining_ram = L2_SRAM_BASE + L2_SRAM_SIZE - - (adsp_mm_get_unused_l2_start_aligned() + - CONFIG_SOF_ZEPHYR_VIRTUAL_HEAP_REGION_SIZE + - CONFIG_LIBRARY_REGION_SIZE); - - ret = adsp_add_virtual_memory_region(adsp_mm_get_unused_l2_start_aligned() + - CONFIG_SOF_ZEPHYR_VIRTUAL_HEAP_REGION_SIZE, - remaining_ram, VIRTUAL_REGION_VPAGES_ATTR); - if (ret) - return ret; + uintptr_t l2_end = L2_SRAM_BASE + L2_SRAM_SIZE; + uintptr_t start = adsp_mm_get_unused_l2_start_aligned() + + CONFIG_SOF_ZEPHYR_VIRTUAL_HEAP_REGION_SIZE; + uintptr_t end; + size_t remaining_ram; k_mutex_init(&vpage_ctx.lock); + if (start + CONFIG_LIBRARY_REGION_SIZE + CONFIG_MM_DRV_PAGE_SIZE <= l2_end) { + /* virtual heap and library regions fit in L2, vpages get the rest */ + end = l2_end - CONFIG_LIBRARY_REGION_SIZE; + } else { + /* + * Small L2 (e.g. NVL-S): the virtual heap and library regions are only + * virtual address space backed on demand, so they need not fit in L2 + * together with the vpages. Use the virtual space up to the library + * region (or the end of virtual memory) and cap it below by the + * physical L2 left after the image, as vpages can never use more. + */ + end = L2_VIRTUAL_SRAM_BASE + L2_VIRTUAL_SRAM_SIZE; + if (IS_ENABLED(CONFIG_LIBRARY_MANAGER) && CONFIG_LIBRARY_BASE_ADDRESS < end) + end = CONFIG_LIBRARY_BASE_ADDRESS; + } + + /* the image or virtual heap may leave no room, don't let this underflow */ + if (end < start + CONFIG_MM_DRV_PAGE_SIZE) { + LOG_ERR("error: no space left for vpages, start %#lx end %#lx", + (unsigned long)start, (unsigned long)end); + return -ENOMEM; + } + + remaining_ram = end - start; + if (end > l2_end) { + uintptr_t image_end = adsp_mm_get_unused_l2_start_aligned(); + size_t phys_left = image_end < l2_end ? l2_end - image_end : 0; + + remaining_ram = MIN(remaining_ram, phys_left); + } + + /* revalidate, the physical cap can leave less than a page */ + if (remaining_ram < CONFIG_MM_DRV_PAGE_SIZE) { + LOG_ERR("error: no physical L2 left for vpages"); + return -ENOMEM; + } + + ret = adsp_add_virtual_memory_region(start, remaining_ram, VIRTUAL_REGION_VPAGES_ATTR); + if (ret) + return ret; + /* now find the virtual region in all memory regions */ virtual_memory_regions = sys_mm_drv_query_memory_regions(); SYS_MM_DRV_MEMORY_REGION_FOREACH(virtual_memory_regions, region) { From 81400fb02b5e15afd8849bd03c8941913067dde6 Mon Sep 17 00:00:00 2001 From: Liam Girdwood Date: Wed, 21 Oct 2026 17:40:28 +0100 Subject: [PATCH 4/4] DNM: west.yml: use zephyr PR 122801 for CI testing Temporary: pull in "drivers: mm: intel_adsp: tlb: map uncached alias in the MMU" (zephyrproject-rtos/zephyr#122801), one commit on top of the currently pinned beeac475263e. Without it, coherent (uncached) accesses to vpages beyond the end of physical L2 fault on NVL-S. Drop this commit and bump the zephyr pin once the Zephyr PR is merged. Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Liam Girdwood --- west.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/west.yml b/west.yml index b66c3a7deac1..0c06913a9df5 100644 --- a/west.yml +++ b/west.yml @@ -11,6 +11,8 @@ manifest: url-base: https://github.com/thesofproject - name: zephyrproject url-base: https://github.com/zephyrproject-rtos + - name: kv2019i + url-base: https://github.com/kv2019i # When upgrading projects here please run git log --oneline in the # project and if not too long then include the output in your commit @@ -43,8 +45,8 @@ manifest: - name: zephyr repo-path: zephyr - revision: beeac475263ea55a74a7b61274cd4867c53c973d - remote: zephyrproject + revision: 7280c15b8cff01f73f2ef4107b886c213baacc0a + remote: kv2019i # Import some projects listed in zephyr/west.yml@revision #