From 879c274cb96d985d3f21f2ec5fe59004578928a7 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 15 Aug 2017 15:55:09 -0700 Subject: [PATCH 1/2] 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 2/2] 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];