-
Notifications
You must be signed in to change notification settings - Fork 351
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
Exception dump hook #10517
Changes from all commits
ba001a4
809a04d
56104c5
06ce324
4b39680
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -10,11 +10,12 @@ | |||
| #include <rtos/string.h> | ||||
| #include <user/debug_stream.h> | ||||
| #include <user/debug_stream_slot.h> | ||||
| #include <zephyr/kernel.h> | ||||
|
|
||||
| LOG_MODULE_REGISTER(debug_stream_slot); | ||||
|
|
||||
| struct cpu_mutex { | ||||
| struct k_mutex m; | ||||
| struct k_spinlock l; | ||||
| } __aligned(CONFIG_DCACHE_LINE_SIZE); | ||||
|
|
||||
| /* CPU specific mutexes for each circular buffer */ | ||||
|
|
@@ -66,6 +67,7 @@ int debug_stream_slot_send_record(struct debug_stream_record *rec) | |||
| debug_stream_get_circular_buffer(&desc, arch_proc_id()); | ||||
| uint32_t record_size = rec->size_words; | ||||
| uint32_t record_start, buf_remain; | ||||
| k_spinlock_key_t key; | ||||
|
|
||||
| LOG_DBG("Sending record %u id %u len %u", rec->seqno, rec->id, rec->size_words); | ||||
|
|
||||
|
|
@@ -77,7 +79,7 @@ int debug_stream_slot_send_record(struct debug_stream_record *rec) | |||
| desc.buf_words, desc.core_id, desc.buf_words, desc.offset); | ||||
| return -ENOMEM; | ||||
| } | ||||
| k_mutex_lock(&cpu_mutex[arch_proc_id()].m, K_FOREVER); | ||||
| key = k_spin_lock(&cpu_mutex[arch_proc_id()].l); | ||||
|
|
||||
| rec->seqno = buf->next_seqno++; | ||||
| rec->size_words = record_size + 1; /* +1 for size at the end of record */ | ||||
|
|
@@ -105,7 +107,7 @@ int debug_stream_slot_send_record(struct debug_stream_record *rec) | |||
| buf->data[buf->w_ptr] = record_size + 1; | ||||
| buf->w_ptr = (buf->w_ptr + 1) % desc.buf_words; | ||||
|
|
||||
| k_mutex_unlock(&cpu_mutex[arch_proc_id()].m); | ||||
| k_spin_unlock(&cpu_mutex[arch_proc_id()].l, key); | ||||
|
|
||||
| LOG_DBG("Record %u id %u len %u sent", rec->seqno, rec->id, record_size); | ||||
| return 0; | ||||
|
|
@@ -159,14 +161,6 @@ static int debug_stream_slot_init(void) | |||
|
|
||||
| buf->next_seqno = 0; | ||||
| buf->w_ptr = 0; | ||||
| k_mutex_init(&cpu_mutex[i].m); | ||||
| /* The core specific mutexes are now .bss which is uncached so the | ||||
| * following line is commented out. However, since the mutexes are | ||||
| * core specific there should be nothing preventing from having them | ||||
| * in cached memory. | ||||
| * | ||||
| * sys_cache_data_flush_range(&cpu_mutex[i], sizeof(cpu_mutex[i])); | ||||
| */ | ||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SOF has one sof/zephyr/include/rtos/spinlock.h Line 13 in a32d983
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's arguably quite confusing we use Zephyr native calls, but have a SOF extension. I'd only use k_spinlock_init if we really need to re-initialize. |
||||
| } | ||||
| LOG_INF("Debug stream slot initialized"); | ||||
|
|
||||
|
|
||||
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.
Hmm, this is ok for this overlay, but needs attention when/if we try to make this the default. The CI systems we have in place do not have debugstream runnign on DUTs, only mtrace, so this would in effect make dumps unavailable in CI.
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.
I should probably test still if this is needed, but with the earlier version it looked like on PTL the dumping hung in the middle of the logging was used.
Uh oh!
There was an error while loading. Please reload this page.
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.
Yep, the normal dumping still hangs on PTL after first line if logging is on.