From 0d74c8689d96d1f04344b5bebc84ac9069c5ee70 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 14 Aug 2017 15:02:19 -0700 Subject: [PATCH 01/14] Fix block memory reads on slow targets. The interesting new code concerns ignore_prev_addr and this_is_last_read. Additionally, I tweaked some debug output, and optimized riscv_batch_run() when the batch is empty. --- src/target/riscv/batch.c | 5 +++++ src/target/riscv/riscv-013.c | 24 ++++++++++++++++-------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/target/riscv/batch.c b/src/target/riscv/batch.c index cb5a10856..d6e8b7941 100644 --- a/src/target/riscv/batch.c +++ b/src/target/riscv/batch.c @@ -44,6 +44,11 @@ bool riscv_batch_full(struct riscv_batch *batch) void riscv_batch_run(struct riscv_batch *batch) { + if (batch->used_scans == 0) { + LOG_DEBUG("Ignoring empty batch."); + return; + } + LOG_DEBUG("running a batch of %ld scans", (long)batch->used_scans); riscv_batch_add_nop(batch); diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 284f6a1ba..5fc3175ca 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1353,7 +1353,7 @@ 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; + bool ignore_prev_addr = true; bool this_is_last_read = false; LOG_DEBUG("reading until final address 0x%" PRIx64, fin_addr); while (count > 1 && !this_is_last_read) { @@ -1361,9 +1361,9 @@ static int read_memory(struct target *target, target_addr_t address, LOG_DEBUG("transferring burst starting at address 0x%" TARGET_PRIxADDR " (previous burst was 0x%" TARGET_PRIxADDR ")", cur_addr, prev_addr); - assert(first || prev_addr < cur_addr); - first = false; + assert(ignore_prev_addr || prev_addr < cur_addr); prev_addr = cur_addr; + ignore_prev_addr = false; riscv_addr_t start = (cur_addr - address) / size; assert (cur_addr >= address); struct riscv_batch *batch = riscv_batch_alloc( @@ -1409,10 +1409,11 @@ static int read_memory(struct target *target, target_addr_t address, info->cmderr = get_field(abstractcs, DMI_ABSTRACTCS_CMDERR); switch (info->cmderr) { case CMDERR_NONE: - LOG_DEBUG("successful (partial?) memory write"); + LOG_DEBUG("successful (partial?) memory read"); break; case CMDERR_BUSY: - LOG_DEBUG("memory write resulted in busy response"); + LOG_DEBUG("memory read resulted in busy response; " + "this_is_last_read=%d", this_is_last_read); riscv013_clear_abstract_error(target); increase_ac_busy_delay(target); retry_batch_transaction = true; @@ -1427,7 +1428,11 @@ static int read_memory(struct target *target, target_addr_t address, riscv_batch_free(batch); return ERROR_FAIL; } - if (retry_batch_transaction) continue; + if (retry_batch_transaction) { + this_is_last_read = false; + ignore_prev_addr = true; + continue; + } for (size_t i = start; i < start + reads; ++i) { riscv_addr_t offset = size*i; @@ -1437,8 +1442,11 @@ static int read_memory(struct target *target, target_addr_t address, if (this_is_last_read && i == start + reads - 1) { riscv013_set_autoexec(target, d_data, 0); - // access debug buffer without executing a program - this address logic was taken from program.c - int const off = (r_data - riscv_debug_buffer_addr(program.target)) / sizeof(program.debug_buffer[0]); + // Access debug buffer without executing a program. This + // address logic was taken from program.c. + int const off = (r_data - + riscv_debug_buffer_addr(program.target)) / + sizeof(program.debug_buffer[0]); value = riscv_read_debug_buffer(target, off); } else { uint64_t dmi_out = riscv_batch_get_dmi_read(batch, rereads); From 5c39079a62d0a6834bd4f37b61fd9f3108cadea9 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 15 Aug 2017 14:29:24 -0700 Subject: [PATCH 02/14] Remove some unnecessary casts. --- src/target/riscv/riscv-013.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 284f6a1ba..2adf9767e 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1319,7 +1319,7 @@ static int read_memory(struct target *target, target_addr_t address, } uint32_t value = riscv_program_read_ram(&program, r_data); - LOG_DEBUG("M[0x%" TARGET_PRIxADDR "] reads 0x%08lx", address, (long)value); + LOG_DEBUG("M[0x%" TARGET_PRIxADDR "] reads 0x%08x", address, value); switch (size) { case 1: buffer[0] = value; @@ -1466,7 +1466,7 @@ static int read_memory(struct target *target, target_addr_t address, return ERROR_FAIL; } - LOG_DEBUG("M[0x%08lx] reads 0x%08lx", (long)t_addr, (long)value); + LOG_DEBUG("M[0x%08lx] reads 0x%08x", (long)t_addr, value); } riscv_batch_free(batch); } @@ -1555,7 +1555,7 @@ static int write_memory(struct target *target, target_addr_t address, } riscv_program_write_ram(&program, r_data, value); - LOG_DEBUG("M[0x%08lx] writes 0x%08lx", (long)address, (long)value); + LOG_DEBUG("M[0x%08lx] writes 0x%08x", (long)address, value); if (riscv_program_exec(&program, target) != ERROR_OK) { uint32_t acs = dmi_read(target, DMI_ABSTRACTCS); @@ -1615,7 +1615,7 @@ static int write_memory(struct target *target, target_addr_t address, return ERROR_FAIL; } - LOG_DEBUG("M[0x%08lx] writes 0x%08lx", (long)t_addr, (long)value); + LOG_DEBUG("M[0x%08lx] writes 0x%08x", (long)t_addr, value); riscv_batch_add_dmi_write( batch, From 0ff4103a2659de06b76bcc6d96fe82afa5fd44cd Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 15 Aug 2017 15:47:35 -0700 Subject: [PATCH 03/14] Reset address if target was busy during bust write Improve Issue #98. DebugCompareSections is still failing for me (with an instrumented sometimes-slow spike), but MemTestBlock now passes reliably. --- src/target/riscv/riscv-013.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 5fc3175ca..d4cb51ced 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1358,13 +1358,13 @@ static int read_memory(struct target *target, target_addr_t address, LOG_DEBUG("reading until final address 0x%" PRIx64, fin_addr); while (count > 1 && !this_is_last_read) { cur_addr = riscv_read_debug_buffer_x(target, d_addr); - LOG_DEBUG("transferring burst starting at address 0x%" TARGET_PRIxADDR - " (previous burst was 0x%" TARGET_PRIxADDR ")", cur_addr, - prev_addr); + riscv_addr_t start = (cur_addr - address) / size; + LOG_DEBUG("reading burst at address 0x%" TARGET_PRIxADDR + "; prev_addr=0x%" TARGET_PRIxADDR "; start=0x%" + TARGET_PRIxADDR, cur_addr, prev_addr, start); assert(ignore_prev_addr || prev_addr < cur_addr); prev_addr = cur_addr; ignore_prev_addr = false; - riscv_addr_t start = (cur_addr - address) / size; assert (cur_addr >= address); struct riscv_batch *batch = riscv_batch_alloc( target, @@ -1431,6 +1431,18 @@ static int read_memory(struct target *target, target_addr_t address, if (retry_batch_transaction) { this_is_last_read = false; ignore_prev_addr = true; + + switch (riscv_xlen(target)) { + case 64: + riscv013_write_debug_buffer(target, d_addr + 4, (cur_addr - size) >> 32); + case 32: + riscv013_write_debug_buffer(target, d_addr, (cur_addr - size)); + break; + default: + LOG_ERROR("unknown XLEN %d", riscv_xlen(target)); + return ERROR_FAIL; + } + continue; } From 879c274cb96d985d3f21f2ec5fe59004578928a7 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 15 Aug 2017 15:55:09 -0700 Subject: [PATCH 04/14] riscv: Add commands for setting timeouts --- src/target/riscv/riscv-011.c | 47 ++++++++-------- src/target/riscv/riscv-013.c | 18 +++--- src/target/riscv/riscv.c | 105 +++++++++++++++++++++++++++++------ src/target/riscv/riscv.h | 9 +++ 4 files changed, 131 insertions(+), 48 deletions(-) diff --git a/src/target/riscv/riscv-011.c b/src/target/riscv/riscv-011.c index b126cf6fc..6b6666b4d 100644 --- a/src/target/riscv/riscv-011.c +++ b/src/target/riscv/riscv-011.c @@ -39,26 +39,26 @@ * * There are a few functions to just instantly shift a register and get its * value: - * dtmcontrol_scan - * idcode_scan - * dbus_scan + * dtmcontrol_scan + * idcode_scan + * dbus_scan * * Because doing one scan and waiting for the result is slow, most functions * batch up a bunch of dbus writes and then execute them all at once. They use * the scans "class" for this: - * scans_new - * scans_delete - * scans_execute - * scans_add_... + * scans_new + * scans_delete + * scans_execute + * scans_add_... * Usually you new(), call a bunch of add functions, then execute() and look * at the results by calling scans_get...() * * Optimized functions will directly use the scans class above, but slightly * lazier code will use the cache functions that in turn use the scans * functions: - * cache_get... - * cache_set... - * cache_write + * cache_get... + * cache_set... + * cache_write * cache_set... update a local structure, which is then synced to the target * with cache_write(). Only Debug RAM words that are actually changed are sent * to the target. Afterwards use cache_get... to read results. @@ -80,10 +80,10 @@ #define CSR_BPCONTROL_BPMATCH (0xf<<7) #define CSR_BPCONTROL_BPACTION (0xff<<11) -#define DEBUG_ROM_START 0x800 -#define DEBUG_ROM_RESUME (DEBUG_ROM_START + 4) -#define DEBUG_ROM_EXCEPTION (DEBUG_ROM_START + 8) -#define DEBUG_RAM_START 0x400 +#define DEBUG_ROM_START 0x800 +#define DEBUG_ROM_RESUME (DEBUG_ROM_START + 4) +#define DEBUG_ROM_EXCEPTION (DEBUG_ROM_START + 8) +#define DEBUG_RAM_START 0x400 #define SETHALTNOT 0x10c @@ -154,7 +154,6 @@ typedef enum slot { /*** Info about the core being debugged. ***/ #define DBUS_ADDRESS_UNKNOWN 0xffff -#define WALL_CLOCK_TIMEOUT 2 // gdb's register list is defined in riscv_gdb_reg_names gdb/riscv-tdep.c in // its source tree. We must interpret the numbers the same here. @@ -730,8 +729,9 @@ static int wait_for_debugint_clear(struct target *target, bool ignore_first) if (!bits.interrupt) { return ERROR_OK; } - if (time(NULL) - start > WALL_CLOCK_TIMEOUT) { - LOG_ERROR("Timed out waiting for debug int to clear."); + if (time(NULL) - start > riscv_command_timeout_sec) { + LOG_ERROR("Timed out waiting for debug int to clear." + "Increase timeout with riscv set_command_timeout_sec."); return ERROR_FAIL; } } @@ -864,7 +864,7 @@ static int cache_write(struct target *target, unsigned int address, bool run) if (last == info->dramsize) { // Nothing needs to be written to RAM. - dbus_write(target, DMCONTROL, DMCONTROL_HALTNOT | (run ? DMCONTROL_INTERRUPT : 0)); + dbus_write(target, DMCONTROL, DMCONTROL_HALTNOT | (run ? DMCONTROL_INTERRUPT : 0)); } else { for (unsigned int i = 0; i < info->dramsize; i++) { @@ -1016,8 +1016,9 @@ static int wait_for_state(struct target *target, enum target_state state) if (target->state == state) { return ERROR_OK; } - if (time(NULL) - start > WALL_CLOCK_TIMEOUT) { - LOG_ERROR("Timed out waiting for state %d.", state); + if (time(NULL) - start > riscv_command_timeout_sec) { + LOG_ERROR("Timed out waiting for state %d. " + "Increase timeout with riscv set_command_timeout_sec.", state); return ERROR_FAIL; } } @@ -1174,8 +1175,9 @@ static int full_step(struct target *target, bool announce) return result; if (target->state != TARGET_DEBUG_RUNNING) break; - if (time(NULL) - start > WALL_CLOCK_TIMEOUT) { - LOG_ERROR("Timed out waiting for step to complete."); + if (time(NULL) - start > riscv_command_timeout_sec) { + LOG_ERROR("Timed out waiting for step to complete." + "Increase timeout with riscv set_command_timeout_sec"); return ERROR_FAIL; } } @@ -1471,6 +1473,7 @@ static int init_target(struct command_context *cmd_ctx, riscv_info_t *generic_info = (riscv_info_t *) target->arch_info; 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; diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 284f6a1ba..7dd4a7f48 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -121,9 +121,6 @@ typedef enum slot { /*** Info about the core being debugged. ***/ -#define WALL_CLOCK_TIMEOUT 2 -#define WALL_CLOCK_RESET_TIMEOUT 30 - struct trigger { uint64_t address; uint32_t length; @@ -562,7 +559,7 @@ static int wait_for_idle(struct target *target, uint32_t *abstractcs) return ERROR_OK; } - if (time(NULL) - start > WALL_CLOCK_TIMEOUT) { + if (time(NULL) - start > riscv_command_timeout_sec) { info->cmderr = get_field(*abstractcs, DMI_ABSTRACTCS_CMDERR); if (info->cmderr != CMDERR_NONE) { const char *errors[8] = { @@ -579,8 +576,10 @@ static int wait_for_idle(struct target *target, uint32_t *abstractcs) errors[info->cmderr], *abstractcs); } - LOG_ERROR("Timed out waiting for busy to go low. (abstractcs=0x%x)", - *abstractcs); + LOG_ERROR("Timed out after %ds waiting for busy to go low. (abstractcs=0x%x)" + "Increase the timeout with riscv set_command_timeout_sec.", + riscv_command_timeout_sec, + *abstractcs); return ERROR_FAIL; } } @@ -909,7 +908,6 @@ static int init_target(struct command_context *cmd_ctx, generic_info->fill_dmi_nop_u64 = &riscv013_fill_dmi_nop_u64; generic_info->dmi_write_u64_bits = &riscv013_dmi_write_u64_bits; generic_info->reset_current_hart = &riscv013_reset_current_hart; - generic_info->version_specific = calloc(1, sizeof(riscv013_info_t)); if (!generic_info->version_specific) return ERROR_FAIL; @@ -1918,9 +1916,11 @@ void riscv013_reset_current_hart(struct target *target) if (get_field(dmstatus, DMI_DMSTATUS_ALLHALTED)) { break; } - if (time(NULL) - start > WALL_CLOCK_RESET_TIMEOUT) { + if (time(NULL) - start > riscv_reset_timeout_sec) { LOG_ERROR("Hart didn't halt coming out of reset in %ds; " - "dmstatus=0x%x", WALL_CLOCK_RESET_TIMEOUT, dmstatus); + "dmstatus=0x%x" + "Increase the timeout with riscv set_reset_timeout_sec.", + riscv_reset_timeout_sec, dmstatus); return; } } diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index e1c7c1c55..db2066a95 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -29,32 +29,32 @@ * Code structure * * At the bottom of the stack are the OpenOCD JTAG functions: - * jtag_add_[id]r_scan - * jtag_execute_query - * jtag_add_runtest + * jtag_add_[id]r_scan + * jtag_execute_query + * jtag_add_runtest * * There are a few functions to just instantly shift a register and get its * value: - * dtmcontrol_scan - * idcode_scan - * dbus_scan + * dtmcontrol_scan + * idcode_scan + * dbus_scan * * Because doing one scan and waiting for the result is slow, most functions * batch up a bunch of dbus writes and then execute them all at once. They use * the scans "class" for this: - * scans_new - * scans_delete - * scans_execute - * scans_add_... + * scans_new + * scans_delete + * scans_execute + * scans_add_... * Usually you new(), call a bunch of add functions, then execute() and look * at the results by calling scans_get...() * * Optimized functions will directly use the scans class above, but slightly * lazier code will use the cache functions that in turn use the scans * functions: - * cache_get... - * cache_set... - * cache_write + * cache_get... + * cache_set... + * cache_write * cache_set... update a local structure, which is then synced to the target * with cache_write(). Only Debug RAM words that are actually changed are sent * to the target. Afterwards use cache_get... to read results. @@ -77,8 +77,8 @@ #define CSR_BPCONTROL_BPACTION (0xff<<11) #define DEBUG_ROM_START 0x800 -#define DEBUG_ROM_RESUME (DEBUG_ROM_START + 4) -#define DEBUG_ROM_EXCEPTION (DEBUG_ROM_START + 8) +#define DEBUG_ROM_RESUME (DEBUG_ROM_START + 4) +#define DEBUG_ROM_EXCEPTION (DEBUG_ROM_START + 8) #define DEBUG_RAM_START 0x400 #define SETHALTNOT 0x10c @@ -150,7 +150,6 @@ typedef enum slot { /*** Info about the core being debugged. ***/ #define DBUS_ADDRESS_UNKNOWN 0xffff -#define WALL_CLOCK_TIMEOUT 2 // gdb's register list is defined in riscv_gdb_reg_names gdb/riscv-tdep.c in // its source tree. We must interpret the numbers the same here. @@ -195,6 +194,12 @@ struct trigger { int unique_id; }; +/* Wall-clock timeout for a command/access. Settable via RISC-V Target commands.*/ +int riscv_command_timeout_sec = DEFAULT_COMMAND_TIMEOUT_SEC; + +/* Wall-clock timeout after reset. Settable via RISC-V Target commands.*/ +int riscv_reset_timeout_sec = DEFAULT_RESET_TIMEOUT_SEC; + static uint32_t dtmcontrol_scan(struct target *target, uint32_t out) { struct scan_field field; @@ -997,7 +1002,7 @@ int riscv_openocd_poll(struct target *target) /* If we're here then at least one hart triggered. That means * we want to go and halt _every_ hart in the system, as that's - * the invariant we hold here. Some harts might have already + * the invariant we hold here. Some harts might have already * halted (as we're either in single-step mode or they also * triggered a breakpoint), so don't attempt to halt those * harts. */ @@ -1167,6 +1172,8 @@ struct target_type riscv_target = .arch_state = riscv_arch_state, .run_algorithm = riscv_run_algorithm, + + .commands = riscv_command_handlers }; /*** RISC-V Interface ***/ @@ -1602,6 +1609,70 @@ int riscv_enumerate_triggers(struct target *target) return ERROR_OK; } +/* Command Handlers */ +COMMAND_HANDLER(riscv_set_command_timeout_sec) { + + if (CMD_ARGC != 1) { + LOG_ERROR("Command takes exactly 1 parameter"); + return ERROR_COMMAND_SYNTAX_ERROR; + } + int timeout = atoi(CMD_ARGV[0]); + if (timeout <= 0){ + LOG_ERROR("%s is not a valid integer argument for command.", CMD_ARGV[0]); + return ERROR_FAIL; + } + + riscv_command_timeout_sec = timeout; + + return ERROR_OK; +} + +COMMAND_HANDLER(riscv_set_reset_timeout_sec) { + + if (CMD_ARGC != 1) { + LOG_ERROR("Command takes exactly 1 parameter"); + return ERROR_COMMAND_SYNTAX_ERROR; + } + int timeout = atoi(CMD_ARGV[0]); + if (timeout <= 0){ + LOG_ERROR("%s is not a valid integer argument for command.", CMD_ARGV[0]); + return ERROR_FAIL; + } + + riscv_reset_timeout_sec = timeout; + return ERROR_OK; +} + + +static const struct command_registration riscv_exec_command_handlers[] = { + { + .name = "set_command_timeout_sec", + .handler = riscv_set_command_timeout_sec, + .mode = COMMAND_ANY, + .usage = "riscv set_command_timeout_sec [sec]", + .help = "Set the wall-clock timeout (in seconds) for individual commands" + }, + { + .name = "set_reset_timeout_sec", + .handler = riscv_set_reset_timeout_sec, + .mode = COMMAND_ANY, + .usage = "riscv set_reset_timeout_sec [sec]", + .help = "Set the wall-clock timeout (in seconds) after reset is deasserted" + }, + COMMAND_REGISTRATION_DONE +}; + +const struct command_registration riscv_command_handlers[] = { + { + .name = "riscv", + .mode = COMMAND_ANY, + .help = "RISC-V Command Group", + .usage = "", + .chain = riscv_exec_command_handlers + }, + COMMAND_REGISTRATION_DONE +}; + const char *gdb_regno_name(enum gdb_regno regno) { static char buf[32]; diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 7bc9a3772..391b1a69e 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -13,6 +13,9 @@ struct riscv_program; #define RISCV_MAX_TRIGGERS 32 #define RISCV_MAX_HWBPS 16 +#define DEFAULT_COMMAND_TIMEOUT_SEC 2 +#define DEFAULT_RESET_TIMEOUT_SEC 30 + extern struct target_type riscv011_target; extern struct target_type riscv013_target; @@ -103,6 +106,12 @@ typedef struct { void (*reset_current_hart)(struct target *target); } riscv_info_t; +/* Wall-clock timeout for a command/access. Settable via RISC-V Target commands.*/ +extern int riscv_command_timeout_sec; + +/* Wall-clock timeout after reset. Settable via RISC-V Target commands.*/ +extern int riscv_reset_timeout_sec; + /* Everything needs the RISC-V specific info structure, so here's a nice macro * that provides that. */ static inline riscv_info_t *riscv_info(const struct target *target) __attribute__((unused)); From 94de39c221c8dda4de6be3330674f674f6db0135 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 15 Aug 2017 17:04:59 -0700 Subject: [PATCH 05/14] riscv: Put commandd_handlers before they are needed. Tabs vs spaces. --- src/target/riscv/riscv.c | 132 +++++++++++++++++++-------------------- 1 file changed, 66 insertions(+), 66 deletions(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index db2066a95..69aea1564 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1137,6 +1137,71 @@ int riscv_openocd_deassert_reset(struct target *target) return ERROR_OK; } + +/* Command Handlers */ +COMMAND_HANDLER(riscv_set_command_timeout_sec) { + + if (CMD_ARGC != 1) { + LOG_ERROR("Command takes exactly 1 parameter"); + return ERROR_COMMAND_SYNTAX_ERROR; + } + int timeout = atoi(CMD_ARGV[0]); + if (timeout <= 0){ + LOG_ERROR("%s is not a valid integer argument for command.", CMD_ARGV[0]); + return ERROR_FAIL; + } + + riscv_command_timeout_sec = timeout; + + return ERROR_OK; +} + +COMMAND_HANDLER(riscv_set_reset_timeout_sec) { + + if (CMD_ARGC != 1) { + LOG_ERROR("Command takes exactly 1 parameter"); + return ERROR_COMMAND_SYNTAX_ERROR; + } + int timeout = atoi(CMD_ARGV[0]); + if (timeout <= 0){ + LOG_ERROR("%s is not a valid integer argument for command.", CMD_ARGV[0]); + return ERROR_FAIL; + } + + riscv_reset_timeout_sec = timeout; + return ERROR_OK; +} + + +static const struct command_registration riscv_exec_command_handlers[] = { + { + .name = "set_command_timeout_sec", + .handler = riscv_set_command_timeout_sec, + .mode = COMMAND_ANY, + .usage = "riscv set_command_timeout_sec [sec]", + .help = "Set the wall-clock timeout (in seconds) for individual commands" + }, + { + .name = "set_reset_timeout_sec", + .handler = riscv_set_reset_timeout_sec, + .mode = COMMAND_ANY, + .usage = "riscv set_reset_timeout_sec [sec]", + .help = "Set the wall-clock timeout (in seconds) after reset is deasserted" + }, + COMMAND_REGISTRATION_DONE +}; + +const struct command_registration riscv_command_handlers[] = { + { + .name = "riscv", + .mode = COMMAND_ANY, + .help = "RISC-V Command Group", + .usage = "", + .chain = riscv_exec_command_handlers + }, + COMMAND_REGISTRATION_DONE +}; + struct target_type riscv_target = { .name = "riscv", @@ -1173,7 +1238,7 @@ struct target_type riscv_target = .run_algorithm = riscv_run_algorithm, - .commands = riscv_command_handlers + .commands = riscv_command_handlers }; /*** RISC-V Interface ***/ @@ -1583,7 +1648,6 @@ int riscv_enumerate_triggers(struct target *target) tselect_rb &= ~(1ULL << (riscv_xlen(target)-1)); if (tselect_rb != 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))); @@ -1609,70 +1673,6 @@ int riscv_enumerate_triggers(struct target *target) return ERROR_OK; } -/* Command Handlers */ -COMMAND_HANDLER(riscv_set_command_timeout_sec) { - - if (CMD_ARGC != 1) { - LOG_ERROR("Command takes exactly 1 parameter"); - return ERROR_COMMAND_SYNTAX_ERROR; - } - int timeout = atoi(CMD_ARGV[0]); - if (timeout <= 0){ - LOG_ERROR("%s is not a valid integer argument for command.", CMD_ARGV[0]); - return ERROR_FAIL; - } - - riscv_command_timeout_sec = timeout; - - return ERROR_OK; -} - -COMMAND_HANDLER(riscv_set_reset_timeout_sec) { - - if (CMD_ARGC != 1) { - LOG_ERROR("Command takes exactly 1 parameter"); - return ERROR_COMMAND_SYNTAX_ERROR; - } - int timeout = atoi(CMD_ARGV[0]); - if (timeout <= 0){ - LOG_ERROR("%s is not a valid integer argument for command.", CMD_ARGV[0]); - return ERROR_FAIL; - } - - riscv_reset_timeout_sec = timeout; - return ERROR_OK; -} - - -static const struct command_registration riscv_exec_command_handlers[] = { - { - .name = "set_command_timeout_sec", - .handler = riscv_set_command_timeout_sec, - .mode = COMMAND_ANY, - .usage = "riscv set_command_timeout_sec [sec]", - .help = "Set the wall-clock timeout (in seconds) for individual commands" - }, - { - .name = "set_reset_timeout_sec", - .handler = riscv_set_reset_timeout_sec, - .mode = COMMAND_ANY, - .usage = "riscv set_reset_timeout_sec [sec]", - .help = "Set the wall-clock timeout (in seconds) after reset is deasserted" - }, - COMMAND_REGISTRATION_DONE -}; - -const struct command_registration riscv_command_handlers[] = { - { - .name = "riscv", - .mode = COMMAND_ANY, - .help = "RISC-V Command Group", - .usage = "", - .chain = riscv_exec_command_handlers - }, - COMMAND_REGISTRATION_DONE -}; - const char *gdb_regno_name(enum gdb_regno regno) { static char buf[32]; From b42bc76e2ebe1525895440ffccffa59b5921cda2 Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 25 Aug 2017 14:53:45 +0300 Subject: [PATCH 06/14] server.c: fix clang warning /Users/ilg/Work/openocd/openocd.git/src/server/server.c:305:22: error: incompatible pointer types passing 'struct sockaddr_in *' to parameter of type 'struct sockaddr *' [-Werror,-Wincompatible-pointer-types] getsockname(c->fd, &addr_in, &addr_in_size); ^~~~~~~~ /usr/include/sys/socket.h:687:50: note: passing argument to parameter here int getsockname(int, struct sockaddr * __restrict, socklen_t * __restrict) --- src/server/server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/server.c b/src/server/server.c index 99bdc35ec..f3ae34ebe 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -302,7 +302,7 @@ int add_service(char *name, struct sockaddr_in addr_in; socklen_t addr_in_size = sizeof(addr_in); - getsockname(c->fd, &addr_in, &addr_in_size); + getsockname(c->fd, (struct sockaddr *)&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) { From a9bcc48064010931ca316222e53aaddb82824d26 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 24 Aug 2017 14:24:40 -0700 Subject: [PATCH 07/14] Remove unnecessary newlines. --- src/target/riscv/riscv-013.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index f5fe37824..5cdf8b192 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -471,7 +471,7 @@ static uint64_t dmi_read(struct target *target, uint16_t address) } else if (status == DMI_STATUS_SUCCESS) { break; } else { - LOG_ERROR("failed read (NOP) at 0x%x, status=%d\n", address, status); + LOG_ERROR("failed read (NOP) at 0x%x, status=%d", address, status); break; } } @@ -500,13 +500,13 @@ static void dmi_write(struct target *target, uint16_t address, uint64_t value) } else if (status == DMI_STATUS_SUCCESS) { break; } else { - LOG_ERROR("failed write to 0x%x, status=%d\n", address, status); + LOG_ERROR("failed write to 0x%x, status=%d", address, status); break; } } if (status != DMI_STATUS_SUCCESS) { - LOG_ERROR("Failed write to 0x%x;, status=%d\n", + LOG_ERROR("Failed write to 0x%x;, status=%d", address, status); abort(); } @@ -521,12 +521,12 @@ static void dmi_write(struct target *target, uint16_t address, uint64_t value) } else if (status == DMI_STATUS_SUCCESS) { break; } else { - LOG_ERROR("failed write (NOP) at 0x%x, status=%d\n", address, status); + LOG_ERROR("failed write (NOP) at 0x%x, status=%d", address, status); break; } } if (status != DMI_STATUS_SUCCESS) { - LOG_ERROR("failed to write (NOP) 0x%" PRIx64 " to 0x%x; status=%d\n", value, address, status); + LOG_ERROR("failed to write (NOP) 0x%" PRIx64 " to 0x%x; status=%d", value, address, status); abort(); } } @@ -1146,7 +1146,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", i, (long)r->debug_buffer_addr[i]); abort(); } From 92ef3281613e6a33755ed6286802a677010d17ce Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Fri, 25 Aug 2017 12:00:06 -0700 Subject: [PATCH 08/14] Don't reset DMI when an abstract command is busy. --- src/target/riscv/riscv-013.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 5cdf8b192..093ae5625 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -364,17 +364,6 @@ static void increase_dmi_busy_delay(struct target *target) dtmcontrol_scan(target, DTM_DTMCS_DMIRESET); } -static void increase_ac_busy_delay(struct target *target) -{ - riscv013_info_t *info = get_info(target); - info->ac_busy_delay += info->ac_busy_delay / 10 + 1; - LOG_INFO("dtmcontrol_idle=%d, dmi_busy_delay=%d, ac_busy_delay=%d", - info->dtmcontrol_idle, info->dmi_busy_delay, - info->ac_busy_delay); - - dtmcontrol_scan(target, DTM_DTMCS_DMIRESET); -} - /** * exec: If this is set, assume the scan results in an execution, so more * run-test/idle cycles may be required. @@ -531,6 +520,23 @@ static void dmi_write(struct target *target, uint16_t address, uint64_t value) } } +static void increase_ac_busy_delay(struct target *target) +{ + riscv013_info_t *info = get_info(target); + info->ac_busy_delay += info->ac_busy_delay / 10 + 1; + LOG_INFO("dtmcontrol_idle=%d, dmi_busy_delay=%d, ac_busy_delay=%d", + info->dtmcontrol_idle, info->dmi_busy_delay, + info->ac_busy_delay); + + // Wait for busy to go away. + uint32_t abstractcs = dmi_read(target, DMI_ABSTRACTCS); + while (get_field(abstractcs, DMI_ABSTRACTCS_BUSY)) { + abstractcs = dmi_read(target, DMI_ABSTRACTCS); + } + // Clear the error status. + dmi_write(target, DMI_ABSTRACTCS, abstractcs & DMI_ABSTRACTCS_CMDERR); +} + uint32_t abstract_register_size(unsigned width) { switch (width) { @@ -1412,8 +1418,8 @@ static int read_memory(struct target *target, target_addr_t address, case CMDERR_BUSY: LOG_DEBUG("memory read resulted in busy response; " "this_is_last_read=%d", this_is_last_read); - riscv013_clear_abstract_error(target); increase_ac_busy_delay(target); + riscv013_clear_abstract_error(target); retry_batch_transaction = true; riscv_batch_free(batch); break; From 8bcec87cc1f0bb4b45d05977d2a1c69822806e0f Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Sat, 26 Aug 2017 16:53:00 -0700 Subject: [PATCH 09/14] Remove unnecessary \n --- src/target/riscv/riscv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 69aea1564..62628a179 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -629,7 +629,7 @@ static int riscv_examine(struct target *target) { LOG_DEBUG("riscv_examine()"); if (target_was_examined(target)) { - LOG_DEBUG("Target was already examined.\n"); + LOG_DEBUG("Target was already examined."); return ERROR_OK; } From 5bdee8bc669e5fdeb1c978726025a5c0ce2f188c Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Sat, 26 Aug 2017 17:49:13 -0700 Subject: [PATCH 10/14] Fix off-by-3 error on 64-bit targets. This caused everything to fall apart when debugging slow 64-bit targets. --- src/target/riscv/riscv-013.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 093ae5625..6ea56ab83 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1438,7 +1438,7 @@ static int read_memory(struct target *target, target_addr_t address, switch (riscv_xlen(target)) { case 64: - riscv013_write_debug_buffer(target, d_addr + 4, (cur_addr - size) >> 32); + riscv013_write_debug_buffer(target, d_addr + 1, (cur_addr - size) >> 32); case 32: riscv013_write_debug_buffer(target, d_addr, (cur_addr - size)); break; From eef9442aa7f6e27efa64f4cde3f93c37a9c3178a Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Sat, 26 Aug 2017 17:50:05 -0700 Subject: [PATCH 11/14] Remove redundant code. --- src/target/riscv/riscv-013.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 6ea56ab83..e1f807d38 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -527,14 +527,6 @@ static void increase_ac_busy_delay(struct target *target) LOG_INFO("dtmcontrol_idle=%d, dmi_busy_delay=%d, ac_busy_delay=%d", info->dtmcontrol_idle, info->dmi_busy_delay, info->ac_busy_delay); - - // Wait for busy to go away. - uint32_t abstractcs = dmi_read(target, DMI_ABSTRACTCS); - while (get_field(abstractcs, DMI_ABSTRACTCS_BUSY)) { - abstractcs = dmi_read(target, DMI_ABSTRACTCS); - } - // Clear the error status. - dmi_write(target, DMI_ABSTRACTCS, abstractcs & DMI_ABSTRACTCS_CMDERR); } uint32_t abstract_register_size(unsigned width) @@ -2087,6 +2079,11 @@ int riscv013_debug_buffer_register(struct target *target, riscv_addr_t addr) void riscv013_clear_abstract_error(struct target *target) { - uint32_t acs = dmi_read(target, DMI_ABSTRACTCS); - dmi_write(target, DMI_ABSTRACTCS, acs); + // Wait for busy to go away. + uint32_t abstractcs = dmi_read(target, DMI_ABSTRACTCS); + while (get_field(abstractcs, DMI_ABSTRACTCS_BUSY)) { + abstractcs = dmi_read(target, DMI_ABSTRACTCS); + } + // Clear the error status. + dmi_write(target, DMI_ABSTRACTCS, abstractcs & DMI_ABSTRACTCS_CMDERR); } From 5f53655e658ee7ffce0ae24e692762321b233c04 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Sat, 26 Aug 2017 18:25:10 -0700 Subject: [PATCH 12/14] Fix off-by-one error. --- src/target/riscv/riscv-013.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index e1f807d38..e7efb9c79 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1430,9 +1430,9 @@ static int read_memory(struct target *target, target_addr_t address, switch (riscv_xlen(target)) { case 64: - riscv013_write_debug_buffer(target, d_addr + 1, (cur_addr - size) >> 32); + riscv013_write_debug_buffer(target, d_addr + 1, cur_addr >> 32); case 32: - riscv013_write_debug_buffer(target, d_addr, (cur_addr - size)); + riscv013_write_debug_buffer(target, d_addr, cur_addr); break; default: LOG_ERROR("unknown XLEN %d", riscv_xlen(target)); From 2efc415db47789e3d10c8be3d57855d190e36403 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Sun, 27 Aug 2017 13:25:54 -0700 Subject: [PATCH 13/14] Finally nailed memory read on slow targets The downloaded program now post-increments, and there's no longer an attempt to read the current address from the target. This made it easier to fix the problem where at the start of the loop the current address was already read (in regular entry) or has not yet been read (when the first round through the loop encountered busy more than once, or busy was encountered at least once later on). --- src/target/riscv/riscv-013.c | 57 +++++++++++++----------------------- 1 file changed, 21 insertions(+), 36 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index e7efb9c79..bd48fe9b0 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1273,7 +1273,6 @@ static int read_memory(struct target *target, target_addr_t address, riscv_addr_t r_addr = riscv_program_alloc_x(&program); riscv_program_fence(&program); riscv_program_lx(&program, GDB_REGNO_S0, r_addr); - riscv_program_addi(&program, GDB_REGNO_S0, GDB_REGNO_S0, size); switch (size) { case 1: riscv_program_lbr(&program, GDB_REGNO_S1, GDB_REGNO_S0, 0); @@ -1288,6 +1287,7 @@ static int read_memory(struct target *target, target_addr_t address, LOG_ERROR("Unsupported size: %d", size); return ERROR_FAIL; } + riscv_program_addi(&program, GDB_REGNO_S0, GDB_REGNO_S0, size); riscv_program_sw(&program, GDB_REGNO_S1, r_data); riscv_program_sx(&program, GDB_REGNO_S0, r_addr); @@ -1295,9 +1295,9 @@ static int read_memory(struct target *target, target_addr_t address, * program execution mechanism. */ switch (riscv_xlen(target)) { case 64: - riscv_program_write_ram(&program, r_addr + 4, (((riscv_addr_t) address) - size) >> 32); + riscv_program_write_ram(&program, r_addr + 4, ((riscv_addr_t) address) >> 32); case 32: - riscv_program_write_ram(&program, r_addr, ((riscv_addr_t) address) - size); + riscv_program_write_ram(&program, r_addr, (riscv_addr_t) address); break; default: LOG_ERROR("unknown XLEN %d", riscv_xlen(target)); @@ -1314,27 +1314,6 @@ static int read_memory(struct target *target, target_addr_t address, return ERROR_FAIL; } - uint32_t value = riscv_program_read_ram(&program, r_data); - LOG_DEBUG("M[0x%" TARGET_PRIxADDR "] reads 0x%08x", address, value); - switch (size) { - case 1: - buffer[0] = value; - break; - case 2: - buffer[0] = value; - buffer[1] = value >> 8; - break; - case 4: - buffer[0] = value; - buffer[1] = value >> 8; - buffer[2] = value >> 16; - buffer[3] = value >> 24; - break; - default: - LOG_ERROR("unsupported access size: %d", size); - return ERROR_FAIL; - } - /* The rest of this program is designed to be fast so it reads various * DMI registers directly. */ int d_data = (r_data - riscv_debug_buffer_addr(target)) / 4; @@ -1346,22 +1325,16 @@ static int read_memory(struct target *target, target_addr_t address, * case we need to back off a bit and try again. There's two * termination conditions to this loop: a non-BUSY error message, or * the data was all copied. */ - riscv_addr_t cur_addr = 0xbadbeef; + riscv_addr_t cur_addr = address; riscv_addr_t fin_addr = address + (count * size); - riscv_addr_t prev_addr = ((riscv_addr_t) address) - size; - bool ignore_prev_addr = true; bool this_is_last_read = false; LOG_DEBUG("reading until final address 0x%" PRIx64, fin_addr); - while (count > 1 && !this_is_last_read) { - cur_addr = riscv_read_debug_buffer_x(target, d_addr); + while (!this_is_last_read) { riscv_addr_t start = (cur_addr - address) / size; LOG_DEBUG("reading burst at address 0x%" TARGET_PRIxADDR - "; prev_addr=0x%" TARGET_PRIxADDR "; start=0x%" - TARGET_PRIxADDR, cur_addr, prev_addr, start); - assert(ignore_prev_addr || prev_addr < cur_addr); - prev_addr = cur_addr; - ignore_prev_addr = false; - assert (cur_addr >= address); + "; start=0x%" + TARGET_PRIxADDR, cur_addr, start); + assert(cur_addr >= address && cur_addr < fin_addr); struct riscv_batch *batch = riscv_batch_alloc( target, 32, @@ -1426,7 +1399,6 @@ static int read_memory(struct target *target, target_addr_t address, } if (retry_batch_transaction) { this_is_last_read = false; - ignore_prev_addr = true; switch (riscv_xlen(target)) { case 64: @@ -1439,6 +1411,16 @@ static int read_memory(struct target *target, target_addr_t address, return ERROR_FAIL; } + if (riscv013_execute_debug_buffer(target) != ERROR_OK) { + uint32_t acs = dmi_read(target, DMI_ABSTRACTCS); + LOG_ERROR("failed to execute program, abstractcs=0x%08x", acs); + riscv013_clear_abstract_error(target); + riscv_set_register(target, GDB_REGNO_S0, s0); + riscv_set_register(target, GDB_REGNO_S1, s1); + LOG_ERROR(" exiting with ERROR_FAIL"); + return ERROR_FAIL; + } + continue; } @@ -1446,6 +1428,7 @@ static int read_memory(struct target *target, target_addr_t address, riscv_addr_t offset = size*i; riscv_addr_t t_addr = address + offset; uint8_t *t_buffer = buffer + offset; + uint32_t value; if (this_is_last_read && i == start + reads - 1) { riscv013_set_autoexec(target, d_data, 0); @@ -1485,6 +1468,8 @@ static int read_memory(struct target *target, target_addr_t address, LOG_DEBUG("M[0x%08lx] reads 0x%08x", (long)t_addr, value); } riscv_batch_free(batch); + + cur_addr += reads * size; } riscv013_set_autoexec(target, d_data, 0); From 6721988ce373d0160dd4994e33d6c50584eac66b Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 29 Aug 2017 17:25:04 -0700 Subject: [PATCH 14/14] Ensure read_memory() only reads each address once. Previously it might read an address multiple times if an abstract command took longer to execute than expected. The new implementations reads from the target how far it has gotten along reading memory, and resumes from there if cmderr=busy. This ended up being a bigger change than I envisioned, but in the end it deleted more lines than it added, so I'm happy. :-) --- src/target/riscv/riscv-013.c | 168 +++++++++++++++-------------------- 1 file changed, 72 insertions(+), 96 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index bd48fe9b0..dbf2ce3db 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1248,6 +1248,31 @@ static int deassert_reset(struct target *target) return ERROR_OK; } +static void write_to_buf(uint8_t *buffer, uint64_t value, unsigned size) +{ + switch (size) { + case 8: + buffer[7] = value >> 56; + buffer[6] = value >> 48; + buffer[5] = value >> 40; + buffer[4] = value >> 32; + case 4: + buffer[3] = value >> 24; + buffer[2] = value >> 16; + case 2: + buffer[1] = value >> 8; + case 1: + buffer[0] = value; + break; + default: + assert(false); + } +} + +/** + * Read the requested memory, taking care to execute every read exactly once, + * even if cmderr=busy is encountered. + */ static int read_memory(struct target *target, target_addr_t address, uint32_t size, uint32_t count, uint8_t *buffer) { @@ -1314,6 +1339,9 @@ static int read_memory(struct target *target, target_addr_t address, return ERROR_FAIL; } + // Program has been executed once. d_addr contains address+size, and d_data + // contains *address. + /* The rest of this program is designed to be fast so it reads various * DMI registers directly. */ int d_data = (r_data - riscv_debug_buffer_addr(target)) / 4; @@ -1325,15 +1353,17 @@ static int read_memory(struct target *target, target_addr_t address, * case we need to back off a bit and try again. There's two * termination conditions to this loop: a non-BUSY error message, or * the data was all copied. */ - riscv_addr_t cur_addr = address; + riscv_addr_t cur_addr = riscv_read_debug_buffer_x(target, d_addr); riscv_addr_t fin_addr = address + (count * size); - bool this_is_last_read = false; LOG_DEBUG("reading until final address 0x%" PRIx64, fin_addr); - while (!this_is_last_read) { - riscv_addr_t start = (cur_addr - address) / size; - LOG_DEBUG("reading burst at address 0x%" TARGET_PRIxADDR - "; start=0x%" - TARGET_PRIxADDR, cur_addr, start); + while (cur_addr < fin_addr) { + // Invariant: + // d_data contains *addr + // d_addr contains addr + size + + unsigned start = (cur_addr - address) / size; + LOG_DEBUG("creating burst to read address 0x%" TARGET_PRIxADDR + " up to 0x%" TARGET_PRIxADDR "; start=0x%d", cur_addr, fin_addr, start); assert(cur_addr >= address && cur_addr < fin_addr); struct riscv_batch *batch = riscv_batch_alloc( target, @@ -1341,23 +1371,12 @@ static int read_memory(struct target *target, target_addr_t address, info->dmi_busy_delay + info->ac_busy_delay); size_t reads = 0; - size_t rereads = reads; - for (riscv_addr_t i = start; i < count; ++i) { - if (i == count - 1) { - // don't do actual read in this batch, - // we will do it later after we disable autoexec - // - // this is done to avoid reading more memory than requested - // which in some special cases(like reading stack located - // at the very top of RAM) may cause an exception - this_is_last_read = true; - } else { - size_t const index = - riscv_batch_add_dmi_read( + for (riscv_addr_t addr = cur_addr; addr < fin_addr; addr += size) { + size_t const index = + riscv_batch_add_dmi_read( batch, riscv013_debug_buffer_register(target, r_data)); - assert(index == reads); - } + assert(index == reads); reads++; if (riscv_batch_full(batch)) @@ -1366,27 +1385,21 @@ static int read_memory(struct target *target, target_addr_t address, riscv_batch_run(batch); - // Note that if the scan resulted in a Busy DMI response, it - // is this read to abstractcs that will cause the dmi_busy_delay - // to be incremented if necessary. The loop condition above - // catches the case where no writes went through at all. - - bool retry_batch_transaction = false; + // Wait for the target to finish performing the last abstract command, + // and update our copy of cmderr. uint32_t abstractcs = dmi_read(target, DMI_ABSTRACTCS); while (get_field(abstractcs, DMI_ABSTRACTCS_BUSY)) abstractcs = dmi_read(target, DMI_ABSTRACTCS); info->cmderr = get_field(abstractcs, DMI_ABSTRACTCS_CMDERR); + switch (info->cmderr) { case CMDERR_NONE: LOG_DEBUG("successful (partial?) memory read"); break; case CMDERR_BUSY: - LOG_DEBUG("memory read resulted in busy response; " - "this_is_last_read=%d", this_is_last_read); + LOG_DEBUG("memory read resulted in busy response"); increase_ac_busy_delay(target); riscv013_clear_abstract_error(target); - retry_batch_transaction = true; - riscv_batch_free(batch); break; default: LOG_ERROR("error when reading memory, abstractcs=0x%08lx", (long)abstractcs); @@ -1397,82 +1410,45 @@ static int read_memory(struct target *target, target_addr_t address, riscv_batch_free(batch); return ERROR_FAIL; } - if (retry_batch_transaction) { - this_is_last_read = false; - switch (riscv_xlen(target)) { - case 64: - riscv013_write_debug_buffer(target, d_addr + 1, cur_addr >> 32); - case 32: - riscv013_write_debug_buffer(target, d_addr, cur_addr); - break; - default: - LOG_ERROR("unknown XLEN %d", riscv_xlen(target)); - return ERROR_FAIL; - } + // Figure out how far we managed to read. + riscv_addr_t next_addr = riscv_read_debug_buffer_x(target, d_addr); + LOG_DEBUG("Batch read [0x%" TARGET_PRIxADDR ", 0x%" TARGET_PRIxADDR + "); reads=%d", cur_addr, next_addr, (unsigned) reads); + assert(next_addr >= address && next_addr <= fin_addr); + assert(info->cmderr != CMDERR_NONE || + next_addr == cur_addr + reads * size); - if (riscv013_execute_debug_buffer(target) != ERROR_OK) { - uint32_t acs = dmi_read(target, DMI_ABSTRACTCS); - LOG_ERROR("failed to execute program, abstractcs=0x%08x", acs); - riscv013_clear_abstract_error(target); - riscv_set_register(target, GDB_REGNO_S0, s0); - riscv_set_register(target, GDB_REGNO_S1, s1); - LOG_ERROR(" exiting with ERROR_FAIL"); - return ERROR_FAIL; - } + // Now read whatever we got out of the batch. + unsigned rereads = 0; + for (riscv_addr_t addr = cur_addr - size; addr < next_addr - size; + addr += size) { + riscv_addr_t offset = addr - address; - continue; - } + uint64_t dmi_out = riscv_batch_get_dmi_read(batch, rereads); + uint32_t value = get_field(dmi_out, DTM_DMI_DATA); + write_to_buf(buffer + offset, value, size); - for (size_t i = start; i < start + reads; ++i) { - riscv_addr_t offset = size*i; - riscv_addr_t t_addr = address + offset; - uint8_t *t_buffer = buffer + offset; - uint32_t value; - - if (this_is_last_read && i == start + reads - 1) { - riscv013_set_autoexec(target, d_data, 0); - - // Access debug buffer without executing a program. This - // address logic was taken from program.c. - int const off = (r_data - - riscv_debug_buffer_addr(program.target)) / - sizeof(program.debug_buffer[0]); - value = riscv_read_debug_buffer(target, off); - } else { - uint64_t dmi_out = riscv_batch_get_dmi_read(batch, rereads); - value = get_field(dmi_out, DTM_DMI_DATA); - } - rereads++; - switch (size) { - case 1: - t_buffer[0] = value; - break; - case 2: - t_buffer[0] = value; - t_buffer[1] = value >> 8; - break; - case 4: - t_buffer[0] = value; - t_buffer[1] = value >> 8; - t_buffer[2] = value >> 16; - t_buffer[3] = value >> 24; - break; - default: - LOG_ERROR("unsupported access size: %d", size); - return ERROR_FAIL; - } - - LOG_DEBUG("M[0x%08lx] reads 0x%08x", (long)t_addr, value); + LOG_DEBUG("M[0x%" TARGET_PRIxADDR "] reads 0x%08x", addr, value); } riscv_batch_free(batch); - cur_addr += reads * size; + cur_addr = next_addr; } riscv013_set_autoexec(target, d_data, 0); + + // Read the last word. + + // Access debug buffer without executing a program. This + // address logic was taken from program.c. + uint32_t value = riscv013_read_debug_buffer(target, d_data); + riscv_addr_t addr = cur_addr - size; + write_to_buf(buffer + addr - address, value, size); + LOG_DEBUG("M[0x%" TARGET_PRIxADDR "] reads 0x%08x", addr, value); + riscv_set_register(target, GDB_REGNO_S0, s0); riscv_set_register(target, GDB_REGNO_S1, s1); return ERROR_OK;