diff --git a/zephyr/lib/vpage.c b/zephyr/lib/vpage.c index ce0da7b5ac97..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; @@ -114,6 +118,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; } @@ -175,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); @@ -219,12 +239,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; @@ -269,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) { 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); +}