vpage / vregion: a minimal version with a test by lyakh · Pull Request #10636 · thesofproject/sof

lyakh · 2026-03-20T14:39:45Z

This is a slightly simplified version of #10281 with an added test

Add a simple virtual page allocator that uses the Zephyr memory mapping infrastructure to allocate pages from a virtual region and map them to physical pages. Due to simplicity, the number of allocations is limited to CONFIG_SOF_VPAGE_MAX_ALLOCS Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com> Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

Add support for per pipeline and per module virtual memory regions. The intention is to provide a single virtual memory region per pipeline or per DP module that can simplify module/pipeline memory management. The region takes advantage of the way pipeline and modules are constructed, destroyed and used during their lifetimes. 1) memory tracking - 1 pointer/size per pipeline or DP module. 2) memory read/write/execute permissions 3) cache utilization. Modules and pipelines will allocate from their region only and this will be abstracted via the allocation APIs. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com> Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

Add a boot-time test for basic vregion functionality. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

Copilot

Pull request overview

Introduces a minimal “virtual pages” allocator and “virtual regions” (partitioned into interim + lifetime allocators) for SOF on Zephyr/ACE, along with a basic boot test and the required Kconfig/build/board configuration plumbing.

Changes:

Add vpage contiguous virtual-page allocator and vregion allocator built on top of it.
Wire new sources and configs into Zephyr build + Kconfig (including new virtual region attribute).
Add a Ztest-based boot test for vregion, and bump ACE board virtual region count.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
`zephyr/test/vregion.c`	Adds a basic `vregion` allocator test under boot tests.
`zephyr/test/CMakeLists.txt`	Builds the new test when `CONFIG_SOF_VREGIONS` is enabled.
`zephyr/lib/vregion.c`	Implements `vregion` (interim `k_heap` + lifetime linear allocator).
`zephyr/lib/vpages.c`	Implements `vpage` allocator and registers its init via `SYS_INIT`.
`zephyr/lib/alloc.c`	Adjusts virtual heap bundle sizing/comments to match new region sizing.
`zephyr/include/sof/lib/vregion.h`	Public API for `vregion`.
`zephyr/include/sof/lib/vpages.h`	Public API for `vpage`.
`zephyr/include/sof/lib/regions_mm.h`	Adds `VIRTUAL_REGION_VPAGES_ATTR`.
`zephyr/Kconfig`	Adds `SOF_VREGIONS`, `SOF_VPAGE_MAX_ALLOCS`, `SOF_ZEPHYR_VIRTUAL_PAGE_REGION_SIZE`, and updates heap region sizing defaults/wording.
`zephyr/CMakeLists.txt`	Builds `vpages.c`/`vregion.c` when `CONFIG_SOF_VREGIONS` is enabled.
`app/boards/intel_adsp_ace40_nvls.conf`	Increases virtual region count to 3 for VPAGES region.
`app/boards/intel_adsp_ace40_nvl.conf`	Increases virtual region count to 3 for VPAGES region.
`app/boards/intel_adsp_ace30_wcl.conf`	Increases virtual region count to 3 for VPAGES region.
`app/boards/intel_adsp_ace30_ptl.conf`	Increases virtual region count to 3 for VPAGES region.
`app/boards/intel_adsp_ace20_lnl.conf`	Increases virtual region count to 3 for VPAGES region.
`app/boards/intel_adsp_ace15_mtpm.conf`	Increases virtual region count to 3 for VPAGES region.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-20T14:49:05Z

zephyr/include/sof/lib/vpages.h

+/**
+ * @brief Initialize virtual page allocator
+ *
+ * Initializes a virtual page allocator that manages a virtual memory region
+ * using a page table and block structures.
+ *
+ * @retval 0 if successful.
+ * @retval -ENOMEM on creation failure.
+ */
+int vpage_init(void);
+


vpages.h declares int vpage_init(void);, but zephyr/lib/vpages.c only defines init_vpages() as a SYS_INIT hook and does not provide a vpage_init() symbol. This makes the public header/API inconsistent and will cause link failures for any caller. Either add an exported vpage_init() wrapper or remove the declaration if initialization is exclusively via SYS_INIT.

Suggested change

/**

* @brief Initialize virtual page allocator

*

* Initializes a virtual page allocator that manages a virtual memory region

* using a page table and block structures.

*

* @retval 0 if successful.

* @retval -ENOMEM on creation failure.

*/

int vpage_init(void);

#ifdef __cplusplus

extern "C" {

#endif

/**

* @brief Allocate virtual pages

* Allocates a specified number of contiguous virtual memory pages by mapping

* physical pages.

*

* @param[in] pages Number of 4kB pages to allocate.

*

* @return Pointer to the allocated virtual memory region, or NULL on failure.

*/

void *vpage_alloc(unsigned int pages);

/**

* @brief Free virtual pages

* Frees previously allocated virtual memory pages and unmaps them.

*

* @param[in] ptr Pointer to the memory pages to free.

*/

void vpage_free(void *ptr);

Copilot · 2026-03-20T14:49:06Z

zephyr/lib/vregion.c

+	if (!lifetime_size || !interim_size) {
+		LOG_ERR("error: invalid vregion lifetime size %d or interim size %d",
+			lifetime_size, interim_size);
+		return NULL;


vregion_create() logs lifetime_size/interim_size with %d, but these are size_t. This is a format mismatch that can cause build failures with -Wformat and incorrect output. Please use %zu (and similarly update other LOG_* size/used logs in this file).

Copilot · 2026-03-20T14:49:06Z

zephyr/lib/vregion.c

+	LOG_INF("new at base %p size %#x pages %d struct embedded at %p",
+		(void *)vr->base, total_size, pages, (void *)vr);
+	LOG_DBG(" interim size %#x at %p", interim_size, (void *)vr->interim.heap.heap.init_mem);
+	LOG_DBG(" lifetime size %#x at %p", lifetime_size, (void *)vr->lifetime.base);


The LOG_INF/LOG_DBG calls here print total_size/interim_size/lifetime_size using %#x, but these are size_t. On 64-bit builds this can truncate and can fail with -Wformat. Prefer %zu or %#zx for sizes.

Copilot · 2026-03-20T14:49:06Z

zephyr/lib/vregion.c

+	if (!alignment)
+		alignment = 4; /* default align 4 bytes */
+
+	switch (type) {
+	case VREGION_MEM_TYPE_INTERIM:
+		return interim_alloc(&vr->interim, size, alignment);
+	case VREGION_MEM_TYPE_LIFETIME:
+		return lifetime_alloc(&vr->lifetime, size, alignment);


vregion_alloc_align() accepts any non-zero alignment and passes it to k_heap_aligned_alloc() / uses it in ALIGN_UP(). Both typically require a power-of-two alignment; non-power-of-two values can assert or behave incorrectly. Consider validating alignment (power-of-two and at least sizeof(void *) if required) and returning NULL with an error log on invalid input.

Copilot · 2026-03-20T14:49:06Z

zephyr/lib/vregion.c

+	LOG_DBG("destroy %p size %#x pages %d", (void *)vr->base, vr->size, vr->pages);
+	LOG_DBG(" lifetime used %d free count %d", vr->lifetime.used, vr->lifetime.free_count);


vregion_destroy() logs vr->size and vr->lifetime.used with %#x/%d, but these are size_t. Please update these format strings to the correct size_t specifiers to avoid -Wformat issues.

Copilot · 2026-03-20T14:49:07Z

zephyr/lib/vpages.c

+	/* 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) {
+		if (region->attr == VIRTUAL_REGION_VPAGES_ATTR) {
+			vpage_ctx.virtual_region = region;
+			break;
+		}
+	}
+	sys_mm_drv_query_memory_regions_free(virtual_memory_regions);
+
+	/* check for a valid region */
+	if (!vpage_ctx.virtual_region) {
+		LOG_ERR("error: no valid virtual region found");
+		k_panic();
+	}
+
+	block_count = region->size / CONFIG_MM_DRV_PAGE_SIZE;
+	if (block_count == 0) {
+		LOG_ERR("error: virtual region too small %d", region->size);
+		k_panic();


init_vpages() stores vpage_ctx.virtual_region = region from sys_mm_drv_query_memory_regions(), then calls sys_mm_drv_query_memory_regions_free(). If the returned list owns the region storage (as the _free API suggests), vpage_ctx.virtual_region and the subsequent region->size access become dangling/use-after-free. Consider either keeping virtual_memory_regions for the allocator lifetime, or copying the found sys_mm_drv_region into persistent storage and using vpage_ctx.virtual_region (not the iterator variable) afterwards.

Copilot · 2026-03-20T14:49:07Z

zephyr/lib/vpages.c

+	/* find the allocation element */
+	for (int i = 0; i < VPAGE_MAX_ALLOCS; i++) {
+		if (vpage_ctx.velems[i].pages > 0 && vpage_ctx.velems[i].vpage == alloc_idx) {
+			pages = vpage_ctx.velems[i].pages;
+
+			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);
+
+			/* clear the element */
+			vpage_ctx.velems[i].pages = 0;
+			vpage_ctx.velems[i].vpage = 0;
+			vpage_ctx.num_elems_in_use--;
+			break;
+		}
+	}
+
+	/* check we found allocation element */
+	if (pages == 0) {
+		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);
+	if (ret < 0) {
+		LOG_ERR("error: failed to unmap virtual region %p pages %u, error %d",
+			ptr, pages, ret);
+		return ret;
+	}


vpages_free_and_unmap() clears the allocation tracking element (and decrements num_elems_in_use) before unmapping/freeing. If sys_mm_drv_unmap_region() or sys_mem_blocks_free_contiguous() fails, the allocator will have lost the bookkeeping for a still-allocated range, making the allocation unfreeable and corrupting accounting. Update bookkeeping only after the unmap+free operations succeed (or restore it on failure).

Copilot · 2026-03-20T14:49:08Z

zephyr/lib/vpages.c

+	if (ret < 0)
+		LOG_ERR("vpage_alloc failed %d for %d pages, total %d free %d",
+			ret, pages, vpage_ctx.total_pages, vpage_ctx.free_pages);
+	else
+		LOG_INF("vpage_alloc ptr %p pages %u free %u/%u", ptr, pages, vpage_ctx.free_pages,
+			vpage_ctx.total_pages);


Several LOG_* calls here use %d for unsigned values (e.g. pages, total_pages/free_pages). This can trigger -Wformat warnings (often treated as errors) and prints incorrectly for large values. Please use the correct unsigned specifiers like %u.

Copilot · 2026-03-20T14:49:08Z

zephyr/lib/vpages.c

+	block_count = region->size / CONFIG_MM_DRV_PAGE_SIZE;
+	if (block_count == 0) {
+		LOG_ERR("error: virtual region too small %d", region->size);
+		k_panic();


init_vpages() logs and checks region->size using %d, but size is typically size_t. This is a format mismatch and can break builds with -Wformat. Use %zu/%#zx as appropriate and prefer vpage_ctx.virtual_region->size once the region is selected.

Copilot · 2026-03-20T14:49:08Z

zephyr/lib/vpages.c

+	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(&vpage_ctx.vpage_blocks, pages, &vaddr);


In vpages_alloc_and_map(), the error path after sys_mm_drv_map_region_safe() uses sys_mem_blocks_free(&vpage_ctx.vpage_blocks, pages, &vaddr). Since the allocation was done with sys_mem_blocks_alloc_contiguous(), this should be freed with the matching contiguous free API; otherwise the virtual blocks may remain allocated and leak.

Suggested change

sys_mem_blocks_free(&vpage_ctx.vpage_blocks, pages, &vaddr);

sys_mem_blocks_free_contiguous(&vpage_ctx.vpage_blocks, pages, &vaddr);

lrgirdwo and others added 3 commits March 19, 2026 15:43

test: add a vregion ztest

9aae1f6

Add a boot-time test for basic vregion functionality. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

lyakh requested a review from dabekjakub as a code owner March 20, 2026 14:39

Copilot AI review requested due to automatic review settings March 20, 2026 14:39

lyakh requested review from abonislawski, dbaluta, iuliana-prodan, kv2019i, lbetlej, lgirdwood, mmaka1 and plbossart as code owners March 20, 2026 14:39

Copilot started reviewing on behalf of lyakh March 20, 2026 14:42 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vpage / vregion: a minimal version with a test#10636

vpage / vregion: a minimal version with a test#10636
lyakh wants to merge 3 commits intothesofproject:mainfrom
lyakh:vreg

lyakh commented Mar 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-/**
- * @brief Initialize virtual page allocator
- *
- * Initializes a virtual page allocator that manages a virtual memory region
- * using a page table and block structures.
- *
- * @retval 0 if successful.
- * @retval -ENOMEM on creation failure.
- */
-int vpage_init(void);
+#ifdef __cplusplus
+extern "C" {
+#endif
+/**
+ * @brief Allocate virtual pages
+ * Allocates a specified number of contiguous virtual memory pages by mapping
+ * physical pages.
+ *
+ * @param[in] pages Number of 4kB pages to allocate.
+ *
+ * @return Pointer to the allocated virtual memory region, or NULL on failure.
+ */
+void *vpage_alloc(unsigned int pages);
+/**
+ * @brief Free virtual pages
+ * Frees previously allocated virtual memory pages and unmaps them.
+ *
+ * @param[in] ptr Pointer to the memory pages to free.
+ */
+void vpage_free(void *ptr);

		LOG_DBG("destroy %p size %#x pages %d", (void *)vr->base, vr->size, vr->pages);
		LOG_DBG(" lifetime used %d free count %d", vr->lifetime.used, vr->lifetime.free_count);

	sys_mem_blocks_free(&vpage_ctx.vpage_blocks, pages, &vaddr);
	sys_mem_blocks_free_contiguous(&vpage_ctx.vpage_blocks, pages, &vaddr);

Conversation

lyakh commented Mar 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants