From 1c5cf8023c44605f25b7d9eab9b726a5986bdb75 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 9 May 2023 11:13:29 -0700 Subject: [PATCH 1/4] target/riscv: Reset DTM when it reports an error. The error state is sticky, so this has to be done to recover. Change-Id: I589f3cdab0f2351fd25f89951830cbc16c39bd93 Signed-off-by: Tim Newsome --- src/target/riscv/riscv-013.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index b4e165595..52c1fc1b8 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -590,6 +590,7 @@ static int dmi_op_timeout(struct target *target, uint32_t *data_in, break; } else { LOG_ERROR("failed %s at 0x%x, status=%d", op_name, address, status); + dtmcontrol_scan(target, DTM_DTMCS_DMIRESET); return ERROR_FAIL; } if (time(NULL) - start > timeout_sec) @@ -598,6 +599,7 @@ static int dmi_op_timeout(struct target *target, uint32_t *data_in, if (status != DMI_STATUS_SUCCESS) { LOG_ERROR("Failed %s at 0x%x; status=%d", op_name, address, status); + dtmcontrol_scan(target, DTM_DTMCS_DMIRESET); return ERROR_FAIL; } @@ -622,6 +624,7 @@ static int dmi_op_timeout(struct target *target, uint32_t *data_in, LOG_ERROR("Failed %s (NOP) at 0x%x; status=%d", op_name, address, status); } + dtmcontrol_scan(target, DTM_DTMCS_DMIRESET); return ERROR_FAIL; } if (time(NULL) - start > timeout_sec) From 82ed02f92aeb073232d04d9c3be3bb57ed7cebce Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 9 May 2023 11:14:15 -0700 Subject: [PATCH 2/4] target/riscv: Always clear progbuf cache in examine(). When a DM was powered down, we end up in examine() again, and clearly if the DM was powered down we need to invalidate that cache. Change-Id: I5eb6a289939f313e06c09cac22245db083026aa3 Signed-off-by: Tim Newsome --- src/target/riscv/riscv-013.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 52c1fc1b8..facab1393 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1623,10 +1623,10 @@ static int examine(struct target *target) dmi_write(target, DM_DMCONTROL, 0); dmi_write(target, DM_DMCONTROL, DM_DMCONTROL_DMACTIVE); dm->was_reset = true; - - /* The DM gets reset, so forget any cached progbuf entries. */ - riscv013_invalidate_cached_debug_buffer(target); } + /* We're here because we're uncertain about the state of the target. That + * includes our progbuf cache. */ + riscv013_invalidate_cached_debug_buffer(target); dmi_write(target, DM_DMCONTROL, DM_DMCONTROL_HARTSELLO | DM_DMCONTROL_HARTSELHI | DM_DMCONTROL_DMACTIVE | From 21433e83eeda7abe621027f12d588ebdea7760d3 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 16 May 2023 10:21:57 -0700 Subject: [PATCH 3/4] target: poll() failure does not mean the target halted. Poll failure just means poll failed. It's safer to assume the target is still running, because then if it is running and subsequently halts we can relay this to gdb correctly. We can't do the other way around, because once gdb thinks the target has halted, it can't deal with it spontaneously running. Change-Id: Idb56137f1d6baa9afc1b0e55e4a48f407b8ebe83 Signed-off-by: Tim Newsome --- src/target/riscv/riscv-013.c | 12 ++++++------ src/target/target.c | 12 +++++++----- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index facab1393..eded3b1bf 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -589,17 +589,15 @@ static int dmi_op_timeout(struct target *target, uint32_t *data_in, } else if (status == DMI_STATUS_SUCCESS) { break; } else { - LOG_ERROR("failed %s at 0x%x, status=%d", op_name, address, status); dtmcontrol_scan(target, DTM_DTMCS_DMIRESET); - return ERROR_FAIL; + break; } if (time(NULL) - start > timeout_sec) return ERROR_TIMEOUT_REACHED; } if (status != DMI_STATUS_SUCCESS) { - LOG_ERROR("Failed %s at 0x%x; status=%d", op_name, address, status); - dtmcontrol_scan(target, DTM_DTMCS_DMIRESET); + LOG_TARGET_ERROR(target, "Failed DMI %s at 0x%x; status=%d", op_name, address, status); return ERROR_FAIL; } @@ -618,10 +616,12 @@ static int dmi_op_timeout(struct target *target, uint32_t *data_in, break; } else { if (data_in) { - LOG_ERROR("Failed %s (NOP) at 0x%x; value=0x%x, status=%d", + LOG_TARGET_ERROR(target, + "Failed DMI %s (NOP) at 0x%x; value=0x%x, status=%d", op_name, address, *data_in, status); } else { - LOG_ERROR("Failed %s (NOP) at 0x%x; status=%d", op_name, address, + LOG_TARGET_ERROR(target, + "Failed DMI %s (NOP) at 0x%x; status=%d", op_name, address, status); } dtmcontrol_scan(target, DTM_DTMCS_DMIRESET); diff --git a/src/target/target.c b/src/target/target.c index 62ef4d224..581f5ef58 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -3055,9 +3055,12 @@ static int handle_target(void *priv) /* Increase interval between polling up to 5000ms */ target->backoff.interval = MAX(polling_interval, MIN(target->backoff.interval * 2 + 1, 5000)); - /* Tell GDB to halt the debugger. This allows the user to run - * monitor commands to handle the situation. */ - target_call_event_callbacks(target, TARGET_EVENT_GDB_HALT); + /* Do *not* tell gdb the target halted. This might just + * be a hiccup. We have no reason to believe the target + * is halted, and if it is running while gdb thinks it's + * halted things just get unnecessarily confused. gdb + * users can hit ^C if the need to interact with the + * target. */ } target->backoff.next_attempt = timeval_ms() + target->backoff.interval; LOG_TARGET_DEBUG(target, "target_poll() -> %d, next attempt in %dms", @@ -3067,8 +3070,7 @@ static int handle_target(void *priv) target_reset_examined(target); retval = target_examine_one(target); if (retval != ERROR_OK) { - LOG_TARGET_DEBUG(target, "Examination failed, GDB will be halted. " - "Polling again in %dms", + LOG_TARGET_DEBUG(target, "Examination failed. Polling again in %dms", target->backoff.interval); return retval; } From f0898155d1e83c0f8691396eb01103cd3beb470c Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 23 May 2023 10:31:55 -0700 Subject: [PATCH 4/4] target/riscv: Set dcsr.ebreak* during examine() This way if you connect to a running target, before it's hit a breakpoint, then when it does hit the breakpoint OpenOCD will catch it. Change-Id: I6f1e5f169fa385f46759015786e664693c3872e4 Signed-off-by: Tim Newsome --- src/target/riscv/riscv-013.c | 43 ++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index eded3b1bf..1647e7139 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1555,6 +1555,24 @@ static int wait_for_authbusy(struct target *target, uint32_t *dmstatus) return ERROR_OK; } +static int update_dcsr(struct target *target, bool step) +{ + riscv_reg_t dcsr; + /* We want to twiddle some bits in the debug CSR so debugging works. */ + int result = register_read_direct(target, &dcsr, GDB_REGNO_DCSR); + if (result != ERROR_OK) + return result; + dcsr = set_field(dcsr, CSR_DCSR_STEP, step); + dcsr = set_field(dcsr, CSR_DCSR_EBREAKM, riscv_ebreakm); + dcsr = set_field(dcsr, CSR_DCSR_EBREAKS, riscv_ebreaks); + dcsr = set_field(dcsr, CSR_DCSR_EBREAKU, riscv_ebreaku); + dcsr = set_field(dcsr, CSR_DCSR_EBREAKVS, riscv_ebreaku); + dcsr = set_field(dcsr, CSR_DCSR_EBREAKVU, riscv_ebreaku); + if (riscv_set_register(target, GDB_REGNO_DCSR, dcsr) != ERROR_OK) + return ERROR_FAIL; + return ERROR_OK; +} + /*** OpenOCD target functions. ***/ static void deinit_target(struct target *target) @@ -1826,10 +1844,6 @@ static int examine(struct target *target) r->mtopi_readable = false; } - /* Now init registers based on what we discovered. */ - if (riscv_init_registers(target) != ERROR_OK) - return ERROR_FAIL; - /* Display this as early as possible to help people who are using * really slow simulators. */ LOG_TARGET_DEBUG(target, " XLEN=%d, misa=0x%" PRIx64, r->xlen, r->misa); @@ -1847,6 +1861,13 @@ static int examine(struct target *target) target->state = saved_tgt_state; target->debug_reason = saved_dbg_reason; + /* Now init registers based on what we discovered. */ + if (riscv_init_registers(target) != ERROR_OK) + return ERROR_FAIL; + + if (update_dcsr(target, false) != ERROR_OK) + return ERROR_FAIL; + if (!halted) { riscv013_step_or_resume_current_hart(target, false); target->state = TARGET_RUNNING; @@ -4605,19 +4626,9 @@ static int riscv013_on_step_or_resume(struct target *target, bool step) if (maybe_execute_fence_i(target) != ERROR_OK) return ERROR_FAIL; - /* We want to twiddle some bits in the debug CSR so debugging works. */ - riscv_reg_t dcsr; - int result = riscv_get_register(target, &dcsr, GDB_REGNO_DCSR); - if (result != ERROR_OK) - return result; - dcsr = set_field(dcsr, CSR_DCSR_STEP, step); - dcsr = set_field(dcsr, CSR_DCSR_EBREAKM, riscv_ebreakm); - dcsr = set_field(dcsr, CSR_DCSR_EBREAKS, riscv_ebreaks); - dcsr = set_field(dcsr, CSR_DCSR_EBREAKU, riscv_ebreaku); - dcsr = set_field(dcsr, CSR_DCSR_EBREAKVS, riscv_ebreaku); - dcsr = set_field(dcsr, CSR_DCSR_EBREAKVU, riscv_ebreaku); - if (riscv_set_register(target, GDB_REGNO_DCSR, dcsr) != ERROR_OK) + if (update_dcsr(target, step) != ERROR_OK) return ERROR_FAIL; + if (riscv_flush_registers(target) != ERROR_OK) return ERROR_FAIL; return ERROR_OK;