Debug stream text msg improvements#10845
Conversation
jsarha
requested review from
a team,
abonislawski,
dbaluta,
iuliana-prodan,
kv2019i,
lbetlej,
lgirdwood,
lyakh,
mmaka1 and
plbossart
as code owners
There was a problem hiding this comment.
Pull request overview
This PR improves debug stream text messaging for Zephyr-based SOF builds by making ds_send_text_record() callable from user space via a syscall and by exporting ds_msg() for use from module code.
Changes:
- Register
debug_stream_text_msg.hfor Zephyr syscall header generation. - Add
ds_send_text_record()API as a Zephyr syscall (whenCONFIG_USERSPACEis enabled) and introduceDS_TEXT_MSG_MAX_LEN. - Implement syscall plumbing (
z_impl_*/z_vrfy_*+ marshalling include) and exportds_msg().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
zephyr/CMakeLists.txt |
Adds the header to Zephyr’s syscall generation inputs. |
src/include/user/debug_stream_text_msg.h |
Introduces DS_TEXT_MSG_MAX_LEN and the ds_send_text_record() syscall/public prototype. |
src/debug/debug_stream/debug_stream_text_msg.c |
Implements the syscall handler + verifier and exports ds_msg(); refactors text record sending path. |
Export ds_msg() to allow using it in loadable modules. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
jsarha
force-pushed
the
debug_stream_text_msg_improvements
branch
2 times, most recently
from
d9364e5 to
59795e2
Compare
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Just a quick sanity check whether the simpler boilerplate could be used. Otherwise looks ready to go.
|
|
||
| #if defined(__ZEPHYR__) && defined(CONFIG_USERSPACE) | ||
| #include <zephyr/syscalls/debug_stream_slot.h> | ||
| #endif |
There was a problem hiding this comment.
Is this really called from any non-Zephyr build? If this is Zephyr only, you could use the simpler boilet plate of having __syscall declaration always and let Zephyr build system generate the boiletplate for userspace and non-userspace builds. We can't use this for syscalls that are only used in some user-space builds, but this seems generic enough, so the simpler boilerplate could work.
There was a problem hiding this comment.
@kv2019i Ok, what exactly are you suggesting? I searched for other syscall implementations, but all seem to follow this pattern, with minor variations.
There was a problem hiding this comment.
@jsarha I mean like "work/zephyr/include/zephyr/device.h", just mark with "__syscall" without any ifdefs.
In implementation file work/zephyr/kernel/device.c you have the z_impl_foo() always and only have the verification code under ifdefs. We can't do this for many of our added syscalls in SOF as they are not enabled fo rall user-space configs, but if something is enabled whenever CONFIG_USERSPACE is set, we can use the standard boilerplate.
There was a problem hiding this comment.
@kv2019i it does not work with CONFIG_USERSPACE=n without #define debug_stream_slot_send_record z_impl_debug_stream_slot_send_record:
/home/oku/xcc/install/tools/RI-2022.10-linux/XtensaTools/bin/xtensa-elf-ld: modules/sof/libmodules_sof.a(debug_stream_text_msg.c.obj):(.literal.ds_msg+0x0): undefined reference to `debug_stream_slot_send_record'
What am I missing?
There was a problem hiding this comment.
Oh, got it. I need to take from around:
#include <zephyr/syscalls/debug_stream_slot.h>
too.
Make debug_stream_slot_send_record() a Zephyr syscall. This allows user-space threads to send debug stream records directly. Rename the implementation to z_impl_debug_stream_slot_send_record() and add z_vrfy_debug_stream_slot_send_record() with K_SYSCALL_MEMORY_READ validation of the record buffer. Register the syscall header in CMakeLists.txt. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
jsarha
force-pushed
the
debug_stream_text_msg_improvements
branch
from
59795e2 to
ca2117b
Compare
Make ds_send_text_record() a syscall to make userspace debugging easier. And export ds_msg() to allow its usage in module code.