diff --git a/src/rtos/riscv_debug.c b/src/rtos/riscv_debug.c index 3677466d8..b2ef66e13 100644 --- a/src/rtos/riscv_debug.c +++ b/src/rtos/riscv_debug.c @@ -248,10 +248,12 @@ static int riscv_gdb_v_packet(struct connection *connection, const char *packet, } if (strcmp(packet_stttrr, "vCont;c") == 0) { - riscv_resume_all_harts(target); target->state = TARGET_RUNNING; - target_call_event_callbacks(target, TARGET_EVENT_GDB_START); gdb_set_frontend_state_running(connection); + target_call_event_callbacks(target, TARGET_EVENT_GDB_START); + target_call_event_callbacks(target, TARGET_EVENT_RESUME_START); + riscv_resume_all_harts(target); + target_call_event_callbacks(target, TARGET_EVENT_RESUME_END); return JIM_OK; } diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 58ad93754..105a860ec 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -616,29 +616,19 @@ struct target_type riscv_target = static int riscv_poll_hart(struct target *target, int hartid) { RISCV_INFO(r); - LOG_DEBUG("polling hart %d", hartid); - - /* Polling can only detect one state change: a hart that was previously - * running but has gone to sleep. A state change in the other - * direction is invalid and indicates that one of the previous calls - * didn't correctly block. */ riscv_set_current_hartid(target, hartid); - if (riscv_was_halted(target) && !riscv_is_halted(target)) { - LOG_ERROR("unexpected wakeup on hart %d", hartid); - abort(); + + LOG_INFO("polling hart %d, target->state=%d (TARGET_HALTED=%d)", hartid, target->state, TARGET_HALTED); + + /* If OpenOCD this we're running but this hart is halted then it's time + * to raise an event. */ + if (target->state != TARGET_HALTED && riscv_is_halted(target)) { + LOG_INFO(" triggered a halt"); + r->on_halt(target); + return 1; } - /* If there's no new event then there's nothing to do. */ - if (riscv_was_halted(target) || !riscv_is_halted(target)) - return 0; - - /* If we got here then this must be the first poll during which this - * hart halted. We need to synchronize the hart's state with the - * debugger, and inform the outer polling loop that there's something - * to do. */ - r->hart_state[hartid] = RISCV_HART_HALTED; - r->on_halt(target); - return 1; + return 0; } /*** OpenOCD Interface ***/ @@ -661,7 +651,7 @@ int riscv_openocd_poll(struct target *target) } } if (triggered_hart == -1) { - LOG_DEBUG(" no harts halted"); + LOG_DEBUG(" no harts just halted, target->state=%d", target->state); return ERROR_OK; } LOG_DEBUG(" hart %d halted", triggered_hart); @@ -691,6 +681,7 @@ int riscv_openocd_poll(struct target *target) target->rtos->current_threadid = triggered_hart + 1; target->rtos->current_thread = triggered_hart + 1; + target->state = TARGET_HALTED; target_call_event_callbacks(target, TARGET_EVENT_HALTED); return ERROR_OK; } else { @@ -700,13 +691,16 @@ int riscv_openocd_poll(struct target *target) int riscv_openocd_halt(struct target *target) { - int out = riscv_halt_all_harts(target); - if (out != ERROR_OK) - return out; + LOG_DEBUG("halting all harts"); - /* Don't change the target state right here, it'll get updated by the - * poll. */ - riscv_openocd_poll(target); + int out = riscv_halt_all_harts(target); + if (out != ERROR_OK) { + LOG_ERROR("Unable to halt all harts"); + return out; + } + + target->state = TARGET_HALTED; + target_call_event_callbacks(target, TARGET_EVENT_HALTED); return out; } @@ -717,17 +711,21 @@ int riscv_openocd_resume( int handle_breakpoints, int debug_execution ) { + LOG_DEBUG("resuming all harts"); + if (!current) { LOG_ERROR("resume-at-pc unimplemented"); return ERROR_FAIL; } int out = riscv_resume_all_harts(target); - if (out != ERROR_OK) + if (out != ERROR_OK) { + LOG_ERROR("unable to resume all harts"); return out; + } target->state = TARGET_RUNNING; - riscv_openocd_poll(target); + target_call_event_callbacks(target, TARGET_EVENT_RESUMED); return out; } @@ -737,6 +735,8 @@ int riscv_openocd_step( uint32_t address, int handle_breakpoints ) { + LOG_DEBUG("stepping rtos hart"); + RISCV_INFO(r); if (!current) { @@ -745,13 +745,10 @@ int riscv_openocd_step( } int out = riscv_step_rtos_hart(target); - if (out != ERROR_OK) + if (out != ERROR_OK) { + LOG_ERROR("unable to step rtos hart"); return out; - - /* step_rtos_hart blocks until the hart has actually stepped, but we - * need to cycle through OpenOCD to actually get this to trigger. */ - target->state = TARGET_RUNNING; - riscv_openocd_poll(target); + } return out; } @@ -768,7 +765,6 @@ void riscv_info_init(riscv_info_t *r) for (size_t h = 0; h < RISCV_MAX_HARTS; ++h) { r->xlen[h] = -1; - r->hart_state[h] = RISCV_HART_UNKNOWN; r->debug_buffer_addr[h] = -1; for (size_t e = 0; e < RISCV_MAX_REGISTERS; ++e) @@ -793,23 +789,12 @@ int riscv_halt_one_hart(struct target *target, int hartid) RISCV_INFO(r); LOG_DEBUG("halting hart %d", hartid); riscv_set_current_hartid(target, hartid); - - if (r->hart_state[hartid] == RISCV_HART_UNKNOWN) { - r->hart_state[hartid] = riscv_is_halted(target) ? RISCV_HART_HALTED : RISCV_HART_RUNNING; - if (riscv_was_halted(target)) { - LOG_WARNING("Connected to hart %d, which was halted. s0, s1, and pc were overwritten by your previous debugger session and cannot be restored.", hartid); - r->on_halt(target); - } - } - - if (riscv_was_halted(target)) { + if (riscv_is_halted(target)) { LOG_DEBUG(" hart %d requested halt, but was already halted", hartid); return ERROR_OK; } r->halt_current_hart(target); - /* Here we don't actually update 'hart_state' because we want poll to - * pick that up. We can't actually wait until */ return ERROR_OK; } @@ -830,23 +815,13 @@ int riscv_resume_one_hart(struct target *target, int hartid) RISCV_INFO(r); LOG_DEBUG("resuming hart %d", hartid); riscv_set_current_hartid(target, hartid); - - if (r->hart_state[hartid] == RISCV_HART_UNKNOWN) { - r->hart_state[hartid] = riscv_is_halted(target) ? RISCV_HART_HALTED : RISCV_HART_RUNNING; - if (!riscv_was_halted(target)) { - LOG_ERROR("Asked to resume hart %d, which was in an unknown state", hartid); - r->on_resume(target); - } - } - - if (!riscv_was_halted(target)) { + if (!riscv_is_halted(target)) { LOG_DEBUG(" hart %d requested resume, but was already resumed", hartid); return ERROR_OK; } r->on_resume(target); r->resume_current_hart(target); - r->hart_state[hartid] = RISCV_HART_RUNNING; return ERROR_OK; } @@ -864,12 +839,10 @@ int riscv_step_rtos_hart(struct target *target) riscv_set_current_hartid(target, hartid); LOG_DEBUG("stepping hart %d", hartid); - assert(r->hart_state[hartid] == RISCV_HART_HALTED); + assert(riscv_is_halted(target)); r->on_step(target); r->step_current_hart(target); - r->hart_state[hartid] = RISCV_HART_RUNNING; r->on_halt(target); - r->hart_state[hartid] = RISCV_HART_HALTED; assert(riscv_is_halted(target)); return ERROR_OK; } @@ -991,13 +964,6 @@ bool riscv_is_halted(struct target *target) return r->is_halted(target); } -bool riscv_was_halted(struct target *target) -{ - RISCV_INFO(r); - assert(r->hart_state[r->current_hartid] != RISCV_HART_UNKNOWN); - return r->hart_state[r->current_hartid] == RISCV_HART_HALTED; -} - enum riscv_halt_reason riscv_halt_reason(struct target *target, int hartid) { RISCV_INFO(r); diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 84ed411db..3aedf2dd1 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -22,12 +22,6 @@ typedef uint64_t riscv_reg_t; typedef uint32_t riscv_insn_t; typedef int64_t riscv_addr_t; -enum riscv_hart_state { - RISCV_HART_UNKNOWN, - RISCV_HART_HALTED, - RISCV_HART_RUNNING, -}; - enum riscv_halt_reason { RISCV_HALT_INTERRUPT, RISCV_HALT_BREAKPOINT, @@ -67,9 +61,6 @@ typedef struct { /* It's possible that each core has a different supported ISA set. */ int xlen[RISCV_MAX_HARTS]; - /* The state of every hart. */ - enum riscv_hart_state hart_state[RISCV_MAX_HARTS]; - /* The number of triggers per hart. */ int trigger_count[RISCV_MAX_HARTS]; @@ -188,9 +179,8 @@ riscv_reg_t riscv_get_register(struct target *target, enum gdb_regno i); riscv_reg_t riscv_get_register_on_hart(struct target *target, int hid, enum gdb_regno rid); /* Checks the state of the current hart -- "is_halted" checks the actual - * on-device register, while "was_halted" checks the machine's state. */ + * on-device register. */ bool riscv_is_halted(struct target *target); -bool riscv_was_halted(struct target *target); enum riscv_halt_reason riscv_halt_reason(struct target *target, int hartid); /* Returns the number of triggers availiable to either the current hart or to diff --git a/src/target/target.c b/src/target/target.c index 42a8caca8..9c0bbcf85 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -1088,7 +1088,7 @@ int target_add_breakpoint(struct target *target, struct breakpoint *breakpoint) { if ((target->state != TARGET_HALTED) && (breakpoint->type != BKPT_HARD)) { - LOG_WARNING("target %s is not halted", target_name(target)); + LOG_WARNING("target %s is not halted (add breakpoint)", target_name(target)); return ERROR_TARGET_NOT_HALTED; } return target->type->add_breakpoint(target, breakpoint); @@ -1098,7 +1098,7 @@ int target_add_context_breakpoint(struct target *target, struct breakpoint *breakpoint) { if (target->state != TARGET_HALTED) { - LOG_WARNING("target %s is not halted", target_name(target)); + LOG_WARNING("target %s is not halted (add context breakpoint)", target_name(target)); return ERROR_TARGET_NOT_HALTED; } return target->type->add_context_breakpoint(target, breakpoint); @@ -1108,7 +1108,7 @@ int target_add_hybrid_breakpoint(struct target *target, struct breakpoint *breakpoint) { if (target->state != TARGET_HALTED) { - LOG_WARNING("target %s is not halted", target_name(target)); + LOG_WARNING("target %s is not halted (add hybrid breakpoint)", target_name(target)); return ERROR_TARGET_NOT_HALTED; } return target->type->add_hybrid_breakpoint(target, breakpoint); @@ -1124,7 +1124,7 @@ int target_add_watchpoint(struct target *target, struct watchpoint *watchpoint) { if (target->state != TARGET_HALTED) { - LOG_WARNING("target %s is not halted", target_name(target)); + LOG_WARNING("target %s is not halted (add watchpoint)", target_name(target)); return ERROR_TARGET_NOT_HALTED; } return target->type->add_watchpoint(target, watchpoint); @@ -1138,7 +1138,7 @@ int target_hit_watchpoint(struct target *target, struct watchpoint **hit_watchpoint) { if (target->state != TARGET_HALTED) { - LOG_WARNING("target %s is not halted", target->cmd_name); + LOG_WARNING("target %s is not halted (hit watchpoint)", target->cmd_name); return ERROR_TARGET_NOT_HALTED; } @@ -1167,7 +1167,8 @@ int target_step(struct target *target, int target_get_gdb_fileio_info(struct target *target, struct gdb_fileio_info *fileio_info) { if (target->state != TARGET_HALTED) { - LOG_WARNING("target %s is not halted", target->cmd_name); + LOG_WARNING("target %s is not halted (gdb fileio)", target->cmd_name); + abort(); return ERROR_TARGET_NOT_HALTED; } return target->type->get_gdb_fileio_info(target, fileio_info); @@ -1176,7 +1177,7 @@ int target_get_gdb_fileio_info(struct target *target, struct gdb_fileio_info *fi int target_gdb_fileio_end(struct target *target, int retcode, int fileio_errno, bool ctrl_c) { if (target->state != TARGET_HALTED) { - LOG_WARNING("target %s is not halted", target->cmd_name); + LOG_WARNING("target %s is not halted (gdb fileio end)", target->cmd_name); return ERROR_TARGET_NOT_HALTED; } return target->type->gdb_fileio_end(target, retcode, fileio_errno, ctrl_c); @@ -1186,7 +1187,7 @@ int target_profiling(struct target *target, uint32_t *samples, uint32_t max_num_samples, uint32_t *num_samples, uint32_t seconds) { if (target->state != TARGET_HALTED) { - LOG_WARNING("target %s is not halted", target->cmd_name); + LOG_WARNING("target %s is not halted (profiling)", target->cmd_name); return ERROR_TARGET_NOT_HALTED; } return target->type->profiling(target, samples, max_num_samples,