From bd266161ca0d8b99adc72320b2e2427993763f8a Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 27 Apr 2022 10:48:10 -0700 Subject: Fix typo in comment. Change-Id: If847aaedc704857f30220da8d6af703f1b57ad1d --- src/target/riscv/riscv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index f3b7530..59680de 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -216,7 +216,7 @@ typedef struct { struct reg_data_type_union vector_union; struct reg_data_type type_vector; - /* Set when trigger registers are changed by the user. This indicates we eed + /* Set when trigger registers are changed by the user. This indicates we need * to beware that we may hit a trigger that we didn't realize had been set. */ bool manual_hwbp_set; -- cgit v1.1 From e8b05455e21079d0e0698235bf4b85bda6023875 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 27 Apr 2022 12:41:13 -0700 Subject: Make watchpoint.unique_id a uint32_t Now it matches breakpoint.unique_id. Change-Id: I06f24b2cede2ee56bdeac8666b5235f923b18659 --- src/target/breakpoints.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/breakpoints.h b/src/target/breakpoints.h index 0a59495..fc5b50b 100644 --- a/src/target/breakpoints.h +++ b/src/target/breakpoints.h @@ -56,7 +56,7 @@ struct watchpoint { bool is_set; unsigned int number; struct watchpoint *next; - int unique_id; + uint32_t unique_id; }; void breakpoint_clear_target(struct target *target); -- cgit v1.1 From dc320d26f03d2a9932947c3eb2f065bd35c80f44 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 27 Apr 2022 12:46:09 -0700 Subject: Small code cleanup. Change-Id: I563b7c62494987287b13d9ed52a923e6f49a64be --- src/target/riscv/riscv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 484db74..174568f 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1025,8 +1025,6 @@ int riscv_remove_watchpoint(struct target *target, * and new value. */ int riscv_hit_watchpoint(struct target *target, struct watchpoint **hit_watchpoint) { - struct watchpoint *wp = target->watchpoints; - LOG_DEBUG("Current hartid = %d", riscv_current_hartid(target)); /*TODO instead of disassembling the instruction that we think caused the @@ -1081,6 +1079,7 @@ int riscv_hit_watchpoint(struct target *target, struct watchpoint **hit_watchpoi return ERROR_FAIL; } + struct watchpoint *wp = target->watchpoints; while (wp) { /*TODO support length/mask */ if (wp->address == mem_addr) { -- cgit v1.1 From a6f32126846a4094c60c60b31393b9e1ab0b22fe Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 27 Apr 2022 12:58:09 -0700 Subject: Create riscv_hit_trigger_hit_bit() function. This goes through the triggers OpenOCD set to find out if one of them has the hit bit set. Change-Id: I5b9f1c19273c7d40392a0cc278277ca6c94d2eae --- src/target/riscv/riscv.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 174568f..51ec64c 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1017,6 +1017,67 @@ int riscv_remove_watchpoint(struct target *target, return ERROR_OK; } +/** + * Look at the trigger hit bits to find out which trigger is the reason we're + * halted. Sets *unique_id to the unique ID of that trigger. If *unique_id is + * ~0, no match was found. + */ +static int riscv_hit_trigger_hit_bit(struct target *target, uint32_t *unique_id) +{ + RISCV_INFO(r); + + riscv_reg_t tselect; + if (riscv_get_register(target, &tselect, GDB_REGNO_TSELECT) != ERROR_OK) + return ERROR_FAIL; + + *unique_id = ~0; + for (unsigned int i = 0; i < r->trigger_count; i++) { + if (r->trigger_unique_id[i] == -1) + continue; + + if (riscv_set_register(target, GDB_REGNO_TSELECT, i) != ERROR_OK) + return ERROR_FAIL; + + uint64_t tdata1; + if (riscv_get_register(target, &tdata1, GDB_REGNO_TDATA1) != ERROR_OK) + return ERROR_FAIL; + int type = get_field(tdata1, MCONTROL_TYPE(riscv_xlen(target))); + + uint64_t hit_mask = 0; + switch (type) { + case 1: + /* Doesn't support hit bit. */ + break; + case 2: + hit_mask = CSR_MCONTROL_HIT; + break; + case 6: + hit_mask = CSR_MCONTROL6_HIT; + break; + default: + LOG_DEBUG("trigger %d has unknown type %d", i, type); + continue; + } + + /* Note: If we ever use chained triggers, then this logic needs + * to be changed to ignore triggers that are not the last one in + * the chain. */ + if (tdata1 & hit_mask) { + LOG_DEBUG("Trigger %d (unique_id=%d) has hit bit set.", i, r->trigger_unique_id[i]); + if (riscv_set_register(target, GDB_REGNO_TDATA1, tdata1 & ~hit_mask) != ERROR_OK) + return ERROR_FAIL; + + *unique_id = r->trigger_unique_id[i]; + break; + } + } + + if (riscv_set_register(target, GDB_REGNO_TSELECT, tselect) != ERROR_OK) + return ERROR_FAIL; + + return ERROR_OK; +} + /* Sets *hit_watchpoint to the first watchpoint identified as causing the * current halt. * -- cgit v1.1 From edcfcab8903afa96563a31852266b7bc265f32ee Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 27 Apr 2022 12:58:57 -0700 Subject: Add trigger_hit field to riscv_info Change-Id: If4e1b5c37da4ab9301d91f41ba4789662b677a29 --- src/target/riscv/riscv.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 59680de..59fdb38 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -124,6 +124,10 @@ typedef struct { * target controls, while otherwise only a single hart is controlled. */ int trigger_unique_id[RISCV_MAX_HWBPS]; + /* The unique id of the trigger that caused the most recent halt. If the + * most recent halt was not caused by a trigger, then this is -1. */ + uint32_t trigger_hit; + /* The number of entries in the debug buffer. */ int debug_buffer_size; -- cgit v1.1 From 68e41dc1c8cc3815290a6997763f19a2dd52324a Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 27 Apr 2022 12:59:43 -0700 Subject: Remove empty line. Change-Id: Id00bfd73363e60e109b339e86d620c1ed7d5198a --- src/target/riscv/riscv.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 51ec64c..a03de6c 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1159,7 +1159,6 @@ int riscv_hit_watchpoint(struct target *target, struct watchpoint **hit_watchpoi return ERROR_FAIL; } - static int oldriscv_step(struct target *target, int current, uint32_t address, int handle_breakpoints) { -- cgit v1.1 From d67a5bf0645f099f1d6457e4007ddb22b38a744b Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 27 Apr 2022 13:00:47 -0700 Subject: During polling, check which trigger has `hit` set. Change-Id: If226810ed930e5d7a2bab277a9f5b0f3ded86ffa --- src/target/riscv/riscv.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index a03de6c..ee58381 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1252,11 +1252,15 @@ int riscv_flush_registers(struct target *target) /* 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) { + RISCV_INFO(r); + r->trigger_hit = -1; switch (halt_reason) { case RISCV_HALT_BREAKPOINT: target->debug_reason = DBG_REASON_BREAKPOINT; break; case RISCV_HALT_TRIGGER: + if (riscv_hit_trigger_hit_bit(target, &r->trigger_hit) != ERROR_OK) + return ERROR_FAIL; target->debug_reason = DBG_REASON_WATCHPOINT; break; case RISCV_HALT_INTERRUPT: -- cgit v1.1 From 73199226dfaf2df1c009509358c3a48fc3b0970d Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 27 Apr 2022 13:01:14 -0700 Subject: Report some triggers as hardware breakpoints. Instead of reporting them all as watchpoints. Change-Id: If43d282a168f64f8fed6f659bcebbe2ef72f23e9 --- src/target/riscv/riscv.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index ee58381..26824a9 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1262,6 +1262,11 @@ int set_debug_reason(struct target *target, enum riscv_halt_reason halt_reason) if (riscv_hit_trigger_hit_bit(target, &r->trigger_hit) != ERROR_OK) return ERROR_FAIL; target->debug_reason = DBG_REASON_WATCHPOINT; + /* Check if we hit a hardware breakpoint. */ + for (struct breakpoint *bp = target->breakpoints; bp; bp = bp->next) { + if (bp->unique_id == r->trigger_hit) + target->debug_reason = DBG_REASON_BREAKPOINT; + } break; case RISCV_HALT_INTERRUPT: case RISCV_HALT_GROUP: -- cgit v1.1 From 1979ad5594ba1ebf1f5e0876f780c4c1c8f187cb Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 27 Apr 2022 13:02:00 -0700 Subject: If we know which trigger hit, don't disassemble. Change-Id: I1d7b6ffa91b0557e2e74e544e4b35033ed3e3553 Signed-off-by: Tim Newsome --- src/target/riscv/riscv.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 26824a9..4ab6d99 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1086,12 +1086,19 @@ static int riscv_hit_trigger_hit_bit(struct target *target, uint32_t *unique_id) * and new value. */ int riscv_hit_watchpoint(struct target *target, struct watchpoint **hit_watchpoint) { + RISCV_INFO(r); + LOG_DEBUG("Current hartid = %d", riscv_current_hartid(target)); - /*TODO instead of disassembling the instruction that we think caused the - * trigger, check the hit bit of each watchpoint first. The hit bit is - * simpler and more reliable to check but as it is optional and relatively - * new, not all hardware will implement it */ + /* If we identified which trigger caused the halt earlier, then just use + * that. */ + for (struct watchpoint *wp = target->watchpoints; wp; wp = wp->next) { + if (wp->unique_id == r->trigger_hit) { + *hit_watchpoint = wp; + return ERROR_OK; + } + } + riscv_reg_t dpc; riscv_get_register(target, &dpc, GDB_REGNO_DPC); const uint8_t length = 4; -- cgit v1.1