Skip to content

[userspace LL] vregion related syscalls - #11108

Open
lyakh wants to merge 8 commits into
thesofproject:mainfrom
lyakh:vreg
Open

[userspace LL] vregion related syscalls#11108
lyakh wants to merge 8 commits into
thesofproject:mainfrom
lyakh:vreg

Conversation

@lyakh

@lyakh lyakh commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

vregion system calls, needed when integrating userspace LL and DP
part of #10945

Copilot AI lite review requested due to automatic review settings August 20, 2026 14:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Zephyr userspace support for SOF “vregion” allocations to enable userspace LL + DP integration (part of #10945), including syscall plumbing and module-adapter wiring to map vregion memory into the LL userspace memory domain.

Changes:

  • Introduces Zephyr syscall handlers/marshalling for vregion alloc/free/get/put/set_interim.
  • Refactors Zephyr vregion implementation entrypoints to z_impl_* to back the new syscalls.
  • Extends module-adapter allocation flow to create/map/unmap vregions for DP modules and plumbs vregion start/size through mod_alloc_ctx.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
zephyr/syscall/vregion.c New syscall verification + marshalling includes for vregion APIs.
zephyr/lib/vregion.c Switches vregion APIs to z_impl_* entrypoints and adjusts symbol exports accordingly.
zephyr/Kconfig Adds SOF_USERSPACE_INTERFACE_VREGION and selects it from SOF_USERSPACE_LL.
zephyr/include/rtos/alloc.h Extends mod_alloc_ctx with vregion start/size metadata for domain mapping.
zephyr/CMakeLists.txt Adds syscall header generation and builds the new vregion syscall source.
src/include/sof/lib/vregion.h Marks vregion APIs as __syscall and includes generated syscall header.
src/include/sof/audio/module_adapter/module/generic.h Exposes module-adapter vregion map/unmap as syscalls for full Zephyr app.
src/audio/module_adapter/module_adapter.c Implements vregion creation + mem-domain partition mapping and adds syscall verifiers.
src/audio/buffers/comp_buffer.c Routes vregion-backed buffer free through the new shared vregion-free helper.
Suppressed comments (2)

src/audio/module_adapter/module_adapter.c:134

  • module_adapter_vreg_free() decrements the vregion refcount (and may free the vregion pages) before removing the user mem-domain partitions. If vregion_put() frees the pages, the user partition remains until module_adapter_vreg_unmap() runs, creating a window where freed (and potentially reallocated) pages stay user-accessible. Consider unmapping first and freeing the vregion atomically in kernel code when the refcount reaches 0.
void module_adapter_vreg_free(struct mod_alloc_ctx *alloc)
{
	if (vregion_put(alloc->vreg))
		return;

	module_adapter_vreg_unmap(alloc);

	sof_heap_free(alloc->heap, alloc);

src/audio/module_adapter/module_adapter.c:255

  • The syscall verifier for module_adapter_vreg_unmap() only checks that the alloc struct is readable. In userspace-LL configurations alloc lives in user-writable memory, so a caller can forge vreg_start/vreg_size and attempt to remove arbitrary partitions from zephyr_ll_mem_domain(). Add validation that alloc refers to an expected allocation and that (vreg_start,vreg_size) match the vregion’s actual mem_info (or avoid taking alloc from user-space entirely).
void z_vrfy_module_adapter_vreg_unmap(const struct mod_alloc_ctx *alloc)
{
	K_OOPS(K_SYSCALL_MEMORY_READ(alloc, sizeof(*alloc)));
	z_impl_module_adapter_vreg_unmap(alloc);
}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/audio/module_adapter/module_adapter.c Outdated
Comment thread src/audio/module_adapter/module_adapter.c Outdated
Comment thread zephyr/syscall/vregion.c Outdated
Comment thread src/include/sof/lib/vregion.h
Comment thread src/include/sof/audio/module_adapter/module/generic.h Outdated
Comment thread zephyr/CMakeLists.txt Outdated

@kv2019i kv2019i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check inline, concern with the syscall verify function.

Comment thread src/audio/module_adapter/module_adapter.c Outdated
@lyakh
lyakh force-pushed the vreg branch 2 times, most recently from 6f6c216 to c223bae Compare August 24, 2026 10:59
@lyakh
lyakh force-pushed the vreg branch 4 times, most recently from 9e4a135 to b15a16c Compare August 24, 2026 13:40

@kv2019i kv2019i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new check in vregion_verify() could be enough, please see comments inline. At least I'd add a comment how this protects against invalid/fabricated "vr" objects.

Comment thread src/audio/module_adapter/module_adapter.c Outdated
Comment thread zephyr/syscall/vregion.c Outdated
size_t vr_size = 0;
uintptr_t vr_start;

vregion_mem_info(vr, &vr_size, &vr_start);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here too we'd need to verify "vr" is a valid kernel vr object.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kv2019i this is your commit :-) I tried to modify it as little as possible. Checks are added in the next commit.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't claim error free output :)

Comment thread zephyr/syscall/vregion.c Outdated
Comment thread zephyr/lib/vregion.c
return false;

/* vregion instances must not be accessible to the userspace. */
K_OOPS(!K_SYSCALL_MEMORY_READ(vr, sizeof(*vr)));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok this could be potentially enough. I still wonder if this is secure enough. but this is definitely a fast check to make, versus looking up a list/array of all kernel vr objects.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kv2019i well, look at z_vrfy_mod_alloc_ext() - how secure do you find it?.. I suppose, yes, we need to add multiple Zephyr kernel object types.

scheduler_dp_task_init() currently only runs in privileged mode, add
a comment and a check for that.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
lyakh and others added 2 commits August 25, 2026 10:29
The entire user_access_to_mailbox() function is already under
an #ifdef CONFIG_SOF_USERSPACE_LL condition. Remove an additional
identical check inside the function.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Make vregion_alloc(), vregion_alloc_coherent(),
vregion_alloc_align(), vregion_alloc_coherent_align(),
and vregion_free() available as Zephyr system calls for
user-space threads.

Add K_SYSCALL_MEMORY_WRITE verification to all syscall
handlers to validate the calling thread has access to
the vregion's managed memory area.

Add CONFIG_SOF_USERSPACE_INTERFACE_VREGION Kconfig option
to control the feature. It is auto-selected by
SOF_USERSPACE_LL when SOF_VREGIONS is enabled.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
@lyakh
lyakh force-pushed the vreg branch 2 times, most recently from 8381e0b to 6f1c250 Compare August 25, 2026 09:51
Comment thread zephyr/syscall/vregion.c Outdated
size_t vr_size = 0;
uintptr_t vr_start;

vregion_mem_info(vr, &vr_size, &vr_start);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't claim error free output :)

Comment thread src/audio/module_adapter/module_adapter.c Outdated
Extract common syscall verification code into a function. Also add a
a check that the underlying metadata object is inaccessible to the
userspace context.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Comment thread src/include/sof/lib/vregion.h Outdated
void *vregion_alloc(struct vregion *vr, size_t size);
__syscall void *vregion_alloc(struct vregion *vr, size_t size);

void *z_impl_vregion_alloc(struct vregion *vr, size_t size);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a cost in adding syscalls? Couldn't we just have one vregion_alloc_align_ext(truct vregion *vr, size_t size, size_t alignment, bool coherent) syscall, and then inline functions calling this single syscall?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsarha adding syscalls is "free," but calling them from the userspace has a cost. The good news is that calling a syscall from the kernel mode is cheap - it resolves to just a thin wrapper around a function call. ATM I'm trying to make everything work with only a minor optimisation effort. I expect a lot of follow-up improvements and optimisations once the functionality is there.
That being said, this specific case is indeed a bit of an overkill and would work best as an inline function. If @kv2019i doesn't mind, we can change that in that his commit, or we can optimise it in a follow-up.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lyakh Good with both options, feel free to modify the commit directly.

vregion_get(), vregion_put() and vregion_set_interim() should also be
callable from the userspace. Make them syscalls. Also remove
redundant symbol exporting since the vregion API shouldn't be used
directly by LLEXT modules.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
@lyakh lyakh added the DNM Do Not Merge tag label Aug 27, 2026
@lyakh lyakh changed the title [userspace LL] vregion related syscalls [DNM][userspace LL] vregion related syscalls Aug 27, 2026
@lyakh

lyakh commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

marking "DNM" - let me refactor this a bit

@lyakh lyakh changed the title [DNM][userspace LL] vregion related syscalls [userspace LL] vregion related syscalls Aug 27, 2026
@lyakh lyakh removed the DNM Do Not Merge tag label Aug 27, 2026
@lyakh

lyakh commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

"DNM" removed

lyakh added 3 commits August 27, 2026 14:46
When userspace LL scheduling is enabled, some vregions have to be
accessible to the LL userspace domain. Add a new system call to
create vregions with such a mapping and add unmapping to freeing for
such mapped vregions.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
z_impl_* functions are declared in automatically generated Zephyr
headers, no need to declare them again.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Make vregion_alloc() and vregion_alloc_coherent() static inlines,
calling their respective aligned versions with zero alignment.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants