From 450307b66f42f402113c50aefa0fe950ed20cf7d Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 3 Jul 2017 12:17:07 -0700 Subject: [PATCH 1/9] Fix 32-bit build errors. I only compiled the source. Didn't have the tooling installed to link. Hopefully that's good enough. Fixes #71. --- src/target/riscv/batch.c | 4 +++- src/target/riscv/riscv-013.c | 11 ++++++----- src/target/riscv/riscv.c | 2 +- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/target/riscv/batch.c b/src/target/riscv/batch.c index 5d680d5fa..cb5a10856 100644 --- a/src/target/riscv/batch.c +++ b/src/target/riscv/batch.c @@ -94,7 +94,9 @@ size_t riscv_batch_add_dmi_read(struct riscv_batch *batch, unsigned address) riscv_batch_add_nop(batch); batch->read_keys[batch->read_keys_used] = batch->used_scans - 1; - LOG_DEBUG("read key %ld for batch 0x%p is %ld (0x%p)", batch->read_keys_used, batch, batch->used_scans - 1, (uint64_t*)batch->data_in + (batch->used_scans + 1)); + LOG_DEBUG("read key %u for batch 0x%p is %u (0x%p)", + (unsigned) batch->read_keys_used, batch, (unsigned) (batch->used_scans - 1), + (uint64_t*)batch->data_in + (batch->used_scans + 1)); return batch->read_keys_used++; } diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 60846debe..9d176e948 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1617,9 +1617,10 @@ static int write_memory(struct target *target, target_addr_t address, * the data was all copied. */ riscv_addr_t cur_addr = 0xbadbeef; riscv_addr_t fin_addr = address + (count * size); - LOG_DEBUG("writing until final address 0x%016lx", fin_addr); + LOG_DEBUG("writing until final address 0x%016" PRIx64, fin_addr); while ((cur_addr = riscv_read_debug_buffer_x(target, d_addr)) < fin_addr) { - LOG_DEBUG("transferring burst starting at address 0x%016lx", cur_addr); + LOG_DEBUG("transferring burst starting at address 0x%016" PRIx64, + cur_addr); riscv_addr_t start = (cur_addr - address) / size; assert (cur_addr > address); struct riscv_batch *batch = riscv_batch_alloc( @@ -1744,7 +1745,7 @@ static riscv_reg_t riscv013_get_register(struct target *target, int hid, int rid register_read_direct(target, &out, rid); } else if (rid == GDB_REGNO_PC) { register_read_direct(target, &out, GDB_REGNO_DPC); - LOG_DEBUG("read PC from DPC: 0x%016lx", out); + LOG_DEBUG("read PC from DPC: 0x%016" PRIx64, out); } else if (rid == GDB_REGNO_PRIV) { uint64_t dcsr; register_read_direct(target, &dcsr, CSR_DCSR); @@ -1772,11 +1773,11 @@ static void riscv013_set_register(struct target *target, int hid, int rid, uint6 if (rid <= GDB_REGNO_XPR31) { register_write_direct(target, rid, value); } else if (rid == GDB_REGNO_PC) { - LOG_DEBUG("writing PC to DPC: 0x%016lx", value); + LOG_DEBUG("writing PC to DPC: 0x%016" PRIx64, value); register_write_direct(target, GDB_REGNO_DPC, value); uint64_t actual_value; register_read_direct(target, &actual_value, GDB_REGNO_DPC); - LOG_DEBUG(" actual DPC written: 0x%016lx", actual_value); + LOG_DEBUG(" actual DPC written: 0x%016" PRIx64, actual_value); assert(value == actual_value); } else if (rid == GDB_REGNO_PRIV) { uint64_t dcsr; diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 7a40467f1..83e73eff3 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1030,7 +1030,7 @@ void riscv_set_current_hartid(struct target *target, int hartid) /* 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_XPR0].size == (long)riscv_xlen(target) + && 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; From 3be99ac884fb956cca67e13065098c5ca0be370a Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 5 Jul 2017 10:33:42 -0700 Subject: [PATCH 2/9] Perform regular build with travis. --- .travis.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 000000000..20f21de24 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,5 @@ +language: c +dist: trusty + +script: + - ./bootstrap && ./configure && make From 708b05ba0765de42cc7eaf8ce56fcc67109af637 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 6 Jul 2017 14:53:14 -0700 Subject: [PATCH 3/9] Build 32- and 64-bit binaries with Travis. --- .travis.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.travis.yml b/.travis.yml index 20f21de24..320799516 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,18 @@ language: c dist: trusty +matrix: + include: + - os: linux + env: BUILD=x86_64-linux-gnu + + - os: linux + env: BUILD=i686-linux-gnu CFLAGS=-m32 + addons: + apt: + packages: + - gcc-multilib + script: - ./bootstrap && ./configure && make + - file src/openocd From 21e06e1d89cac1a946876caf28bb5ea2dbea353c Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 6 Jul 2017 14:33:28 -0700 Subject: [PATCH 4/9] Fix 32-bit build. Code taken from http://openocd.zylin.com/#/c/4178/ --- src/target/mips32_pracc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/mips32_pracc.h b/src/target/mips32_pracc.h index b8b93c649..888c847c0 100644 --- a/src/target/mips32_pracc.h +++ b/src/target/mips32_pracc.h @@ -114,7 +114,7 @@ int mips32_cp0_read(struct mips_ejtag *ejtag_info, int mips32_cp0_write(struct mips_ejtag *ejtag_info, uint32_t val, uint32_t cp0_reg, uint32_t cp0_sel); -inline void pracc_swap16_array(struct mips_ejtag *ejtag_info, uint32_t *buf, int count) +static inline void pracc_swap16_array(struct mips_ejtag *ejtag_info, uint32_t *buf, int count) { if (ejtag_info->isa && ejtag_info->endianness) for (int i = 0; i != count; i++) From 4072fa493b71077647b225305d43b5867a1a8ad2 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 10 Jul 2017 10:26:24 -0700 Subject: [PATCH 5/9] Disable debugger-set triggers on connect When first connecting to a target, have the debugger disable any hardware triggers that are set by a previously connected debugger. The 0.11 code already did this, but 0.13 did not. To achieve this I decided to share the code to enumerate triggers between 0.11 and 0.13, which required me to implement get_register() and set_register() for 0.11, which made the whole change a lot larger than you might have guessed. Hopefully this sets us up to in the future share the code to set/remove triggers as well. --- src/target/riscv/riscv-011.c | 136 +++++++++++++++++++---------------- src/target/riscv/riscv-013.c | 16 +---- src/target/riscv/riscv.c | 56 ++++++++++++++- src/target/riscv/riscv.h | 9 ++- 4 files changed, 135 insertions(+), 82 deletions(-) diff --git a/src/target/riscv/riscv-011.c b/src/target/riscv/riscv-011.c index 95328037b..6e7b97065 100644 --- a/src/target/riscv/riscv-011.c +++ b/src/target/riscv/riscv-011.c @@ -216,8 +216,6 @@ typedef struct { // unique_id of the breakpoint/watchpoint that is using it. int trigger_unique_id[MAX_HWBPS]; - unsigned int trigger_count; - // Number of run-test/idle cycles the target requests we do after each dbus // access. unsigned int dtmcontrol_idle; @@ -1045,7 +1043,7 @@ static int read_csr(struct target *target, uint64_t *value, uint32_t csr) uint32_t exception = cache_get32(target, info->dramsize-1); if (exception) { - LOG_ERROR("Got exception 0x%x when reading CSR 0x%x", exception, csr); + LOG_WARNING("Got exception 0x%x when reading CSR 0x%x", exception, csr); *value = ~0; return ERROR_FAIL; } @@ -1260,22 +1258,54 @@ static int update_mstatus_actual(struct target *target) /*** OpenOCD target functions. ***/ +static int register_read(struct target *target, riscv_reg_t *value, int regnum) +{ + riscv011_info_t *info = get_info(target); + if (regnum >= REG_CSR0 && regnum <= REG_CSR4095) { + cache_set32(target, 0, csrr(S0, regnum - REG_CSR0)); + cache_set_store(target, 1, S0, SLOT0); + cache_set_jump(target, 2); + } else { + LOG_ERROR("Don't know how to read register %d", regnum); + return ERROR_FAIL; + } + + if (cache_write(target, 4, true) != ERROR_OK) { + return ERROR_FAIL; + } + + uint32_t exception = cache_get32(target, info->dramsize-1); + if (exception) { + LOG_WARNING("Got exception 0x%x when reading register %d", exception, + regnum); + *value = ~0; + return ERROR_FAIL; + } + + *value = cache_get(target, SLOT0); + LOG_DEBUG("reg[%d]=0x%" PRIx64, regnum, *value); + + if (regnum == REG_MSTATUS) { + info->mstatus_actual = *value; + } + + return ERROR_OK; +} + static int register_get(struct reg *reg) { struct target *target = (struct target *) reg->arch_info; riscv011_info_t *info = get_info(target); maybe_write_tselect(target); + riscv_reg_t value = ~0; if (reg->number <= REG_XPR31) { - buf_set_u64(reg->value, 0, riscv_xlen(target), reg_cache_get(target, reg->number)); + value = reg_cache_get(target, reg->number); LOG_DEBUG("%s=0x%" PRIx64, reg->name, reg_cache_get(target, reg->number)); - return ERROR_OK; } else if (reg->number == REG_PC) { - buf_set_u32(reg->value, 0, 32, info->dpc); - reg->valid = true; + value = info->dpc; LOG_DEBUG("%s=0x%" PRIx64 " (cached)", reg->name, info->dpc); - return ERROR_OK; } else if (reg->number >= REG_FPR0 && reg->number <= REG_FPR31) { int result = update_mstatus_actual(target); if (result != ERROR_OK) { @@ -1295,44 +1325,24 @@ static int register_get(struct reg *reg) cache_set32(target, i++, fsd(reg->number - REG_FPR0, 0, DEBUG_RAM_START + 16)); } cache_set_jump(target, i++); - } else if (reg->number >= REG_CSR0 && reg->number <= REG_CSR4095) { - cache_set32(target, 0, csrr(S0, reg->number - REG_CSR0)); - cache_set_store(target, 1, S0, SLOT0); - cache_set_jump(target, 2); - } else if (reg->number == REG_PRIV) { - buf_set_u64(reg->value, 0, 8, get_field(info->dcsr, DCSR_PRV)); - LOG_DEBUG("%s=%d (cached)", reg->name, - (int) get_field(info->dcsr, DCSR_PRV)); - return ERROR_OK; + + if (cache_write(target, 4, true) != ERROR_OK) { + return ERROR_FAIL; + } } else { - LOG_ERROR("Don't know how to read register %d (%s)", reg->number, reg->name); - return ERROR_FAIL; + if (register_read(target, &value, reg->number) != ERROR_OK) + return ERROR_FAIL; } - - if (cache_write(target, 4, true) != ERROR_OK) { - return ERROR_FAIL; - } - - uint32_t exception = cache_get32(target, info->dramsize-1); - if (exception) { - LOG_ERROR("Got exception 0x%x when reading register %d", exception, - reg->number); - buf_set_u64(reg->value, 0, riscv_xlen(target), ~0); - return ERROR_FAIL; - } - - uint64_t value = cache_get(target, SLOT0); - LOG_DEBUG("%s=0x%" PRIx64, reg->name, value); buf_set_u64(reg->value, 0, riscv_xlen(target), value); if (reg->number == REG_MSTATUS) { - info->mstatus_actual = value; reg->valid = true; } return ERROR_OK; } +// Write the register. No caching or games. static int register_write(struct target *target, unsigned int number, uint64_t value) { @@ -1396,7 +1406,7 @@ static int register_write(struct target *target, unsigned int number, uint32_t exception = cache_get32(target, info->dramsize-1); if (exception) { - LOG_ERROR("Got exception 0x%x when writing register %d", exception, + LOG_WARNING("Got exception 0x%x when writing register %d", exception, number); return ERROR_FAIL; } @@ -1423,6 +1433,25 @@ static struct reg_arch_type riscv_reg_arch_type = { .set = register_set }; +static riscv_reg_t get_register(struct target *target, int hartid, int regid) +{ + assert(hartid == 0); + riscv_reg_t value; + if (register_read(target, &value, regid) != ERROR_OK) { + // TODO: propagate errors + value = ~0; + } + return value; +} + +static void set_register(struct target *target, int hartid, int regid, + uint64_t value) +{ + assert(hartid == 0); + // TODO: propagate errors + register_write(target, regid, value); +} + static int halt(struct target *target) { LOG_DEBUG("riscv_halt()"); @@ -1446,7 +1475,8 @@ static int init_target(struct command_context *cmd_ctx, { LOG_DEBUG("init"); riscv_info_t *generic_info = (riscv_info_t *) target->arch_info; - generic_info->get_register = NULL; + generic_info->get_register = get_register; + generic_info->set_register = set_register; generic_info->version_specific = calloc(1, sizeof(riscv011_info_t)); if (!generic_info->version_specific) return ERROR_FAIL; @@ -1605,11 +1635,12 @@ static int maybe_add_trigger_t2(struct target *target, struct trigger *trigger, 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 < info->trigger_count; i++) { + for (i = 0; i < r->trigger_count[0]; i++) { if (info->trigger_unique_id[i] != -1) { continue; } @@ -1642,7 +1673,7 @@ static int add_trigger(struct target *target, struct trigger *trigger) info->trigger_unique_id[i] = trigger->unique_id; break; } - if (i >= info->trigger_count) { + if (i >= r->trigger_count[0]) { LOG_ERROR("Couldn't find an available hardware trigger."); return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; } @@ -1652,17 +1683,18 @@ static int add_trigger(struct target *target, struct trigger *trigger) 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 < info->trigger_count; i++) { + for (i = 0; i < r->trigger_count[0]; i++) { if (info->trigger_unique_id[i] == trigger->unique_id) { break; } } - if (i >= info->trigger_count) { + if (i >= r->trigger_count[0]) { LOG_ERROR("Couldn't find the hardware resources used by hardware " "trigger."); return ERROR_FAIL; @@ -2200,30 +2232,10 @@ static int handle_halt(struct target *target, bool announce) if (info->never_halted) { info->never_halted = false; - // Disable any hardware triggers that have dmode set. We can't have set - // them ourselves. Maybe they're left over from some killed debug - // session. - // Count the number of triggers while we're at it. - int result = maybe_read_tselect(target); if (result != ERROR_OK) return result; - for (info->trigger_count = 0; info->trigger_count < MAX_HWBPS; info->trigger_count++) { - write_csr(target, CSR_TSELECT, info->trigger_count); - uint64_t tselect_rb; - read_csr(target, &tselect_rb, CSR_TSELECT); - // Mask off the top bit, which is used as tdrmode in old - // implementations. - tselect_rb &= ~(1ULL << (riscv_xlen(target)-1)); - if (info->trigger_count != tselect_rb) - break; - uint64_t tdata1; - read_csr(target, &tdata1, CSR_TDATA1); - if ((tdata1 & MCONTROL_DMODE(riscv_xlen(target))) && - (tdata1 & (MCONTROL_EXECUTE | MCONTROL_STORE | MCONTROL_LOAD))) { - write_csr(target, CSR_TDATA1, 0); - } - } + riscv_enumerate_triggers(target); } if (announce) { diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 96187f2f7..fc7f3aa10 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1239,21 +1239,7 @@ static int examine(struct target *target) } /* Then we check the number of triggers availiable to each hart. */ - for (int i = 0; i < riscv_count_harts(target); ++i) { - if (!riscv_hart_enabled(target, i)) - continue; - - for (uint32_t t = 0; t < RISCV_MAX_TRIGGERS; ++t) { - riscv_set_current_hartid(target, i); - - r->trigger_count[i] = t; - register_write_direct(target, GDB_REGNO_TSELECT, t); - uint64_t tselect = t+1; - register_read_direct(target, &tselect, GDB_REGNO_TSELECT); - if (tselect != t) - break; - } - } + riscv_enumerate_triggers(target); /* Resumes all the harts, so the debugger can later pause them. */ riscv_resume_all_harts(target); diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 4fe027b59..5560b4242 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1103,10 +1103,12 @@ void riscv_set_register(struct target *target, enum gdb_regno r, riscv_reg_t v) return riscv_set_register_on_hart(target, riscv_current_hartid(target), r, v); } -void riscv_set_register_on_hart(struct target *target, int hartid, enum gdb_regno regid, uint64_t value) +void riscv_set_register_on_hart(struct target *target, int hartid, + enum gdb_regno regid, uint64_t value) { RISCV_INFO(r); - LOG_DEBUG("writing register %d on hart %d", regid, hartid); + LOG_DEBUG("[%d] reg[%d] <- %" PRIx64, hartid, regid, value); + assert(r->set_register); return r->set_register(target, hartid, regid, value); } @@ -1243,3 +1245,53 @@ bool riscv_hart_enabled(struct target *target, int hartid) return hartid == target->coreid; } + +/** + * Count triggers, and initialize trigger_count for each hart. + * trigger_count is initialized even if this function fails to discover + * something. + * Disable any hardware triggers that have dmode set. We can't have set them + * ourselves. Maybe they're left over from some killed debug session. + * */ +int riscv_enumerate_triggers(struct target *target) +{ + RISCV_INFO(r); + + for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) { + if (!riscv_hart_enabled(target, hartid)) + continue; + + 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, + 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) + break; + + uint64_t tdata1 = riscv_get_register_on_hart(target, hartid, + GDB_REGNO_TDATA1); + int type = get_field(tdata1, MCONTROL_TYPE(riscv_xlen(target))); + switch (type) { + case 1: + // On these older cores we don't support software using + // triggers. + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TDATA1, 0); + break; + case 2: + if (tdata1 & MCONTROL_DMODE(riscv_xlen(target))) { + riscv_set_register_on_hart(target, hartid, GDB_REGNO_TDATA1, 0); + } + break; + } + } + + LOG_DEBUG("[%d] Found %d triggers", hartid, r->trigger_count[hartid]); + } + + return ERROR_OK; +} diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 94a6080f2..51d7cc933 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -57,7 +57,7 @@ typedef struct { int xlen[RISCV_MAX_HARTS]; /* The number of triggers per hart. */ - int trigger_count[RISCV_MAX_HARTS]; + unsigned trigger_count[RISCV_MAX_HARTS]; /* The address of the debug RAM buffer. */ riscv_addr_t debug_buffer_addr[RISCV_MAX_HARTS]; @@ -70,8 +70,9 @@ typedef struct { /* Helper functions that target the various RISC-V debug spec * implementations. */ - riscv_reg_t (*get_register)(struct target *, int, int); - void (*set_register)(struct target *, int, int, uint64_t); + riscv_reg_t (*get_register)(struct target *, int hartid, int regid); + void (*set_register)(struct target *, int hartid, int regid, + uint64_t value); void (*select_current_hart)(struct target *); bool (*is_halted)(struct target *target); void (*halt_current_hart)(struct target *); @@ -218,4 +219,6 @@ void riscv_invalidate_register_cache(struct target *target); /* Returns TRUE when a hart is enabled in this target. */ bool riscv_hart_enabled(struct target *target, int hartid); +int riscv_enumerate_triggers(struct target *target); + #endif From 10a61000b5430f7651745eeac27bc11f8a5bbeeb Mon Sep 17 00:00:00 2001 From: Palmer Dabbelt Date: Mon, 10 Jul 2017 13:43:55 -0700 Subject: [PATCH 6/9] Use LL for 64-bit defines, as Windows is LLP64 This should also fix bugs on ILP32 systems. --- src/target/riscv/debug_defines.h | 54 ++++++++++++++++---------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/target/riscv/debug_defines.h b/src/target/riscv/debug_defines.h index e5f929105..c53cec7d5 100644 --- a/src/target/riscv/debug_defines.h +++ b/src/target/riscv/debug_defines.h @@ -106,7 +106,7 @@ */ #define DTM_DMI_DATA_OFFSET 2 #define DTM_DMI_DATA_LENGTH 32 -#define DTM_DMI_DATA (0xffffffffL << DTM_DMI_DATA_OFFSET) +#define DTM_DMI_DATA (0xffffffffLL << DTM_DMI_DATA_OFFSET) /* * When the debugger writes this field, it has the following meaning: * @@ -151,7 +151,7 @@ */ #define DTM_DMI_OP_OFFSET 0 #define DTM_DMI_OP_LENGTH 2 -#define DTM_DMI_OP (0x3L << DTM_DMI_OP_OFFSET) +#define DTM_DMI_OP (0x3LL << DTM_DMI_OP_OFFSET) #define CSR_DCSR 0x7b0 /* * 0: There is no external debug support. @@ -285,7 +285,7 @@ */ #define CSR_TDATA1_TYPE_OFFSET XLEN-4 #define CSR_TDATA1_TYPE_LENGTH 4 -#define CSR_TDATA1_TYPE (0xfL << CSR_TDATA1_TYPE_OFFSET) +#define CSR_TDATA1_TYPE (0xfLL << CSR_TDATA1_TYPE_OFFSET) /* * 0: Both Debug and M Mode can write the {\tt tdata} registers at the * selected \Rtselect. @@ -297,7 +297,7 @@ */ #define CSR_TDATA1_HMODE_OFFSET XLEN-5 #define CSR_TDATA1_HMODE_LENGTH 1 -#define CSR_TDATA1_HMODE (0x1L << CSR_TDATA1_HMODE_OFFSET) +#define CSR_TDATA1_HMODE (0x1LL << CSR_TDATA1_HMODE_OFFSET) /* * Trigger-specific data. */ @@ -315,10 +315,10 @@ #define CSR_MCONTROL 0x7a1 #define CSR_MCONTROL_TYPE_OFFSET XLEN-4 #define CSR_MCONTROL_TYPE_LENGTH 4 -#define CSR_MCONTROL_TYPE (0xfL << CSR_MCONTROL_TYPE_OFFSET) +#define CSR_MCONTROL_TYPE (0xfLL << CSR_MCONTROL_TYPE_OFFSET) #define CSR_MCONTROL_DMODE_OFFSET XLEN-5 #define CSR_MCONTROL_DMODE_LENGTH 1 -#define CSR_MCONTROL_DMODE (0x1L << CSR_MCONTROL_DMODE_OFFSET) +#define CSR_MCONTROL_DMODE (0x1LL << CSR_MCONTROL_DMODE_OFFSET) /* * Specifies the largest naturally aligned powers-of-two (NAPOT) range * supported by the hardware. The value is the logarithm base 2 of the @@ -329,7 +329,7 @@ */ #define CSR_MCONTROL_MASKMAX_OFFSET XLEN-11 #define CSR_MCONTROL_MASKMAX_LENGTH 6 -#define CSR_MCONTROL_MASKMAX (0x3fL << CSR_MCONTROL_MASKMAX_OFFSET) +#define CSR_MCONTROL_MASKMAX (0x3fLL << CSR_MCONTROL_MASKMAX_OFFSET) /* * 0: Perform a match on the virtual address. * @@ -338,7 +338,7 @@ */ #define CSR_MCONTROL_SELECT_OFFSET 19 #define CSR_MCONTROL_SELECT_LENGTH 1 -#define CSR_MCONTROL_SELECT (0x1L << CSR_MCONTROL_SELECT_OFFSET) +#define CSR_MCONTROL_SELECT (0x1LL << CSR_MCONTROL_SELECT_OFFSET) /* * 0: The action for this trigger will be taken just before the * instruction that triggered it is executed, but after all preceding @@ -366,7 +366,7 @@ */ #define CSR_MCONTROL_TIMING_OFFSET 18 #define CSR_MCONTROL_TIMING_LENGTH 1 -#define CSR_MCONTROL_TIMING (0x1L << CSR_MCONTROL_TIMING_OFFSET) +#define CSR_MCONTROL_TIMING (0x1LL << CSR_MCONTROL_TIMING_OFFSET) /* * Determines what happens when this trigger matches. * @@ -387,7 +387,7 @@ */ #define CSR_MCONTROL_ACTION_OFFSET 12 #define CSR_MCONTROL_ACTION_LENGTH 6 -#define CSR_MCONTROL_ACTION (0x3fL << CSR_MCONTROL_ACTION_OFFSET) +#define CSR_MCONTROL_ACTION (0x3fLL << CSR_MCONTROL_ACTION_OFFSET) /* * 0: When this trigger matches, the configured action is taken. * @@ -396,7 +396,7 @@ */ #define CSR_MCONTROL_CHAIN_OFFSET 11 #define CSR_MCONTROL_CHAIN_LENGTH 1 -#define CSR_MCONTROL_CHAIN (0x1L << CSR_MCONTROL_CHAIN_OFFSET) +#define CSR_MCONTROL_CHAIN (0x1LL << CSR_MCONTROL_CHAIN_OFFSET) /* * 0: Matches when the value equals \Rtdatatwo. * @@ -420,57 +420,57 @@ */ #define CSR_MCONTROL_MATCH_OFFSET 7 #define CSR_MCONTROL_MATCH_LENGTH 4 -#define CSR_MCONTROL_MATCH (0xfL << CSR_MCONTROL_MATCH_OFFSET) +#define CSR_MCONTROL_MATCH (0xfLL << CSR_MCONTROL_MATCH_OFFSET) /* * When set, enable this trigger in M mode. */ #define CSR_MCONTROL_M_OFFSET 6 #define CSR_MCONTROL_M_LENGTH 1 -#define CSR_MCONTROL_M (0x1L << CSR_MCONTROL_M_OFFSET) +#define CSR_MCONTROL_M (0x1LL << CSR_MCONTROL_M_OFFSET) /* * When set, enable this trigger in H mode. */ #define CSR_MCONTROL_H_OFFSET 5 #define CSR_MCONTROL_H_LENGTH 1 -#define CSR_MCONTROL_H (0x1L << CSR_MCONTROL_H_OFFSET) +#define CSR_MCONTROL_H (0x1LL << CSR_MCONTROL_H_OFFSET) /* * When set, enable this trigger in S mode. */ #define CSR_MCONTROL_S_OFFSET 4 #define CSR_MCONTROL_S_LENGTH 1 -#define CSR_MCONTROL_S (0x1L << CSR_MCONTROL_S_OFFSET) +#define CSR_MCONTROL_S (0x1LL << CSR_MCONTROL_S_OFFSET) /* * When set, enable this trigger in U mode. */ #define CSR_MCONTROL_U_OFFSET 3 #define CSR_MCONTROL_U_LENGTH 1 -#define CSR_MCONTROL_U (0x1L << CSR_MCONTROL_U_OFFSET) +#define CSR_MCONTROL_U (0x1LL << CSR_MCONTROL_U_OFFSET) /* * When set, the trigger fires on the virtual address or opcode of an * instruction that is executed. */ #define CSR_MCONTROL_EXECUTE_OFFSET 2 #define CSR_MCONTROL_EXECUTE_LENGTH 1 -#define CSR_MCONTROL_EXECUTE (0x1L << CSR_MCONTROL_EXECUTE_OFFSET) +#define CSR_MCONTROL_EXECUTE (0x1LL << CSR_MCONTROL_EXECUTE_OFFSET) /* * When set, the trigger fires on the virtual address or data of a store. */ #define CSR_MCONTROL_STORE_OFFSET 1 #define CSR_MCONTROL_STORE_LENGTH 1 -#define CSR_MCONTROL_STORE (0x1L << CSR_MCONTROL_STORE_OFFSET) +#define CSR_MCONTROL_STORE (0x1LL << CSR_MCONTROL_STORE_OFFSET) /* * When set, the trigger fires on the virtual address or data of a load. */ #define CSR_MCONTROL_LOAD_OFFSET 0 #define CSR_MCONTROL_LOAD_LENGTH 1 -#define CSR_MCONTROL_LOAD (0x1L << CSR_MCONTROL_LOAD_OFFSET) +#define CSR_MCONTROL_LOAD (0x1LL << CSR_MCONTROL_LOAD_OFFSET) #define CSR_ICOUNT 0x7a1 #define CSR_ICOUNT_TYPE_OFFSET XLEN-4 #define CSR_ICOUNT_TYPE_LENGTH 4 -#define CSR_ICOUNT_TYPE (0xfL << CSR_ICOUNT_TYPE_OFFSET) +#define CSR_ICOUNT_TYPE (0xfLL << CSR_ICOUNT_TYPE_OFFSET) #define CSR_ICOUNT_DMODE_OFFSET XLEN-5 #define CSR_ICOUNT_DMODE_LENGTH 1 -#define CSR_ICOUNT_DMODE (0x1L << CSR_ICOUNT_DMODE_OFFSET) +#define CSR_ICOUNT_DMODE (0x1LL << CSR_ICOUNT_DMODE_OFFSET) /* * When count is decremented to 0, the trigger fires. Instead of * changing \Fcount from 1 to 0, it is also acceptable for hardware to @@ -479,35 +479,35 @@ */ #define CSR_ICOUNT_COUNT_OFFSET 10 #define CSR_ICOUNT_COUNT_LENGTH 14 -#define CSR_ICOUNT_COUNT (0x3fffL << CSR_ICOUNT_COUNT_OFFSET) +#define CSR_ICOUNT_COUNT (0x3fffLL << CSR_ICOUNT_COUNT_OFFSET) /* * When set, every instruction completed or exception taken in M mode decrements \Fcount * by 1. */ #define CSR_ICOUNT_M_OFFSET 9 #define CSR_ICOUNT_M_LENGTH 1 -#define CSR_ICOUNT_M (0x1L << CSR_ICOUNT_M_OFFSET) +#define CSR_ICOUNT_M (0x1LL << CSR_ICOUNT_M_OFFSET) /* * When set, every instruction completed or exception taken in in H mode decrements \Fcount * by 1. */ #define CSR_ICOUNT_H_OFFSET 8 #define CSR_ICOUNT_H_LENGTH 1 -#define CSR_ICOUNT_H (0x1L << CSR_ICOUNT_H_OFFSET) +#define CSR_ICOUNT_H (0x1LL << CSR_ICOUNT_H_OFFSET) /* * When set, every instruction completed or exception taken in S mode decrements \Fcount * by 1. */ #define CSR_ICOUNT_S_OFFSET 7 #define CSR_ICOUNT_S_LENGTH 1 -#define CSR_ICOUNT_S (0x1L << CSR_ICOUNT_S_OFFSET) +#define CSR_ICOUNT_S (0x1LL << CSR_ICOUNT_S_OFFSET) /* * When set, every instruction completed or exception taken in U mode decrements \Fcount * by 1. */ #define CSR_ICOUNT_U_OFFSET 6 #define CSR_ICOUNT_U_LENGTH 1 -#define CSR_ICOUNT_U (0x1L << CSR_ICOUNT_U_OFFSET) +#define CSR_ICOUNT_U (0x1LL << CSR_ICOUNT_U_OFFSET) /* * Determines what happens when this trigger matches. * @@ -528,7 +528,7 @@ */ #define CSR_ICOUNT_ACTION_OFFSET 0 #define CSR_ICOUNT_ACTION_LENGTH 6 -#define CSR_ICOUNT_ACTION (0x3fL << CSR_ICOUNT_ACTION_OFFSET) +#define CSR_ICOUNT_ACTION (0x3fLL << CSR_ICOUNT_ACTION_OFFSET) #define DMI_DMSTATUS 0x11 /* * This field is 1 when all currently selected harts have acknowledged the previous \Fresumereq. From f37e93bbc091f2c523ad64d520b2af9b2630f947 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 3 Jul 2017 13:55:20 -0700 Subject: [PATCH 7/9] Try using abstract commands to read registers This is the only way the spec guarantees that GPRs are accessible, and depending on the implementation this might be the only way that CSRs are accessible. Also changed the debug code that parses out DMI fields to be simpler to maintain (albeit a little slower). riscv013_execute_debug_buffer() now automatically clears cmderr if the command fails. That feels like the right behavior. (It does return the error to its caller.) --- src/target/riscv/riscv-013.c | 221 +++++++++++++++++++++++++---------- 1 file changed, 159 insertions(+), 62 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index fc7f3aa10..7df035c75 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -187,55 +187,60 @@ typedef struct { // Some memoized values int progbuf_size, progbuf_addr, data_addr, data_size; + + bool abstract_read_csr_supported; + bool abstract_write_csr_supported; + bool abstract_read_fpr_supported; + bool abstract_write_fpr_supported; } riscv013_info_t; static void decode_dmi(char *text, unsigned address, unsigned data) { + static const struct { + unsigned address; + uint64_t mask; + const char *name; + } description[] = { + { DMI_DMSTATUS, DMI_DMSTATUS_ALLRESUMEACK, "allresumeack" }, + { DMI_DMSTATUS, DMI_DMSTATUS_ANYRESUMEACK, "anyresumeack" }, + { DMI_DMSTATUS, DMI_DMSTATUS_ALLNONEXISTENT, "allnonexistent" }, + { DMI_DMSTATUS, DMI_DMSTATUS_ANYNONEXISTENT, "anynonexistent" }, + { DMI_DMSTATUS, DMI_DMSTATUS_ALLUNAVAIL, "allunavail" }, + { DMI_DMSTATUS, DMI_DMSTATUS_ANYUNAVAIL, "anyunavail" }, + { DMI_DMSTATUS, DMI_DMSTATUS_ALLRUNNING, "allrunning" }, + { DMI_DMSTATUS, DMI_DMSTATUS_ANYRUNNING, "anyrunning" }, + { DMI_DMSTATUS, DMI_DMSTATUS_ALLHALTED, "allhalted" }, + { DMI_DMSTATUS, DMI_DMSTATUS_ANYHALTED, "anyhalted" }, + { DMI_DMSTATUS, DMI_DMSTATUS_AUTHENTICATED, "authenticated" }, + { DMI_DMSTATUS, DMI_DMSTATUS_AUTHBUSY, "authbusy" }, + { DMI_DMSTATUS, DMI_DMSTATUS_CFGSTRVALID, "cfgstrvalid" }, + { DMI_DMSTATUS, DMI_DMSTATUS_VERSION, "version" }, + + { DMI_ABSTRACTCS, DMI_ABSTRACTCS_PROGSIZE, "progsize" }, + { DMI_ABSTRACTCS, DMI_ABSTRACTCS_BUSY, "busy" }, + { DMI_ABSTRACTCS, DMI_ABSTRACTCS_CMDERR, "cmderr" }, + { DMI_ABSTRACTCS, DMI_ABSTRACTCS_DATACOUNT, "datacount" }, + + { DMI_COMMAND, DMI_COMMAND_CMDTYPE, "cmdtype" }, + }; + text[0] = 0; - switch (address) { - case DMI_DMSTATUS: - if (get_field(data, DMI_DMSTATUS_ALLRESUMEACK)) { - strcat(text, " allresumeack"); + for (unsigned i = 0; i < DIM(description); i++) { + if (description[i].address == address) { + uint64_t mask = description[i].mask; + unsigned value = get_field(data, mask); + if (value) { + if (i > 0) + *(text++) = ' '; + if (mask & (mask >> 1)) { + // If the field is more than 1 bit wide. + sprintf(text, "%s=%d", description[i].name, value); + } else { + strcpy(text, description[i].name); + } + text += strlen(text); } - if (get_field(data, DMI_DMSTATUS_ANYRESUMEACK)) { - strcat(text, " anyresumeack"); - } - if (get_field(data, DMI_DMSTATUS_ALLNONEXISTENT)) { - strcat(text, " allnonexistent"); - } - if (get_field(data, DMI_DMSTATUS_ANYNONEXISTENT)) { - strcat(text, " anynonexistent"); - } - if (get_field(data, DMI_DMSTATUS_ALLUNAVAIL)) { - strcat(text, " allunavail"); - } - if (get_field(data, DMI_DMSTATUS_ANYUNAVAIL)) { - strcat(text, " anyunavail"); - } - if (get_field(data, DMI_DMSTATUS_ALLRUNNING)) { - strcat(text, " allrunning"); - } - if (get_field(data, DMI_DMSTATUS_ANYRUNNING)) { - strcat(text, " anyrunning"); - } - if (get_field(data, DMI_DMSTATUS_ALLHALTED)) { - strcat(text, " allhalted"); - } - if (get_field(data, DMI_DMSTATUS_ANYHALTED)) { - strcat(text, " anyhalted"); - } - if (get_field(data, DMI_DMSTATUS_AUTHENTICATED)) { - strcat(text, " authenticated"); - } - if (get_field(data, DMI_DMSTATUS_AUTHBUSY)) { - strcat(text, " authbusy"); - } - if (get_field(data, DMI_DMSTATUS_CFGSTRVALID)) { - strcat(text, " cfgstrvalid"); - } - sprintf(text + strlen(text), " version=%d", get_field(data, - DMI_DMSTATUS_VERSION)); - break; + } } } @@ -612,9 +617,109 @@ static int register_write_direct(struct target *target, unsigned number, return ERROR_OK; } +static int execute_abstract_command(struct target *target, uint32_t command) +{ + LOG_DEBUG("command=0x%x", command); + dmi_write(target, DMI_COMMAND, command); + + { + uint32_t dmstatus = 0; + wait_for_idle(target, &dmstatus); + } + + uint32_t cs = dmi_read(target, DMI_ABSTRACTCS); + unsigned cmderr = get_field(cs, DMI_ABSTRACTCS_CMDERR); + if (cmderr != 0) { + LOG_DEBUG("command 0x%x failed; abstractcs=0x%x", command, cs); + // Clear the error. + dmi_write(target, DMI_ABSTRACTCS, set_field(0, DMI_ABSTRACTCS_CMDERR, + cmderr)); + return ERROR_FAIL; + } + + return ERROR_OK; +} + +static uint64_t read_abstract_arg(struct target *target, unsigned index) +{ + uint64_t value = 0; + unsigned xlen = riscv_xlen(target); + unsigned offset = index * xlen / 32; + switch (xlen) { + default: + LOG_ERROR("Unsupported xlen: %d", xlen); + return ~0; + case 64: + value |= ((uint64_t) dmi_read(target, DMI_DATA0 + offset + 1)) << 32; + case 32: + value |= dmi_read(target, DMI_DATA0 + offset); + } + return value; +} + +static int register_read_abstract(struct target *target, uint64_t *value, + uint32_t number, unsigned size) +{ + RISCV013_INFO(r); + + uint32_t command = set_field(0, DMI_COMMAND_CMDTYPE, 0); + switch (size) { + case 32: + command = set_field(command, AC_ACCESS_REGISTER_SIZE, 2); + break; + case 64: + command = set_field(command, AC_ACCESS_REGISTER_SIZE, 3); + break; + default: + LOG_ERROR("Unsupported abstract register read size: %d", size); + return ERROR_FAIL; + } + command = set_field(command, AC_ACCESS_REGISTER_POSTEXEC, 0); + command = set_field(command, AC_ACCESS_REGISTER_TRANSFER, 1); + command = set_field(command, AC_ACCESS_REGISTER_WRITE, 0); + + if (number <= GDB_REGNO_XPR31) { + command = set_field(command, AC_ACCESS_REGISTER_REGNO, + 0x1000 + number - GDB_REGNO_XPR0); + } else if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { + if (!r->abstract_read_fpr_supported) + return ERROR_FAIL; + command = set_field(command, AC_ACCESS_REGISTER_REGNO, + 0x1020 + number - GDB_REGNO_FPR0); + } else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) { + if (!r->abstract_read_csr_supported) + return ERROR_FAIL; + command = set_field(command, AC_ACCESS_REGISTER_REGNO, + number - GDB_REGNO_CSR0); + } else { + return ERROR_FAIL; + } + + int result = execute_abstract_command(target, command); + if (result != ERROR_OK) { + if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { + r->abstract_read_fpr_supported = false; + LOG_INFO("Disabling abstract command reads from FPRs."); + } else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) { + r->abstract_read_csr_supported = false; + LOG_INFO("Disabling abstract command reads from CSRs."); + } + return result; + } + + *value = read_abstract_arg(target, 0); + + return ERROR_OK; +} + /** Actually read registers from the target right now. */ static int register_read_direct(struct target *target, uint64_t *value, uint32_t number) { + int result = register_read_abstract(target, value, number, + riscv_xlen(target)); + if (result == ERROR_OK) + return ERROR_OK; + struct riscv_program program; riscv_program_init(&program, target); riscv_addr_t output = riscv_program_alloc_d(&program); @@ -742,6 +847,13 @@ static int init_target(struct command_context *cmd_ctx, info->dmi_busy_delay = 0; info->ac_busy_delay = 0; + // Assume all these abstract commands are supported until we learn + // otherwise. + info->abstract_read_csr_supported = true; + info->abstract_write_csr_supported = true; + info->abstract_read_fpr_supported = true; + info->abstract_write_fpr_supported = true; + target->reg_cache = calloc(1, sizeof(*target->reg_cache)); target->reg_cache->name = "RISC-V Registers"; target->reg_cache->num_regs = GDB_REGNO_COUNT; @@ -1207,9 +1319,9 @@ static int examine(struct target *target) riscv_program_insert(&program64, sd(GDB_REGNO_S0, GDB_REGNO_S0, offset)); riscv_program_csrrw(&program64, GDB_REGNO_S0, GDB_REGNO_S0, GDB_REGNO_DSCRATCH); riscv_program_fence(&program64); - riscv_program_exec(&program64, target); + int result = riscv_program_exec(&program64, target); - if (get_field(dmi_read(target, DMI_ABSTRACTCS), DMI_ABSTRACTCS_CMDERR) == 0) { + if (result == ERROR_OK) { r->debug_buffer_addr[i] = (dmi_read(target, DMI_PROGBUF0 + (8 + offset) / 4) << 32) + dmi_read(target, DMI_PROGBUF0 + (4 + offset) / 4) @@ -1742,7 +1854,7 @@ struct target_type riscv013_target = .arch_state = arch_state, }; -/*** 0.13-specific implementations of various RISC-V hepler functions. ***/ +/*** 0.13-specific implementations of various RISC-V helper functions. ***/ static riscv_reg_t riscv013_get_register(struct target *target, int hid, int rid) { LOG_DEBUG("reading register 0x%08x on hart %d", rid, hid); @@ -1914,28 +2026,13 @@ riscv_insn_t riscv013_read_debug_buffer(struct target *target, unsigned index) int riscv013_execute_debug_buffer(struct target *target) { - riscv013_clear_abstract_error(target); - uint32_t run_program = 0; run_program = set_field(run_program, AC_ACCESS_REGISTER_SIZE, 2); run_program = set_field(run_program, AC_ACCESS_REGISTER_POSTEXEC, 1); run_program = set_field(run_program, AC_ACCESS_REGISTER_TRANSFER, 0); run_program = set_field(run_program, AC_ACCESS_REGISTER_REGNO, 0x1000); - dmi_write(target, DMI_COMMAND, run_program); - { - uint32_t dmstatus = 0; - wait_for_idle(target, &dmstatus); - } - - uint32_t cs = dmi_read(target, DMI_ABSTRACTCS); - if (get_field(cs, DMI_ABSTRACTCS_CMDERR) != 0) { - LOG_ERROR("unable to execute program: (abstractcs=0x%08x)", cs); - dmi_read(target, DMI_DMSTATUS); - return ERROR_FAIL; - } - - return ERROR_OK; + return execute_abstract_command(target, run_program); } void riscv013_fill_dmi_write_u64(struct target *target, char *buf, int a, uint64_t d) From 856f70fe449e4591fe9445ccdcbee0e83b334746 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Sat, 8 Jul 2017 11:12:24 -0700 Subject: [PATCH 8/9] Try abstract register writes as well. --- src/target/riscv/riscv-013.c | 188 +++++++++++++++++++++++++---------- 1 file changed, 133 insertions(+), 55 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 7df035c75..731289b58 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -578,45 +578,6 @@ static int wait_for_idle(struct target *target, uint32_t *abstractcs) } } -static int register_read_direct(struct target *target, uint64_t *value, uint32_t number); - -static int register_write_direct(struct target *target, unsigned number, - uint64_t value) -{ - struct riscv_program program; - - LOG_DEBUG("[%d] reg[0x%x] <- 0x%" PRIx64, riscv_current_hartid(target), - number, value); - - riscv_program_init(&program, target); - - riscv_addr_t input = riscv_program_alloc_d(&program); - riscv_program_write_ram(&program, input + 4, value >> 32); - riscv_program_write_ram(&program, input, value); - - assert(GDB_REGNO_XPR0 == 0); - if (number <= GDB_REGNO_XPR31) { - riscv_program_lx(&program, number, input); - } else if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { - riscv_program_fld(&program, number, input); - } else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) { - enum gdb_regno temp = riscv_program_gettemp(&program); - riscv_program_lx(&program, temp, input); - riscv_program_csrw(&program, temp, number); - } else { - LOG_ERROR("Unsupported register (enum gdb_regno)(%d)", number); - abort(); - } - - int exec_out = riscv_program_exec(&program, target); - if (exec_out != ERROR_OK) { - riscv013_clear_abstract_error(target); - return ERROR_FAIL; - } - - return ERROR_OK; -} - static int execute_abstract_command(struct target *target, uint32_t command) { LOG_DEBUG("command=0x%x", command); @@ -640,9 +601,9 @@ static int execute_abstract_command(struct target *target, uint32_t command) return ERROR_OK; } -static uint64_t read_abstract_arg(struct target *target, unsigned index) +static riscv_reg_t read_abstract_arg(struct target *target, unsigned index) { - uint64_t value = 0; + riscv_reg_t value = 0; unsigned xlen = riscv_xlen(target); unsigned offset = index * xlen / 32; switch (xlen) { @@ -657,6 +618,23 @@ static uint64_t read_abstract_arg(struct target *target, unsigned index) return value; } +static int write_abstract_arg(struct target *target, unsigned index, + riscv_reg_t value) +{ + unsigned xlen = riscv_xlen(target); + unsigned offset = index * xlen / 32; + switch (xlen) { + default: + LOG_ERROR("Unsupported xlen: %d", xlen); + return ~0; + case 64: + dmi_write(target, DMI_DATA0 + offset + 1, value >> 32); + case 32: + dmi_write(target, DMI_DATA0 + offset, value); + } + return ERROR_OK; +} + static int register_read_abstract(struct target *target, uint64_t *value, uint32_t number, unsigned size) { @@ -712,30 +690,91 @@ static int register_read_abstract(struct target *target, uint64_t *value, return ERROR_OK; } -/** Actually read registers from the target right now. */ -static int register_read_direct(struct target *target, uint64_t *value, uint32_t number) +static int register_write_abstract(struct target *target, uint32_t number, + uint64_t value, unsigned size) { - int result = register_read_abstract(target, value, number, + RISCV013_INFO(r); + + uint32_t command = set_field(0, DMI_COMMAND_CMDTYPE, 0); + switch (size) { + case 32: + command = set_field(command, AC_ACCESS_REGISTER_SIZE, 2); + break; + case 64: + command = set_field(command, AC_ACCESS_REGISTER_SIZE, 3); + break; + default: + LOG_ERROR("Unsupported abstract register read size: %d", size); + return ERROR_FAIL; + } + command = set_field(command, AC_ACCESS_REGISTER_POSTEXEC, 0); + command = set_field(command, AC_ACCESS_REGISTER_TRANSFER, 1); + command = set_field(command, AC_ACCESS_REGISTER_WRITE, 1); + + if (number <= GDB_REGNO_XPR31) { + command = set_field(command, AC_ACCESS_REGISTER_REGNO, + 0x1000 + number - GDB_REGNO_XPR0); + } else if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { + if (!r->abstract_read_fpr_supported) + return ERROR_FAIL; + command = set_field(command, AC_ACCESS_REGISTER_REGNO, + 0x1020 + number - GDB_REGNO_FPR0); + } else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) { + if (!r->abstract_read_csr_supported) + return ERROR_FAIL; + command = set_field(command, AC_ACCESS_REGISTER_REGNO, + number - GDB_REGNO_CSR0); + } else { + return ERROR_FAIL; + } + + if (write_abstract_arg(target, 0, value) != ERROR_OK) { + return ERROR_FAIL; + } + + int result = execute_abstract_command(target, command); + if (result != ERROR_OK) { + if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { + r->abstract_write_fpr_supported = false; + LOG_INFO("Disabling abstract command writes to FPRs."); + } else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) { + r->abstract_write_csr_supported = false; + LOG_INFO("Disabling abstract command writes to CSRs."); + } + return result; + } + + return ERROR_OK; +} + +static int register_write_direct(struct target *target, unsigned number, + uint64_t value) +{ + LOG_DEBUG("[%d] reg[0x%x] <- 0x%" PRIx64, riscv_current_hartid(target), + number, value); + + int result = register_write_abstract(target, number, value, riscv_xlen(target)); if (result == ERROR_OK) return ERROR_OK; struct riscv_program program; + riscv_program_init(&program, target); - riscv_addr_t output = riscv_program_alloc_d(&program); - riscv_program_write_ram(&program, output + 4, 0); - riscv_program_write_ram(&program, output, 0); + + riscv_addr_t input = riscv_program_alloc_d(&program); + riscv_program_write_ram(&program, input + 4, value >> 32); + riscv_program_write_ram(&program, input, value); assert(GDB_REGNO_XPR0 == 0); if (number <= GDB_REGNO_XPR31) { - riscv_program_sx(&program, number, output); + riscv_program_lx(&program, number, input); } else if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { - riscv_program_fsd(&program, number, output); + riscv_program_fld(&program, number, input); } else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) { - LOG_DEBUG("reading CSR index=0x%03x", number - GDB_REGNO_CSR0); enum gdb_regno temp = riscv_program_gettemp(&program); - riscv_program_csrr(&program, temp, number); - riscv_program_sx(&program, temp, output); + riscv_program_lx(&program, temp, input); + riscv_program_csrw(&program, temp, number); } else { LOG_ERROR("Unsupported register (enum gdb_regno)(%d)", number); abort(); @@ -747,9 +786,48 @@ static int register_read_direct(struct target *target, uint64_t *value, uint32_t return ERROR_FAIL; } - *value = 0; - *value |= ((uint64_t)(riscv_program_read_ram(&program, output + 4))) << 32; - *value |= riscv_program_read_ram(&program, output); + return ERROR_OK; +} + +/** Actually read registers from the target right now. */ +static int register_read_direct(struct target *target, uint64_t *value, uint32_t number) +{ + int result = register_read_abstract(target, value, number, + riscv_xlen(target)); + + if (result != ERROR_OK) { + struct riscv_program program; + riscv_program_init(&program, target); + riscv_addr_t output = riscv_program_alloc_d(&program); + riscv_program_write_ram(&program, output + 4, 0); + riscv_program_write_ram(&program, output, 0); + + assert(GDB_REGNO_XPR0 == 0); + if (number <= GDB_REGNO_XPR31) { + riscv_program_sx(&program, number, output); + } else if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { + riscv_program_fsd(&program, number, output); + } else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) { + LOG_DEBUG("reading CSR index=0x%03x", number - GDB_REGNO_CSR0); + enum gdb_regno temp = riscv_program_gettemp(&program); + riscv_program_csrr(&program, temp, number); + riscv_program_sx(&program, temp, output); + } else { + LOG_ERROR("Unsupported register (enum gdb_regno)(%d)", number); + abort(); + } + + int exec_out = riscv_program_exec(&program, target); + if (exec_out != ERROR_OK) { + riscv013_clear_abstract_error(target); + return ERROR_FAIL; + } + + *value = 0; + *value |= ((uint64_t)(riscv_program_read_ram(&program, output + 4))) << 32; + *value |= riscv_program_read_ram(&program, output); + } + LOG_DEBUG("[%d] reg[0x%x] = 0x%" PRIx64, riscv_current_hartid(target), number, *value); return ERROR_OK; From 09bf86e31a76e401e516d3cb515d0054aa6f78f7 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 12 Jul 2017 14:36:09 -0700 Subject: [PATCH 9/9] Keep around cmderr for callers to inspect. Use this to only change abstract register access behavior when cmderr explicitly says the requested operation is unsupported. --- src/target/riscv/riscv-013.c | 65 +++++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 731289b58..758e1d3c0 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -192,6 +192,11 @@ typedef struct { bool abstract_write_csr_supported; bool abstract_read_fpr_supported; bool abstract_write_fpr_supported; + + // When a function returns some error due to a failure indicated by the + // target in cmderr, the caller can look here to see what that error was. + // (Compare with errno.) + unsigned cmderr; } riscv013_info_t; static void decode_dmi(char *text, unsigned address, unsigned data) @@ -546,6 +551,7 @@ uint32_t abstract_register_size(unsigned width) static int wait_for_idle(struct target *target, uint32_t *abstractcs) { + RISCV013_INFO(info); time_t start = time(NULL); while (1) { *abstractcs = dmi_read(target, DMI_ABSTRACTCS); @@ -555,7 +561,8 @@ static int wait_for_idle(struct target *target, uint32_t *abstractcs) } if (time(NULL) - start > WALL_CLOCK_TIMEOUT) { - if (get_field(*abstractcs, DMI_ABSTRACTCS_CMDERR) != CMDERR_NONE) { + info->cmderr = get_field(*abstractcs, DMI_ABSTRACTCS_CMDERR); + if (info->cmderr != CMDERR_NONE) { const char *errors[8] = { "none", "busy", @@ -567,8 +574,7 @@ static int wait_for_idle(struct target *target, uint32_t *abstractcs) "other" }; LOG_ERROR("Abstract command ended in error '%s' (abstractcs=0x%x)", - errors[get_field(*abstractcs, DMI_ABSTRACTCS_CMDERR)], - *abstractcs); + errors[info->cmderr], *abstractcs); } LOG_ERROR("Timed out waiting for busy to go low. (abstractcs=0x%x)", @@ -580,6 +586,7 @@ static int wait_for_idle(struct target *target, uint32_t *abstractcs) static int execute_abstract_command(struct target *target, uint32_t command) { + RISCV013_INFO(info); LOG_DEBUG("command=0x%x", command); dmi_write(target, DMI_COMMAND, command); @@ -589,12 +596,12 @@ static int execute_abstract_command(struct target *target, uint32_t command) } uint32_t cs = dmi_read(target, DMI_ABSTRACTCS); - unsigned cmderr = get_field(cs, DMI_ABSTRACTCS_CMDERR); - if (cmderr != 0) { + info->cmderr = get_field(cs, DMI_ABSTRACTCS_CMDERR); + if (info->cmderr != 0) { LOG_DEBUG("command 0x%x failed; abstractcs=0x%x", command, cs); // Clear the error. dmi_write(target, DMI_ABSTRACTCS, set_field(0, DMI_ABSTRACTCS_CMDERR, - cmderr)); + info->cmderr)); return ERROR_FAIL; } @@ -675,12 +682,14 @@ static int register_read_abstract(struct target *target, uint64_t *value, int result = execute_abstract_command(target, command); if (result != ERROR_OK) { - if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { - r->abstract_read_fpr_supported = false; - LOG_INFO("Disabling abstract command reads from FPRs."); - } else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) { - r->abstract_read_csr_supported = false; - LOG_INFO("Disabling abstract command reads from CSRs."); + if (r->cmderr == CMDERR_NOT_SUPPORTED) { + if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { + r->abstract_read_fpr_supported = false; + LOG_INFO("Disabling abstract command reads from FPRs."); + } else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) { + r->abstract_read_csr_supported = false; + LOG_INFO("Disabling abstract command reads from CSRs."); + } } return result; } @@ -693,7 +702,7 @@ static int register_read_abstract(struct target *target, uint64_t *value, static int register_write_abstract(struct target *target, uint32_t number, uint64_t value, unsigned size) { - RISCV013_INFO(r); + RISCV013_INFO(info); uint32_t command = set_field(0, DMI_COMMAND_CMDTYPE, 0); switch (size) { @@ -715,12 +724,12 @@ static int register_write_abstract(struct target *target, uint32_t number, command = set_field(command, AC_ACCESS_REGISTER_REGNO, 0x1000 + number - GDB_REGNO_XPR0); } else if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { - if (!r->abstract_read_fpr_supported) + if (!info->abstract_read_fpr_supported) return ERROR_FAIL; command = set_field(command, AC_ACCESS_REGISTER_REGNO, 0x1020 + number - GDB_REGNO_FPR0); } else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) { - if (!r->abstract_read_csr_supported) + if (!info->abstract_read_csr_supported) return ERROR_FAIL; command = set_field(command, AC_ACCESS_REGISTER_REGNO, number - GDB_REGNO_CSR0); @@ -734,12 +743,14 @@ static int register_write_abstract(struct target *target, uint32_t number, int result = execute_abstract_command(target, command); if (result != ERROR_OK) { - if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { - r->abstract_write_fpr_supported = false; - LOG_INFO("Disabling abstract command writes to FPRs."); - } else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) { - r->abstract_write_csr_supported = false; - LOG_INFO("Disabling abstract command writes to CSRs."); + if (info->cmderr == CMDERR_NOT_SUPPORTED) { + if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { + info->abstract_write_fpr_supported = false; + LOG_INFO("Disabling abstract command writes to FPRs."); + } else if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095) { + info->abstract_write_csr_supported = false; + LOG_INFO("Disabling abstract command writes to CSRs."); + } } return result; } @@ -1415,7 +1426,7 @@ static int examine(struct target *target) r->xlen[i], r->debug_buffer_addr[i]); if (riscv_program_gah(&program64, r->debug_buffer_addr[i])) { - LOG_ERROR("This implementation will not work with hart %d with debug_buffer_addr of 0x%lx\n", i, + LOG_ERROR("This implementation will not work with hart %d with debug_buffer_addr of 0x%lx\n", i, (long)r->debug_buffer_addr[i]); abort(); } @@ -1479,7 +1490,7 @@ static int assert_reset(struct target *target) static int deassert_reset(struct target *target) { RISCV_INFO(r); - RISCV013_INFO(info); + RISCV013_INFO(info); select_dmi(target); /*FIXME -- this only works for Single Hart*/ @@ -1637,7 +1648,7 @@ static int read_memory(struct target *target, target_addr_t address, size_t reads = 0; size_t rereads = reads; for (riscv_addr_t i = start; i < count; ++i) { - size_t index = + size_t index = riscv_batch_add_dmi_read( batch, riscv013_debug_buffer_register(target, r_data)); @@ -1690,7 +1701,8 @@ static int read_memory(struct target *target, target_addr_t address, uint32_t abstractcs = dmi_read(target, DMI_ABSTRACTCS); while (get_field(abstractcs, DMI_ABSTRACTCS_BUSY)) abstractcs = dmi_read(target, DMI_ABSTRACTCS); - switch (get_field(abstractcs, DMI_ABSTRACTCS_CMDERR)) { + info->cmderr = get_field(abstractcs, DMI_ABSTRACTCS_CMDERR); + switch (info->cmderr) { case CMDERR_NONE: LOG_DEBUG("successful (partial?) memory write"); break; @@ -1874,7 +1886,8 @@ static int write_memory(struct target *target, target_addr_t address, uint32_t abstractcs = dmi_read(target, DMI_ABSTRACTCS); while (get_field(abstractcs, DMI_ABSTRACTCS_BUSY)) abstractcs = dmi_read(target, DMI_ABSTRACTCS); - switch (get_field(abstractcs, DMI_ABSTRACTCS_CMDERR)) { + info->cmderr = get_field(abstractcs, DMI_ABSTRACTCS_CMDERR); + switch (info->cmderr) { case CMDERR_NONE: LOG_DEBUG("successful (partial?) memory write"); break;