userspace: reduce fast-get scope and simplify preprocessor conditionals#10549
userspace: reduce fast-get scope and simplify preprocessor conditionals#10549lyakh wants to merge 2 commits intothesofproject:mainfrom
Conversation
Kernel threads have access to all the memory, no need to additionally grant access to them. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
fast-get is only needed when DRAM is used, i.e. when CONFIG_COLD_STORE_EXECUTE_DRAM is selectef. It's also unneeded in modules when CONFIG_LLEXT_TYPE_ELF_SHAREDLIB is used, i.e. in gcc builds, because also in those builds no .cold and .coldrodata sections are created. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
There was a problem hiding this comment.
Pull request overview
This pull request optimizes the fast-get functionality and simplifies preprocessor conditionals for userspace isolation in the SOF (Sound Open Firmware) Zephyr integration.
Changes:
- Restricts fast-get implementation compilation to when .cold/.coldrodata sections are actually created
- Adds inline stub functions for fast-get when the full implementation isn't needed
- Simplifies userspace preprocessor conditionals by moving them to call sites
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| zephyr/CMakeLists.txt | Conditionally compiles fast-get.c only when CONFIG_COLD_STORE_EXECUTE_DRAM is enabled |
| src/include/sof/lib/fast-get.h | Adds preprocessor logic to provide inline stub implementations when fast-get isn't needed |
| src/schedule/zephyr_dp_schedule_thread.c | Moves k_thread_access_grant and scheduler_dp_grant calls inside CONFIG_USERSPACE guard |
| src/schedule/zephyr_dp_schedule.c | Removes CONFIG_USERSPACE preprocessor guard from scheduler_dp_grant function body |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| add_subdirectory(../src/module module_unused_install/) | ||
|
|
||
| if(CONFIG_COLD_STORE_EXECUTE_DRAM) | ||
| zephyr_library_sources_ifdef(CONFIG_FAST_GET lib/fast-get.c) |
There was a problem hiding this comment.
The compilation of fast-get.c now depends on both CONFIG_COLD_STORE_EXECUTE_DRAM and CONFIG_FAST_GET being enabled. However, the header file (src/include/sof/lib/fast-get.h lines 15-17) declares the real fast_get/fast_put functions based on CONFIG_COLD_STORE_EXECUTE_DRAM and other conditions, without checking CONFIG_FAST_GET. This could lead to a linking error if CONFIG_COLD_STORE_EXECUTE_DRAM is enabled but CONFIG_FAST_GET is not, since the header would declare the functions but the implementation wouldn't be compiled. Consider either: 1) removing the CONFIG_FAST_GET check here if CONFIG_FAST_GET is always enabled when CONFIG_COLD_STORE_EXECUTE_DRAM is true, or 2) updating the header to also check CONFIG_FAST_GET.
| zephyr_library_sources_ifdef(CONFIG_FAST_GET lib/fast-get.c) | |
| zephyr_library_sources(lib/fast-get.c) |
two simple commits: