diff options
author | Tim Newsome <tim@sifive.com> | 2018-05-01 11:47:22 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-05-01 11:47:22 -0700 |
commit | 15993bc8db71aa6fd12c208917e45512d07ff885 (patch) | |
tree | 0a02bae7a34dd3db3f410885b79c71d171bd9b2f | |
parent | 8956edd8aa2dbfed06c1d4fe5b6d2e26540f670b (diff) | |
parent | b62c014bdc7a4b9a88ac6f44328c01ea4974c46a (diff) | |
download | riscv-openocd-15993bc8db71aa6fd12c208917e45512d07ff885.zip riscv-openocd-15993bc8db71aa6fd12c208917e45512d07ff885.tar.gz riscv-openocd-15993bc8db71aa6fd12c208917e45512d07ff885.tar.bz2 |
Merge pull request #226 from riscv/notice_reset
Notice when a hart is reset while it's being debugged, and let the user know that it happened
-rw-r--r-- | src/helper/log.h | 2 | ||||
-rw-r--r-- | src/target/riscv/batch.c | 5 | ||||
-rw-r--r-- | src/target/riscv/riscv-013.c | 304 | ||||
-rw-r--r-- | src/target/riscv/riscv.c | 102 | ||||
-rw-r--r-- | src/target/riscv/riscv.h | 7 |
5 files changed, 242 insertions, 178 deletions
diff --git a/src/helper/log.h b/src/helper/log.h index 512bcc5..d60587f 100644 --- a/src/helper/log.h +++ b/src/helper/log.h @@ -149,6 +149,8 @@ extern int debug_level; */ #define ERROR_FAIL (-4) #define ERROR_WAIT (-5) +/* ERROR_TIMEOUT is already taken by winerror.h. */ +#define ERROR_TIMEOUT_REACHED (-6) #endif /* OPENOCD_HELPER_LOG_H */ diff --git a/src/target/riscv/batch.c b/src/target/riscv/batch.c index 117119d..9327cb3 100644 --- a/src/target/riscv/batch.c +++ b/src/target/riscv/batch.c @@ -160,11 +160,10 @@ void dump_field(const struct scan_field *field) log_printf_lf(LOG_LVL_DEBUG, __FILE__, __LINE__, __PRETTY_FUNCTION__, - "%db %s %08x @%02x -> %s %08x @%02x [0x%p -> 0x%p]", + "%db %s %08x @%02x -> %s %08x @%02x", field->num_bits, op_string[out_op], out_data, out_address, - status_string[in_op], in_data, in_address, - field->out_value, field->in_value); + status_string[in_op], in_data, in_address); } else { log_printf_lf(LOG_LVL_DEBUG, __FILE__, __LINE__, __PRETTY_FUNCTION__, "%db %s %08x @%02x -> ?", diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index cec01f3..4488bda 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -39,7 +39,7 @@ static void riscv013_clear_abstract_error(struct target *target); static int riscv013_get_register(struct target *target, riscv_reg_t *value, int hid, int rid); static int riscv013_set_register(struct target *target, int hartid, int regid, uint64_t value); -static void riscv013_select_current_hart(struct target *target); +static int riscv013_select_current_hart(struct target *target); static int riscv013_halt_current_hart(struct target *target); static int riscv013_resume_current_hart(struct target *target); static int riscv013_step_current_hart(struct target *target); @@ -279,8 +279,11 @@ static void decode_dmi(char *text, unsigned address, unsigned data) { DMI_DMCONTROL, ((1L<<10)-1) << DMI_DMCONTROL_HARTSEL_OFFSET, "hartsel" }, { DMI_DMCONTROL, DMI_DMCONTROL_NDMRESET, "ndmreset" }, { DMI_DMCONTROL, DMI_DMCONTROL_DMACTIVE, "dmactive" }, + { DMI_DMCONTROL, DMI_DMCONTROL_ACKHAVERESET, "ackhavereset" }, { DMI_DMSTATUS, DMI_DMSTATUS_IMPEBREAK, "impebreak" }, + { DMI_DMSTATUS, DMI_DMSTATUS_ALLHAVERESET, "allhavereset" }, + { DMI_DMSTATUS, DMI_DMSTATUS_ANYHAVERESET, "anyhavereset" }, { DMI_DMSTATUS, DMI_DMSTATUS_ALLRESUMEACK, "allresumeack" }, { DMI_DMSTATUS, DMI_DMSTATUS_ANYRESUMEACK, "anyresumeack" }, { DMI_DMSTATUS, DMI_DMSTATUS_ALLNONEXISTENT, "allnonexistent" }, @@ -478,58 +481,79 @@ static dmi_status_t dmi_scan(struct target *target, uint32_t *address_in, return buf_get_u32(in, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH); } -static int dmi_read(struct target *target, uint32_t *value, uint32_t address) +static int dmi_op_timeout(struct target *target, uint32_t *data_in, int dmi_op, + uint32_t address, uint32_t data_out, int timeout_sec) { select_dmi(target); dmi_status_t status; uint32_t address_in; - unsigned i = 0; + const char *op_name; + switch (dmi_op) { + case DMI_OP_NOP: + op_name = "nop"; + break; + case DMI_OP_READ: + op_name = "read"; + break; + case DMI_OP_WRITE: + op_name = "write"; + break; + default: + LOG_ERROR("Invalid DMI operation: %d", dmi_op); + return ERROR_FAIL; + } - /* This first loop ensures that the read request was actually sent - * to the target. Note that if for some reason this stays busy, - * it is actually due to the previous dmi_read or dmi_write. */ - for (i = 0; i < 256; i++) { - status = dmi_scan(target, NULL, NULL, DMI_OP_READ, address, 0, + time_t start = time(NULL); + /* This first loop performs the request. Note that if for some reason this + * stays busy, it is actually due to the previous access. */ + while (1) { + status = dmi_scan(target, NULL, NULL, dmi_op, address, data_out, false); if (status == DMI_STATUS_BUSY) { increase_dmi_busy_delay(target); } else if (status == DMI_STATUS_SUCCESS) { break; } else { - LOG_ERROR("failed read from 0x%x, status=%d", address, status); + LOG_ERROR("failed %s at 0x%x, status=%d", op_name, address, status); return ERROR_FAIL; } + if (time(NULL) - start > timeout_sec) + return ERROR_TIMEOUT_REACHED; } if (status != DMI_STATUS_SUCCESS) { - LOG_ERROR("Failed read from 0x%x; status=%d", address, status); + LOG_ERROR("Failed %s at 0x%x; status=%d", op_name, address, status); return ERROR_FAIL; } - /* This second loop ensures that we got the read - * data back. Note that NOP can result in a 'busy' result as well, but - * that would be noticed on the next DMI access we do. */ - for (i = 0; i < 256; i++) { - status = dmi_scan(target, &address_in, value, DMI_OP_NOP, address, 0, + /* This second loop ensures the request succeeded, and gets back data. + * Note that NOP can result in a 'busy' result as well, but that would be + * noticed on the next DMI access we do. */ + while (1) { + status = dmi_scan(target, &address_in, data_in, DMI_OP_NOP, address, 0, false); if (status == DMI_STATUS_BUSY) { increase_dmi_busy_delay(target); } else if (status == DMI_STATUS_SUCCESS) { break; } else { - LOG_ERROR("failed read (NOP) at 0x%x, status=%d", address, status); + LOG_ERROR("failed %s (NOP) at 0x%x, status=%d", op_name, address, + status); return ERROR_FAIL; } + if (time(NULL) - start > timeout_sec) + return ERROR_TIMEOUT_REACHED; } if (status != DMI_STATUS_SUCCESS) { - if (status == DMI_STATUS_FAILED) { - LOG_ERROR("Failed read (NOP) from 0x%x; status=%d", address, status); + if (status == DMI_STATUS_FAILED || !data_in) { + LOG_ERROR("Failed %s (NOP) at 0x%x; status=%d", op_name, address, + status); } else { - LOG_ERROR("Failed read (NOP) from 0x%x; value=0x%x, status=%d", - address, *value, status); + LOG_ERROR("Failed %s (NOP) at 0x%x; value=0x%x, status=%d", + op_name, address, *data_in, status); } return ERROR_FAIL; } @@ -537,60 +561,38 @@ static int dmi_read(struct target *target, uint32_t *value, uint32_t address) return ERROR_OK; } -static int dmi_write(struct target *target, uint32_t address, uint32_t value) +static int dmi_op(struct target *target, uint32_t *data_in, int dmi_op, + uint32_t address, uint32_t data_out) { - select_dmi(target); - dmi_status_t status = DMI_STATUS_BUSY; - unsigned i = 0; - - /* The first loop ensures that we successfully sent the write request. */ - for (i = 0; i < 256; i++) { - status = dmi_scan(target, NULL, NULL, DMI_OP_WRITE, address, value, - address == DMI_COMMAND); - if (status == DMI_STATUS_BUSY) { - increase_dmi_busy_delay(target); - } else if (status == DMI_STATUS_SUCCESS) { - break; - } else { - LOG_ERROR("failed write to 0x%x, status=%d", address, status); - break; - } - } - - if (status != DMI_STATUS_SUCCESS) { - LOG_ERROR("Failed write to 0x%x;, status=%d", - address, status); + int result = dmi_op_timeout(target, data_in, dmi_op, address, data_out, + riscv_command_timeout_sec); + if (result == ERROR_TIMEOUT_REACHED) { + LOG_ERROR("DMI operation didn't complete in %d seconds. The target is " + "either really slow or broken. You could increase the " + "timeout with riscv set_command_timeout_sec.", + riscv_command_timeout_sec); return ERROR_FAIL; } + return result; +} - /* The second loop isn't strictly necessary, but would ensure that the - * write is complete/ has no non-busy errors before returning from this - * function. */ - for (i = 0; i < 256; i++) { - status = dmi_scan(target, NULL, NULL, DMI_OP_NOP, address, 0, - false); - if (status == DMI_STATUS_BUSY) { - increase_dmi_busy_delay(target); - } else if (status == DMI_STATUS_SUCCESS) { - break; - } else { - LOG_ERROR("failed write (NOP) at 0x%x, status=%d", address, status); - break; - } - } - if (status != DMI_STATUS_SUCCESS) { - LOG_ERROR("failed to write (NOP) 0x%x to 0x%x; status=%d", value, address, status); - return ERROR_FAIL; - } +static int dmi_read(struct target *target, uint32_t *value, uint32_t address) +{ + return dmi_op(target, value, DMI_OP_READ, address, 0); +} - return ERROR_OK; +static int dmi_write(struct target *target, uint32_t address, uint32_t value) +{ + return dmi_op(target, NULL, DMI_OP_WRITE, address, value); } -int dmstatus_read(struct target *target, uint32_t *dmstatus, - bool authenticated) +int dmstatus_read_timeout(struct target *target, uint32_t *dmstatus, + bool authenticated, unsigned timeout_sec) { - if (dmi_read(target, dmstatus, DMI_DMSTATUS) != ERROR_OK) - return ERROR_FAIL; + int result = dmi_op_timeout(target, dmstatus, DMI_OP_READ, DMI_DMSTATUS, 0, + timeout_sec); + if (result != ERROR_OK) + return result; if (authenticated && !get_field(*dmstatus, DMI_DMSTATUS_AUTHENTICATED)) { LOG_ERROR("Debugger is not authenticated to target Debug Module. " "(dmstatus=0x%x). Use `riscv authdata_read` and " @@ -600,6 +602,13 @@ int dmstatus_read(struct target *target, uint32_t *dmstatus, return ERROR_OK; } +int dmstatus_read(struct target *target, uint32_t *dmstatus, + bool authenticated) +{ + return dmstatus_read_timeout(target, dmstatus, authenticated, + riscv_command_timeout_sec); +} + static void increase_ac_busy_delay(struct target *target) { riscv013_info_t *info = get_info(target); @@ -1124,10 +1133,9 @@ static int register_read_direct(struct target *target, uint64_t *value, uint32_t int result = register_read_abstract(target, value, number, register_size(target, number)); - if (result != ERROR_OK && info->progbufsize + r->impebreak >= 2 && - riscv_is_halted(target)) { - assert(number != GDB_REGNO_S0); - + if (result != ERROR_OK && + info->progbufsize + r->impebreak >= 2 && + number > GDB_REGNO_XPR31) { struct riscv_program program; riscv_program_init(&program, target); @@ -1353,7 +1361,8 @@ static int examine(struct target *target) continue; r->current_hartid = i; - riscv013_select_current_hart(target); + if (riscv013_select_current_hart(target) != ERROR_OK) + return ERROR_FAIL; uint32_t s; if (dmstatus_read(target, &s, true) != ERROR_OK) @@ -1362,6 +1371,11 @@ static int examine(struct target *target) break; r->hart_count = i + 1; + if (get_field(s, DMI_DMSTATUS_ANYHAVERESET)) + dmi_write(target, DMI_DMCONTROL, + set_field(DMI_DMCONTROL_DMACTIVE | DMI_DMCONTROL_ACKHAVERESET, + hartsel_mask(target), i)); + if (!riscv_is_halted(target)) { if (riscv013_halt_current_hart(target) != ERROR_OK) { LOG_ERROR("Fatal: Hart %d failed to halt during examine()", i); @@ -1532,7 +1546,7 @@ static int assert_reset(struct target *target) /* TODO: Try to use hasel in dmcontrol */ - /* Set haltreq/resumereq for each hart. */ + /* Set haltreq for each hart. */ uint32_t control = control_base; for (int i = 0; i < riscv_count_harts(target); ++i) { if (!riscv_hart_enabled(target, i)) @@ -1553,20 +1567,8 @@ static int assert_reset(struct target *target) r->current_hartid); control = set_field(control, DMI_DMCONTROL_HALTREQ, target->reset_halt ? 1 : 0); - control = set_field(control, DMI_DMCONTROL_HARTRESET, 1); + control = set_field(control, DMI_DMCONTROL_NDMRESET, 1); dmi_write(target, DMI_DMCONTROL, control); - - /* Read back to check if hartreset is supported. */ - uint32_t rb; - if (dmi_read(target, &rb, DMI_DMCONTROL) != ERROR_OK) - return ERROR_FAIL; - if (!get_field(rb, DMI_DMCONTROL_HARTRESET)) { - /* Use ndmreset instead. That will reset the entire device, but - * that's probably what OpenOCD wants anyway. */ - control = set_field(control, DMI_DMCONTROL_HARTRESET, 0); - control = set_field(control, DMI_DMCONTROL_NDMRESET, 1); - dmi_write(target, DMI_DMCONTROL, control); - } } target->state = TARGET_RESET; @@ -1580,57 +1582,77 @@ static int deassert_reset(struct target *target) RISCV013_INFO(info); select_dmi(target); - LOG_DEBUG("%d", r->current_hartid); - /* Clear the reset, but make sure haltreq is still set */ uint32_t control = 0; control = set_field(control, DMI_DMCONTROL_HALTREQ, target->reset_halt ? 1 : 0); - control = set_field(control, hartsel_mask(target), r->current_hartid); control = set_field(control, DMI_DMCONTROL_DMACTIVE, 1); - dmi_write(target, DMI_DMCONTROL, control); + dmi_write(target, DMI_DMCONTROL, + set_field(control, hartsel_mask(target), r->current_hartid)); uint32_t dmstatus; int dmi_busy_delay = info->dmi_busy_delay; time_t start = time(NULL); - if (target->reset_halt) { - LOG_DEBUG("Waiting for hart to be halted."); - do { - if (dmstatus_read(target, &dmstatus, true) != ERROR_OK) - return ERROR_FAIL; - if (time(NULL) - start > riscv_reset_timeout_sec) { - LOG_ERROR("Hart didn't halt coming out of reset in %ds; " - "dmstatus=0x%x; " - "Increase the timeout with riscv set_reset_timeout_sec.", - riscv_reset_timeout_sec, dmstatus); - return ERROR_FAIL; - } - target->state = TARGET_HALTED; - } while (get_field(dmstatus, DMI_DMSTATUS_ALLHALTED) == 0); - - control = set_field(control, DMI_DMCONTROL_HALTREQ, 0); - dmi_write(target, DMI_DMCONTROL, control); + for (int i = 0; i < riscv_count_harts(target); ++i) { + int index = i; + if (target->rtos) { + if (!riscv_hart_enabled(target, index)) + continue; + dmi_write(target, DMI_DMCONTROL, + set_field(control, hartsel_mask(target), index)); + } else { + index = r->current_hartid; + } - } else { - LOG_DEBUG("Waiting for hart to be running."); - do { - if (dmstatus_read(target, &dmstatus, true) != ERROR_OK) - return ERROR_FAIL; - if (get_field(dmstatus, DMI_DMSTATUS_ANYHALTED) || - get_field(dmstatus, DMI_DMSTATUS_ANYUNAVAIL)) { - LOG_ERROR("Unexpected hart status during reset. dmstatus=0x%x", - dmstatus); + char *operation; + uint32_t expected_field; + uint32_t unexpected_field; + if (target->reset_halt) { + operation = "halt"; + expected_field = DMI_DMSTATUS_ALLHALTED; + unexpected_field = DMI_DMSTATUS_ANYRUNNING; + } else { + operation = "run"; + expected_field = DMI_DMSTATUS_ALLRUNNING; + unexpected_field = DMI_DMSTATUS_ANYHALTED; + } + LOG_DEBUG("Waiting for hart %d to %s out of reset.", index, operation); + while (1) { + int result = dmstatus_read_timeout(target, &dmstatus, true, + riscv_reset_timeout_sec); + if (result == ERROR_TIMEOUT_REACHED) + LOG_ERROR("Hart %d didn't complete a DMI read coming out of " + "reset in %ds; Increase the timeout with riscv " + "set_reset_timeout_sec.", + index, riscv_reset_timeout_sec); + if (result != ERROR_OK) + return result; + if (get_field(dmstatus, unexpected_field)) { + LOG_ERROR("Unexpected hart %d status during reset. dmstatus=0x%x", + index, dmstatus); return ERROR_FAIL; } + if (get_field(dmstatus, expected_field)) + break; if (time(NULL) - start > riscv_reset_timeout_sec) { - LOG_ERROR("Hart didn't run coming out of reset in %ds; " + LOG_ERROR("Hart %d didn't %s coming out of reset in %ds; " "dmstatus=0x%x; " "Increase the timeout with riscv set_reset_timeout_sec.", - riscv_reset_timeout_sec, dmstatus); + index, operation, riscv_reset_timeout_sec, dmstatus); return ERROR_FAIL; } - } while (get_field(dmstatus, DMI_DMSTATUS_ALLRUNNING) == 0); - target->state = TARGET_RUNNING; + } + target->state = TARGET_HALTED; + + if (get_field(dmstatus, DMI_DMSTATUS_ALLHAVERESET)) { + /* Ack reset. */ + dmi_write(target, DMI_DMCONTROL, + set_field(control, hartsel_mask(target), index) | + DMI_DMCONTROL_ACKHAVERESET); + } + + if (!target->rtos) + break; } info->dmi_busy_delay = dmi_busy_delay; return ERROR_OK; @@ -2655,14 +2677,15 @@ static int riscv013_set_register(struct target *target, int hid, int rid, uint64 return ERROR_OK; } -static void riscv013_select_current_hart(struct target *target) +static int riscv013_select_current_hart(struct target *target) { RISCV_INFO(r); uint32_t dmcontrol; - dmi_read(target, &dmcontrol, DMI_DMCONTROL); + if (dmi_read(target, &dmcontrol, DMI_DMCONTROL) != ERROR_OK) + return ERROR_FAIL; dmcontrol = set_field(dmcontrol, hartsel_mask(target), r->current_hartid); - dmi_write(target, DMI_DMCONTROL, dmcontrol); + return dmi_write(target, DMI_DMCONTROL, dmcontrol); } static int riscv013_halt_current_hart(struct target *target) @@ -2732,9 +2755,25 @@ static bool riscv013_is_halted(struct target *target) if (dmstatus_read(target, &dmstatus, true) != ERROR_OK) return false; if (get_field(dmstatus, DMI_DMSTATUS_ANYUNAVAIL)) - LOG_ERROR("hart %d is unavailiable", riscv_current_hartid(target)); + LOG_ERROR("Hart %d is unavailable.", riscv_current_hartid(target)); if (get_field(dmstatus, DMI_DMSTATUS_ANYNONEXISTENT)) - LOG_ERROR("hart %d doesn't exist", riscv_current_hartid(target)); + LOG_ERROR("Hart %d doesn't exist.", riscv_current_hartid(target)); + if (get_field(dmstatus, DMI_DMSTATUS_ANYHAVERESET)) { + int hartid = riscv_current_hartid(target); + LOG_INFO("Hart %d unexpectedly reset!", hartid); + /* TODO: Can we make this more obvious to eg. a gdb user? */ + uint32_t dmcontrol = DMI_DMCONTROL_DMACTIVE | + DMI_DMCONTROL_ACKHAVERESET; + dmcontrol = set_field(dmcontrol, hartsel_mask(target), hartid); + /* If we had been halted when we reset, request another halt. If we + * ended up running out of reset, then the user will (hopefully) get a + * message that a reset happened, that the target is running, and then + * that it is halted again once the request goes through. + */ + if (target->state == TARGET_HALTED) + dmcontrol |= DMI_DMCONTROL_HALTREQ; + dmi_write(target, DMI_DMCONTROL, dmcontrol); + } return get_field(dmstatus, DMI_DMSTATUS_ALLHALTED); } @@ -2863,11 +2902,9 @@ static int riscv013_step_or_resume_current_hart(struct target *target, bool step return ERROR_FAIL; /* Issue the resume command, and then wait for the current hart to resume. */ - uint32_t dmcontrol; - if (dmi_read(target, &dmcontrol, DMI_DMCONTROL) != ERROR_OK) - return ERROR_FAIL; - dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 1); - dmi_write(target, DMI_DMCONTROL, dmcontrol); + uint32_t dmcontrol = DMI_DMCONTROL_DMACTIVE; + dmcontrol = set_field(dmcontrol, hartsel_mask(target), r->current_hartid); + dmi_write(target, DMI_DMCONTROL, dmcontrol | DMI_DMCONTROL_RESUMEREQ); uint32_t dmstatus; for (size_t i = 0; i < 256; ++i) { @@ -2879,17 +2916,16 @@ static int riscv013_step_or_resume_current_hart(struct target *target, bool step if (step && get_field(dmstatus, DMI_DMSTATUS_ALLHALTED) == 0) continue; - dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 0); dmi_write(target, DMI_DMCONTROL, dmcontrol); return ERROR_OK; } - if (dmstatus_read(target, &dmstatus, true) != ERROR_OK) - return ERROR_FAIL; + LOG_ERROR("unable to resume hart %d", r->current_hartid); if (dmi_read(target, &dmcontrol, DMI_DMCONTROL) != ERROR_OK) return ERROR_FAIL; - LOG_ERROR("unable to resume hart %d", r->current_hartid); LOG_ERROR(" dmcontrol=0x%08x", dmcontrol); + if (dmstatus_read(target, &dmstatus, true) != ERROR_OK) + return ERROR_FAIL; LOG_ERROR(" dmstatus =0x%08x", dmstatus); if (step) { @@ -2914,7 +2950,7 @@ void riscv013_clear_abstract_error(struct target *target) LOG_ERROR("abstractcs.busy is not going low after %d seconds " "(abstractcs=0x%x). The target is either really slow or " "broken. You could increase the timeout with riscv " - "set_reset_timeout_sec.", + "set_command_timeout_sec.", riscv_command_timeout_sec, abstractcs); break; } diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 4a0bc66..bdae76a 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -741,19 +741,20 @@ static int old_or_new_riscv_resume( return riscv_openocd_resume(target, current, address, handle_breakpoints, debug_execution); } -static void riscv_select_current_hart(struct target *target) +static int riscv_select_current_hart(struct target *target) { RISCV_INFO(r); if (r->rtos_hartid != -1 && riscv_rtos_enabled(target)) - riscv_set_current_hartid(target, r->rtos_hartid); + return riscv_set_current_hartid(target, r->rtos_hartid); else - riscv_set_current_hartid(target, target->coreid); + return riscv_set_current_hartid(target, target->coreid); } static int riscv_read_memory(struct target *target, target_addr_t address, uint32_t size, uint32_t count, uint8_t *buffer) { - riscv_select_current_hart(target); + if (riscv_select_current_hart(target) != ERROR_OK) + return ERROR_FAIL; struct target_type *tt = get_target_type(target); return tt->read_memory(target, address, size, count, buffer); } @@ -761,7 +762,8 @@ static int riscv_read_memory(struct target *target, target_addr_t address, static int riscv_write_memory(struct target *target, target_addr_t address, uint32_t size, uint32_t count, const uint8_t *buffer) { - riscv_select_current_hart(target); + if (riscv_select_current_hart(target) != ERROR_OK) + return ERROR_FAIL; struct target_type *tt = get_target_type(target); return tt->write_memory(target, address, size, count, buffer); } @@ -779,7 +781,8 @@ static int riscv_get_gdb_reg_list(struct target *target, return ERROR_FAIL; } - riscv_select_current_hart(target); + if (riscv_select_current_hart(target) != ERROR_OK) + return ERROR_FAIL; switch (reg_class) { case REG_CLASS_GENERAL: @@ -968,50 +971,61 @@ int riscv_blank_check_memory(struct target *target, /*** OpenOCD Helper Functions ***/ -/* 0 means nothing happened, 1 means the hart's state changed (and thus the - * poll should terminate), and -1 means there was an error. */ -static int riscv_poll_hart(struct target *target, int hartid) +enum riscv_poll_hart { + RPH_NO_CHANGE, + RPH_DISCOVERED_HALTED, + RPH_DISCOVERED_RUNNING, + RPH_ERROR +}; +static enum riscv_poll_hart riscv_poll_hart(struct target *target, int hartid) { RISCV_INFO(r); - riscv_set_current_hartid(target, hartid); + if (riscv_set_current_hartid(target, hartid) != ERROR_OK) + return RPH_ERROR; - LOG_DEBUG("polling hart %d, target->state=%d (TARGET_HALTED=%d)", hartid, target->state, TARGET_HALTED); + LOG_DEBUG("polling hart %d, target->state=%d", hartid, target->state); - /* If OpenOCD this we're running but this hart is halted then it's time + /* If OpenOCD thinks 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)) { + bool halted = riscv_is_halted(target); + if (target->state != TARGET_HALTED && halted) { LOG_DEBUG(" triggered a halt"); r->on_halt(target); - return 1; + return RPH_DISCOVERED_HALTED; + } else if (target->state != TARGET_RUNNING && !halted) { + LOG_DEBUG(" triggered running"); + target->state = TARGET_RUNNING; + return RPH_DISCOVERED_RUNNING; } - return 0; + return RPH_NO_CHANGE; } /*** OpenOCD Interface ***/ int riscv_openocd_poll(struct target *target) { LOG_DEBUG("polling all harts"); - int triggered_hart = -1; + int halted_hart = -1; if (riscv_rtos_enabled(target)) { /* Check every hart for an event. */ for (int i = 0; i < riscv_count_harts(target); ++i) { - int out = riscv_poll_hart(target, i); + enum riscv_poll_hart out = riscv_poll_hart(target, i); switch (out) { - case 0: + case RPH_NO_CHANGE: + case RPH_DISCOVERED_RUNNING: continue; - case 1: - triggered_hart = i; + case RPH_DISCOVERED_HALTED: + halted_hart = i; break; - case -1: + case RPH_ERROR: return ERROR_FAIL; } } - if (triggered_hart == -1) { + if (halted_hart == -1) { LOG_DEBUG(" no harts just halted, target->state=%d", target->state); return ERROR_OK; } - LOG_DEBUG(" hart %d halted", triggered_hart); + LOG_DEBUG(" hart %d halted", halted_hart); /* If we're here then at least one hart triggered. That means * we want to go and halt _every_ hart in the system, as that's @@ -1022,15 +1036,19 @@ int riscv_openocd_poll(struct target *target) for (int i = 0; i < riscv_count_harts(target); ++i) riscv_halt_one_hart(target, i); } else { - if (riscv_poll_hart(target, riscv_current_hartid(target)) == 0) + enum riscv_poll_hart out = riscv_poll_hart(target, + riscv_current_hartid(target)); + if (out == RPH_NO_CHANGE || out == RPH_DISCOVERED_RUNNING) return ERROR_OK; + else if (out == RPH_ERROR) + return ERROR_FAIL; - triggered_hart = riscv_current_hartid(target); - LOG_DEBUG(" hart %d halted", triggered_hart); + halted_hart = riscv_current_hartid(target); + LOG_DEBUG(" hart %d halted", halted_hart); } target->state = TARGET_HALTED; - switch (riscv_halt_reason(target, triggered_hart)) { + switch (riscv_halt_reason(target, halted_hart)) { case RISCV_HALT_BREAKPOINT: target->debug_reason = DBG_REASON_BREAKPOINT; break; @@ -1046,11 +1064,13 @@ int riscv_openocd_poll(struct target *target) case RISCV_HALT_UNKNOWN: target->debug_reason = DBG_REASON_UNDEFINED; break; + case RISCV_HALT_ERROR: + return ERROR_FAIL; } if (riscv_rtos_enabled(target)) { - target->rtos->current_threadid = triggered_hart + 1; - target->rtos->current_thread = triggered_hart + 1; + target->rtos->current_threadid = halted_hart + 1; + target->rtos->current_thread = halted_hart + 1; } target->state = TARGET_HALTED; @@ -1582,7 +1602,8 @@ 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 (riscv_set_current_hartid(target, hartid) != ERROR_OK) + return ERROR_FAIL; if (riscv_is_halted(target)) { LOG_DEBUG(" hart %d requested halt, but was already halted", hartid); return ERROR_OK; @@ -1608,7 +1629,8 @@ 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 (riscv_set_current_hartid(target, hartid) != ERROR_OK) + return ERROR_FAIL; if (!riscv_is_halted(target)) { LOG_DEBUG(" hart %d requested resume, but was already resumed", hartid); return ERROR_OK; @@ -1629,7 +1651,8 @@ int riscv_step_rtos_hart(struct target *target) hartid = 0; } } - riscv_set_current_hartid(target, hartid); + if (riscv_set_current_hartid(target, hartid) != ERROR_OK) + return ERROR_FAIL; LOG_DEBUG("stepping hart %d", hartid); if (!riscv_is_halted(target)) { @@ -1679,33 +1702,35 @@ bool riscv_rtos_enabled(const struct target *target) return target->rtos != NULL; } -void riscv_set_current_hartid(struct target *target, int hartid) +int riscv_set_current_hartid(struct target *target, int hartid) { RISCV_INFO(r); if (!r->select_current_hart) - return; + return ERROR_FAIL; int previous_hartid = riscv_current_hartid(target); r->current_hartid = hartid; assert(riscv_hart_enabled(target, hartid)); LOG_DEBUG("setting hartid to %d, was %d", hartid, previous_hartid); - r->select_current_hart(target); + if (r->select_current_hart(target) != ERROR_OK) + return ERROR_FAIL; /* This might get called during init, in which case we shouldn't be * setting up the register cache. */ if (!target_was_examined(target)) - return; + return ERROR_OK; /* Avoid invalidating the register cache all the time. */ if (r->registers_initialized && (!riscv_rtos_enabled(target) || (previous_hartid == hartid)) && target->reg_cache->reg_list[GDB_REGNO_ZERO].size == (unsigned)riscv_xlen(target) && (!riscv_rtos_enabled(target) || (r->rtos_hartid != -1))) { - return; + return ERROR_OK; } else LOG_DEBUG("Initializing registers: xlen=%d", riscv_xlen(target)); riscv_invalidate_register_cache(target); + return ERROR_OK; } void riscv_invalidate_register_cache(struct target *target) @@ -1795,7 +1820,8 @@ bool riscv_is_halted(struct target *target) enum riscv_halt_reason riscv_halt_reason(struct target *target, int hartid) { RISCV_INFO(r); - riscv_set_current_hartid(target, hartid); + if (riscv_set_current_hartid(target, hartid) != ERROR_OK) + return RISCV_HALT_ERROR; if (!riscv_is_halted(target)) { LOG_ERROR("Hart is not halted!"); return RISCV_HALT_UNKNOWN; diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 8e06bd5..63a3b79 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -31,7 +31,8 @@ enum riscv_halt_reason { RISCV_HALT_BREAKPOINT, RISCV_HALT_SINGLESTEP, RISCV_HALT_TRIGGER, - RISCV_HALT_UNKNOWN + RISCV_HALT_UNKNOWN, + RISCV_HALT_ERROR }; typedef struct { @@ -92,7 +93,7 @@ typedef struct { riscv_reg_t *value, int hid, int rid); int (*set_register)(struct target *, int hartid, int regid, uint64_t value); - void (*select_current_hart)(struct target *); + int (*select_current_hart)(struct target *); bool (*is_halted)(struct target *target); int (*halt_current_hart)(struct target *); int (*resume_current_hart)(struct target *target); @@ -192,7 +193,7 @@ bool riscv_rtos_enabled(const struct target *target); /* Sets the current hart, which is the hart that will actually be used when * issuing debug commands. */ -void riscv_set_current_hartid(struct target *target, int hartid); +int riscv_set_current_hartid(struct target *target, int hartid); int riscv_current_hartid(const struct target *target); /*** Support functions for the RISC-V 'RTOS', which provides multihart support |