From 2deb02695e851dc461bc24d49340a7e9b4a2c8e7 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 12 Jul 2017 17:51:38 -0700 Subject: [PATCH 1/8] Forgot to commit this follow up to PR #79 --- src/target/riscv/riscv-013.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 758e1d3c0..3589e6f28 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -938,6 +938,9 @@ static int init_target(struct command_context *cmd_ctx, // Assume all these abstract commands are supported until we learn // otherwise. + // TODO: The spec allows eg. one CSR to be able to be accessed abstractly + // while another one isn't. We don't track that this closely here, but in + // the future we probably should. info->abstract_read_csr_supported = true; info->abstract_write_csr_supported = true; info->abstract_read_fpr_supported = true; From d60dbd60e8d04b37de5fb39a366eabeab23547b2 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 11 Jul 2017 14:49:35 -0700 Subject: [PATCH 2/8] Share trigger code between 0.11 and 0.13 code. The actual implementation of triggers didn't change between those two versions, so there's no need to duplicate the code. In the process, I also fixed a minor multicore bug where tselect didn't always get written on all harts. --- src/target/riscv/riscv-011.c | 339 ++-------------------------------- src/target/riscv/riscv-013.c | 300 +----------------------------- src/target/riscv/riscv.c | 348 ++++++++++++++++++++++++++++++++--- src/target/riscv/riscv.h | 15 ++ 4 files changed, 359 insertions(+), 643 deletions(-) diff --git a/src/target/riscv/riscv-011.c b/src/target/riscv/riscv-011.c index 6e7b97065..b126cf6fc 100644 --- a/src/target/riscv/riscv-011.c +++ b/src/target/riscv/riscv-011.c @@ -171,7 +171,6 @@ enum { REG_COUNT }; -#define MAX_HWBPS 16 #define DRAM_CACHE_SIZE 16 struct trigger { @@ -196,7 +195,6 @@ typedef struct { unsigned int dramsize; uint64_t dcsr; uint64_t dpc; - uint64_t misa; uint64_t tselect; bool tselect_dirty; /* The value that mstatus actually has on the target right now. This is not @@ -212,10 +210,6 @@ typedef struct { /* Single buffer that contains all register values. */ void *reg_values; - // For each physical trigger, contains -1 if the hwbp is available, or the - // unique_id of the breakpoint/watchpoint that is using it. - int trigger_unique_id[MAX_HWBPS]; - // Number of run-test/idle cycles the target requests we do after each dbus // access. unsigned int dtmcontrol_idle; @@ -1519,9 +1513,6 @@ static int init_target(struct command_context *cmd_ctx, reg_name += strlen(reg_name) + 1; assert(reg_name < info->reg_names + REG_COUNT * max_reg_name_len); } - update_reg_list(target); - - memset(info->trigger_unique_id, 0xff, sizeof(info->trigger_unique_id)); return ERROR_OK; } @@ -1534,306 +1525,6 @@ static void deinit_target(struct target *target) info->version_specific = NULL; } -static int maybe_add_trigger_t1(struct target *target, struct trigger *trigger, - uint64_t tdata1) -{ - riscv011_info_t *info = get_info(target); - - const uint32_t bpcontrol_x = 1<<0; - const uint32_t bpcontrol_w = 1<<1; - const uint32_t bpcontrol_r = 1<<2; - const uint32_t bpcontrol_u = 1<<3; - const uint32_t bpcontrol_s = 1<<4; - const uint32_t bpcontrol_h = 1<<5; - const uint32_t bpcontrol_m = 1<<6; - const uint32_t bpcontrol_bpmatch = 0xf << 7; - const uint32_t bpcontrol_bpaction = 0xff << 11; - - if (tdata1 & (bpcontrol_r | bpcontrol_w | bpcontrol_x)) { - // Trigger is already in use, presumably by user code. - return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; - } - - tdata1 = set_field(tdata1, bpcontrol_r, trigger->read); - tdata1 = set_field(tdata1, bpcontrol_w, trigger->write); - tdata1 = set_field(tdata1, bpcontrol_x, trigger->execute); - tdata1 = set_field(tdata1, bpcontrol_u, !!(info->misa & (1 << ('U' - 'A')))); - tdata1 = set_field(tdata1, bpcontrol_s, !!(info->misa & (1 << ('S' - 'A')))); - tdata1 = set_field(tdata1, bpcontrol_h, !!(info->misa & (1 << ('H' - 'A')))); - tdata1 |= bpcontrol_m; - tdata1 = set_field(tdata1, bpcontrol_bpmatch, 0); // exact match - tdata1 = set_field(tdata1, bpcontrol_bpaction, 0); // cause bp exception - - write_csr(target, CSR_TDATA1, tdata1); - - uint64_t tdata1_rb; - read_csr(target, &tdata1_rb, CSR_TDATA1); - LOG_DEBUG("tdata1=0x%" PRIx64, tdata1_rb); - - if (tdata1 != tdata1_rb) { - LOG_DEBUG("Trigger doesn't support what we need; After writing 0x%" - PRIx64 " to tdata1 it contains 0x%" PRIx64, - tdata1, tdata1_rb); - write_csr(target, CSR_TDATA1, 0); - return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; - } - - write_csr(target, CSR_TDATA2, trigger->address); - - return ERROR_OK; -} - -static int maybe_add_trigger_t2(struct target *target, struct trigger *trigger, - uint64_t tdata1) -{ - riscv011_info_t *info = get_info(target); - // tselect is already set - if (tdata1 & (MCONTROL_EXECUTE | MCONTROL_STORE | MCONTROL_LOAD)) { - // Trigger is already in use, presumably by user code. - return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; - } - - // address/data match trigger - tdata1 |= MCONTROL_DMODE(riscv_xlen(target)); - tdata1 = set_field(tdata1, MCONTROL_ACTION, - MCONTROL_ACTION_DEBUG_MODE); - tdata1 = set_field(tdata1, MCONTROL_MATCH, MCONTROL_MATCH_EQUAL); - tdata1 |= MCONTROL_M; - if (info->misa & (1 << ('H' - 'A'))) - tdata1 |= MCONTROL_H; - if (info->misa & (1 << ('S' - 'A'))) - tdata1 |= MCONTROL_S; - if (info->misa & (1 << ('U' - 'A'))) - tdata1 |= MCONTROL_U; - - if (trigger->execute) - tdata1 |= MCONTROL_EXECUTE; - if (trigger->read) - tdata1 |= MCONTROL_LOAD; - if (trigger->write) - tdata1 |= MCONTROL_STORE; - - write_csr(target, CSR_TDATA1, tdata1); - - uint64_t tdata1_rb; - read_csr(target, &tdata1_rb, CSR_TDATA1); - LOG_DEBUG("tdata1=0x%" PRIx64, tdata1_rb); - - if (tdata1 != tdata1_rb) { - LOG_DEBUG("Trigger doesn't support what we need; After writing 0x%" - PRIx64 " to tdata1 it contains 0x%" PRIx64, - tdata1, tdata1_rb); - write_csr(target, CSR_TDATA1, 0); - return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; - } - - write_csr(target, CSR_TDATA2, trigger->address); - - return ERROR_OK; -} - -static int add_trigger(struct target *target, struct trigger *trigger) -{ - riscv011_info_t *info = get_info(target); - RISCV_INFO(r); - - maybe_read_tselect(target); - - unsigned int i; - for (i = 0; i < r->trigger_count[0]; i++) { - if (info->trigger_unique_id[i] != -1) { - continue; - } - - write_csr(target, CSR_TSELECT, i); - - uint64_t tdata1; - read_csr(target, &tdata1, CSR_TDATA1); - int type = get_field(tdata1, MCONTROL_TYPE(riscv_xlen(target))); - - int result; - switch (type) { - case 1: - result = maybe_add_trigger_t1(target, trigger, tdata1); - break; - case 2: - result = maybe_add_trigger_t2(target, trigger, tdata1); - break; - default: - LOG_DEBUG("trigger %d has unknown type %d", i, type); - continue; - } - - if (result != ERROR_OK) { - continue; - } - - LOG_DEBUG("Using resource %d for bp %d", i, - trigger->unique_id); - info->trigger_unique_id[i] = trigger->unique_id; - break; - } - if (i >= r->trigger_count[0]) { - LOG_ERROR("Couldn't find an available hardware trigger."); - return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; - } - - return ERROR_OK; -} - -static int remove_trigger(struct target *target, struct trigger *trigger) -{ - RISCV_INFO(r); - riscv011_info_t *info = get_info(target); - - maybe_read_tselect(target); - - unsigned int i; - for (i = 0; i < r->trigger_count[0]; i++) { - if (info->trigger_unique_id[i] == trigger->unique_id) { - break; - } - } - if (i >= r->trigger_count[0]) { - LOG_ERROR("Couldn't find the hardware resources used by hardware " - "trigger."); - return ERROR_FAIL; - } - LOG_DEBUG("Stop using resource %d for bp %d", i, trigger->unique_id); - write_csr(target, CSR_TSELECT, i); - write_csr(target, CSR_TDATA1, 0); - info->trigger_unique_id[i] = -1; - - return ERROR_OK; -} - -static void trigger_from_breakpoint(struct trigger *trigger, - const struct breakpoint *breakpoint) -{ - trigger->address = breakpoint->address; - trigger->length = breakpoint->length; - trigger->mask = ~0LL; - trigger->read = false; - trigger->write = false; - trigger->execute = true; - // unique_id is unique across both breakpoints and watchpoints. - trigger->unique_id = breakpoint->unique_id; -} - -static void trigger_from_watchpoint(struct trigger *trigger, - const struct watchpoint *watchpoint) -{ - trigger->address = watchpoint->address; - trigger->length = watchpoint->length; - trigger->mask = watchpoint->mask; - trigger->value = watchpoint->value; - trigger->read = (watchpoint->rw == WPT_READ || watchpoint->rw == WPT_ACCESS); - trigger->write = (watchpoint->rw == WPT_WRITE || watchpoint->rw == WPT_ACCESS); - trigger->execute = false; - // unique_id is unique across both breakpoints and watchpoints. - trigger->unique_id = watchpoint->unique_id; -} - -static int add_breakpoint(struct target *target, - struct breakpoint *breakpoint) -{ - if (breakpoint->type == BKPT_SOFT) { - if (target_read_memory(target, breakpoint->address, breakpoint->length, 1, - breakpoint->orig_instr) != ERROR_OK) { - LOG_ERROR("Failed to read original instruction at 0x%" TARGET_PRIxADDR, - breakpoint->address); - return ERROR_FAIL; - } - - int retval; - if (breakpoint->length == 4) { - retval = target_write_u32(target, breakpoint->address, ebreak()); - } else { - retval = target_write_u16(target, breakpoint->address, ebreak_c()); - } - if (retval != ERROR_OK) { - LOG_ERROR("Failed to write %d-byte breakpoint instruction at 0x%" - TARGET_PRIxADDR, breakpoint->length, breakpoint->address); - return ERROR_FAIL; - } - - } else if (breakpoint->type == BKPT_HARD) { - struct trigger trigger; - trigger_from_breakpoint(&trigger, breakpoint); - int result = add_trigger(target, &trigger); - if (result != ERROR_OK) { - return result; - } - - } else { - LOG_INFO("OpenOCD only supports hardware and software breakpoints."); - return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; - } - - breakpoint->set = true; - - return ERROR_OK; -} - -static int remove_breakpoint(struct target *target, - struct breakpoint *breakpoint) -{ - if (breakpoint->type == BKPT_SOFT) { - if (target_write_memory(target, breakpoint->address, breakpoint->length, 1, - breakpoint->orig_instr) != ERROR_OK) { - LOG_ERROR("Failed to restore instruction for %d-byte breakpoint at " - "0x%" TARGET_PRIxADDR, breakpoint->length, breakpoint->address); - return ERROR_FAIL; - } - - } else if (breakpoint->type == BKPT_HARD) { - struct trigger trigger; - trigger_from_breakpoint(&trigger, breakpoint); - int result = remove_trigger(target, &trigger); - if (result != ERROR_OK) { - return result; - } - - } else { - LOG_INFO("OpenOCD only supports hardware and software breakpoints."); - return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; - } - - breakpoint->set = false; - - return ERROR_OK; -} - -static int add_watchpoint(struct target *target, - struct watchpoint *watchpoint) -{ - struct trigger trigger; - trigger_from_watchpoint(&trigger, watchpoint); - - int result = add_trigger(target, &trigger); - if (result != ERROR_OK) { - return result; - } - watchpoint->set = true; - - return ERROR_OK; -} - -static int remove_watchpoint(struct target *target, - struct watchpoint *watchpoint) -{ - struct trigger trigger; - trigger_from_watchpoint(&trigger, watchpoint); - - int result = remove_trigger(target, &trigger); - if (result != ERROR_OK) { - return result; - } - watchpoint->set = false; - - return ERROR_OK; -} - static int strict_step(struct target *target, bool announce) { riscv011_info_t *info = get_info(target); @@ -1842,13 +1533,13 @@ static int strict_step(struct target *target, bool announce) struct breakpoint *breakpoint = target->breakpoints; while (breakpoint) { - remove_breakpoint(target, breakpoint); + riscv_remove_breakpoint(target, breakpoint); breakpoint = breakpoint->next; } struct watchpoint *watchpoint = target->watchpoints; while (watchpoint) { - remove_watchpoint(target, watchpoint); + riscv_remove_watchpoint(target, watchpoint); watchpoint = watchpoint->next; } @@ -1858,13 +1549,13 @@ static int strict_step(struct target *target, bool announce) breakpoint = target->breakpoints; while (breakpoint) { - add_breakpoint(target, breakpoint); + riscv_add_breakpoint(target, breakpoint); breakpoint = breakpoint->next; } watchpoint = target->watchpoints; while (watchpoint) { - add_watchpoint(target, watchpoint); + riscv_add_watchpoint(target, watchpoint); watchpoint = watchpoint->next; } @@ -1962,6 +1653,9 @@ static int examine(struct target *target) return ERROR_FAIL; } + // Pretend this is a 32-bit system until we have found out the true value. + r->xlen[0] = 32; + // Figure out XLEN, and test writing all of Debug RAM while we're at it. cache_set32(target, 0, xori(S1, ZERO, -1)); // 0xffffffff 0xffffffff:ffffffff 0xffffffff:ffffffff:ffffffff:ffffffff @@ -2007,12 +1701,14 @@ static int examine(struct target *target) // Update register list to match discovered XLEN. update_reg_list(target); - if (read_csr(target, &info->misa, CSR_MISA) != ERROR_OK) { - LOG_WARNING("Failed to read misa at 0x%x.", CSR_MISA); - if (read_csr(target, &info->misa, 0xf10) != ERROR_OK) { + if (read_csr(target, &r->misa, CSR_MISA) != ERROR_OK) { + const unsigned old_csr_misa = 0xf10; + LOG_WARNING("Failed to read misa at 0x%x; trying 0x%x.", CSR_MISA, + old_csr_misa); + if (read_csr(target, &r->misa, old_csr_misa) != ERROR_OK) { // Maybe this is an old core that still has $misa at the old // address. - LOG_ERROR("Failed to read misa at 0x%x.", 0xf10); + LOG_ERROR("Failed to read misa at 0x%x.", old_csr_misa); return ERROR_FAIL; } } @@ -2028,7 +1724,8 @@ static int examine(struct target *target) riscv_set_current_hartid(target, 0); for (size_t i = 0; i < 32; ++i) reg_cache_set(target, i, -1); - LOG_INFO("Examined RISCV core; XLEN=%d, misa=0x%" PRIx64, riscv_xlen(target), info->misa); + LOG_INFO("Examined RISCV core; XLEN=%d, misa=0x%" PRIx64, + riscv_xlen(target), r->misa); return ERROR_OK; } @@ -2699,11 +2396,5 @@ struct target_type riscv011_target = .read_memory = read_memory, .write_memory = write_memory, - .add_breakpoint = add_breakpoint, - .remove_breakpoint = remove_breakpoint, - - .add_watchpoint = add_watchpoint, - .remove_watchpoint = remove_watchpoint, - .arch_state = arch_state, }; diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 3589e6f28..f82b9bc14 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -123,8 +123,6 @@ typedef enum slot { #define WALL_CLOCK_TIMEOUT 2 -#define MAX_HWBPS 16 - struct trigger { uint64_t address; uint32_t length; @@ -149,7 +147,6 @@ typedef struct { unsigned progsize; /* Number of Program Buffer registers. */ /* Number of words in Debug RAM. */ - uint64_t misa; uint64_t tselect; bool tselect_dirty; /* The value that mstatus actually has on the target right now. This is not @@ -163,10 +160,6 @@ typedef struct { /* Single buffer that contains all register values. */ void *reg_values; - // For each physical trigger, contains -1 if the hwbp is available, or the - // unique_id of the breakpoint/watchpoint that is using it. - int trigger_unique_id[MAX_HWBPS]; - // Number of run-test/idle cycles the target requests we do after each dbus // access. unsigned int dtmcontrol_idle; @@ -298,7 +291,7 @@ static int register_get(struct reg *reg); bool supports_extension(struct target *target, char letter) { - riscv013_info_t *info = get_info(target); + RISCV_INFO(r); unsigned num; if (letter >= 'a' && letter <= 'z') { num = letter - 'a'; @@ -307,7 +300,7 @@ bool supports_extension(struct target *target, char letter) } else { return false; } - return info->misa & (1 << num); + return r->misa & (1 << num); } static void select_dmi(struct target *target) @@ -844,20 +837,6 @@ static int register_read_direct(struct target *target, uint64_t *value, uint32_t return ERROR_OK; } -static int maybe_read_tselect(struct target *target) -{ - riscv013_info_t *info = get_info(target); - - if (info->tselect_dirty) { - int result = register_read_direct(target, &info->tselect, GDB_REGNO_TSELECT); - if (result != ERROR_OK) - return result; - info->tselect_dirty = false; - } - - return ERROR_OK; -} - /*** OpenOCD target functions. ***/ static int register_get(struct reg *reg) @@ -900,7 +879,6 @@ static int init_target(struct command_context *cmd_ctx, LOG_DEBUG("init"); riscv_info_t *generic_info = (riscv_info_t *) target->arch_info; - riscv_info_init(target, generic_info); generic_info->get_register = &riscv013_get_register; generic_info->set_register = &riscv013_set_register; generic_info->select_current_hart = &riscv013_select_current_hart; @@ -983,11 +961,6 @@ static int init_target(struct command_context *cmd_ctx, reg_name += strlen(reg_name) + 1; assert(reg_name < info->reg_names + GDB_REGNO_COUNT * max_reg_name_len); } -#if 0 - update_reg_list(target); -#endif - - memset(info->trigger_unique_id, 0xff, sizeof(info->trigger_unique_id)); return ERROR_OK; } @@ -1000,269 +973,6 @@ static void deinit_target(struct target *target) info->version_specific = NULL; } -static int add_trigger(struct target *target, struct trigger *trigger) -{ - riscv013_info_t *info = get_info(target); - - // While we're using threads to fake harts, both gdb and OpenOCD assume - // that hardware breakpoints are shared among threads. Make this true by - // setting the same breakpoints on all harts. - - // Assume that all triggers are configured the same on all harts. - riscv_set_current_hartid(target, 0); - - maybe_read_tselect(target); - - int i; - for (i = 0; i < riscv_count_triggers(target); i++) { - if (info->trigger_unique_id[i] != -1) { - continue; - } - - register_write_direct(target, GDB_REGNO_TSELECT, i); - - uint64_t tdata1; - register_read_direct(target, &tdata1, GDB_REGNO_TDATA1); - int type = get_field(tdata1, MCONTROL_TYPE(riscv_xlen(target))); - - if (type != 2) { - continue; - } - - if (tdata1 & (MCONTROL_EXECUTE | MCONTROL_STORE | MCONTROL_LOAD)) { - // Trigger is already in use, presumably by user code. - continue; - } - - uint64_t tdata1_rb; - for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) { - if (!riscv_hart_enabled(target, hartid)) - continue; - - riscv_set_current_hartid(target, hartid); - - if (hartid > 0) { - register_write_direct(target, GDB_REGNO_TSELECT, i); - } - - // address/data match trigger - tdata1 |= MCONTROL_DMODE(riscv_xlen(target)); - tdata1 = set_field(tdata1, MCONTROL_ACTION, - MCONTROL_ACTION_DEBUG_MODE); - tdata1 = set_field(tdata1, MCONTROL_MATCH, MCONTROL_MATCH_EQUAL); - tdata1 |= MCONTROL_M; - if (info->misa & (1 << ('H' - 'A'))) - tdata1 |= MCONTROL_H; - if (info->misa & (1 << ('S' - 'A'))) - tdata1 |= MCONTROL_S; - if (info->misa & (1 << ('U' - 'A'))) - tdata1 |= MCONTROL_U; - - if (trigger->execute) - tdata1 |= MCONTROL_EXECUTE; - if (trigger->read) - tdata1 |= MCONTROL_LOAD; - if (trigger->write) - tdata1 |= MCONTROL_STORE; - - register_write_direct(target, GDB_REGNO_TDATA1, tdata1); - - register_read_direct(target, &tdata1_rb, GDB_REGNO_TDATA1); - LOG_DEBUG("tdata1=0x%" PRIx64, tdata1_rb); - - if (tdata1 != tdata1_rb) { - LOG_DEBUG("Trigger %d doesn't support what we need; After writing 0x%" - PRIx64 " to tdata1 it contains 0x%" PRIx64, - i, tdata1, tdata1_rb); - register_write_direct(target, GDB_REGNO_TDATA1, 0); - if (hartid > 0) { - LOG_ERROR("Setting hardware breakpoints requires " - "homogeneous harts."); - return ERROR_FAIL; - } - break; - } - - register_write_direct(target, GDB_REGNO_TDATA2, trigger->address); - } - - if (tdata1 != tdata1_rb) { - continue; - } - - LOG_DEBUG("Using resource %d for bp %d", i, - trigger->unique_id); - info->trigger_unique_id[i] = trigger->unique_id; - break; - } - if (i >= riscv_count_triggers(target)) { - LOG_ERROR("Couldn't find an available hardware trigger."); - return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; - } - - return ERROR_OK; -} - -static int remove_trigger(struct target *target, struct trigger *trigger) -{ - riscv013_info_t *info = get_info(target); - - // Assume that all triggers are configured the same on all harts. - riscv_set_current_hartid(target, 0); - - maybe_read_tselect(target); - - int i; - for (i = 0; i < riscv_count_triggers(target); i++) { - if (info->trigger_unique_id[i] == trigger->unique_id) { - break; - } - } - if (i >= riscv_count_triggers(target)) { - LOG_ERROR("Couldn't find the hardware resources used by hardware " - "trigger."); - return ERROR_FAIL; - } - LOG_DEBUG("Stop using resource %d for bp %d", i, trigger->unique_id); - for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) { - if (!riscv_hart_enabled(target, hartid)) - continue; - - riscv_set_current_hartid(target, hartid); - register_write_direct(target, GDB_REGNO_TSELECT, i); - register_write_direct(target, GDB_REGNO_TDATA1, 0); - } - info->trigger_unique_id[i] = -1; - - return ERROR_OK; -} - -static void trigger_from_breakpoint(struct trigger *trigger, - const struct breakpoint *breakpoint) -{ - trigger->address = breakpoint->address; - trigger->length = breakpoint->length; - trigger->mask = ~0LL; - trigger->read = false; - trigger->write = false; - trigger->execute = true; - // unique_id is unique across both breakpoints and watchpoints. - trigger->unique_id = breakpoint->unique_id; -} - -static void trigger_from_watchpoint(struct trigger *trigger, - const struct watchpoint *watchpoint) -{ - trigger->address = watchpoint->address; - trigger->length = watchpoint->length; - trigger->mask = watchpoint->mask; - trigger->value = watchpoint->value; - trigger->read = (watchpoint->rw == WPT_READ || watchpoint->rw == WPT_ACCESS); - trigger->write = (watchpoint->rw == WPT_WRITE || watchpoint->rw == WPT_ACCESS); - trigger->execute = false; - // unique_id is unique across both breakpoints and watchpoints. - trigger->unique_id = watchpoint->unique_id; -} - -static int add_breakpoint(struct target *target, - struct breakpoint *breakpoint) -{ - if (breakpoint->type == BKPT_SOFT) { - if (target_read_memory(target, breakpoint->address, breakpoint->length, 1, - breakpoint->orig_instr) != ERROR_OK) { - LOG_ERROR("Failed to read original instruction at 0x%" - TARGET_PRIxADDR, breakpoint->address); - return ERROR_FAIL; - } - - int retval; - if (breakpoint->length == 4) { - retval = target_write_u32(target, breakpoint->address, ebreak()); - } else { - retval = target_write_u16(target, breakpoint->address, ebreak_c()); - } - if (retval != ERROR_OK) { - LOG_ERROR("Failed to write %d-byte breakpoint instruction at 0x%" - TARGET_PRIxADDR, breakpoint->length, breakpoint->address); - return ERROR_FAIL; - } - - } else if (breakpoint->type == BKPT_HARD) { - struct trigger trigger; - trigger_from_breakpoint(&trigger, breakpoint); - int result = add_trigger(target, &trigger); - if (result != ERROR_OK) { - return result; - } - } else { - LOG_INFO("OpenOCD only supports hardware and software breakpoints."); - return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; - } - - breakpoint->set = true; - - return ERROR_OK; -} - -static int remove_breakpoint(struct target *target, - struct breakpoint *breakpoint) -{ - if (breakpoint->type == BKPT_SOFT) { - if (target_write_memory(target, breakpoint->address, breakpoint->length, 1, - breakpoint->orig_instr) != ERROR_OK) { - LOG_ERROR("Failed to restore instruction for %d-byte breakpoint at " - "0x%" TARGET_PRIxADDR, breakpoint->length, - breakpoint->address); - return ERROR_FAIL; - } - - } else if (breakpoint->type == BKPT_HARD) { - struct trigger trigger; - trigger_from_breakpoint(&trigger, breakpoint); - int result = remove_trigger(target, &trigger); - if (result != ERROR_OK) { - return result; - } - } else { - LOG_INFO("OpenOCD only supports hardware and software breakpoints."); - return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; - } - - breakpoint->set = false; - - return ERROR_OK; -} - -static int add_watchpoint(struct target *target, - struct watchpoint *watchpoint) -{ - struct trigger trigger; - trigger_from_watchpoint(&trigger, watchpoint); - - int result = add_trigger(target, &trigger); - if (result != ERROR_OK) { - return result; - } - watchpoint->set = true; - - return ERROR_OK; -} - -static int remove_watchpoint(struct target *target, - struct watchpoint *watchpoint) -{ - struct trigger trigger; - trigger_from_watchpoint(&trigger, watchpoint); - - int result = remove_trigger(target, &trigger); - if (result != ERROR_OK) { - return result; - } - watchpoint->set = false; - - return ERROR_OK; -} - static int examine(struct target *target) { // Don't need to select dbus, since the first thing we do is read dtmcontrol. @@ -1939,12 +1649,6 @@ struct target_type riscv013_target = .read_memory = read_memory, .write_memory = write_memory, - .add_breakpoint = add_breakpoint, - .remove_breakpoint = remove_breakpoint, - - .add_watchpoint = add_watchpoint, - .remove_watchpoint = remove_watchpoint, - .arch_state = arch_state, }; diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 5560b4242..b0ce59264 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -248,6 +248,7 @@ static int riscv_init_target(struct command_context *cmd_ctx, if (!target->arch_info) return ERROR_FAIL; riscv_info_t *info = (riscv_info_t *) target->arch_info; + riscv_info_init(target, info); info->cmd_ctx = cmd_ctx; select_dtmcontrol.num_bits = target->tap->ir_length; @@ -273,32 +274,329 @@ static int oldriscv_halt(struct target *target) return tt->halt(target); } -static int riscv_add_breakpoint(struct target *target, - struct breakpoint *breakpoint) +static void trigger_from_breakpoint(struct trigger *trigger, + const struct breakpoint *breakpoint) { - struct target_type *tt = get_target_type(target); - return tt->add_breakpoint(target, breakpoint); + trigger->address = breakpoint->address; + trigger->length = breakpoint->length; + trigger->mask = ~0LL; + trigger->read = false; + trigger->write = false; + trigger->execute = true; + // unique_id is unique across both breakpoints and watchpoints. + trigger->unique_id = breakpoint->unique_id; } -static int riscv_remove_breakpoint(struct target *target, +static int maybe_add_trigger_t1(struct target *target, unsigned hartid, + struct trigger *trigger, uint64_t tdata1) +{ + RISCV_INFO(r); + + const uint32_t bpcontrol_x = 1<<0; + const uint32_t bpcontrol_w = 1<<1; + const uint32_t bpcontrol_r = 1<<2; + const uint32_t bpcontrol_u = 1<<3; + const uint32_t bpcontrol_s = 1<<4; + const uint32_t bpcontrol_h = 1<<5; + const uint32_t bpcontrol_m = 1<<6; + const uint32_t bpcontrol_bpmatch = 0xf << 7; + const uint32_t bpcontrol_bpaction = 0xff << 11; + + if (tdata1 & (bpcontrol_r | bpcontrol_w | bpcontrol_x)) { + // Trigger is already in use, presumably by user code. + return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; + } + + tdata1 = set_field(tdata1, bpcontrol_r, trigger->read); + tdata1 = set_field(tdata1, bpcontrol_w, trigger->write); + tdata1 = set_field(tdata1, bpcontrol_x, trigger->execute); + tdata1 = set_field(tdata1, bpcontrol_u, !!(r->misa & (1 << ('U' - 'A')))); + tdata1 = set_field(tdata1, bpcontrol_s, !!(r->misa & (1 << ('S' - 'A')))); + tdata1 = set_field(tdata1, bpcontrol_h, !!(r->misa & (1 << ('H' - 'A')))); + tdata1 |= bpcontrol_m; + tdata1 = set_field(tdata1, bpcontrol_bpmatch, 0); // exact match + tdata1 = set_field(tdata1, bpcontrol_bpaction, 0); // cause bp exception + + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TDATA1, tdata1); + + riscv_reg_t tdata1_rb = riscv_get_register_on_hart(target, hartid, + GDB_REGNO_TDATA1); + LOG_DEBUG("tdata1=0x%" PRIx64, tdata1_rb); + + if (tdata1 != tdata1_rb) { + LOG_DEBUG("Trigger doesn't support what we need; After writing 0x%" + PRIx64 " to tdata1 it contains 0x%" PRIx64, + tdata1, tdata1_rb); + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TDATA1, 0); + return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; + } + + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TDATA2, trigger->address); + + return ERROR_OK; +} + +static int maybe_add_trigger_t2(struct target *target, unsigned hartid, + struct trigger *trigger, uint64_t tdata1) +{ + RISCV_INFO(r); + + // tselect is already set + if (tdata1 & (MCONTROL_EXECUTE | MCONTROL_STORE | MCONTROL_LOAD)) { + // Trigger is already in use, presumably by user code. + return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; + } + + // address/data match trigger + tdata1 |= MCONTROL_DMODE(riscv_xlen(target)); + tdata1 = set_field(tdata1, MCONTROL_ACTION, + MCONTROL_ACTION_DEBUG_MODE); + tdata1 = set_field(tdata1, MCONTROL_MATCH, MCONTROL_MATCH_EQUAL); + tdata1 |= MCONTROL_M; + if (r->misa & (1 << ('H' - 'A'))) + tdata1 |= MCONTROL_H; + if (r->misa & (1 << ('S' - 'A'))) + tdata1 |= MCONTROL_S; + if (r->misa & (1 << ('U' - 'A'))) + tdata1 |= MCONTROL_U; + + if (trigger->execute) + tdata1 |= MCONTROL_EXECUTE; + if (trigger->read) + tdata1 |= MCONTROL_LOAD; + if (trigger->write) + tdata1 |= MCONTROL_STORE; + + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TDATA1, tdata1); + + uint64_t tdata1_rb = riscv_get_register_on_hart(target, hartid, GDB_REGNO_TDATA1); + LOG_DEBUG("tdata1=0x%" PRIx64, tdata1_rb); + + if (tdata1 != tdata1_rb) { + LOG_DEBUG("Trigger doesn't support what we need; After writing 0x%" + PRIx64 " to tdata1 it contains 0x%" PRIx64, + tdata1, tdata1_rb); + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TDATA1, 0); + return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; + } + + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TDATA2, trigger->address); + + return ERROR_OK; +} + +static int add_trigger(struct target *target, struct trigger *trigger) +{ + RISCV_INFO(r); + + riscv_reg_t tselect[RISCV_MAX_HARTS]; + + for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) { + if (!riscv_hart_enabled(target, hartid)) + continue; + tselect[hartid] = riscv_get_register_on_hart(target, hartid, + GDB_REGNO_TSELECT); + } + + unsigned int i; + for (i = 0; i < r->trigger_count[0]; i++) { + if (r->trigger_unique_id[i] != -1) { + continue; + } + + riscv_set_register_on_hart(target, 0, GDB_REGNO_TSELECT, i); + + uint64_t tdata1 = riscv_get_register_on_hart(target, 0, GDB_REGNO_TDATA1); + int type = get_field(tdata1, MCONTROL_TYPE(riscv_xlen(target))); + + int result = ERROR_OK; + for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) { + if (!riscv_hart_enabled(target, hartid)) + continue; + if (hartid > 0) { + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TSELECT, i); + } + switch (type) { + case 1: + result = maybe_add_trigger_t1(target, hartid, trigger, tdata1); + break; + case 2: + result = maybe_add_trigger_t2(target, hartid, trigger, tdata1); + break; + default: + LOG_DEBUG("trigger %d has unknown type %d", i, type); + continue; + } + + if (result != ERROR_OK) { + continue; + } + } + + if (result != ERROR_OK) { + continue; + } + + LOG_DEBUG("Using resource %d for bp %d", i, + trigger->unique_id); + r->trigger_unique_id[i] = trigger->unique_id; + break; + } + + for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) { + if (!riscv_hart_enabled(target, hartid)) + continue; + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TSELECT, + tselect[hartid]); + } + + if (i >= r->trigger_count[0]) { + LOG_ERROR("Couldn't find an available hardware trigger."); + return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; + } + + return ERROR_OK; +} + +int riscv_add_breakpoint(struct target *target, struct breakpoint *breakpoint) +{ + if (breakpoint->type == BKPT_SOFT) { + if (target_read_memory(target, breakpoint->address, breakpoint->length, 1, + breakpoint->orig_instr) != ERROR_OK) { + LOG_ERROR("Failed to read original instruction at 0x%" TARGET_PRIxADDR, + breakpoint->address); + return ERROR_FAIL; + } + + int retval; + if (breakpoint->length == 4) { + retval = target_write_u32(target, breakpoint->address, ebreak()); + } else { + retval = target_write_u16(target, breakpoint->address, ebreak_c()); + } + if (retval != ERROR_OK) { + LOG_ERROR("Failed to write %d-byte breakpoint instruction at 0x%" + TARGET_PRIxADDR, breakpoint->length, breakpoint->address); + return ERROR_FAIL; + } + + } else if (breakpoint->type == BKPT_HARD) { + struct trigger trigger; + trigger_from_breakpoint(&trigger, breakpoint); + int result = add_trigger(target, &trigger); + if (result != ERROR_OK) { + return result; + } + + } else { + LOG_INFO("OpenOCD only supports hardware and software breakpoints."); + return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; + } + + breakpoint->set = true; + + return ERROR_OK; +} + +static int remove_trigger(struct target *target, struct trigger *trigger) +{ + RISCV_INFO(r); + + unsigned int i; + for (i = 0; i < r->trigger_count[0]; i++) { + if (r->trigger_unique_id[i] == trigger->unique_id) { + break; + } + } + if (i >= r->trigger_count[0]) { + LOG_ERROR("Couldn't find the hardware resources used by hardware " + "trigger."); + return ERROR_FAIL; + } + LOG_DEBUG("Stop using resource %d for bp %d", i, trigger->unique_id); + for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) { + if (!riscv_hart_enabled(target, hartid)) + continue; + riscv_reg_t tselect = riscv_get_register_on_hart(target, hartid, GDB_REGNO_TSELECT); + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TSELECT, i); + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TDATA1, 0); + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TSELECT, tselect); + } + r->trigger_unique_id[i] = -1; + + return ERROR_OK; +} + +int riscv_remove_breakpoint(struct target *target, struct breakpoint *breakpoint) { - struct target_type *tt = get_target_type(target); - return tt->remove_breakpoint(target, breakpoint); + if (breakpoint->type == BKPT_SOFT) { + if (target_write_memory(target, breakpoint->address, breakpoint->length, 1, + breakpoint->orig_instr) != ERROR_OK) { + LOG_ERROR("Failed to restore instruction for %d-byte breakpoint at " + "0x%" TARGET_PRIxADDR, breakpoint->length, breakpoint->address); + return ERROR_FAIL; + } + + } else if (breakpoint->type == BKPT_HARD) { + struct trigger trigger; + trigger_from_breakpoint(&trigger, breakpoint); + int result = remove_trigger(target, &trigger); + if (result != ERROR_OK) { + return result; + } + + } else { + LOG_INFO("OpenOCD only supports hardware and software breakpoints."); + return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; + } + + breakpoint->set = false; + + return ERROR_OK; } -static int riscv_add_watchpoint(struct target *target, - struct watchpoint *watchpoint) +static void trigger_from_watchpoint(struct trigger *trigger, + const struct watchpoint *watchpoint) { - struct target_type *tt = get_target_type(target); - return tt->add_watchpoint(target, watchpoint); + trigger->address = watchpoint->address; + trigger->length = watchpoint->length; + trigger->mask = watchpoint->mask; + trigger->value = watchpoint->value; + trigger->read = (watchpoint->rw == WPT_READ || watchpoint->rw == WPT_ACCESS); + trigger->write = (watchpoint->rw == WPT_WRITE || watchpoint->rw == WPT_ACCESS); + trigger->execute = false; + // unique_id is unique across both breakpoints and watchpoints. + trigger->unique_id = watchpoint->unique_id; } -static int riscv_remove_watchpoint(struct target *target, +int riscv_add_watchpoint(struct target *target, struct watchpoint *watchpoint) +{ + struct trigger trigger; + trigger_from_watchpoint(&trigger, watchpoint); + + int result = add_trigger(target, &trigger); + if (result != ERROR_OK) { + return result; + } + watchpoint->set = true; + + return ERROR_OK; +} + +int riscv_remove_watchpoint(struct target *target, struct watchpoint *watchpoint) { - struct target_type *tt = get_target_type(target); - return tt->remove_watchpoint(target, watchpoint); + struct trigger trigger; + trigger_from_watchpoint(&trigger, watchpoint); + + int result = remove_trigger(target, &trigger); + if (result != ERROR_OK) { + return result; + } + watchpoint->set = false; + + return ERROR_OK; } static int oldriscv_step(struct target *target, int current, uint32_t address, @@ -880,6 +1178,8 @@ void riscv_info_init(struct target *target, riscv_info_t *r) r->registers_initialized = false; r->current_hartid = target->coreid; + memset(r->trigger_unique_id, 0xff, sizeof(r->trigger_unique_id)); + for (size_t h = 0; h < RISCV_MAX_HARTS; ++h) { r->xlen[h] = -1; r->debug_buffer_addr[h] = -1; @@ -1033,7 +1333,6 @@ void riscv_set_current_hartid(struct target *target, int hartid) && (!riscv_rtos_enabled(target) || (previous_hartid == hartid)) && target->reg_cache->reg_list[GDB_REGNO_XPR0].size == (unsigned)riscv_xlen(target) && (!riscv_rtos_enabled(target) || (r->rtos_hartid != -1))) { - LOG_DEBUG("registers already initialized, skipping"); return; } else LOG_DEBUG("Initializing registers: xlen=%d", riscv_xlen(target)); @@ -1107,7 +1406,7 @@ void riscv_set_register_on_hart(struct target *target, int hartid, enum gdb_regno regid, uint64_t value) { RISCV_INFO(r); - LOG_DEBUG("[%d] reg[%d] <- %" PRIx64, hartid, regid, value); + LOG_DEBUG("[%d] reg[0x%x] <- %" PRIx64, hartid, regid, value); assert(r->set_register); return r->set_register(target, hartid, regid, value); } @@ -1120,13 +1419,15 @@ riscv_reg_t riscv_get_register(struct target *target, enum gdb_regno r) uint64_t riscv_get_register_on_hart(struct target *target, int hartid, enum gdb_regno regid) { RISCV_INFO(r); - LOG_DEBUG("reading register %d on hart %d", regid, hartid); - return r->get_register(target, hartid, regid); + uint64_t value = r->get_register(target, hartid, regid); + LOG_DEBUG("[%d] reg[0x%x] = %" PRIx64, hartid, regid, value); + return value; } bool riscv_is_halted(struct target *target) { RISCV_INFO(r); + assert(r->is_halted); return r->is_halted(target); } @@ -1261,16 +1562,19 @@ int riscv_enumerate_triggers(struct target *target) if (!riscv_hart_enabled(target, hartid)) continue; + riscv_reg_t tselect = riscv_get_register_on_hart(target, hartid, + GDB_REGNO_TSELECT); + for (unsigned t = 0; t < RISCV_MAX_TRIGGERS; ++t) { r->trigger_count[hartid] = t; riscv_set_register_on_hart(target, hartid, GDB_REGNO_TSELECT, t); - uint64_t tselect = riscv_get_register_on_hart(target, hartid, + uint64_t tselect_rb = riscv_get_register_on_hart(target, hartid, GDB_REGNO_TSELECT); // Mask off the top bit, which is used as tdrmode in old // implementations. - tselect &= ~(1ULL << (riscv_xlen(target)-1)); - if (tselect != t) + tselect_rb &= ~(1ULL << (riscv_xlen(target)-1)); + if (tselect_rb != t) break; uint64_t tdata1 = riscv_get_register_on_hart(target, hartid, @@ -1290,6 +1594,8 @@ int riscv_enumerate_triggers(struct target *target) } } + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TSELECT, tselect); + LOG_DEBUG("[%d] Found %d triggers", hartid, r->trigger_count[hartid]); } diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 51d7cc933..7bc9a3772 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -11,6 +11,7 @@ struct riscv_program; #define RISCV_MAX_HARTS 32 #define RISCV_MAX_REGISTERS 5000 #define RISCV_MAX_TRIGGERS 32 +#define RISCV_MAX_HWBPS 16 extern struct target_type riscv011_target; extern struct target_type riscv013_target; @@ -30,6 +31,9 @@ enum riscv_halt_reason { typedef struct { unsigned dtm_version; + + riscv_reg_t misa; + struct command_context *cmd_ctx; void *version_specific; @@ -59,6 +63,10 @@ typedef struct { /* The number of triggers per hart. */ unsigned trigger_count[RISCV_MAX_HARTS]; + /* For each physical trigger, contains -1 if the hwbp is available, or the + * unique_id of the breakpoint/watchpoint that is using it. */ + int trigger_unique_id[RISCV_MAX_HWBPS]; + /* The address of the debug RAM buffer. */ riscv_addr_t debug_buffer_addr[RISCV_MAX_HARTS]; @@ -221,4 +229,11 @@ bool riscv_hart_enabled(struct target *target, int hartid); int riscv_enumerate_triggers(struct target *target); +int riscv_add_breakpoint(struct target *target, struct breakpoint *breakpoint); +int riscv_remove_breakpoint(struct target *target, + struct breakpoint *breakpoint); +int riscv_add_watchpoint(struct target *target, struct watchpoint *watchpoint); +int riscv_remove_watchpoint(struct target *target, + struct watchpoint *watchpoint); + #endif From f0f1df1061622e564f6984dd0ffeeeb2b612ced2 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Fri, 14 Jul 2017 12:50:11 -0700 Subject: [PATCH 3/8] Fix infinite loop in reset. --- src/target/riscv/riscv-013.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index f82b9bc14..7f532f224 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -199,6 +199,14 @@ static void decode_dmi(char *text, unsigned address, unsigned data) uint64_t mask; const char *name; } description[] = { + { DMI_DMCONTROL, DMI_DMCONTROL_HALTREQ, "haltreq" }, + { DMI_DMCONTROL, DMI_DMCONTROL_RESUMEREQ, "resumereq" }, + { DMI_DMCONTROL, DMI_DMCONTROL_HARTRESET, "hartreset" }, + { DMI_DMCONTROL, DMI_DMCONTROL_HASEL, "hasel" }, + { DMI_DMCONTROL, DMI_DMCONTROL_HARTSEL, "hartsel" }, + { DMI_DMCONTROL, DMI_DMCONTROL_NDMRESET, "ndmreset" }, + { DMI_DMCONTROL, DMI_DMCONTROL_DMACTIVE, "dmactive" }, + { DMI_DMSTATUS, DMI_DMSTATUS_ALLRESUMEACK, "allresumeack" }, { DMI_DMSTATUS, DMI_DMSTATUS_ANYRESUMEACK, "anyresumeack" }, { DMI_DMSTATUS, DMI_DMSTATUS_ALLNONEXISTENT, "allnonexistent" }, @@ -1874,7 +1882,16 @@ void riscv013_reset_current_hart(struct target *target) control = set_field(control, DMI_DMCONTROL_NDMRESET, 0); dmi_write(target, DMI_DMCONTROL, control); - while (get_field(dmi_read(target, DMI_DMSTATUS), DMI_DMSTATUS_ALLHALTED) == 0); + for (unsigned i = 0; i < 256; i++) { + uint32_t dmstatus = dmi_read(target, DMI_DMSTATUS); + if (get_field(dmstatus, DMI_DMSTATUS_ALLHALTED)) { + break; + } + if (i == 255) { + LOG_ERROR("Hart didn't halt coming out of reset; dmstatus=0x%x", + dmstatus); + } + } control = set_field(control, DMI_DMCONTROL_HALTREQ, 0); dmi_write(target, DMI_DMCONTROL, control); From b032eb1bcc53d43c9cb7a7f04bf2d62ea39af26c Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Sun, 16 Jul 2017 11:16:49 -0700 Subject: [PATCH 4/8] Use a wall clock timeout to complete reset. --- src/target/riscv/riscv-013.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 7f532f224..4b0f71b6e 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -121,7 +121,8 @@ typedef enum slot { /*** Info about the core being debugged. ***/ -#define WALL_CLOCK_TIMEOUT 2 +#define WALL_CLOCK_TIMEOUT 2 +#define WALL_CLOCK_RESET_TIMEOUT 30 struct trigger { uint64_t address; @@ -1882,14 +1883,17 @@ void riscv013_reset_current_hart(struct target *target) control = set_field(control, DMI_DMCONTROL_NDMRESET, 0); dmi_write(target, DMI_DMCONTROL, control); - for (unsigned i = 0; i < 256; i++) { + time_t start = time(NULL); + + while (1) { uint32_t dmstatus = dmi_read(target, DMI_DMSTATUS); if (get_field(dmstatus, DMI_DMSTATUS_ALLHALTED)) { break; } - if (i == 255) { - LOG_ERROR("Hart didn't halt coming out of reset; dmstatus=0x%x", - dmstatus); + if (time(NULL) - start > WALL_CLOCK_RESET_TIMEOUT) { + LOG_ERROR("Hart didn't halt coming out of reset in %ds; " + "dmstatus=0x%x", WALL_CLOCK_RESET_TIMEOUT, dmstatus); + return; } } From 753d15e22ce910ea75fd876851f91c2b3ab179fb Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 25 Jul 2017 14:08:10 -0700 Subject: [PATCH 5/8] Print out which port OpenOCD is listening on. This is essential when a test environment asks OpenOCD to listen on port 0, so that the environment can easily discover which port is actually being used. --- src/server/server.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/server/server.c b/src/server/server.c index 8009d408f..65da53f6d 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -298,6 +298,12 @@ int add_service(char *name, free_service(c); return ERROR_FAIL; } + + struct sockaddr_in addr_in; + socklen_t addr_in_size = sizeof(addr_in); + getsockname(c->fd, &addr_in, &addr_in_size); + LOG_INFO("Listening on port %d for %s connections", + ntohs(addr_in.sin_port), name); } else if (c->type == CONNECTION_STDINOUT) { c->fd = fileno(stdin); From 46b5f913c73df49b609f64eda8c5b753ca69707d Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 27 Jul 2017 13:45:26 -0700 Subject: [PATCH 6/8] Display register numbers in a more usable format. --- src/target/riscv/gdb_regs.h | 2 ++ src/target/riscv/riscv.c | 57 ++++++++++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/target/riscv/gdb_regs.h b/src/target/riscv/gdb_regs.h index e16fa7fb5..0ad02e9ef 100644 --- a/src/target/riscv/gdb_regs.h +++ b/src/target/riscv/gdb_regs.h @@ -25,4 +25,6 @@ enum gdb_regno { GDB_REGNO_COUNT }; +const char *gdb_regno_name(enum gdb_regno regno); + #endif diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index b0ce59264..e1c7c1c55 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -437,7 +437,7 @@ static int add_trigger(struct target *target, struct trigger *trigger) continue; } - LOG_DEBUG("Using resource %d for bp %d", i, + LOG_DEBUG("Using trigger %d (type %d) for bp %d", i, type, trigger->unique_id); r->trigger_unique_id[i] = trigger->unique_id; break; @@ -1406,7 +1406,7 @@ void riscv_set_register_on_hart(struct target *target, int hartid, enum gdb_regno regid, uint64_t value) { RISCV_INFO(r); - LOG_DEBUG("[%d] reg[0x%x] <- %" PRIx64, hartid, regid, value); + LOG_DEBUG("[%d] %s <- %" PRIx64, hartid, gdb_regno_name(regid), value); assert(r->set_register); return r->set_register(target, hartid, regid, value); } @@ -1420,7 +1420,7 @@ uint64_t riscv_get_register_on_hart(struct target *target, int hartid, enum gdb_ { RISCV_INFO(r); uint64_t value = r->get_register(target, hartid, regid); - LOG_DEBUG("[%d] reg[0x%x] = %" PRIx64, hartid, regid, value); + LOG_DEBUG("[%d] %s: %" PRIx64, hartid, gdb_regno_name(regid), value); return value; } @@ -1596,8 +1596,57 @@ int riscv_enumerate_triggers(struct target *target) riscv_set_register_on_hart(target, hartid, GDB_REGNO_TSELECT, tselect); - LOG_DEBUG("[%d] Found %d triggers", hartid, r->trigger_count[hartid]); + LOG_INFO("[%d] Found %d triggers", hartid, r->trigger_count[hartid]); } return ERROR_OK; } + +const char *gdb_regno_name(enum gdb_regno regno) +{ + static char buf[32]; + + switch (regno) { + case GDB_REGNO_ZERO: + return "zero"; + case GDB_REGNO_S0: + return "s0"; + case GDB_REGNO_S1: + return "s1"; + case GDB_REGNO_PC: + return "pc"; + case GDB_REGNO_FPR0: + return "fpr0"; + case GDB_REGNO_FPR31: + return "fpr31"; + case GDB_REGNO_CSR0: + return "csr0"; + case GDB_REGNO_TSELECT: + return "tselect"; + case GDB_REGNO_TDATA1: + return "tdata1"; + case GDB_REGNO_TDATA2: + return "tdata2"; + case GDB_REGNO_MISA: + return "misa"; + case GDB_REGNO_DPC: + return "dpc"; + case GDB_REGNO_DCSR: + return "dcsr"; + case GDB_REGNO_DSCRATCH: + return "dscratch"; + case GDB_REGNO_MSTATUS: + return "mstatus"; + case GDB_REGNO_PRIV: + return "priv"; + default: + if (regno <= GDB_REGNO_XPR31) { + sprintf(buf, "x%d", regno - GDB_REGNO_XPR0); + } else if (regno >= GDB_REGNO_CSR0 && regno <= GDB_REGNO_CSR4095) { + sprintf(buf, "csr%d", regno - GDB_REGNO_CSR0); + } else { + sprintf(buf, "gdb_regno_%d", regno); + } + return buf; + } +} From b89780722448007ac1168e77b6e803560a100365 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 7 Aug 2017 13:55:37 -0700 Subject: [PATCH 7/8] When gdb_port is 0, don't increment it. Usually incrementing to get the next port is a good idea, but when set to 0 the idea is to find an arbitrary unallocated port. 1 is almost certainly not helpful. --- src/server/gdb_server.c | 8 +++++++- src/server/server.c | 3 ++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c index 6ad95bdfa..ecf46add3 100644 --- a/src/server/gdb_server.c +++ b/src/server/gdb_server.c @@ -3113,7 +3113,13 @@ static int gdb_target_add_one(struct target *target) if (!*end) { if (parse_long(gdb_port_next, &portnumber) == ERROR_OK) { free(gdb_port_next); - gdb_port_next = alloc_printf("%d", portnumber+1); + if (portnumber) { + gdb_port_next = alloc_printf("%d", portnumber+1); + } else { + /* Don't increment if gdb_port is 0, since we're just + * trying to allocate an unused port. */ + gdb_port_next = alloc_printf("0"); + } } } } diff --git a/src/server/server.c b/src/server/server.c index 65da53f6d..99bdc35ec 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -273,7 +273,8 @@ int add_service(char *name, c->sin.sin_port = htons(c->portnumber); if (bind(c->fd, (struct sockaddr *)&c->sin, sizeof(c->sin)) == -1) { - LOG_ERROR("couldn't bind %s to socket: %s", name, strerror(errno)); + LOG_ERROR("couldn't bind %s to socket on port %d: %s", name, + c->portnumber, strerror(errno)); close_socket(c->fd); free_service(c); return ERROR_FAIL; From efcfcf555fd52922fd0ce0683ac490abee6c9896 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 9 Aug 2017 12:42:17 -0700 Subject: [PATCH 8/8] Fix assertion failure when reading from address 0. --- src/target/riscv/riscv-013.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 4b0f71b6e..99a0280e9 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1353,12 +1353,14 @@ static int read_memory(struct target *target, target_addr_t address, riscv_addr_t cur_addr = 0xbadbeef; riscv_addr_t fin_addr = address + (count * size); riscv_addr_t prev_addr = ((riscv_addr_t) address) - size; + bool first = true; LOG_DEBUG("writing until final address 0x%" PRIx64, fin_addr); while (count > 1 && (cur_addr = riscv_read_debug_buffer_x(target, d_addr)) < fin_addr) { LOG_DEBUG("transferring burst starting at address 0x%" TARGET_PRIxADDR " (previous burst was 0x%" TARGET_PRIxADDR ")", cur_addr, prev_addr); - assert(prev_addr < cur_addr); + assert(first || prev_addr < cur_addr); + first = false; prev_addr = cur_addr; riscv_addr_t start = (cur_addr - address) / size; assert (cur_addr >= address);