-
-
Notifications
You must be signed in to change notification settings - Fork 774
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
riscv : fix the inverted logc interrupt-are-disabled-while-single-ste… #1767
riscv : fix the inverted logc interrupt-are-disabled-while-single-ste… #1767
Conversation
Hello! I would really like to advance the state of the BMP code for using with ch32v30x , please give me a usage scenario - I will try to test the work. |
Having finally managed to actually review the debug manual and this patch, the original behaviour is intended in that, with interrupts disabled it'll substantially change execution of the user firmware while stepping. Most parts we've seen seem lockstep their peripherals to core halt, allowing you to step through exactly how the firmware would execute without the debugger present. It would be interesting to see if the part you're debugging does not do this, leading to wanting interrupts while stepping to be disabled. In either case though, if this patch is actually warranted, then not manipulating STEPIE at all would be more correct as the bit defaults to 0 on reset, and does not want to be on when executing normally - only potentially while single-stepping. |
Thank you for the review |
I did a couple of quick tryouts :
Tested on the CH32V3xx, probably the same for CH32V2XX. |
Please rebase this PR on |
With your permission, we'll do the rebase ourself so we can get progress on this PR and try to get it merged for v2.0 |
632674d
to
10ec2da
Compare
Sorry, i didnt notice the previous notification |
10ec2da
to
3b50115
Compare
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.
If you can please rebase the 3 commits into one, keeping only the commit message with the first, fixed to remove the extra
in front of the :
, then we will get this merged. This looks good as implemented.
Thank you for the contribution!
…nter and timer activity).
3b50115
to
5652979
Compare
did that too quickly, should be okay now |
Detailed description
There is a small inverted logic in the "disable-interrupt-while-single-stepping" code
The STEPIE bit actually enables the interrupt while single stepping
The change inverts the logic (and let is "off" when leaving single step mode)
With that patch you dont go into timer interrupt as soon as you hit next with gdb
(on some platforms that bit is glued to zero, so the problem may not show)
Your checklist for this pull request