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

Remove the hart_state variable 

The new code always keeps harts in consistant states, so it's no longer
necessary to track their states manually.  There's still some flakyness
with halt/resume over breakpoints.
This commit is contained in:
Palmer Dabbelt 2017-04-14 22:16:03 -07:00
parent bb2cceac25
commit 4b21319d7e
4 changed files with 49 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/rtos/riscv_debug.c
View File

@ -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;
}

104
src/target/riscv/riscv.c
View File

@ -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);

12
src/target/riscv/riscv.h
View File

@ -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

17
src/target/target.c
View File

@ -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,