cpu/cortexm: rework bkpt instruction call on panic#20616
Merged
maribu merged 2 commits intoRIOT-OS:masterfrom Apr 24, 2024
Merged
cpu/cortexm: rework bkpt instruction call on panic#20616maribu merged 2 commits intoRIOT-OS:masterfrom
maribu merged 2 commits intoRIOT-OS:masterfrom
Conversation
github-actions
bot
added
Platform: ARM
Contributor
|
I like this idea! |
benpicco
added
the
CI: ready for build
maribu
approved these changes
Apr 24, 2024
Member
maribu
left a comment
maribu
left a comment
There was a problem hiding this comment.
lgtm, thx. Please squash the nitpicks right away.
When you have the file open anyway, care to sneak in a style fix as separate commit in line 30?
- if(ipsr) {
+ if (ipsr) {Only call this instruction if a debug session is active otherwise it will trigger a hardfault Signed-off-by: Dylan Laduranty <[email protected]>
Signed-off-by: Dylan Laduranty <[email protected]>
dylad
force-pushed
the
pr/cpu/cortexm/rework_break_on_panic
branch
from
679536e to
e784794
Compare
Member
Author
|
@maribu I've addressed your comments and squashed. |
maribu
approved these changes
Apr 24, 2024
Member
Author
|
Thanks! |
Member
|
Thx for the enhancement :) It will be a real quality of life improvement when the MCU no longer gets locked up :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
Rework the way
bkptinstruction is call on panic for Cortex-M MCU.Currently, this instruction is called everytime on panic. However, when a debug session is not active (no GDB running), and if a panic occured, the
bkptinstruction will either escalate the current fault to hardfault or the CPU will enter LOCKUP state (it may reset automatically depending on how vendors handle this signal internally).To mitigate this issue, one can check the
C_DEBUGENinCoreDebug->DHCSR. If this bit is set, it means a debug session is running, thusbkptinstruction can be used safely. Otherwise, skip this instruction. The execution will then continue untilpm_off()orwhile(1) {}is called.However, this bit is NOT accessible by the CPU on CortexM0+ (Only accessible by GDB). So the
bkptinstruction is completely skip for this one.Testing procedure
Trigger a fault on any cortexm-based board
for instance
Such code will trigger a hardfault (technically a busfault for ARMv7-M or ARMv8-M Mainline, but it escalates to hardfault as the busfault is currently not enable).
On panic, if GDB is NOT used, the execution will 'stop' on the
bkptinstruction. On nRF5340DK, a reset is issued (Probably because the processor enters LOCKUP state). Nevertheless, the execution should continue up to here.Issues/PRs references
None.