Debug stream debugfs #5154
Debug stream debugfs #5154
Conversation
jsarha
requested review from
dbaluta,
lgirdwood,
plbossart,
ranj063,
kv2019i,
bardliao and
ujfalusi
as code owners
sound/soc/sof/ipc4-debug-stream.c
Outdated
| if (!buf) | ||
| return -ENOMEM; | ||
|
|
||
| sof_mailbox_read(sdev, offset, buf, SOF_IPC4_DEBUG_SLOT_SIZE); |
There was a problem hiding this comment.
Would not make sense to skip the magic number as the telemetry does?
If so, then probably creating a single function to get the content of a debug slot by type and use that in both?
There was a problem hiding this comment.
Sorry @ujfalusi , I do not follow. What magic number you are referring to?
About unifying the telemetry and debug-stream code I agree on general level, but there is the exception part in the current telemetry code, that I do not fully comprehend. And then telemetry debugfs does not support seek and there are some other subtle differences. To unify the code I should know how telemetry debugfs file is used, so that I can verify it works with code borrowed from debug-stream -side. The other way around wont work, as my python debug-stream client disagrees with the telemetry debugfs implementation.
There was a problem hiding this comment.
@jsarha, the first u32 in a slot is the magic number or tag or slot type, it is SOF_IPC4_DEBUG_SLOT_DEBUG_STREAM in this case. For telemetry it is SOF_IPC4_DEBUG_SLOT_TELEMETRY.
The real data is after this magic number.
The sof_ipc4_query_exception_address() is just wrapping the sof_ipc4_find_debug_slot_offset_by_type() call and skips the magic number to get a pointer for the slot data.
The telemetry.c uses memcpy_fromio() instead sof_mailbox_read(), they do the same at the end.
If I look at the two files from a distance, they are 99% identical with the only difference is:
SOF_IPC4_DEBUG_SLOT_DEBUG_STREAM vs SOF_IPC4_DEBUG_SLOT_TELEMETRY
"debug_stream" vs "exception"
The only tricky part is how to know which slot you need to return, for which the debug-dsp-ops.c file gives an answer.
There was a problem hiding this comment.
Yes, I know how the debug slots work, and I understand the basic structure, but while I was implementing debug-stream, I started off with how telemetry debugfs was implemented, but needed to change couple of things to get it to work.
Only thing is, that I would like to test the telemetry still works, if I unify the debugfs behind a single function that follows the debug-stream implementation... but I have no idea how telemetry should be used and I can not find any client etc. But yes, maybe I just do it and hope the possible issues will come up in review.
There was a problem hiding this comment.
@jsarha, the first u32 in a slot is the magic number or tag or slot type, it is SOF_IPC4_DEBUG_SLOT_DEBUG_STREAM in this case. For telemetry it is SOF_IPC4_DEBUG_SLOT_TELEMETRY. The real data is after this magic number.
Oh, sorry @ujfalusi I read hastily. This is not how the debug slots work. The slot allocation is done in the descriptor slot:
And the whole allocated 4k slot can be used freely, no need to skip any words. And I rather not skip any words in the generic debug slot debufs mapping, just because its required in telemetry slot when its used for exception dumping.
Maybe I could parameterize it, thou that would look funny too. It would be really helpful to know how (with what tool?) the exception file is used. I'd rather skip the first word in the exception interpreting tool than in in the generic debug slot debugfs implementation.
There was a problem hiding this comment.
Right, yes, you are right. The telemetry slot itself can be split up to different sections and internally it is using 'separator magic' numbers for identification (in theory, but that's another story).
So, here, the ipc4-telemetry.c is exposing the 'exception' section of the telemetry slot, which I guess is the only one present when an exception happens and it will overtake the whole telemetry slot.
This is the reason why the first u32 is skipped I guess, it is a marker in the scope of the telemetry slot.
There was a problem hiding this comment.
@jsarha, what I would do is:
rename the ipc4-telemetry.c to ipc4-debugfs.c or something generic and use this to export the raw slots on the needed base.
You can have a single exported function as entry point, you can even move here the sof_ipc4_find_debug_slot_offset_by_type() function...
I know, mtrace is also uses debug slot, but it has been special cased out...
The ipc4-telemetry.h should remain as it is.
Adds sof_ipc4_create_debug_slot_debugfs_node() -function for mapping an SOF Intel ipc4 debug window slot as debugfs file. The actual slot is specified with slot_type parameter that are defined in include/sound/sof/ipc4/header.h and the slot is found with sof_ipc4_find_debug_slot_offset_by_type(). It also takes the data_offset parameter that specifies where the payload data in the slot begins. The portion that is mapped to the debugfs file is everything after the offset. The last parameter is the name of the file. Signed-off-by: Jyri Sarha <[email protected]>
Add new debug window slot type for debug-stream protocol. For details see src/debug/debug_stream/debug_stream_slot.h under SOF sources [1]. [1] https://github.com/thesofproject/sof Signed-off-by: Jyri Sarha <[email protected]>
Maps debug-stream debug window slot as debugfs file with sof_ipc4_create_debug_slot_debugfs_node() -function. Signed-off-by: Jyri Sarha <[email protected]>
… file Remove ipc4-telemetry.[ch] and use sof_ipc4_create_debug_slot_debugfs_node() instead for mapping "exception" debugfs file. In running system the SOF_IPC4_DEBUG_SLOT_TELEMETRY is used for telemetry data, but if configured, Zephyr exception data is dumped in the same debug window slot, right after the separator word. Signed-off-by: Jyri Sarha <[email protected]>
jsarha
force-pushed
the
debug-stream-debugfs
branch
from
672baba
to
68be79a
Compare
@ujfalusi I took a shot at this. I still try make sure the zephyr tool can still read the execption file. |
ok, we can park now and see how well existing tools work - if slow or inconsistent we can continue here. |
This commit simply maps the identified debug-stream debug window slot as a debugfs file.