interesting
/
riscv-openocd
mirror of https://github.com/riscv/riscv-openocd.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

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 <anatoly.parshintsev@syntacore.com>
This commit is contained in:
Parshintsev Anatoly 2023-08-04 14:54:50 +03:00
parent f061623568
commit 198edca6d0
1 changed files with 16 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
src/target/riscv/riscv-013.c
View File

@ -1870,8 +1870,12 @@ static int set_group(struct target *target, bool *supported, unsigned int group,
static int examine(struct target *target) static int examine(struct target *target)
{ {
/* Don't need to select dbus, since the first thing we do is read dtmcontrol. */ /* We reset target state in case if something goes wrong during examine:
* DTM/DM scans could fail or hart may fail to halt. */
target->state = TARGET_UNKNOWN;
target->debug_reason = DBG_REASON_UNDEFINED;
/* Don't need to select dbus, since the first thing we do is read dtmcontrol. */
LOG_TARGET_DEBUG(target, "dbgbase=0x%x", target->dbgbase); LOG_TARGET_DEBUG(target, "dbgbase=0x%x", target->dbgbase);
uint32_t dtmcontrol = dtmcontrol_scan(target, 0); uint32_t dtmcontrol = dtmcontrol_scan(target, 0);
@ -2033,34 +2037,21 @@ static int examine(struct target *target)
if (dm013_select_hart(target, info->index) != ERROR_OK) if (dm013_select_hart(target, info->index) != ERROR_OK)
return ERROR_FAIL; return ERROR_FAIL;
enum riscv_hart_state state; enum riscv_hart_state state_at_examine_start;
if (riscv_get_hart_state(target, &state) != ERROR_OK) if (riscv_get_hart_state(target, &state_at_examine_start) != ERROR_OK)
return ERROR_FAIL; return ERROR_FAIL;
bool halted = (state == RISCV_STATE_HALTED); const bool hart_halted_at_examine_start = state_at_examine_start == RISCV_STATE_HALTED;
if (!halted) { if (!hart_halted_at_examine_start) {
r->prepped = true; r->prepped = true;
if (riscv013_halt_go(target) != ERROR_OK) { if (riscv013_halt_go(target) != ERROR_OK) {
LOG_TARGET_ERROR(target, "Fatal: Hart %d failed to halt during %s", LOG_TARGET_ERROR(target, "Fatal: Hart %d failed to halt during %s",
info->index, __func__); info->index, __func__);
return ERROR_FAIL; return ERROR_FAIL;
} }
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->state = TARGET_HALTED;
target->debug_reason = DBG_REASON_DBGRQ; 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 /* Without knowing anything else we can at least mess with the
* program buffer. */ * program buffer. */
@ -2134,13 +2125,13 @@ static int examine(struct target *target)
if (set_dcsr_ebreak(target, false) != ERROR_OK) if (set_dcsr_ebreak(target, false) != ERROR_OK)
return ERROR_FAIL; return ERROR_FAIL;
target->state = saved_tgt_state; if (state_at_examine_start == RISCV_STATE_RUNNING) {
target->debug_reason = saved_dbg_reason;
if (!halted) {
riscv013_step_or_resume_current_hart(target, false); riscv013_step_or_resume_current_hart(target, false);
target->state = TARGET_RUNNING; target->state = TARGET_RUNNING;
target->debug_reason = DBG_REASON_NOTHALTED; 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) { if (target->smp) {