[DRAFT] userspace LL/audio test PR (latest ver: V24b)#10558
Conversation
|
Running the pipeline_two_components_user test added in this PR, on a Intel PTL, looks like this: |
9cdb1be to
37b0412
Compare
|
V2 snapshot pushed:
|
37b0412 to
7317b18
Compare
|
V3 snapshot pushed:
|
|
Test output for the new test added in V3: |
| /* works? yes */ | ||
| //return 0; | ||
|
|
||
| printk("ipc %p\n", ipc); |
There was a problem hiding this comment.
Oops, these shouldn't be here. :)
|
|
||
| /* create the pipeline */ | ||
| pipe = pipeline_new(NULL, pipe_desc->primary.r.instance_id, | ||
| pipe = pipeline_new(heap, pipe_desc->primary.r.instance_id, |
There was a problem hiding this comment.
@lyakh @jsarha @lgirdwood This is where you'd plug in the vregions stuff to pass a separate heap to each pipe (based on topology description of its needs). In this series, as a placeholder, I use the zephyr_ll_user_heap() instead.
There was a problem hiding this comment.
right, maybe put a comment there for now to make re-discovery easier
lyakh
left a comment
There was a problem hiding this comment.
last reviewed commit so far "schedule: zephyr_ll: implement thread_init/free domain ops"
| #ifdef CONFIG_SOF_USERSPACE_LL | ||
| void comp_grant_access_to_thread(const struct comp_dev *dev, struct k_thread *th) | ||
| { | ||
| assert(dev->list_mutex); |
There was a problem hiding this comment.
description a bit confusing - this is only granting access to a mutex. Also list_mutex is only added to comp_dev in the next commit.
| } | ||
|
|
||
| stream_addr = rballoc_align(flags, size, align); | ||
| stream_addr = sof_heap_alloc(heap, flags, size, align); |
There was a problem hiding this comment.
the commit, that is mentioned in the commit message, only moved buffer context objects to particular heaps. This commit moves actual data buffers to them too, which is different and (arguably) more risky
| #define HDA_DMA_BUFFER_PERIOD_COUNT 4 | ||
|
|
||
| SHARED_DATA struct sof_dma dma[] = { | ||
| APP_TASK_DATA SHARED_DATA struct sof_dma dma[] = { |
There was a problem hiding this comment.
do I understand correctly, that this kind of userspace access makes that data writable to userspace?
| comp_err(dev, "requested channel %d is busy", hda_chan); | ||
| return -ENODEV; | ||
| } | ||
| hd->chan = &hd->dma->chan[channel]; |
There was a problem hiding this comment.
is this also not needed for the legacy mode? Also below
| uint64_t next_sync; | ||
| uint64_t period_in_cycles; | ||
| #endif | ||
| struct k_heap *heap; |
There was a problem hiding this comment.
wasn't this already referenced in the previous commit?
|
|
||
| k_spinlock_init(&dd->dai->lock); | ||
| #ifdef CONFIG_SOF_USERSPACE_LL | ||
| dd->dai->lock = k_object_alloc(K_OBJ_MUTEX); |
There was a problem hiding this comment.
check for NULL? Possibly in other locations too
| k_mutex_lock(dai->lock, K_FOREVER); | ||
| props = dai_get_properties(dai->dev, direction, stream_id); | ||
| hs_id = props->dma_hs_id; | ||
| ret = dai_get_properties_copy(dai->dev, direction, stream_id, &props); |
There was a problem hiding this comment.
I'm guessing this is made a syscall in one of the commits
| mod_heap = &mod_heap_user->heap; | ||
| } else { | ||
| #ifdef CONFIG_SOF_USERSPACE_LL | ||
| mod_heap = zephyr_ll_user_heap(); |
There was a problem hiding this comment.
looks good, but this else is entered under multiple conditions, might need to double-check
| .schedule_task_before = zephyr_ll_task_schedule_before, | ||
| .schedule_task_after = zephyr_ll_task_schedule_after, | ||
| .schedule_task_free = zephyr_ll_task_free, | ||
| .schedule_task_free = zephyr_ll_task_sched_free, |
There was a problem hiding this comment.
let's "spend" 3 more characters and make it ..._schedule_free()
| return -ENOMEM; | ||
| } | ||
| tr_err(&ll_tr, "Failed to allocate thread object for core %d", core); | ||
| dt->handler = NULL; |
| { | ||
| const struct sof_man_fw_desc *desc = basefw_vendor_get_manifest(); | ||
| const struct sof_man_module *mod; | ||
| uint32_t i; |
| uint32_t i; | ||
|
|
||
| if (!desc) | ||
| return -1; |
| return (int)i; | ||
| } | ||
|
|
||
| return -1; |
| union ipc4_connector_node_id node_id; | ||
| uint32_t dma_buffer_size; | ||
| uint32_t config_length; | ||
| } __packed __aligned(4); |
There was a problem hiding this comment.
does __aligned actually make sense in a type definition?
| pipe_msg.extension.dat = ipc_user->ipc_msg_ext; | ||
|
|
||
| /* Execute pipeline creation in user context */ | ||
| ipc_user->result = ipc_pipeline_new(ipc_user->ipc, (ipc_pipe_new *)&pipe_msg); |
There was a problem hiding this comment.
that's brave! ;-) I'd put a huge "TODO" here to make sure not to ship this by chance :-)
|
|
||
| /* create the pipeline */ | ||
| pipe = pipeline_new(NULL, pipe_desc->primary.r.instance_id, | ||
| pipe = pipeline_new(heap, pipe_desc->primary.r.instance_id, |
There was a problem hiding this comment.
right, maybe put a comment there for now to make re-discovery easier
7317b18 to
c029d49
Compare
|
V4 snapshot pushed:
|
|
Example output with V4 patchset: |
c029d49 to
9c778e4
Compare
|
V5 pushed:
|
9c778e4 to
c4b127f
Compare
|
V6 pushed:
|
c4b127f to
ad359d6
Compare
|
V7 submitted:
|
ad359d6 to
c7e9a37
Compare
|
V8 submitted:
|
c7e9a37 to
8af1be4
Compare
|
V9 submitted:
|
|
V10:
|
aa5efe0 to
35a61b6
Compare
|
V11 pushed:
|
35a61b6 to
214c051
Compare
|
V12 pushed:
|
Make comp_drivers_get() usable from user-space when SOF is built with CONFIG_SOF_USERSPACE_LL. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add CONFIG_SOF_USERSPACE_LL support to chain DMA component: - Allocate comp_dev, private data, and DMA buffer from user heap - Use k_work_delayable for periodic task instead of LL timer scheduler to keep DMA operations in kernel context - Guard all changes with #ifdef CONFIG_SOF_USERSPACE_LL Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Use a module context instead of heap to allocate objects in chain-dma module. This is required to support vregion use. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Commit f515f3b kept chain DMA in kernel context for CONFIG_SOF_USERSPACE_LL by driving chain_task_run() from a k_work_delayable on the system workqueue, because chain DMA used the raw Zephyr DMA API (dma_get_status/dma_reload/dma_start/dma_stop) which is not callable from an unprivileged user thread. Running a ~1ms (500us on AMD) audio DMA cadence on the low-priority system workqueue, rescheduled completion-relative via k_work_reschedule(), is prone to preemption and drift and risks xruns. Port chain DMA to SOF's sof_dma_* syscall wrappers (as host and dai already do) so it can run unprivileged, and schedule it as a normal SOF_SCHEDULE_LL_TIMER task on the user LL thread. This removes the kernel-workqueue path entirely; both userspace and non-userspace builds now share the LL scheduler path with scheduler-managed task lifecycle. Channel handles are stored as integer indices instead of kernel-only struct dma_chan_data pointers. The user-heap allocations for the component, private data and DMA buffer are retained so the user thread can access them. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
If CONFIG_SOF_BOOT_TEST_STANDALONE is set, ipc_init() is terminated early. This ensures SOF will not start to generate or respond to IPC messages that could potentially interfere with standalone test cases (some of which send and receive IPCs). The current implementation leaves the component list uninitialized and this can cause trouble to standalone tests that want to utilzie common IPC code to build messages. Fix this problem by executing more of ipc_init() also in the standalone mode. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
When CONFIG_SOF_USERSPACE_LL is enabled the IPC context must live in user-accessible application memory rather than being allocated from the kernel heap and reached via sof->ipc. Place struct ipc in a dedicated K_APP_BMEM partition and provide an out-of-line ipc_get() returning it. Add the user-space bookkeeping fields to struct ipc (ipc_user_pdata and ll_alloc) and the struct ipc_user descriptor later used to forward commands to a user thread. Rework ipc_init() to set up ll_alloc from the LL user heap and allocate the IPC objects accordingly. A no-op ipc_user_init() stub is added here and implemented later. This also makes the tree build with CONFIG_SOF_USERSPACE_LL again, as chain_dma.c already references ipc_get()->ll_alloc. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
…stic Extract the per-message processing logic for module CONFIG_GET/SET and LARGE_CONFIG_GET/SET into standalone functions that return their results via output parameters instead of writing the global msg_reply directly, and give SET_PIPELINE_STATE external linkage. This is a pure refactor with no functional change. The new ipc4_process_module_config(), ipc4_process_large_config_get(), ipc4_process_large_config_set() and ipc4_set_pipeline_state() can be called from any execution context, which is needed to later dispatch these messages from a separate IPC user-space thread. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
The IPC reply and compound-message completion entry points need to be callable from the IPC user-space thread. Declare ipc_msg_reply(), ipc_compound_pre_start(), ipc_compound_post_start() and ipc_wait_for_compound_msg() as Zephyr syscalls when CONFIG_SOF_USERSPACE_LL is enabled, renaming the implementations to z_impl_* and adding z_vrfy_* verification wrappers. A new ipc_reply.h carries the ipc_msg_reply declaration so it can be pulled in as a syscall header. Register the new syscall headers in CMakeLists.txt. No functional change for non-userspace builds, where the functions keep direct external linkage via z_impl_* defines. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
The pipeline, component and chain-DMA bookkeeping objects must be reachable from the user-space IPC thread, so allocate them from the system user heap via sof_heap_alloc()/sof_heap_free() instead of the generic rzalloc()/rfree(). As sof_heap_alloc() does not zero the allocation, add an explicit memset() to preserve the previous behaviour. Give ipc4_add_comp_dev() external linkage (declared in topology.h) so the user thread can register a created component, and skip the buffer tr_ctx copy for user-space LL builds where the kernel tr_ctx is not mapped. No functional change for non-userspace builds. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add a dedicated user-space thread for handling IPC commands that operate on audio pipelines when CONFIG_SOF_USERSPACE_LL is enabled. The kernel IPC handler forwards IPC4 messages to the user thread via k_event signaling and collects the result through a k_sem handshake. The IPC task_mask IPC_TASK_IN_THREAD bit prevents host completion until the user thread finishes. Component creation (drv->ops.create) runs in the user thread so untrusted module code does not execute with kernel privileges; the kernel resolves the driver and copies it into a user-readable buffer first. HOSTBOX partitions are mapped into the user thread's memory domain for module init parameter reads. Current implementation only covers IPC4, but the generic infra can be extended to cover other major IPC protocol variants. Signed-off-by: Kai Vehmanen <kai.vehmanen@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>
Make ipc_msg_send() a Zephyr system call so audio processing modules running in user-space LL threads can queue IPC messages (e.g. position updates, notifications) back to the host. The change takes effect only if CONFIG_SOF_USERSPACE_LL=y. Follows the same pattern used for ipc_msg_reply(): a dedicated header with __syscall declaration, z_impl/z_vrfy split, and syscall header registration in CMakeLists.txt. The verifier validates that the msg struct is writable (the implementation touches the list linkage) and that the data buffer, when provided, is readable up to msg->tx_size bytes. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com> (cherry picked from commit b25c15c)
Add an optional heap parameter to ipc_msg_w_ext_init() and ipc_msg_init() so callers can direct allocations to a specific heap. When the heap argument is NULL the existing rzalloc() path is used; when non-NULL, sof_heap_alloc()/sof_heap_free() are used instead. This allows IPC messages to be allocated from userspace-accessible heaps. For audio module contexts, introduce mod_ipc_msg_w_ext_init() and mod_ipc_msg_init() in generic.h. These use mod_zalloc()/ mod_free() for allocations that are automatically tracked and freed with the module lifecycle. ipc_msg_w_ext_init() is moved from a static inline in msg.h to a non-inline function in ipc-common.c due to the additional sof_heap_alloc dependency. Update all existing callers: - Module context callers (cadence, tdfb, sound_dose) use the new mod_ipc_msg_*() variants. - host-zephyr.c uses hd->heap, pipeline-graph.c uses the heap parameter from pipeline_new(). - Remaining kernel-context callers pass NULL for the default heap. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com> (cherry picked from commit 11a17d5)
Move user-facing notification functions (send_copier_gateway_xrun_notif_msg, send_gateway_xrun_notif_msg, send_mixer_underrun_notif_msg, send_process_data_error_notif_msg) to a new notification-user.c file so they can run in userspace. The send_resource_notif() function, which depends on the kernel-side notification pool and IPC message infrastructure, is converted to a Zephyr syscall. The implementation is renamed to z_impl_send_resource_notif() and remains in notification.c alongside is_notif_filtered_out() and ipc4_update_notification_mask(). The send_resource_notif() is converted to a system call only if CONFIG_SOF_USERSPACE_LL=y, without it the behaviour is same as befofe. A z_vrfy_send_resource_notif() handler is added to validate the user-provided data buffer and other parameters before forwarding to the kernel implementation. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com> (cherry picked from commit e385f10)
Add a new test to userspace_ll set that tests more of the functionality needed to run full audio pipelines in user-space. The test creates a pipeline with two components (IPC4 host and DAI copiers), does pipeline prepare, one copy cycle in prepared state and tears down the pipeline. One user-space thread is created to manage the pipelines. This would be equivalent to user IPC handler thread. Another user-space thread is created for the LL scheduler. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Place the pipeline position lookup table in the sysuser memory partition and replace k_spinlock with a dynamically allocated k_mutex when CONFIG_SOF_USERSPACE_LL is enabled. Spinlocks disable interrupts which is a privileged operation unavailable from user-mode threads. The mutex pointer is stored in a separate APP_SYSUSER_BSS variable outside the SHARED_DATA struct so Zephyr's kernel object tracking can recognize it for syscall verification. Move pipeline_posn_init() from task_main_start() to primary_core_init() before platform_init(), so the mutex is allocated before ipc_user_init() grants thread access to it. In pipeline_posn_get(), bypass the sof_get() kernel singleton and access the shared structure directly when running in user-space. Grant the ipc_user_init thread access to the pipeline position mutex via new pipeline_posn_grant_access() helper. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
…pace task zephyr_ll_task_sched_free() frees an active (RUNNING/RESCHEDULE) task by setting pdata->freeing and waiting on pdata->sem for the scheduler thread to remove the task from its run list before the memory is released. Under CONFIG_SOF_USERSPACE_LL this function runs in kernel context while pdata->sem is a sys_sem allocated on the user heap. sys_sem_take() returns -EINVAL immediately when called from kernel context, so the wait is a no-op: pdata is freed (and the struct task is subsequently freed by pipeline_free()) while the task is still linked in sch->tasks with n_tasks != 0 and the scheduling domain handler still set. Because n_tasks is non-zero, schedule_free() does not stop the LL timer, and the next timer tick runs zephyr_ll_run() over the dangling task, dereferencing freed memory and taking a load/store-privilege exception (EXCCAUSE 26) in the user-space LL thread. Stop relying on the cross-privilege semaphore handshake in this path. When the task must be waited for, mark it cancelled so that, should it actually be mid-execution on the scheduler's temporary list, it is removed via the cancel path without re-running task->run() on resources the caller may already have freed. If the task is still linked on the run list, the scheduler thread is provably not executing it (a running task is moved off sch->tasks with the lock dropped), so remove it directly and skip the wait. This guarantees the task is delisted (n_tasks -> 0, handler -> NULL) before pdata is freed, eliminating both the dangling list entry and the stray timer wakeups. Verified on PTL with the standalone user-space LL boot tests: the userspace_ll suite, including pipeline_two_components_user, now passes without the fatal exception at teardown. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
If SOF is built with CONFIG_SOF_USERSPACE_LL, the IPC user handled will require access to coldrodata sections to initialize audio modules. This logic is not required for LLEXT modules, which have existing code to add access to coldrodata (and other sections). This commit is needed for builds where LLEXT is not used. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
This is a set of temporary changes to audio code to remove calls to privileged interfaces that are not mandatory to run simple audio tests. These need proper solutions to be able to run all use-cases in user LL version. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
01ce2ff to
9b0dde8
Compare
|
V24b pushed:
|
SOF has recently gained ability to run DP (=preemptable audio tasks) in Zephyr user-space.
This PR is an early stage pull-request for changes to extend this capability to all of the audio pipeline code, and specifically the LL (low-latency) tasks.
This early stage as the design is not set in stone and the PR uses a number of short cuts in order to move (and tests) incrementally larger sets of audio functionality.