From 605d200a0b40c17decf9e507daf85019dfff0b54 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 8a7e54261006..bcddd6f37bf4 100644 --- a/zephyr/lib/vpage.c +++ b/zephyr/lib/vpage.c @@ -231,12 +231,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 c6cab73839f06263dad7967ad2b9f8a76de625e9 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 bcddd6f37bf4..c7f1cc274502 100644 --- a/zephyr/lib/vpage.c +++ b/zephyr/lib/vpage.c @@ -121,6 +121,19 @@ __cold 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 c0936fca165bf5b92d2c1f75e26c9b6f2ab2b031 Mon Sep 17 00:00:00 2001 From: Liam Girdwood Date: Mon, 9 Nov 2026 14:02:53 +0000 Subject: [PATCH 3/4] vpage: size the vpage region without underflow, support small L2 The vpage region size was computed as an unsigned subtraction. The overflow is now caught, but vpages are then simply disabled when the virtual heap and library regions do not fit in L2 together with the image, as on NVL-S (2MB L2). Those regions are only virtual address space backed on demand, and the Zephyr TLB driver now maps the uncached alias of pages beyond physical L2 too. When they do not fit, 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, as vpages cannot use more than that. Disable vpages only if no page is left. Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Liam Girdwood --- zephyr/lib/vpage.c | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/zephyr/lib/vpage.c b/zephyr/lib/vpage.c index c7f1cc274502..c9f6d9df00b7 100644 --- a/zephyr/lib/vpage.c +++ b/zephyr/lib/vpage.c @@ -312,18 +312,40 @@ __cold static int vpage_init(void) end = l2_end - CONFIG_LIBRARY_REGION_SIZE; } else { /* - * TODO: on demand mapping currently only supports - * cached mappings, so we need to limit vpages to - * physical L2 size for now until support is added to - * Zephyr mm_drv_intel_adsp_mtl_tlb.c and friends + * 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. + * Using pages beyond physical L2 needs the Zephyr TLB driver to map + * the uncached alias too, see + * https://github.com/zephyrproject-rtos/zephyr/pull/122801 */ - LOG_WRN("no space left for vpages, start %#lx l2_end %#lx", - (unsigned long)start, (unsigned long)l2_end); + 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_WRN("no space left for vpages, start %#lx end %#lx", + (unsigned long)start, (unsigned long)end); return -ENOMEM; } remaining_ram = end - start; - __ASSERT_NO_MSG(remaining_ram >= CONFIG_MM_DRV_PAGE_SIZE); + 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_WRN("no physical L2 left for vpages"); + return -ENOMEM; + } ret = adsp_add_virtual_memory_region(start, remaining_ram, VIRTUAL_REGION_VPAGES_ATTR); if (ret) From 621fb80f143e865509373482d35cc38bde00de3a Mon Sep 17 00:00:00 2001 From: Liam Girdwood Date: Mon, 9 Nov 2026 15:51:02 +0000 Subject: [PATCH 4/4] test: vpage: cover all vpage APIs, over commit, leaks and sparse maps Add boot tests for vpage_alloc() and vpage_free(): - zero page allocation - alignment, contiguity and full read/write of a multi-page allocation - over commit: more than available, absurd sizes and exhausting all pages fail cleanly and consume nothing - leaks: repeated alloc/free of mixed sizes and orders returns every page, detected by comparing the largest allocatable block before and after - sparse maps: holes left by frees are reused by single pages, larger blocks never overlap live pages and data is never corrupted - invalid frees (NULL, unaligned, mid-allocation, double free) are rejected without disturbing valid allocations - exhausting the allocation tracking elements fails cleanly and recovers Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Liam Girdwood --- zephyr/test/vpage.c | 272 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 272 insertions(+) diff --git a/zephyr/test/vpage.c b/zephyr/test/vpage.c index 98ab174dcf73..71e6205f5d20 100644 --- a/zephyr/test/vpage.c +++ b/zephyr/test/vpage.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -67,3 +68,274 @@ ZTEST(sof_boot, vpage_free_out_of_order) zassert_equal(d, a, "pages leaked after out of order free"); vpage_free(d); } + +#define VPAGE_TEST_PAGE_SIZE CONFIG_MM_DRV_PAGE_SIZE + +/* + * Size in pages of the largest single contiguous allocation that currently + * succeeds. The allocator has no free page accessor, so this is how the tests + * detect leaked virtual or physical pages: it must be the same before and after + * a test has released everything it allocated. + */ +static unsigned int vpage_largest(void) +{ + unsigned int lo = 0, hi = 1; + void *p; + + /* grow until an allocation fails */ + while (hi <= 0x8000) { + p = vpage_alloc(hi); + if (!p) + break; + vpage_free(p); + lo = hi; + hi <<= 1; + } + + /* binary search the limit between the last success and the first failure */ + while (hi - lo > 1) { + unsigned int mid = lo + (hi - lo) / 2; + + p = vpage_alloc(mid); + if (p) { + vpage_free(p); + lo = mid; + } else { + hi = mid; + } + } + + return lo; +} + +static void vpage_fill(void *ptr, unsigned int pages, uint8_t seed) +{ + uint8_t *p = ptr; + + for (size_t i = 0; i < pages * VPAGE_TEST_PAGE_SIZE; i += 61) + p[i] = (uint8_t)(seed + i); +} + +static bool vpage_check(void *ptr, unsigned int pages, uint8_t seed) +{ + uint8_t *p = ptr; + + for (size_t i = 0; i < pages * VPAGE_TEST_PAGE_SIZE; i += 61) + if (p[i] != (uint8_t)(seed + i)) + return false; + + return true; +} + +/* 0 pages is not an allocation */ +ZTEST(sof_boot, vpage_alloc_zero) +{ + zassert_is_null(vpage_alloc(0)); +} + +/* Allocations are page aligned, contiguous, fully mapped and writable */ +ZTEST(sof_boot, vpage_contiguous_rw) +{ + const unsigned int pages = 8; + uint8_t *p = vpage_alloc(pages); + + zassert_not_null(p); + zassert_true(IS_ALIGNED(p, VPAGE_TEST_PAGE_SIZE)); + + /* touch every page, a hole in the mapping would fault */ + for (unsigned int i = 0; i < pages; i++) + memset(p + i * VPAGE_TEST_PAGE_SIZE, 0xa0 + i, VPAGE_TEST_PAGE_SIZE); + + for (unsigned int i = 0; i < pages; i++) + for (size_t j = 0; j < VPAGE_TEST_PAGE_SIZE; j += 37) + zassert_equal(p[i * VPAGE_TEST_PAGE_SIZE + j], (uint8_t)(0xa0 + i)); + + vpage_free(p); +} + +/* Asking for more than is available must fail cleanly and not leak anything */ +ZTEST(sof_boot, vpage_overcommit) +{ + unsigned int max = vpage_largest(); + void *all, *extra; + + zassert_true(max > 0, "no vpages available"); + + /* one page more than the largest block, and absurd sizes */ + zassert_is_null(vpage_alloc(max + 1)); + zassert_is_null(vpage_alloc(0x10000)); + zassert_is_null(vpage_alloc(UINT_MAX)); + + /* failures must not have consumed anything */ + zassert_equal(vpage_largest(), max, "failed allocation leaked pages"); + + /* take everything, then there must be nothing left */ + all = vpage_alloc(max); + zassert_not_null(all); + vpage_fill(all, max, 0x11); + + extra = vpage_alloc(1); + zassert_is_null(extra, "allocated beyond the available pages"); + + zassert_true(vpage_check(all, max, 0x11)); + vpage_free(all); + + zassert_equal(vpage_largest(), max, "pages leaked after full allocation"); +} + +/* Repeated alloc/free of mixed sizes and orders must return all pages */ +ZTEST(sof_boot, vpage_no_leak) +{ + const unsigned int sizes[] = { 1, 2, 3, 5, 8, 1, 4 }; + void *p[ARRAY_SIZE(sizes)]; + unsigned int max = vpage_largest(); + + zassert_true(max > 0, "no vpages available"); + + for (unsigned int round = 0; round < 32; round++) { + for (unsigned int i = 0; i < ARRAY_SIZE(sizes); i++) { + p[i] = vpage_alloc(sizes[i]); + zassert_not_null(p[i]); + vpage_fill(p[i], sizes[i], round + i); + } + + for (unsigned int i = 0; i < ARRAY_SIZE(sizes); i++) + zassert_true(vpage_check(p[i], sizes[i], round + i), "data corrupted"); + + /* alternate between forward and reverse release order */ + for (unsigned int i = 0; i < ARRAY_SIZE(sizes); i++) + vpage_free(p[(round & 1) ? ARRAY_SIZE(sizes) - 1 - i : i]); + } + + zassert_equal(vpage_largest(), max, "pages leaked"); +} + +/* Fragmented (sparse) free space: holes are reused and never overlap live pages */ +ZTEST(sof_boot, vpage_sparse) +{ + void *p[8]; + void *hole[4]; + void *big, *refill; + unsigned int max = vpage_largest(); + unsigned int i, h; + + for (i = 0; i < ARRAY_SIZE(p); i++) { + p[i] = vpage_alloc(1); + zassert_not_null(p[i]); + vpage_fill(p[i], 1, 0x30 + i); + } + + /* + * The hole checks below rely on first fit giving back to back pages, which + * holds unless other users have left free gaps below. + */ + for (i = 1; i < ARRAY_SIZE(p); i++) { + if ((uint8_t *)p[i] != (uint8_t *)p[i - 1] + VPAGE_TEST_PAGE_SIZE) { + for (i = 0; i < ARRAY_SIZE(p); i++) + vpage_free(p[i]); + ztest_test_skip(); + return; + } + } + + /* free every other page, leaving single page holes between live pages */ + for (i = 0, h = 0; i < ARRAY_SIZE(p); i += 2, h++) { + hole[h] = p[i]; + vpage_free(p[i]); + p[i] = NULL; + } + + /* a 2 page block can't fit in a hole and must not overlap a live page */ + big = vpage_alloc(2); + zassert_not_null(big); + vpage_fill(big, 2, 0x70); + for (i = 1; i < ARRAY_SIZE(p); i += 2) { + zassert_true((uint8_t *)p[i] + VPAGE_TEST_PAGE_SIZE <= (uint8_t *)big || + (uint8_t *)big + 2 * VPAGE_TEST_PAGE_SIZE <= (uint8_t *)p[i], + "2 page block overlaps live page %u", i); + } + + /* every single page allocation must take a hole while holes remain */ + for (h = 0; h < ARRAY_SIZE(hole); h++) { + bool found = false; + + refill = vpage_alloc(1); + zassert_not_null(refill); + for (unsigned int k = 0; k < ARRAY_SIZE(hole); k++) + if (refill == hole[k]) { + hole[k] = NULL; + found = true; + } + zassert_true(found, "single page didn't reuse a hole"); + vpage_fill(refill, 1, 0x50 + h); + /* keep it for release below */ + p[h * 2] = refill; + } + + /* nothing was overwritten while the holes were being reused */ + for (i = 1; i < ARRAY_SIZE(p); i += 2) + zassert_true(vpage_check(p[i], 1, 0x30 + i), "live page %u corrupted", i); + zassert_true(vpage_check(big, 2, 0x70)); + + for (i = 0; i < ARRAY_SIZE(p); i++) + vpage_free(p[i]); + vpage_free(big); + + zassert_equal(vpage_largest(), max, "pages leaked"); +} + +/* Invalid frees must be rejected without disturbing valid allocations */ +ZTEST(sof_boot, vpage_invalid_free) +{ + unsigned int max = vpage_largest(); + uint8_t *p = vpage_alloc(2); + + zassert_not_null(p); + vpage_fill(p, 2, 0x42); + + /* NULL, unaligned, inside an allocation but not its start */ + vpage_free(NULL); + vpage_free(p + 1); + vpage_free(p + VPAGE_TEST_PAGE_SIZE); + + zassert_true(vpage_check(p, 2, 0x42), "allocation disturbed by invalid free"); + + vpage_free(p); + + /* double free must not release anything else or corrupt the tracking */ + vpage_free(p); + + zassert_equal(vpage_largest(), max, "invalid free changed the available pages"); +} + +/* Running out of tracking elements must fail cleanly and recover on free */ +ZTEST(sof_boot, vpage_max_allocs) +{ + void *p[CONFIG_SOF_VPAGE_MAX_ALLOCS + 1]; + unsigned int max = vpage_largest(); + unsigned int n; + + for (n = 0; n < ARRAY_SIZE(p); n++) { + p[n] = vpage_alloc(1); + if (!p[n]) + break; + } + + /* either the elements or the pages must have run out before this many */ + zassert_true(n < ARRAY_SIZE(p), "allocated more than %u elements", + CONFIG_SOF_VPAGE_MAX_ALLOCS); + zassert_true(n > 0); + + /* the failure must be sticky until something is freed */ + zassert_is_null(vpage_alloc(1)); + + for (unsigned int i = 0; i < n; i++) + vpage_free(p[i]); + + zassert_equal(vpage_largest(), max, "pages leaked after exhausting allocations"); + + /* the allocator works again */ + p[0] = vpage_alloc(1); + zassert_not_null(p[0]); + vpage_free(p[0]); +}