vpage: fix free tracking and region sizing, enable vpages on NVL-S

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
       #

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) {

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);
+}