From 8fdce4534fb816430545c08eb59439e6ce0b0843 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 29 Jun 2023 16:57:12 -0700 Subject: [PATCH 1/4] rtos/hwthread: Call rtos_free_threadlist() again. I think this was incorrectly removed in a merge. Change-Id: I49fce230f35ae7bd368d2ed780c6c1ffe5939fda --- src/rtos/hwthread.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/rtos/hwthread.c b/src/rtos/hwthread.c index eac72fca6..61ceb66e0 100644 --- a/src/rtos/hwthread.c +++ b/src/rtos/hwthread.c @@ -95,6 +95,9 @@ static int hwthread_update_threads(struct rtos *rtos) target = rtos->target; + /* wipe out previous thread details if any */ + rtos_free_threadlist(rtos); + /* determine the number of "threads" */ if (target->smp) { foreach_smp_target(head, target->smp_targets) { From 162cc1e79d75b56578f5bc607234c0a2c8c32881 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 29 Jun 2023 17:02:26 -0700 Subject: [PATCH 2/4] target/riscv: Fix typo in gdb_regno_cacheable() comment. Change-Id: If8806853d47779b5b208202803ed5da437f7b624 Signed-off-by: Tim Newsome --- src/target/riscv/riscv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 7ea3a08d8..f8eac51d2 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -4663,7 +4663,7 @@ static bool gdb_regno_cacheable(enum gdb_regno regno, bool is_write) case GDB_REGNO_TSELECT: /* I think this should be above, but then it doesn't work. */ case GDB_REGNO_TDATA1: /* Changes value when tselect is changed. */ - case GDB_REGNO_TDATA2: /* Changse value when tselect is changed. */ + case GDB_REGNO_TDATA2: /* Changes value when tselect is changed. */ default: return false; } From 39a4f37f84d52aa38ffa1a7db703ad57deec4fdc Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Fri, 30 Jun 2023 12:03:38 -0700 Subject: [PATCH 3/4] target/breakpoints: Clear software breakpoints from available targets If a target where a software breakpoint was set is not currently available, but there are other targets in the same SMP group that are available, then we can use those to remove the software breakpoint. Change-Id: I9faa427c7b3aee31504e6e6599539e6f29b58d8f Signed-off-by: Tim Newsome --- src/target/breakpoints.c | 132 +++++++++++++++++++++++++++------------ 1 file changed, 93 insertions(+), 39 deletions(-) diff --git a/src/target/breakpoints.c b/src/target/breakpoints.c index 7bdb2a6c1..c2774cc6f 100644 --- a/src/target/breakpoints.c +++ b/src/target/breakpoints.c @@ -220,6 +220,8 @@ int breakpoint_add(struct target *target, return ERROR_OK; } else { + /* For software breakpoints on SMP targets, only set them on a + * single target. We assume that SMP targets share memory. */ return breakpoint_add_internal(target, address, length, type); } } @@ -270,11 +272,17 @@ int hybrid_breakpoint_add(struct target *target, return hybrid_breakpoint_add_internal(target, address, asid, length, type); } -/* free up a breakpoint */ -static void breakpoint_free(struct target *target, struct breakpoint *breakpoint_to_remove) +/* Free the data structures we use to track a breakpoint on data_target. + * Remove the actual breakpoint from breakpoint_target. + * This separation is useful when a software breakpoint is tracked on a target + * that is currently unavailable, but the breakpoint also affects a target that + * is available. + */ +static void breakpoint_free(struct target *data_target, struct target *breakpoint_target, + struct breakpoint *breakpoint_to_remove) { - struct breakpoint *breakpoint = target->breakpoints; - struct breakpoint **breakpoint_p = &target->breakpoints; + struct breakpoint *breakpoint = data_target->breakpoints; + struct breakpoint **breakpoint_p = &data_target->breakpoints; int retval; while (breakpoint) { @@ -287,7 +295,7 @@ static void breakpoint_free(struct target *target, struct breakpoint *breakpoint if (!breakpoint) return; - retval = target_remove_breakpoint(target, breakpoint); + retval = target_remove_breakpoint(breakpoint_target, breakpoint); LOG_DEBUG("free BPID: %" PRIu32 " --> %d", breakpoint->unique_id, retval); (*breakpoint_p) = breakpoint->next; @@ -295,27 +303,6 @@ static void breakpoint_free(struct target *target, struct breakpoint *breakpoint free(breakpoint); } -static int breakpoint_remove_internal(struct target *target, target_addr_t address) -{ - struct breakpoint *breakpoint = target->breakpoints; - - while (breakpoint) { - if ((breakpoint->address == address) || - (breakpoint->address == 0 && breakpoint->asid == address)) - break; - breakpoint = breakpoint->next; - } - - if (breakpoint) { - breakpoint_free(target, breakpoint); - return 1; - } else { - if (!target->smp) - LOG_ERROR("no breakpoint at address " TARGET_ADDR_FMT " found", address); - return 0; - } -} - static void breakpoint_remove_all_internal(struct target *target) { struct breakpoint *breakpoint = target->breakpoints; @@ -323,24 +310,90 @@ static void breakpoint_remove_all_internal(struct target *target) while (breakpoint) { struct breakpoint *tmp = breakpoint; breakpoint = breakpoint->next; - breakpoint_free(target, tmp); + breakpoint_free(target, target, tmp); } } void breakpoint_remove(struct target *target, target_addr_t address) { - if (target->smp) { - unsigned int num_breakpoints = 0; - struct target_list *head; + if (!target->smp) { + struct breakpoint *breakpoint = breakpoint_find(target, address); + if (breakpoint) + breakpoint_free(target, target, breakpoint); + return; + } - foreach_smp_target(head, target->smp_targets) { - struct target *curr = head->target; - num_breakpoints += breakpoint_remove_internal(curr, address); + unsigned int found = 0; + struct target_list *head; + /* Target where we found a software breakpoint. */ + struct target *software_breakpoint_target = NULL; + struct breakpoint *software_breakpoint = NULL; + /* Target that is available. */ + struct target *available_target = NULL; + /* Target that is available and halted. */ + struct target *halted_target = NULL; + + foreach_smp_target(head, target->smp_targets) { + struct target *curr = head->target; + + if (!available_target && curr->state != TARGET_UNAVAILABLE) + available_target = curr; + if (!halted_target && curr->state == TARGET_HALTED) + halted_target = curr; + + struct breakpoint *breakpoint = breakpoint_find(curr, address); + if (!breakpoint) + continue; + + found++; + + if (breakpoint->type == BKPT_SOFT) { + /* Software breakpoints are set on only one of the SMP + * targets. We can remove them through any of the SMP + * targets. */ + if (software_breakpoint_target) { + LOG_TARGET_WARNING(curr, "Already found software breakpoint at " + TARGET_ADDR_FMT " on %s.", address, target_name(software_breakpoint_target)); + } else { + assert(!software_breakpoint_target); + software_breakpoint_target = curr; + software_breakpoint = breakpoint; + } + } else { + breakpoint_free(curr, curr, breakpoint); + } + } + + if (!found) { + LOG_ERROR("no breakpoint at address " TARGET_ADDR_FMT " found", address); + return; + } + + if (software_breakpoint) { + struct target *remove_target; + if (software_breakpoint_target->state == TARGET_HALTED) + remove_target = software_breakpoint_target; + else if (halted_target) + remove_target = halted_target; + else + remove_target = available_target; + + if (remove_target) { + LOG_DEBUG("Removing software breakpoint found on %s using %s (address=" + TARGET_ADDR_FMT ").", + target_name(software_breakpoint_target), + target_name(remove_target), + address); + /* Remove the software breakpoint through + * remove_target, but update the breakpoints structure + * of software_breakpoint_target. */ + /* TODO: If there is an error, can we try to remove the + * same breakpoint from a different target? */ + breakpoint_free(software_breakpoint_target, remove_target, software_breakpoint); + } else { + LOG_WARNING("No halted target found to remove software breakpoint at " + TARGET_ADDR_FMT ".", address); } - if (!num_breakpoints) - LOG_ERROR("no breakpoint at address " TARGET_ADDR_FMT " found", address); - } else { - breakpoint_remove_internal(target, address); } } @@ -363,7 +416,7 @@ static void breakpoint_clear_target_internal(struct target *target) LOG_DEBUG("Delete all breakpoints for target: %s", target_name(target)); while (target->breakpoints) - breakpoint_free(target, target->breakpoints); + breakpoint_free(target, target, target->breakpoints); } void breakpoint_clear_target(struct target *target) @@ -385,7 +438,8 @@ struct breakpoint *breakpoint_find(struct target *target, target_addr_t address) struct breakpoint *breakpoint = target->breakpoints; while (breakpoint) { - if (breakpoint->address == address) + if (breakpoint->address == address || + (breakpoint->address == 0 && breakpoint->asid == address)) return breakpoint; breakpoint = breakpoint->next; } From 122c54b4c2188161e0503df57e50b9e799835bb2 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 27 Jun 2023 10:09:45 -0700 Subject: [PATCH 4/4] target/riscv: Message when harts become available. Change-Id: I3824e215a845ba7df3c7887ce1693378fde94b4b Signed-off-by: Tim Newsome --- src/target/riscv/riscv.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index f8eac51d2..a21be979c 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -2737,6 +2737,9 @@ static int riscv_poll_hart(struct target *target, enum riscv_next_action *next_a if (target->state == TARGET_UNKNOWN || state != previous_riscv_state) { switch (state) { case RISCV_STATE_HALTED: + if (previous_riscv_state == RISCV_STATE_UNAVAILABLE) + LOG_TARGET_INFO(target, "became available (halted)"); + LOG_TARGET_DEBUG(target, " triggered a halt; previous_target_state=%d", previous_target_state); target->state = TARGET_HALTED; @@ -2782,6 +2785,9 @@ static int riscv_poll_hart(struct target *target, enum riscv_next_action *next_a break; case RISCV_STATE_RUNNING: + if (previous_riscv_state == RISCV_STATE_UNAVAILABLE) + LOG_TARGET_INFO(target, "became available (running)"); + LOG_TARGET_DEBUG(target, " triggered running"); target->state = TARGET_RUNNING; target->debug_reason = DBG_REASON_NOTHALTED;