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

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.

zephyr/include/sof/lib/vpages.h

zephyr/lib/vregion.c

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.

zephyr/lib/vregion.c

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.

zephyr/lib/vpages.c

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>

kv2019i

One staging related comment inline. Otherwise looks good and addition of a small test case is great.

zephyr/include/sof/lib/vpages.h

kv2019i · 2026-03-24T15:52:13Z

zephyr/Kconfig

 	hex "Size in bytes of virtual memory region for virtual heap shared for all cores"
 	depends on MM_DRV_INTEL_ADSP_MTL_TLB
-	default 0x100000
+	default 0xc0000


Any downsides to this change when done now? I mean this is enable the feature in many builds by default, but the vregions are not actually used yet. I wonder if these Kconfig changes should be done in the PR when vregion is actually used in pipelines?

@kv2019i it's used by the test!

@lyakh And test is run at boot...? Hmm, right, so maybe ok then. With my recent user-space PR, I've kept the new tests standalone so I don't have to enable new features in Kconfig by default yet. OTOH, the major downside is these tests are not run by any CI then....

softwarecki

Most of the comments in this review were carried over from the previous review. Unfortunately, the code was moved to a new PR, which required re-reviewing the entire change again. Again, the vpage_ctx.velems is a large array (SOF_VPAGE_MAX_ALLOCS = 128). It would be worth optimizing add and remove operations to avoid repeatedly scanning it linearly. Really please consider moving page mapping logic into vregion. For lifetime allocators, mapping pages progressively as needed could help reduce memory usage.

softwarecki · 2026-03-25T12:27:45Z

zephyr/lib/vpages.c

+	vpage_ctx.num_elems_in_use = 0;
+
+	/* bundles are uint32_t of bits */
+	bitmap_num_bundles = SOF_DIV_ROUND_UP(block_count, 32);


32 => sizeof(uint32_t)? Bellow you use sizeof(uint32_t).

softwarecki · 2026-03-25T12:30:02Z

zephyr/lib/vpages.c

+ * This allocator manages the allocation and deallocation of virtual memory pages from
+ * a predefined virtual memory region which is larger than the physical memory region.
+ *
+ * Both memory regions are divided into 4kB pages that are represented as blocks in a


should we really hard-specify the 4kB size?

softwarecki · 2026-03-25T12:35:29Z

zephyr/lib/vpages.c

+	for (i = 0; i < VPAGE_MAX_ALLOCS; i++) {
+		if (vpage_ctx.velems[i].pages == 0) {
+			vpage_ctx.velems[i].pages = pages;
+			vpage_ctx.velems[i].vpage =
+				(POINTER_TO_UINT(vaddr) -
+				 POINTER_TO_UINT(vpage_ctx.virtual_region->addr)) /
+				CONFIG_MM_DRV_PAGE_SIZE;
+			vpage_ctx.num_elems_in_use++;
+			break;
+		}
+	}


To eliminate the loop during page allocation, I suggest:

vpage_ctx.velems[page_context.num_elems_in_use].pages = pages; vpage_ctx.velems[page_context.num_elems_in_use].vpage = (POINTER_TO_UINT(vaddr) - POINTER_TO_UINT(vpage_ctx.virtual_region->addr)) / CONFIG_MM_DRV_PAGE_SIZE; vpage_ctx.num_elems_in_use++;

Especially since this array is large by default: SOF_VPAGE_MAX_ALLOCS = 128

softwarecki · 2026-03-25T12:38:43Z

zephyr/lib/vpages.c

+	if (i == VPAGE_MAX_ALLOCS) {
+		/*
+		 * The caller is holding a lock, so someone must have changed
+		 * vpages state without taking it
+		 */
+		LOG_ERR("data corruption, check for races");
+		return -EPROTO;
+	}


It looks like AI slop. We already check for allocation elements, so we don't expect to find no free entry in this array. Allocations are protected by mutex so there is no "someone".

softwarecki · 2026-03-25T12:41:10Z

zephyr/lib/vpages.c

+	/* check for valid ptr which must be page aligned */
+	if (!IS_ALIGNED(ptr, CONFIG_MM_DRV_PAGE_SIZE)) {
+		LOG_ERR("error: invalid non aligned page pointer %p", ptr);
+		return -EINVAL;


k_panic()? This seems like a serious problem and we should not allow it to leak memory.

softwarecki · 2026-03-25T16:50:37Z

zephyr/include/sof/lib/vregion.h

+ * Allocate aligned memory from the specified virtual region based on the memory type.
+ *
+ * @param[in] vr Pointer to the virtual region instance.
+ * @param[in] type Type of memory to allocate (static, dynamic, or shared static).


softwarecki · 2026-03-25T16:58:30Z

zephyr/lib/vregion.c

+};
+
+/* zephyr k_heap for interim allocations. TODO: make lockless for improved performance */
+struct zephyr_heap {


zephyr_heap? A vague and misleading name. It suggests a Zephyr origin.

softwarecki · 2026-03-25T17:02:19Z

zephyr/lib/vregion.c

+ * information about the base address, size, and allocation status
+ * of the region.
+ *
+ * Currently the virtual region memory region can be partitioned into two areas


one "region" too many?

softwarecki · 2026-03-25T17:11:27Z

zephyr/lib/vregion.c

+	if (align == CONFIG_DCACHE_LINE_SIZE)
+		size = ALIGN_UP(size, CONFIG_DCACHE_LINE_SIZE);
+
+	/* calculate new heap object size for object and alignments */


maybe the comment could be clarified?

softwarecki · 2026-03-25T17:11:59Z

zephyr/lib/vregion.c

+		return;
+	}
+
+	LOG_ERR("error: vregion free invalid pointer %p", ptr);


k_panic()?

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

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

test: add a vregion ztest

33b4313

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

lyakh force-pushed the vreg branch from 9aae1f6 to 33b4313 Compare March 24, 2026 14:54

kv2019i reviewed Mar 24, 2026

View reviewed changes

kv2019i approved these changes Mar 25, 2026

View reviewed changes

softwarecki reviewed Mar 25, 2026

View reviewed changes

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

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!

Uh oh!

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

softwarecki left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

softwarecki left a comment •

edited

Loading