From e2bdf0e5cc99969955537eff8f39e744a2e72a1a Mon Sep 17 00:00:00 2001 From: Parshintsev Anatoly Date: Fri, 4 Aug 2023 14:54:50 +0300 Subject: [PATCH] riscv: simplify state management during examine This also fixes a bug when, after `examine` completion, the target still has `unknown` status. To reproduce this one spike, it is enough to do the following: --- // make sure spike harts are halted openocd ... -c init -c 'echo "[targets]"' --- this behavior is quite dangerous and leads to segfaults in some cases Change-Id: I13915f7038ad6d0251d56d2d519fbad9a2f13c18 Signed-off-by: Parshintsev Anatoly --- src/target/riscv/riscv-013.c | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 8021375a82..cda2242833 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1958,11 +1958,14 @@ static int examine(struct target *target) if (dm013_select_hart(target, info->index) != ERROR_OK) return ERROR_FAIL; - enum riscv_hart_state state; - if (riscv_get_hart_state(target, &state) != ERROR_OK) + target->state = TARGET_UNKNOWN; + target->debug_reason = DBG_REASON_UNDEFINED; + + enum riscv_hart_state state_at_examine_start; + if (riscv_get_hart_state(target, &state_at_examine_start) != ERROR_OK) return ERROR_FAIL; - bool halted = (state == RISCV_STATE_HALTED); - if (!halted) { + const bool hart_halted_at_examine_start = state_at_examine_start == RISCV_STATE_HALTED; + if (!hart_halted_at_examine_start) { r->prepped = true; if (riscv013_halt_go(target) != ERROR_OK) { LOG_TARGET_ERROR(target, "Fatal: Hart %d failed to halt during %s", @@ -1972,20 +1975,9 @@ static int examine(struct target *target) target->state = TARGET_HALTED; target->debug_reason = DBG_REASON_DBGRQ; } - /* FIXME: This is needed since register_read_direct relies on target->state - * to work correctly, so, if target->state does not represent current state - * of target, e.g. if a target is halted, but target->state is - * TARGET_UNKNOWN, it can fail early, (e.g. accessing registers via program - * buffer can not be done atomically on a running hart becuse mstatus can't - * be prepared for a register access and then restored) - * See https://github.com/riscv/riscv-openocd/pull/842#discussion_r1179414089 - */ - const enum target_state saved_tgt_state = target->state; - const enum target_debug_reason saved_dbg_reason = target->debug_reason; - if (target->state != TARGET_HALTED) { - target->state = TARGET_HALTED; - target->debug_reason = DBG_REASON_DBGRQ; - } + + target->state = TARGET_HALTED; + target->debug_reason = hart_halted_at_examine_start ? DBG_REASON_UNDEFINED : DBG_REASON_DBGRQ; /* Without knowing anything else we can at least mess with the * program buffer. */ @@ -2059,13 +2051,13 @@ static int examine(struct target *target) if (set_dcsr_ebreak(target, false) != ERROR_OK) return ERROR_FAIL; - target->state = saved_tgt_state; - target->debug_reason = saved_dbg_reason; - - if (!halted) { + if (state_at_examine_start == RISCV_STATE_RUNNING) { riscv013_step_or_resume_current_hart(target, false); target->state = TARGET_RUNNING; target->debug_reason = DBG_REASON_NOTHALTED; + } else if (state_at_examine_start == RISCV_STATE_HALTED) { + target->state = TARGET_HALTED; + target->debug_reason = DBG_REASON_UNDEFINED; } if (target->smp) {