From 5c0a9a9ee40c94714a5061b4df7033d6716985b2 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 5 Apr 2018 17:59:07 -0700 Subject: Just read abstractcs once when executing a command DebugBreakpoint went from 3.41s to 3.05s! Change-Id: Icfc4ad5fb663b3607bf2027fda744b43be662fc5 --- src/target/riscv/riscv-013.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 8946948..26a8c95 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -670,17 +670,12 @@ 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 abstractcs = 0; - wait_for_idle(target, &abstractcs); - } + uint32_t abstractcs = 0; + wait_for_idle(target, &abstractcs); - uint32_t cs; - if (dmi_read(target, &cs, DMI_ABSTRACTCS) != ERROR_OK) - return ERROR_FAIL; - info->cmderr = get_field(cs, DMI_ABSTRACTCS_CMDERR); + info->cmderr = get_field(abstractcs, DMI_ABSTRACTCS_CMDERR); if (info->cmderr != 0) { - LOG_DEBUG("command 0x%x failed; abstractcs=0x%x", command, cs); + LOG_DEBUG("command 0x%x failed; abstractcs=0x%x", command, abstractcs); /* Clear the error. */ dmi_write(target, DMI_ABSTRACTCS, set_field(0, DMI_ABSTRACTCS_CMDERR, info->cmderr)); -- cgit v1.1 From 238b1e9f06709ce993f3057b7be54311485b5467 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Fri, 6 Apr 2018 15:52:40 -0700 Subject: Cache registers while halted. This saves us from re-reading s0 before doing just about anything program buffer related. Improves DebugBreakpoint from 3.01s to 2.89s. Feels like the improvement should be larger than that. Maybe my metric isn't very good. Change-Id: I85e1a1ddbf09006d76c451a32048be7b773dcfe9 --- src/target/riscv/riscv-013.c | 72 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 57 insertions(+), 15 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 26a8c95..d9e2907 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -57,6 +57,7 @@ static void riscv013_fill_dmi_write_u64(struct target *target, char *buf, int a, static void riscv013_fill_dmi_read_u64(struct target *target, char *buf, int a); static int riscv013_dmi_write_u64_bits(struct target *target); static void riscv013_fill_dmi_nop_u64(struct target *target, char *buf); +static int register_read(struct target *target, uint64_t *value, uint32_t number); 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); @@ -846,7 +847,7 @@ static int examine_progbuf(struct target *target) } uint64_t s0; - if (register_read_direct(target, &s0, GDB_REGNO_S0) != ERROR_OK) + if (register_read(target, &s0, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; struct riscv_program program; @@ -1055,6 +1056,11 @@ static int register_write_direct(struct target *target, unsigned number, int result = register_write_abstract(target, number, value, register_size(target, number)); + if (result == ERROR_OK && target->reg_cache) { + struct reg *reg = &target->reg_cache->reg_list[number]; + buf_set_u64(reg->value, 0, reg->size, value); + reg->valid = true; + } if (result == ERROR_OK || info->progbufsize + r->impebreak < 2 || !riscv_is_halted(target)) return result; @@ -1063,7 +1069,7 @@ static int register_write_direct(struct target *target, unsigned number, riscv_program_init(&program, target); uint64_t s0; - if (register_read_direct(target, &s0, GDB_REGNO_S0) != ERROR_OK) + if (register_read(target, &s0, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31 && @@ -1103,6 +1109,11 @@ static int register_write_direct(struct target *target, unsigned number, int exec_out = riscv_program_exec(&program, target); /* Don't message on error. Probably the register doesn't exist. */ + if (exec_out == ERROR_OK && target->reg_cache) { + struct reg *reg = &target->reg_cache->reg_list[number]; + buf_set_u64(reg->value, 0, reg->size, value); + reg->valid = true; + } /* Restore S0. */ if (register_write_direct(target, GDB_REGNO_S0, s0) != ERROR_OK) @@ -1111,6 +1122,37 @@ static int register_write_direct(struct target *target, unsigned number, return exec_out; } +/** Return the cached value, or read from the target if necessary. */ +static int register_read(struct target *target, uint64_t *value, uint32_t number) +{ + if (number == GDB_REGNO_ZERO) { + *value = 0; + return ERROR_OK; + } + if (target->reg_cache && + (number <= GDB_REGNO_XPR31 || + (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31))) { + /* Only check the cache for registers that we know won't spontaneously + * change. */ + struct reg *reg = &target->reg_cache->reg_list[number]; + if (reg && reg->valid) { + *value = buf_get_u64(reg->value, 0, reg->size); + LOG_INFO(">>> cache hit on %s: 0x%" PRIx64, + gdb_regno_name(number), *value); + return ERROR_OK; + } + } + int result = register_read_direct(target, value, number); + if (result != ERROR_OK) + return ERROR_FAIL; + if (target->reg_cache) { + struct reg *reg = &target->reg_cache->reg_list[number]; + buf_set_u64(reg->value, 0, reg->size, *value); + reg->valid = true; + } + 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) { @@ -1131,14 +1173,14 @@ static int register_read_direct(struct target *target, uint64_t *value, uint32_t bool use_scratch = false; uint64_t s0; - if (register_read_direct(target, &s0, GDB_REGNO_S0) != ERROR_OK) + if (register_read(target, &s0, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; /* Write program to move data into s0. */ uint64_t mstatus; if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { - if (register_read_direct(target, &mstatus, GDB_REGNO_MSTATUS) != ERROR_OK) + if (register_read(target, &mstatus, GDB_REGNO_MSTATUS) != ERROR_OK) return ERROR_FAIL; if ((mstatus & MSTATUS_FS) == 0) if (register_write_direct(target, GDB_REGNO_MSTATUS, @@ -1375,7 +1417,7 @@ static int examine(struct target *target) else r->xlen[i] = 32; - if (register_read_direct(target, &r->misa[i], GDB_REGNO_MISA)) { + if (register_read(target, &r->misa[i], GDB_REGNO_MISA)) { LOG_ERROR("Fatal: Failed to read MISA from hart %d.", i); return ERROR_FAIL; } @@ -1948,9 +1990,9 @@ static int read_memory_progbuf(struct target *target, target_addr_t address, * s1 holds the next data value to write */ uint64_t s0, s1; - if (register_read_direct(target, &s0, GDB_REGNO_S0) != ERROR_OK) + if (register_read(target, &s0, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; - if (register_read_direct(target, &s1, GDB_REGNO_S1) != ERROR_OK) + if (register_read(target, &s1, GDB_REGNO_S1) != ERROR_OK) return ERROR_FAIL; if (execute_fence(target) != ERROR_OK) @@ -2381,9 +2423,9 @@ static int write_memory_progbuf(struct target *target, target_addr_t address, int result = ERROR_OK; uint64_t s0, s1; - if (register_read_direct(target, &s0, GDB_REGNO_S0) != ERROR_OK) + if (register_read(target, &s0, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; - if (register_read_direct(target, &s1, GDB_REGNO_S1) != ERROR_OK) + if (register_read(target, &s1, GDB_REGNO_S1) != ERROR_OK) return ERROR_FAIL; /* Write the program (store, increment) */ @@ -2607,14 +2649,14 @@ static int riscv013_get_register(struct target *target, int result = ERROR_OK; if (rid == GDB_REGNO_PC) { - result = register_read_direct(target, value, GDB_REGNO_DPC); + result = register_read(target, value, GDB_REGNO_DPC); LOG_DEBUG("read PC from DPC: 0x%016" PRIx64, *value); } else if (rid == GDB_REGNO_PRIV) { uint64_t dcsr; - result = register_read_direct(target, &dcsr, GDB_REGNO_DCSR); + result = register_read(target, &dcsr, GDB_REGNO_DCSR); *value = get_field(dcsr, CSR_DCSR_PRV); } else { - result = register_read_direct(target, value, rid); + result = register_read(target, value, rid); if (result != ERROR_OK) *value = -1; } @@ -2644,7 +2686,7 @@ static int riscv013_set_register(struct target *target, int hid, int rid, uint64 } } else if (rid == GDB_REGNO_PRIV) { uint64_t dcsr; - register_read_direct(target, &dcsr, GDB_REGNO_DCSR); + register_read(target, &dcsr, GDB_REGNO_DCSR); dcsr = set_field(dcsr, CSR_DCSR_PRV, value); return register_write_direct(target, GDB_REGNO_DCSR, dcsr); } else { @@ -2740,7 +2782,7 @@ static bool riscv013_is_halted(struct target *target) static enum riscv_halt_reason riscv013_halt_reason(struct target *target) { riscv_reg_t dcsr; - int result = register_read_direct(target, &dcsr, GDB_REGNO_DCSR); + int result = register_read(target, &dcsr, GDB_REGNO_DCSR); if (result != ERROR_OK) return RISCV_HALT_UNKNOWN; @@ -2839,7 +2881,7 @@ static int riscv013_on_step_or_resume(struct target *target, bool step) /* We want to twiddle some bits in the debug CSR so debugging works. */ riscv_reg_t dcsr; - int result = register_read_direct(target, &dcsr, GDB_REGNO_DCSR); + int result = register_read(target, &dcsr, GDB_REGNO_DCSR); if (result != ERROR_OK) return result; dcsr = set_field(dcsr, CSR_DCSR_STEP, step); -- cgit v1.1 From 1fda89c3cef918a87d2b3bcd1f3158c011ef107e Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 9 Apr 2018 15:12:51 -0700 Subject: Only write hartsel if we're changing it. DebugBreakpoint went from 2.94s to 2.74s. Change-Id: Ia3ab857aea89fb83f0bcdd9a6bb69f256bde13dd --- src/target/riscv/riscv-013.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index d9e2907..4d23cd6 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -144,6 +144,8 @@ typedef struct { bool was_reset; /* Targets that are connected to this DM. */ struct list_head target_list; + /* The currently selected hartid on this DM. */ + int current_hartid; } dm013_info_t; typedef struct { @@ -243,6 +245,7 @@ static dm013_info_t *get_dm(struct target *target) if (!dm) { dm = calloc(1, sizeof(dm013_info_t)); dm->abs_chain_position = abs_chain_position; + dm->current_hartid = -1; INIT_LIST_HEAD(&dm->target_list); list_add(&dm->list, &dm_list); } @@ -1137,8 +1140,6 @@ static int register_read(struct target *target, uint64_t *value, uint32_t number struct reg *reg = &target->reg_cache->reg_list[number]; if (reg && reg->valid) { *value = buf_get_u64(reg->value, 0, reg->size); - LOG_INFO(">>> cache hit on %s: 0x%" PRIx64, - gdb_regno_name(number), *value); return ERROR_OK; } } @@ -2700,10 +2701,19 @@ static void riscv013_select_current_hart(struct target *target) { RISCV_INFO(r); + dm013_info_t *dm = get_dm(target); + if (r->current_hartid == dm->current_hartid) { + LOG_DEBUG(">>> nothing to be done"); + return; + } + + LOG_DEBUG(">>> change it"); + uint32_t dmcontrol; dmi_read(target, &dmcontrol, DMI_DMCONTROL); dmcontrol = set_field(dmcontrol, hartsel_mask(target), r->current_hartid); dmi_write(target, DMI_DMCONTROL, dmcontrol); + dm->current_hartid = r->current_hartid; } static int riscv013_halt_current_hart(struct target *target) -- cgit v1.1 From 909c9d4ab2e5ee5180ef81248608fa2109c3dba7 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 3 May 2018 17:58:44 -0700 Subject: Conform to OpenOCD style Change-Id: I3954a8ac254b460560fa1414c5921777e4005645 --- src/target/riscv/riscv-013.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 1869038..f3fe76b 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2722,9 +2722,8 @@ static int riscv013_select_current_hart(struct target *target) RISCV_INFO(r); dm013_info_t *dm = get_dm(target); - if (r->current_hartid == dm->current_hartid) { + if (r->current_hartid == dm->current_hartid) return ERROR_OK; - } uint32_t dmcontrol; if (dmi_read(target, &dmcontrol, DMI_DMCONTROL) != ERROR_OK) -- cgit v1.1