schedule: add support for user-space LL scheduler#10508
schedule: add support for user-space LL scheduler#10508kv2019i wants to merge 6 commits intothesofproject:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds initial support for running SOF Low-Latency (LL) scheduler tasks in Zephyr user-space, providing memory protection and isolation between audio code and kernel resources.
Changes:
- Adds user-space LL scheduler support with dedicated memory domains and heap management
- Replaces spinlocks with mutexes for user-space compatibility
- Introduces test case to validate LL task creation and execution in user-space mode
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| zephyr/test/userspace/test_ll_task.c | New test case validating user-space LL scheduler functionality with task lifecycle management |
| zephyr/test/userspace/README.md | Documentation update describing new LL scheduler test |
| zephyr/test/CMakeLists.txt | Build configuration to include LL task test when CONFIG_SOF_USERSPACE_LL is enabled |
| zephyr/Kconfig | New CONFIG_SOF_USERSPACE_LL option for enabling user-space LL pipelines |
| src/schedule/zephyr_ll.c | Core LL scheduler implementation modified to support user-space execution with dynamic memory allocation |
| src/schedule/zephyr_domain.c | Domain thread management updated for user-space with mutex-based synchronization |
| src/schedule/Kconfig | Statistics logging disabled for user-space LL scheduler |
| src/init/init.c | Initialization hook for user-space LL resources |
| src/include/sof/schedule/ll_schedule_domain.h | Header updates exposing user-space LL APIs and mutex-based locking |
| src/include/sof/schedule/ll_schedule.h | API declarations for user-space LL heap and memory domain management |
| src/debug/telemetry/Kconfig | Telemetry disabled when user-space LL is enabled |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/schedule/zephyr_domain.c
Outdated
| ll_sch_domain_set_pdata(domain, zephyr_domain); | ||
|
|
||
| struct zephyr_domain_thread *dt = zephyr_domain->domain_thread + cpu_get_id(); |
There was a problem hiding this comment.
Variable declaration should be at the beginning of the function or block. Move this declaration to the top of the function for consistency with C89/C90 style if required by the project, or to improve readability.
| ll_sch_domain_set_pdata(domain, zephyr_domain); | |
| struct zephyr_domain_thread *dt = zephyr_domain->domain_thread + cpu_get_id(); | |
| struct zephyr_domain_thread *dt; | |
| ll_sch_domain_set_pdata(domain, zephyr_domain); | |
| dt = zephyr_domain->domain_thread + cpu_get_id(); |
There was a problem hiding this comment.
Zephyr and SOF allow C99 usage: https://docs.zephyrproject.org/latest/develop/languages/c/index.html
|
Example test run (on Intel PTL): |
| #define schedule_task_init_ll zephyr_ll_task_init | ||
|
|
||
| struct task *zephyr_ll_task_alloc(void); | ||
| k_tid_t zephyr_ll_get_thread(int core); |
There was a problem hiding this comment.
nitpick: I think we use struct k_thread * mostly in SOF and it seems to "work better" with various simulation / testing builds, I was getting "undefined" errors when I tried to use k_tid_t
There was a problem hiding this comment.
@lyakh Seems we have a mix in existing code as well. I will switch over a few places, but I won't start changing existing code away from k_tid_t in this PR.
src/schedule/zephyr_domain.c
Outdated
|
|
||
| #if CONFIG_SOF_USERSPACE_LL | ||
|
|
||
| k_tid_t zephyr_domain_thread_tid(struct ll_schedule_domain *domain) |
There was a problem hiding this comment.
This is a clearly a new addition, so let me change this in V2.
zephyr/test/userspace/test_ll_task.c
Outdated
| ZTEST(userspace_ll, ll_task_test) | ||
| { | ||
| ll_task_test(); | ||
| ztest_test_pass(); |
There was a problem hiding this comment.
I actually removed these from my tests, they're doing some long jumps... Are you sure you need this?
There was a problem hiding this comment.
Just following existing tests, but you are right, probably should remove.
I think this should be only used in specific case like: "" * However, if the success case for your test involves a fatal fault, you can call this function from k_sys_fatal_error_handler to indicate that the test passed before aborting the thread.""
|
|
||
| if (CONFIG_SOF_BOOT_TEST_STANDALONE AND CONFIG_SOF_USERSPACE_LL) | ||
| if(CONFIG_SOF_BOOT_TEST_STANDALONE AND CONFIG_SOF_USERSPACE_LL) | ||
| zephyr_library_sources(userspace/test_ll_task.c) |
There was a problem hiding this comment.
looks like aligning to use TABs instead would make the path smaller
There was a problem hiding this comment.
I don't really care either way, but this 2-space indent is the style used in both upstream Cmake and upstream Zephyr, so given the mix-of-style we currently have, I think we should just go with this.
softwarecki
left a comment
There was a problem hiding this comment.
First quick remarks. I still have 2 commits left to review... There is a lot of conditional code added here. Would it not be better to make this a separate scheduler? SOF already supports multiple different schedulers
|
|
||
| #if defined(__ZEPHYR__) && CONFIG_SOF_USERSPACE_LL | ||
| domain = sof_heap_alloc(zephyr_ll_heap(), SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT, | ||
| sizeof(*domain), sizeof(void *)); |
There was a problem hiding this comment.
Ack, right, there were a few ones where I'm replacing rzalloc. I'll address in V2.
There was a problem hiding this comment.
looks like memset() still isn't there? I suppose it works in your tests, and many fields are indeed initialised individually below, but at least the.enabled` array isn't initialised?
There was a problem hiding this comment.
Doh, I fixed these in the .c files, but missed this one in the ll_schedule_domain.h. Fixed in V3.
src/schedule/zephyr_domain.c
Outdated
| (void *)mem_partition.start, | ||
| heap->heap.init_bytes); | ||
|
|
||
| mem_partition.start = (uintptr_t)sys_cache_uncached_ptr_get(heap->heap.init_mem); |
There was a problem hiding this comment.
Zephyr maps cached and non-cached addresses when the double map config is enabled. Maybe it is worth check it here?
src/schedule/zephyr_domain.c
Outdated
| zephyr_domain->timer = k_object_alloc(K_OBJ_TIMER); | ||
| if (!zephyr_domain->timer) { | ||
| tr_err(&ll_tr, "timer allocation failed"); | ||
| rfree(zephyr_domain); |
There was a problem hiding this comment.
heap_free(zephyr_ll_heap(), ...
There was a problem hiding this comment.
Thanks, fixed in V2.
src/schedule/zephyr_domain.c
Outdated
| /* Add zephyr_domain_ops to the memory domain for user thread access */ | ||
| struct k_mem_partition ops_partition; | ||
|
|
||
| ops_partition.start = (uintptr_t)&zephyr_domain_ops; |
There was a problem hiding this comment.
Partition size must be aligned to page size. Consider use APP_TASK_DATA in the zephyr_domain_ops declatarion.
There was a problem hiding this comment.
Added this in V2 @softwarecki . Hopefully this is ok. I tried to follow your usage in user DP code.
There was a problem hiding this comment.
@kv2019i now you label zephyr_domain_ops with APP_TASK_DATA which places it in a partition, which is used in user_memory_init_shared() to grant access to respective threads, but that function is only called when the "thread" DP variant is used for DP tasks. And AFAICS that partition isn't even page-aligned. So this doesn't look quite right to me.
There was a problem hiding this comment.
@lyakh @softwarecki Gah, I misunderstood how userspace_helper.h worked in this case. I assumed the 'common_partition' was registered on Zephyr side and I just need to tag with APP_TASK_DATA, but right, that is not the case.
And true, if/when APP_TASK_DATA works, I no longer need to add the partition here separately.
I refactored the code so that I reuse the common_partition management done in userspace_helper.h, and just add new logic to hook up the common partition to the LL domain. This is better as there is a lot of "ops" structures the LL threads is going to need access to (and are already covered by APP_TASK_ attributes).
Let's see if we have some static const data that we want to expose to LL user thread, but not to DP user threads. If we have any such cases, then we'd need new variant of APP_TASK_ that exposes more. But not sure why we'd hide any const data like this.
jsarha
left a comment
There was a problem hiding this comment.
Went through this once, but did not pick really almost anything that was not picked before. Still many places I do not fully understand. I gather there is a new version coming. I go through this again when its out.
src/schedule/zephyr_domain.c
Outdated
| k_thread_abort(&zephyr_domain->domain_thread[core].ll_thread); | ||
| if (zephyr_domain->domain_thread[core].ll_thread) { | ||
| k_thread_abort(zephyr_domain->domain_thread[core].ll_thread); | ||
| k_object_free(zephyr_domain->domain_thread[core].ll_thread); |
There was a problem hiding this comment.
Shouldn't it be:
#ifdef CONFIG_SOF_USERSPACE_LL
k_object_free(zephyr_domain->domain_thread[core].ll_thread);
else
rfree(zephyr_domain->domain_thread[core].ll_thread);
#endif
lgirdwood
left a comment
There was a problem hiding this comment.
Only some minor things from me.
| config SOF_TELEMETRY | ||
| bool "enable telemetry" | ||
| default n | ||
| depends on !SOF_USERSPACE_LL |
There was a problem hiding this comment.
I assume here we still need to map this page with timer IO for user as RO ?
There was a problem hiding this comment.
@lgirdwood Not sure I understand connection with timer IO. Telemetry does direct writes to the debug window in shared memory and these are writes are down from various call sites in SOF codebase. Mapping the debug window to ll thread is one option, but given this is optional (and not really used in Linux), this is something we can easily tackle later if needed.
There was a problem hiding this comment.
I think we need the addition of the Kconfig entry at least together with this patch, not after it. Even if kconfig doesn't complain and just takes a non-existing option as false, I'd still not do that
There was a problem hiding this comment.
Ack, I'll moved it later in the series. Will be in V3. Doesn't have to be in same commit as SOF_USERSPACE_LL is not yet used and is disabled by default.
| if (!IS_ENABLED(CONFIG_SOF_USERSPACE_LL) || !dt->ll_thread) { | ||
| /* Allocate thread structure dynamically */ | ||
| #if CONFIG_SOF_USERSPACE_LL | ||
| dt->ll_thread = k_object_alloc(K_OBJ_THREAD); |
There was a problem hiding this comment.
Would an object not equally work for LL kernel ? i.e. do we always need to differentiate here ?
There was a problem hiding this comment.
@lgirdwood That's an interesting point. I actually started moving all builds to use k_object_alloc(), but then realized this interface is not available if CONFIG_USERSPACE is not set. So e.g. with Intel PTL, we could use this for all builds, but in build targets where userspace is not used, it won't work. So at least for now, I think we need to differentiate, but there is certainly potential to converge.
The main technical need is to register the objects to Zephyr kernel object database. We need this so we can grant access to the object to user threads.
| struct ll_schedule_domain *ll_domain; /* scheduling domain */ | ||
| unsigned int core; /* core ID of this instance */ | ||
| #if CONFIG_SOF_USERSPACE_LL | ||
| struct k_mutex *lock; /* mutex for userspace */ |
There was a problem hiding this comment.
Same question re differentiation, would a mutex work here too for a kernel thread in this use case ? Not a blocker or anything, it would be nice at some point to merge some of these flows around locking since at the end of the day we are using threads in both kernel/user mode.
There was a problem hiding this comment.
Ack @lgirdwood , I do think there is potential to converge much more. We are running both IPC handling and the LL tasks in threads (not in ISRs), so I don't really think we need to use spinlocks even in kernel builds. So in theory we should be able to use same locking code and when run in user-space, the lock/unlock calls are just trapped as system calls. But now, as LL user is still a developing feature, I want to start keeping modifications to current LL kernel scheduler to a minimum. So continuing on this track with V2.
|
|
||
| if (CONFIG_SOF_BOOT_TEST_STANDALONE AND CONFIG_SOF_USERSPACE_LL) | ||
| if(CONFIG_SOF_BOOT_TEST_STANDALONE AND CONFIG_SOF_USERSPACE_LL) | ||
| zephyr_library_sources(userspace/test_ll_task.c) |
kv2019i
left a comment
There was a problem hiding this comment.
Thanks a lot of the reviews! I answer most comment inline, but no V2 uploaded. I still have a few comments from @softwarecki and @lyakh I need to cover before uploading V2.
@softwarecki I did look at the option to move code to a separate scheduler file. Especially in zephyr_domain.c, this would bring benefit can keep code readability. OTOH, most of the code is still shared and it does look possible we can converge the kernel/user implementations more down the road. I did now implement a bit of a compromise solution where I split out some user-ll specific functions to a separate file, and implemented separate domain register/unregister functions for user/kernel builds. This will make it easier to see I'm not modifying the existing default kernel LL implementation, while still reusing most of the common code. I'll tidy up opens tomorrow and push a V2 for comments.
| #define scheduler_init_ll zephyr_ll_scheduler_init | ||
| #define schedule_task_init_ll zephyr_ll_task_init | ||
|
|
||
| struct task *zephyr_ll_task_alloc(void); |
There was a problem hiding this comment.
Ack, will fix in V2.
| #define schedule_task_init_ll zephyr_ll_task_init | ||
|
|
||
| struct task *zephyr_ll_task_alloc(void); | ||
| k_tid_t zephyr_ll_get_thread(int core); |
There was a problem hiding this comment.
@lyakh Seems we have a mix in existing code as well. I will switch over a few places, but I won't start changing existing code away from k_tid_t in this PR.
|
|
||
| #if defined(__ZEPHYR__) && CONFIG_SOF_USERSPACE_LL | ||
| domain = sof_heap_alloc(zephyr_ll_heap(), SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT, | ||
| sizeof(*domain), sizeof(void *)); |
There was a problem hiding this comment.
Ack, right, there were a few ones where I'm replacing rzalloc. I'll address in V2.
|
|
||
| if (CONFIG_SOF_BOOT_TEST_STANDALONE AND CONFIG_SOF_USERSPACE_LL) | ||
| if(CONFIG_SOF_BOOT_TEST_STANDALONE AND CONFIG_SOF_USERSPACE_LL) | ||
| zephyr_library_sources(userspace/test_ll_task.c) |
There was a problem hiding this comment.
I don't really care either way, but this 2-space indent is the style used in both upstream Cmake and upstream Zephyr, so given the mix-of-style we currently have, I think we should just go with this.
zephyr/test/userspace/test_ll_task.c
Outdated
| ZTEST(userspace_ll, ll_task_test) | ||
| { | ||
| ll_task_test(); | ||
| ztest_test_pass(); |
There was a problem hiding this comment.
Just following existing tests, but you are right, probably should remove.
I think this should be only used in specific case like: "" * However, if the success case for your test involves a fatal fault, you can call this function from k_sys_fatal_error_handler to indicate that the test passed before aborting the thread.""
e57df1a to
b91a460
Compare
|
V2 pushed:
|
kv2019i
left a comment
There was a problem hiding this comment.
Some additional inline replies to comments.
| config SOF_TELEMETRY | ||
| bool "enable telemetry" | ||
| default n | ||
| depends on !SOF_USERSPACE_LL |
There was a problem hiding this comment.
@lgirdwood Not sure I understand connection with timer IO. Telemetry does direct writes to the debug window in shared memory and these are writes are down from various call sites in SOF codebase. Mapping the debug window to ll thread is one option, but given this is optional (and not really used in Linux), this is something we can easily tackle later if needed.
| if (!IS_ENABLED(CONFIG_SOF_USERSPACE_LL) || !dt->ll_thread) { | ||
| /* Allocate thread structure dynamically */ | ||
| #if CONFIG_SOF_USERSPACE_LL | ||
| dt->ll_thread = k_object_alloc(K_OBJ_THREAD); |
There was a problem hiding this comment.
@lgirdwood That's an interesting point. I actually started moving all builds to use k_object_alloc(), but then realized this interface is not available if CONFIG_USERSPACE is not set. So e.g. with Intel PTL, we could use this for all builds, but in build targets where userspace is not used, it won't work. So at least for now, I think we need to differentiate, but there is certainly potential to converge.
The main technical need is to register the objects to Zephyr kernel object database. We need this so we can grant access to the object to user threads.
src/schedule/zephyr_domain.c
Outdated
| /* Add zephyr_domain_ops to the memory domain for user thread access */ | ||
| struct k_mem_partition ops_partition; | ||
|
|
||
| ops_partition.start = (uintptr_t)&zephyr_domain_ops; |
There was a problem hiding this comment.
Added this in V2 @softwarecki . Hopefully this is ok. I tried to follow your usage in user DP code.
| struct ll_schedule_domain *ll_domain; /* scheduling domain */ | ||
| unsigned int core; /* core ID of this instance */ | ||
| #if CONFIG_SOF_USERSPACE_LL | ||
| struct k_mutex *lock; /* mutex for userspace */ |
There was a problem hiding this comment.
Ack @lgirdwood , I do think there is potential to converge much more. We are running both IPC handling and the LL tasks in threads (not in ISRs), so I don't really think we need to use spinlocks even in kernel builds. So in theory we should be able to use same locking code and when run in user-space, the lock/unlock calls are just trapped as system calls. But now, as LL user is still a developing feature, I want to start keeping modifications to current LL kernel scheduler to a minimum. So continuing on this track with V2.
| #ifndef CONFIG_SOF_USERSPACE_LL | ||
| /* TODO: what to do with notifiers? */ | ||
| notifier_event(sch, NOTIFIER_ID_LL_POST_RUN, | ||
| NOTIFIER_TARGET_CORE_LOCAL, NULL, 0); |
There was a problem hiding this comment.
Ack @lyakh , added a TODO entry for now in V2 set.
| config SOF_TELEMETRY | ||
| bool "enable telemetry" | ||
| default n | ||
| depends on !SOF_USERSPACE_LL |
There was a problem hiding this comment.
I think we need the addition of the Kconfig entry at least together with this patch, not after it. Even if kconfig doesn't complain and just takes a non-existing option as false, I'd still not do that
|
|
||
| #if defined(__ZEPHYR__) && CONFIG_SOF_USERSPACE_LL | ||
| domain = sof_heap_alloc(zephyr_ll_heap(), SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT, | ||
| sizeof(*domain), sizeof(void *)); |
There was a problem hiding this comment.
looks like memset() still isn't there? I suppose it works in your tests, and many fields are indeed initialised individually below, but at least the.enabled` array isn't initialised?
src/schedule/zephyr_domain.c
Outdated
| zephyr_ll_mem_domain_add_thread(thread); | ||
| k_thread_access_grant(thread, dt->sem); | ||
| k_thread_access_grant(thread, domain->lock); | ||
| k_thread_access_grant(thread, zephyr_domain->timer); |
There was a problem hiding this comment.
please use just one k_thread_access_grant() with all the objects
There was a problem hiding this comment.
Heh, I kind of like separate calls for readbility, but right, no (other) reason to make multiple function calls. Changed in V3. (although, I will refactor access granting later, currently working on code to grant access to the various devices LL needs)
| if (!zephyr_domain) { | ||
| tr_err(&ll_tr, "domain allocation failed"); | ||
| rfree(domain); | ||
| k_panic(); |
There was a problem hiding this comment.
This is kernel space and k_panic() is more serious (than k_oops()) and that's the intention here. Keeping this.
| zephyr_domain->timer = k_object_alloc(K_OBJ_TIMER); | ||
| if (!zephyr_domain->timer) { | ||
| tr_err(&ll_tr, "timer allocation failed"); | ||
| k_panic(); |
There was a problem hiding this comment.
ditto, possibly at other locations too
There was a problem hiding this comment.
Ditto, in kernel and really serious so keeping k_panic().
src/schedule/zephyr_domain.c
Outdated
| /* Add zephyr_domain_ops to the memory domain for user thread access */ | ||
| struct k_mem_partition ops_partition; | ||
|
|
||
| ops_partition.start = (uintptr_t)&zephyr_domain_ops; |
There was a problem hiding this comment.
@kv2019i now you label zephyr_domain_ops with APP_TASK_DATA which places it in a partition, which is used in user_memory_init_shared() to grant access to respective threads, but that function is only called when the "thread" DP variant is used for DP tasks. And AFAICS that partition isn't even page-aligned. So this doesn't look quite right to me.
src/schedule/zephyr_ll_user.c
Outdated
| k_panic(); | ||
|
|
||
| mem_partition.start = (uintptr_t)sys_cache_uncached_ptr_get(heap->heap.init_mem); | ||
| res = zephyr_ll_mem_domain_add_partition(&mem_partition); |
There was a problem hiding this comment.
.attr shouldn't contain XTENSA_MMU_CACHED_WB for this
src/schedule/zephyr_ll_user.c
Outdated
| struct zephyr_ll_mem_resources { | ||
| struct k_mem_domain mem_domain; /**< Memory domain for LL thread isolation */ | ||
| struct k_heap *heap; /**< Heap allocator for LL scheduler memory */ | ||
| struct k_mutex lock; /**< Mutex protecting memory domain operations */ |
There was a problem hiding this comment.
paraphrasing: "locks should protect data, not 'operations'" ;-) Can we clarify what exactly it protects? That would help understanding / solidifying too IMHO.
There was a problem hiding this comment.
Excellent comment @lyakh . I assumed 'mem_domain' functions don't have their own locking, but alas, I was wrong. So in fact, I removed 'lock' completely and bunch of related wrappers. Not needed as the Zephyr k_kem_domain functions have their internal lock. Fixed in V3.
Userspace_helper.h provides infrastructure to mark SOF objects that should be accessible to all user threads (the "common_partition"). Add function user_memory_attach_common_partition() that allows to attach this partition to a specific domain. This is intended to use e.g. when moving LL tasks to user-space, where a single domain will be created at boot, and should have access to the common partition. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add option to build SOF with support for running LL scheduler in user-space. This commit adds initial support in the scheduler and does not yet allow to run full SOF application using the new scheduler configuration, but has enough functionality to run scheduler level tests. No functional change to default build configuration where LL scheduler is run in kernel mode, or to platforms with no userspace support. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
The telemetry infra is calling privileged timer functions, so if the Low-Latency tasks are run in user-space, telemetry must be disabled. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
The load tracking for Low-Latency tasks depends on low-overhead access to cycle counter (e.g. CCOUNT on xtensa), which is not currently available from user-space tasks. Add a dependency to ensure the LL stats can only be enabled if LL tasks are run in kernel mode. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add a test case to run tasks with low-latency (LL) scheduler in user-space. The test does not yet use any audio pipeline functionality, but uses similar interfaces towards the SOF scheduler interface. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
There are multiple style variants used in SOF for CMakeLists.txt, but this file now contains multiple variants in the same file. Fix this and align style to Zephyr style (2 space for indent, no tabs, no space before opening brackets). Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i
left a comment
There was a problem hiding this comment.
About to push V3 soon. A lot of good comments.
|
|
||
| #if defined(__ZEPHYR__) && CONFIG_SOF_USERSPACE_LL | ||
| domain = sof_heap_alloc(zephyr_ll_heap(), SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT, | ||
| sizeof(*domain), sizeof(void *)); |
There was a problem hiding this comment.
Doh, I fixed these in the .c files, but missed this one in the ll_schedule_domain.h. Fixed in V3.
| config SOF_TELEMETRY | ||
| bool "enable telemetry" | ||
| default n | ||
| depends on !SOF_USERSPACE_LL |
There was a problem hiding this comment.
Ack, I'll moved it later in the series. Will be in V3. Doesn't have to be in same commit as SOF_USERSPACE_LL is not yet used and is disabled by default.
src/schedule/zephyr_domain.c
Outdated
| zephyr_ll_mem_domain_add_thread(thread); | ||
| k_thread_access_grant(thread, dt->sem); | ||
| k_thread_access_grant(thread, domain->lock); | ||
| k_thread_access_grant(thread, zephyr_domain->timer); |
There was a problem hiding this comment.
Heh, I kind of like separate calls for readbility, but right, no (other) reason to make multiple function calls. Changed in V3. (although, I will refactor access granting later, currently working on code to grant access to the various devices LL needs)
| if (!zephyr_domain) { | ||
| tr_err(&ll_tr, "domain allocation failed"); | ||
| rfree(domain); | ||
| k_panic(); |
There was a problem hiding this comment.
This is kernel space and k_panic() is more serious (than k_oops()) and that's the intention here. Keeping this.
| zephyr_domain->timer = k_object_alloc(K_OBJ_TIMER); | ||
| if (!zephyr_domain->timer) { | ||
| tr_err(&ll_tr, "timer allocation failed"); | ||
| k_panic(); |
There was a problem hiding this comment.
Ditto, in kernel and really serious so keeping k_panic().
src/schedule/zephyr_ll_user.c
Outdated
| struct zephyr_ll_mem_resources { | ||
| struct k_mem_domain mem_domain; /**< Memory domain for LL thread isolation */ | ||
| struct k_heap *heap; /**< Heap allocator for LL scheduler memory */ | ||
| struct k_mutex lock; /**< Mutex protecting memory domain operations */ |
There was a problem hiding this comment.
Excellent comment @lyakh . I assumed 'mem_domain' functions don't have their own locking, but alas, I was wrong. So in fact, I removed 'lock' completely and bunch of related wrappers. Not needed as the Zephyr k_kem_domain functions have their internal lock. Fixed in V3.
src/schedule/zephyr_ll_user.c
Outdated
| k_panic(); | ||
|
|
||
| mem_partition.start = (uintptr_t)sys_cache_uncached_ptr_get(heap->heap.init_mem); | ||
| res = zephyr_ll_mem_domain_add_partition(&mem_partition); |
src/schedule/zephyr_domain.c
Outdated
| /* Add zephyr_domain_ops to the memory domain for user thread access */ | ||
| struct k_mem_partition ops_partition; | ||
|
|
||
| ops_partition.start = (uintptr_t)&zephyr_domain_ops; |
There was a problem hiding this comment.
@lyakh @softwarecki Gah, I misunderstood how userspace_helper.h worked in this case. I assumed the 'common_partition' was registered on Zephyr side and I just need to tag with APP_TASK_DATA, but right, that is not the case.
And true, if/when APP_TASK_DATA works, I no longer need to add the partition here separately.
I refactored the code so that I reuse the common_partition management done in userspace_helper.h, and just add new logic to hook up the common partition to the LL domain. This is better as there is a lot of "ops" structures the LL threads is going to need access to (and are already covered by APP_TASK_ attributes).
Let's see if we have some static const data that we want to expose to LL user thread, but not to DP user threads. If we have any such cases, then we'd need new variant of APP_TASK_ that exposes more. But not sure why we'd hide any const data like this.
| { | ||
| #if CONFIG_SOF_USERSPACE_LL | ||
| k_mutex_lock(sch->lock, K_FOREVER); | ||
| #else |
There was a problem hiding this comment.
I'd go with this. We will anyways not be able to disable irqs from a user thread, so if we have recursive locking, it's going to be a problem anyways.
b91a460 to
6531ead
Compare
|
V3 pushed:
|
Series that starts building up support for running SOF LL tasks in user-space (on platforms supporting Zephyr user-space). We already have support for DP tasks, so with both LL and DP supported, in theory all audio can be moved to user-space and run in separate memory space. This will isolate audio code from direct hardware access, protect kernel memory and device driver state.
This PR contains initial support for LL scheduler and adds a separate test case to mimic usage of SOF audio pipeline, without yet bringing in any audio dependencies.