fix confusing status messages during resume #1031
Merged
Conversation
This file contains 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
|
NOTE: this is more like a temporary solution to hide the problem. The situation with these "hidden" step and user-specified callbacks is not quite obvious. I'm going to clarify this with openocd maintainers in upstream. Also, this commit only fixes 0.13 implementations. 0.11 is still affected. Will address this in future |
en-sc
previously approved these changes
Mar 25, 2024
There was a problem hiding this comment.
@JanMatCodasip, @MarekVCodasip, please, take a look at the patch.
I have already reviewed it internally.
src/target/riscv/riscv.c
Outdated
| @@ -3361,7 +3373,8 @@ int riscv_openocd_step(struct target *target, int current, | |||
| LOG_TARGET_ERROR(target, "Unable to restore the disabled breakpoint."); | |||
| } | |||
|
|
|||
| if (success) { | |||
| // TODO: should we update debug_reason if no callback was called? | |||
There was a problem hiding this comment.
I think that we should update the debug reason even if callbacks are not to be handled.
There was a problem hiding this comment.
@MarekVCodasip Marek, can you please quickly re-check this PR now that your suggestion has been addressed. Thanks.
Recently, (after b503fde) OpenOCD started to notify user about hart state updates. This causes confusion in some cases since some internal updates to the hart state should not be visible to the user as these are implementation details. For example situation like this: ``` > reset halt JTAG tap: riscv.tap tap/device found: 0xdeadbeef ... > resume [riscv.cpu0] Found 4 triggers riscv.cpu0 halted due to single-step. [riscv.cpu1] Found 4 triggers riscv.cpu1 halted due to single-step. [riscv.cpu2] Found 4 triggers riscv.cpu2 halted due to single-step. [riscv.cpu3] Found 4 triggers riscv.cpu3 halted due to single-step. ``` likely confuse people. There is no issue with the resume functionality. It`s just that resume internally causes single-step that causes hart state to change. This commit disable calling of user-specified (and default) callbacks during the "hidden" step operation disabling these confusing messages Change-Id: I3412a089e2abdcd315d86cec7ee732fdd18c1601 Signed-off-by: Parshintsev Anatoly <[email protected]>
aap-sc
force-pushed
the
aap-sc/hart_status_info_fixup
branch
from
e3f60f2 to
665fbf6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Recently, (after b503fde) OpenOCD started to notify user about hart state updates. This causes confusion in some cases since some internal updates to the hart state should not be visible to the user as these are implementation details. For example situation like this:
likely confuse people.
There is no issue with the resume functionality. It`s just that resume internally causes single-step that causes hart state to change.
This commit disable calling of user-specified (and default) callbacks during the "hidden" step operation disabling these confusing messages
Change-Id: I3412a089e2abdcd315d86cec7ee732fdd18c1601