From 0b965363a6cd78a05a64527aae1894706238c8a8 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 24 Nov 2021 10:20:15 -0800 Subject: [PATCH] Deal with halt race. (#664) * Deal with halt race. What could happen: 1. hart 1 halts due to a breakpoint 2. OpenOCD polls and notices hart 1 has halted. 3. hart 0 halts because it is in a halt group with halt 1 4. OpenOCD decides to halt hart 0 also. 5. OpenOCD discovers hart 0 is already halted, and doesn't update the debug reason. In that case OpenOCD would tell gdb that hart 0 halted, while the interesting event is that hart 1 halted. Fix this by updating the debug reason when we discover a hart is already halted when we try to halt it. This race was exposed in the Sv* address translation tests, but the bug has nothing to do with address translation. Change-Id: I59d871e8545bd644bf42581266f15234b93e9900 Signed-off-by: Tim Newsome * Comment set_debug_reason Co-authored-by: Jan Matyas <50193733+JanMatCodasip@users.noreply.github.com> Co-authored-by: Jan Matyas <50193733+JanMatCodasip@users.noreply.github.com> --- src/target/riscv/riscv.c | 62 +++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 7eb6b637b..09c1939fe 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1189,6 +1189,34 @@ int riscv_flush_registers(struct target *target) return ERROR_OK; } +/* Convert: RISC-V hart's halt reason --> OpenOCD's generic debug reason */ +int set_debug_reason(struct target *target, enum riscv_halt_reason halt_reason) +{ + switch (halt_reason) { + case RISCV_HALT_BREAKPOINT: + target->debug_reason = DBG_REASON_BREAKPOINT; + break; + case RISCV_HALT_TRIGGER: + target->debug_reason = DBG_REASON_WATCHPOINT; + break; + case RISCV_HALT_INTERRUPT: + case RISCV_HALT_GROUP: + target->debug_reason = DBG_REASON_DBGRQ; + break; + case RISCV_HALT_SINGLESTEP: + target->debug_reason = DBG_REASON_SINGLESTEP; + break; + case RISCV_HALT_UNKNOWN: + target->debug_reason = DBG_REASON_UNDEFINED; + break; + case RISCV_HALT_ERROR: + return ERROR_FAIL; + } + LOG_DEBUG("[%s] debug_reason=%d", target_name(target), target->debug_reason); + + return ERROR_OK; +} + int halt_prep(struct target *target) { RISCV_INFO(r); @@ -1198,8 +1226,14 @@ int halt_prep(struct target *target) if (riscv_select_current_hart(target) != ERROR_OK) return ERROR_FAIL; if (riscv_is_halted(target)) { - LOG_DEBUG("[%s] Hart is already halted (reason=%d).", + LOG_DEBUG("[%s] Hart is already halted (debug_reason=%d).", target_name(target), target->debug_reason); + if (target->debug_reason == DBG_REASON_NOTHALTED) { + enum riscv_halt_reason halt_reason = + riscv_halt_reason(target, r->current_hartid); + if (set_debug_reason(target, halt_reason) != ERROR_OK) + return ERROR_FAIL; + } } else { if (r->halt_prep(target) != ERROR_OK) return ERROR_FAIL; @@ -2153,32 +2187,6 @@ static enum riscv_poll_hart riscv_poll_hart(struct target *target, int hartid) return RPH_NO_CHANGE; } -int set_debug_reason(struct target *target, enum riscv_halt_reason halt_reason) -{ - switch (halt_reason) { - case RISCV_HALT_BREAKPOINT: - target->debug_reason = DBG_REASON_BREAKPOINT; - break; - case RISCV_HALT_TRIGGER: - target->debug_reason = DBG_REASON_WATCHPOINT; - break; - case RISCV_HALT_INTERRUPT: - case RISCV_HALT_GROUP: - target->debug_reason = DBG_REASON_DBGRQ; - break; - case RISCV_HALT_SINGLESTEP: - target->debug_reason = DBG_REASON_SINGLESTEP; - break; - case RISCV_HALT_UNKNOWN: - target->debug_reason = DBG_REASON_UNDEFINED; - break; - case RISCV_HALT_ERROR: - return ERROR_FAIL; - } - LOG_DEBUG("[%s] debug_reason=%d", target_name(target), target->debug_reason); - return ERROR_OK; -} - int sample_memory(struct target *target) { RISCV_INFO(r);