-
Notifications
You must be signed in to change notification settings - Fork 350
Exception dump hook #10517
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Exception dump hook #10517
Conversation
Use spinlock instead of mutex to allow ISR context usage. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
b3c5f35 to
fd97e52
Compare
| * in cached memory. | ||
| * | ||
| * sys_cache_data_flush_range(&cpu_mutex[i], sizeof(cpu_mutex[i])); | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
k_spinlock_init()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm, I forgot that I already checked first time around that Zephyr spinlocks do not have such function or anything like it.
| static struct { | ||
| struct debug_stream_text_msg msg; | ||
| char text[768]; | ||
| } __packed buf = { 0 }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static data is already 0. Also would be helpful to have a longer name with some context, at least ds_buf
| struct debug_stream_text_msg msg; | ||
| char text[768]; | ||
| } __packed buf = { 0 }; | ||
| static int reports_sent_cpu[CONFIG_MP_MAX_NUM_CPUS] = { 0 }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto 0
| buf.msg.hdr.id = DEBUG_STREAM_RECORD_ID_TEXT_MSG; | ||
| buf.msg.hdr.size_words = SOF_DIV_ROUND_UP(sizeof(buf.msg) + pos, | ||
| sizeof(buf.msg.hdr.data[0])); | ||
| memset(buf.text + pos, 0, buf.msg.hdr.size_words * sizeof(uint32_t) - pos); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buf.msg.hdr.size_words * sizeof(uint32_t) - pos can be replaced with sizeof(buf.msg)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be, but is unnecessary. We only need to make sure the string is null terminated if the round up operation adds some extra bytes to the end. Actually setting just one byte to zero would be enough, but just using memset saves an if statement in case so check if there are any extra bytes in the end of the message. I'll add a comment to explain why the memset is there.
| ds_exception_drain(false); | ||
| } | ||
|
|
||
| #if defined(CONFIG_EXCEPTION_DUMP_HOOK) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without CONFIG_EXCEPTION_DUMP_HOOK the functions above aren't needed either?
| #define __SOC_DEBUG_STREAM_TEXT_MSG_H__ | ||
|
|
||
| #include <user/debug_stream_slot.h> | ||
| #include <stdarg.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does anything in the header need it? I guess you wanted this in .c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jsarha This is missing something, CONFIG_EXCEPTION_DUMP_HOOK is not introduced anwhere (as a kconfig).
set_exception_dump_hook() is not implemented in this PR and I couldn't find it in SOF or Zephyr...?
@kv2019i its defined in zephyrproject-rtos/zephyr#103449 , without it this PR does not do much. |
Set exception dump hooks if CONFIG_EXCEPTION_DUMP_HOOK=y. This enables sending a simple text report of fatal exceptions. To get this working one needs these config options: CONFIG_EXCEPTION_DUMP_HOOK=y CONFIG_EXCEPTION_DUMP_HOOK_ONLY=y CONFIG_SOF_DEBUG_STREAM_SLOT=y CONFIG_SOF_DEBUG_STREAM_TEXT_MSG=y CONFIG_SOF_DEBUG_STREAM_SLOT_NUMBER=2 CONFIG_SOF_TELEMETRY=n CONFIG_SOF_TELEMETRY_PERFORMANCE_MEASUREMENTS=n CONFIG_SOF_TELEMETRY_IO_PERFORMANCE_MEASUREMENTS=n If system hangs and an the exception is reported successfully the report can be seen with debug_stream.py (which should be installed in the same directory with cavstool.py). It does matter if the too was not running at the time. The report should be available there in the debug slot window for debug_stream.py to decode as long as system remains up. The report should looks something like this: CPU 2: CPU 2 EXCCAUSE 13 (load/store PIF data error) PC 0xa06b24ba VADDR 0xa0031020 PS 0x60820 (INTLEVEL:0 EXCM: 0 UM:1 RING:0 WOE:1 OWB:8 CALLINC:2) A0 0xa06b10dd SP 0xa00f8b00 A2 0xa A3 0xa01a9354 A4 0xa01a922c A5 0x3c1 A6 0xa01a7ee4 A7 0xa01a7ec4 A8 0xa006e572 A9 0xa00f8ab0 A10 0x4018d8b0 A11 0xa A12 0x14 A13 0x1 A14 0xa A15 (nil) LBEG 0xa0044323 LEND 0xa0044330 LCOUNT 0xa006de66 SAR 0x5 THREADPTR (nil) BT 0xa06b24b7:0xa00f8b00 CORRUPTED Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Skip useless " ** " prefix from exception dumps to save byte is in the debug_stream buffer. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
fd97e52 to
1e537a2
Compare
|
The both PRs, this and zephyrproject-rtos/zephyr#103449 , are now updated and now they should work on PTL. The problem was that logging from the exception function hung the core in question, so that even debug_stream records could not be sent. Also the max dump size was too big for PTL. |
This is experimental debug_stream exception dump handler. It should be able to report fatal exceptions through debug_stream using debug window slot and debug_stream.py. To get it working one needs zephyrproject-rtos/zephyr#103449 too.
I tried to get backtrace dumpped too, but for some reason all my attempts so far rendered everything unusable, maybe I try again after my vacation.