From 53b5b0ada89e06fbe9d6ac22e716bc86ad8b659d Mon Sep 17 00:00:00 2001 From: Evgeniy Naydanov Date: Tue, 22 Oct 2024 12:43:04 +0300 Subject: [PATCH] target/riscv: new `ebreak` controls * Deprecate `riscv set_ebreak*` commands. * Introduce RISC-V-sepecific `configure` parameter `-ebreak` instead. * Separate controls for VS and VU modes. Change-Id: I0ff88318dcb52af6923eb9f20f9d0c056ad09cf0 Signed-off-by: Evgeniy Naydanov --- doc/openocd.texi | 34 ++--- src/target/riscv/riscv-011.c | 22 ++-- src/target/riscv/riscv-013.c | 19 +-- src/target/riscv/riscv.c | 246 +++++++++++++++++++++++++++++------ src/target/riscv/riscv.h | 24 +++- src/target/riscv/startup.tcl | 4 + 6 files changed, 269 insertions(+), 80 deletions(-) diff --git a/doc/openocd.texi b/doc/openocd.texi index 127887365..418c1989c 100644 --- a/doc/openocd.texi +++ b/doc/openocd.texi @@ -11225,6 +11225,25 @@ follows: @end example +@subsection RISC-V @code{$target_name configure} options +@itemize +@item @code{-ebreak} [@option{m}|@option{s}|@option{u}|@option{vs}|@option{vu}] +@option{exception}|@option{halt} -- sets the desired behavior of @code{ebreak} +instruction on the target. Defaults to @option{halt} in all execution modes. + +@itemize +@item The last argument specifies which action should be taken when a hart +executes a @code{ebreak}. + +@item The first argument specifies in which execution mode the @code{ebreak} +behavior should change. If this option is omitted the configuration affects +all execution modes. + +@item @code{cget} returns a TCL @code{dict} of execution mode - @code{ebreak} +action pairs. +@end itemize +@end itemize + @subsection RISC-V Debug Configuration Commands @deffn {Command} {riscv dump_sample_buf} [base64] @@ -11434,21 +11453,6 @@ Keep in mind, disabling the option does not guarantee that single stepping will To make that happen, dcsr.stepie would have to be written to 1 as well. @end deffn -@deffn {Command} {riscv set_ebreakm} [on|off] -Control dcsr.ebreakm. When on (default), M-mode ebreak instructions trap to -OpenOCD. When off, they generate a breakpoint exception handled internally. -@end deffn - -@deffn {Command} {riscv set_ebreaks} [on|off] -Control dcsr.ebreaks. When on (default), S-mode ebreak instructions trap to -OpenOCD. When off, they generate a breakpoint exception handled internally. -@end deffn - -@deffn {Command} {riscv set_ebreaku} [on|off] -Control dcsr.ebreaku. When on (default), U-mode ebreak instructions trap to -OpenOCD. When off, they generate a breakpoint exception handled internally. -@end deffn - The commands below can be used to prevent OpenOCD from using certain RISC-V trigger features. For example in cases when there are known issues in the target hardware. diff --git a/src/target/riscv/riscv-011.c b/src/target/riscv/riscv-011.c index 0e635b3a6..f1f18b7ec 100644 --- a/src/target/riscv/riscv-011.c +++ b/src/target/riscv/riscv-011.c @@ -1074,9 +1074,18 @@ static int maybe_write_tselect(struct target *target) return ERROR_OK; } +static uint64_t set_ebreakx_fields(uint64_t dcsr, const struct target *target) +{ + const struct riscv_private_config * const pc = riscv_private_config(target); + dcsr = set_field(dcsr, DCSR_EBREAKM, pc->dcsr_ebreak_ctls[RISCV_DCSR_EBREAK_M_CTL]); + dcsr = set_field(dcsr, DCSR_EBREAKS, pc->dcsr_ebreak_ctls[RISCV_DCSR_EBREAK_S_CTL]); + dcsr = set_field(dcsr, DCSR_EBREAKU, pc->dcsr_ebreak_ctls[RISCV_DCSR_EBREAK_U_CTL]); + dcsr = set_field(dcsr, DCSR_EBREAKH, 1); + return dcsr; +} + static int execute_resume(struct target *target, bool step) { - RISCV_INFO(r); riscv011_info_t *info = get_info(target); LOG_DEBUG("step=%d", step); @@ -1108,10 +1117,7 @@ static int execute_resume(struct target *target, bool step) } } - info->dcsr = set_field(info->dcsr, DCSR_EBREAKM, r->riscv_ebreakm); - info->dcsr = set_field(info->dcsr, DCSR_EBREAKS, r->riscv_ebreaks); - info->dcsr = set_field(info->dcsr, DCSR_EBREAKU, r->riscv_ebreaku); - info->dcsr = set_field(info->dcsr, DCSR_EBREAKH, 1); + info->dcsr = set_ebreakx_fields(info->dcsr, target); info->dcsr &= ~DCSR_HALT; if (step) @@ -1928,7 +1934,6 @@ static int riscv011_resume(struct target *target, int current, static int assert_reset(struct target *target) { - RISCV_INFO(r); riscv011_info_t *info = get_info(target); /* TODO: Maybe what I implemented here is more like soft_reset_halt()? */ @@ -1942,10 +1947,7 @@ static int assert_reset(struct target *target) /* Not sure what we should do when there are multiple cores. * Here just reset the single hart we're talking to. */ - info->dcsr = set_field(info->dcsr, DCSR_EBREAKM, r->riscv_ebreakm); - info->dcsr = set_field(info->dcsr, DCSR_EBREAKS, r->riscv_ebreaks); - info->dcsr = set_field(info->dcsr, DCSR_EBREAKU, r->riscv_ebreaku); - info->dcsr = set_field(info->dcsr, DCSR_EBREAKH, 1); + info->dcsr = set_ebreakx_fields(info->dcsr, target); info->dcsr |= DCSR_HALT; if (target->reset_halt) info->dcsr |= DCSR_NDRESET; diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 4ab3357dc..dd36d2527 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1604,7 +1604,6 @@ static int set_dcsr_ebreak(struct target *target, bool step) if (dm013_select_target(target) != ERROR_OK) return ERROR_FAIL; - RISCV_INFO(r); RISCV013_INFO(info); riscv_reg_t original_dcsr, dcsr; /* We want to twiddle some bits in the debug CSR so debugging works. */ @@ -1612,11 +1611,12 @@ static int set_dcsr_ebreak(struct target *target, bool step) return ERROR_FAIL; original_dcsr = dcsr; dcsr = set_field(dcsr, CSR_DCSR_STEP, step); - dcsr = set_field(dcsr, CSR_DCSR_EBREAKM, r->riscv_ebreakm); - dcsr = set_field(dcsr, CSR_DCSR_EBREAKS, r->riscv_ebreaks && riscv_supports_extension(target, 'S')); - dcsr = set_field(dcsr, CSR_DCSR_EBREAKU, r->riscv_ebreaku && riscv_supports_extension(target, 'U')); - dcsr = set_field(dcsr, CSR_DCSR_EBREAKVS, r->riscv_ebreaku && riscv_supports_extension(target, 'H')); - dcsr = set_field(dcsr, CSR_DCSR_EBREAKVU, r->riscv_ebreaku && riscv_supports_extension(target, 'H')); + const struct riscv_private_config * const pc = riscv_private_config(target); + dcsr = set_field(dcsr, CSR_DCSR_EBREAKM, pc->dcsr_ebreak_ctls[RISCV_DCSR_EBREAK_M_CTL]); + dcsr = set_field(dcsr, CSR_DCSR_EBREAKS, pc->dcsr_ebreak_ctls[RISCV_DCSR_EBREAK_S_CTL]); + dcsr = set_field(dcsr, CSR_DCSR_EBREAKU, pc->dcsr_ebreak_ctls[RISCV_DCSR_EBREAK_U_CTL]); + dcsr = set_field(dcsr, CSR_DCSR_EBREAKVS, pc->dcsr_ebreak_ctls[RISCV_DCSR_EBREAK_VS_CTL]); + dcsr = set_field(dcsr, CSR_DCSR_EBREAKVU, pc->dcsr_ebreak_ctls[RISCV_DCSR_EBREAK_VU_CTL]); if (dcsr != original_dcsr && riscv_reg_set(target, GDB_REGNO_DCSR, dcsr) != ERROR_OK) return ERROR_FAIL; @@ -2842,8 +2842,11 @@ static int assert_reset(struct target *target) static bool dcsr_ebreak_config_equals_reset_value(const struct target *target) { - RISCV_INFO(r); - return !(r->riscv_ebreakm || r->riscv_ebreaks || r->riscv_ebreaku); + const struct riscv_private_config * const pc = riscv_private_config(target); + for (int i = 0; i < N_RISCV_DCSR_EBREAK_CTL; ++i) + if (pc->dcsr_ebreak_ctls[i]) + return false; + return true; } static int deassert_reset(struct target *target) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 6d32adc83..4406be921 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -480,10 +480,157 @@ static int riscv_create_target(struct target *target, Jim_Interp *interp) return ERROR_OK; } +static struct jim_nvp nvp_ebreak_config_opts[] = { + { .name = "m", .value = RISCV_DCSR_EBREAK_M_CTL }, + { .name = "s", .value = RISCV_DCSR_EBREAK_S_CTL }, + { .name = "u", .value = RISCV_DCSR_EBREAK_U_CTL }, + { .name = "vs", .value = RISCV_DCSR_EBREAK_VS_CTL }, + { .name = "vu", .value = RISCV_DCSR_EBREAK_VU_CTL }, + { .name = NULL, .value = N_RISCV_DCSR_EBREAK_CTL } +}; + +#define RISCV_EBREAK_MODE_INVALID -1 + +static struct jim_nvp nvp_ebreak_mode_opts[] = { + { .name = "exception", .value = false }, + { .name = "halt", .value = true }, + { .name = NULL, .value = RISCV_EBREAK_MODE_INVALID } +}; + +static int jim_configure_ebreak(struct target *target, struct jim_getopt_info *goi) +{ + struct riscv_private_config * const pc = target->private_config; + struct jim_nvp *common_mode_nvp; + if (jim_nvp_name2value_obj(goi->interp, nvp_ebreak_mode_opts, goi->argv[0], + &common_mode_nvp) == JIM_OK) { + /* Here a common "ebreak" action is processed, e.g: + * "riscv.cpu configure -ebreak halt" + */ + for (int ebreak_ctl_i = 0; ebreak_ctl_i < N_RISCV_DCSR_EBREAK_CTL; ++ebreak_ctl_i) + pc->dcsr_ebreak_ctls[ebreak_ctl_i] = common_mode_nvp->value; + return jim_getopt_obj(goi, NULL); + } + + /* Here a "ebreak" action for a specific execution mode is processed, e.g: + * "riscv.cpu configure -ebreak m halt" + */ + struct jim_nvp *ctrl_nvp; + if (jim_getopt_nvp(goi, nvp_ebreak_config_opts, &ctrl_nvp) != JIM_OK) { + jim_getopt_nvp_unknown(goi, nvp_ebreak_config_opts, /*hadprefix*/ true); + return JIM_ERR; + } + struct jim_nvp *mode_nvp; + if (jim_getopt_nvp(goi, nvp_ebreak_mode_opts, &mode_nvp) != JIM_OK) { + jim_getopt_nvp_unknown(goi, nvp_ebreak_mode_opts, /*hadprefix*/ true); + return JIM_ERR; + } + pc->dcsr_ebreak_ctls[ctrl_nvp->value] = mode_nvp->value; + return JIM_OK; +} + +static int str_ebreak_config(struct target *target, char *buffer) +{ + int len = 0; + const char *separator = ""; + for (int ebreak_ctl_i = 0; ebreak_ctl_i < N_RISCV_DCSR_EBREAK_CTL; + ++ebreak_ctl_i) { + const char * const format = "%s%s %s"; + const char * const ctl = jim_nvp_value2name_simple(nvp_ebreak_config_opts, ebreak_ctl_i)->name; + const struct riscv_private_config * const pc = riscv_private_config(target); + const char * const mode = jim_nvp_value2name_simple(nvp_ebreak_mode_opts, + pc->dcsr_ebreak_ctls[ebreak_ctl_i])->name; + if (!buffer) + len += snprintf(NULL, 0, format, separator, ctl, mode); + else + len += sprintf(buffer + len, format, separator, ctl, mode); + + separator = " "; + } + return len; +} + +static int jim_report_ebreak_config(struct target *target, Jim_Interp *interp) +{ + const int len = str_ebreak_config(target, NULL); + char *str = malloc(len + 1); + if (!str) { + LOG_ERROR("Unable to allocate a string of %d bytes.", len + 1); + return JIM_ERR; + } + str_ebreak_config(target, str); + Jim_SetResultString(interp, str, len); + free(str); + return JIM_OK; +} + +enum riscv_cfg_opts { + RISCV_CFG_EBREAK, + RISCV_CFG_INVALID = -1 +}; + +static struct jim_nvp nvp_config_opts[] = { + { .name = "-ebreak", .value = RISCV_CFG_EBREAK }, + { .name = NULL, .value = RISCV_CFG_INVALID } +}; + +static struct riscv_private_config *default_riscv_private_config(void) +{ + struct riscv_private_config * const pc = malloc(sizeof(*pc)); + if (!pc) { + LOG_ERROR("Out of memory!"); + return NULL; + } + + memset(pc->dcsr_ebreak_ctls, true, sizeof(pc->dcsr_ebreak_ctls)); + + return pc; +} + +static int riscv_jim_configure(struct target *target, + struct jim_getopt_info *goi) +{ + struct riscv_private_config *pc = target->private_config; + if (!pc) { + pc = default_riscv_private_config(); + if (!pc) + return JIM_ERR; + target->private_config = pc; + } + if (!goi->argc) + return JIM_OK; + + struct jim_nvp *n; + int e = jim_nvp_name2value_obj(goi->interp, nvp_config_opts, + goi->argv[0], &n); + if (e != JIM_OK) + return JIM_CONTINUE; + + e = jim_getopt_obj(goi, NULL); + if (e != JIM_OK) + return e; + + switch (n->value) { + case RISCV_CFG_EBREAK: + return goi->isconfigure + ? jim_configure_ebreak(target, goi) + : jim_report_ebreak_config(target, goi->interp); + default: + assert(false && "'jim_getopt_nvp' should have returned an error."); + } + return JIM_ERR; +} + static int riscv_init_target(struct command_context *cmd_ctx, struct target *target) { LOG_TARGET_DEBUG(target, "riscv_init_target()"); + struct riscv_private_config *pc = target->private_config; + if (!pc) { + pc = default_riscv_private_config(); + if (!pc) + return ERROR_FAIL; + target->private_config = pc; + } RISCV_INFO(info); info->cmd_ctx = cmd_ctx; info->reset_delays_wait = -1; @@ -537,6 +684,8 @@ static void riscv_deinit_target(struct target *target) { LOG_TARGET_DEBUG(target, "riscv_deinit_target()"); + free(target->private_config); + struct riscv_info *info = target->arch_info; struct target_type *tt = get_target_type(target); if (!tt) @@ -4598,52 +4747,69 @@ COMMAND_HANDLER(riscv_set_autofence) return ERROR_COMMAND_SYNTAX_ERROR; } -COMMAND_HANDLER(riscv_set_ebreakm) +COMMAND_HELPER(ebreakx_deprecation_helper, enum riscv_ebreak_ctl mode) { - struct target *target = get_current_target(CMD_CTX); - RISCV_INFO(r); - + struct target * const target = get_current_target(CMD_CTX); + struct riscv_private_config * const pc = riscv_private_config(target); + const char *mode_str; + switch (mode) { + case RISCV_DCSR_EBREAK_M_CTL: + mode_str = "m"; + break; + case RISCV_DCSR_EBREAK_S_CTL: + mode_str = "s"; + break; + case RISCV_DCSR_EBREAK_U_CTL: + mode_str = "u"; + break; + default: + assert(0 && "Unexpected execution mode"); + mode_str = "unexpected"; + } + if (CMD_ARGC > 1) + return ERROR_COMMAND_SYNTAX_ERROR; if (CMD_ARGC == 0) { - command_print(CMD, "riscv_ebreakm enabled: %s", r->riscv_ebreakm ? "on" : "off"); - return ERROR_OK; - } else if (CMD_ARGC == 1) { - COMMAND_PARSE_ON_OFF(CMD_ARGV[0], r->riscv_ebreakm); + LOG_WARNING("DEPRECATED! use '%s cget -ebreak' not '%s'", + target_name(target), CMD_NAME); + command_print(CMD, "riscv_ebreak%s enabled: %s", mode_str, + pc->dcsr_ebreak_ctls[mode] ? "on" : "off"); return ERROR_OK; } + assert(CMD_ARGC == 1); + command_print(CMD, "DEPRECATED! use '%s configure -ebreak %s' not '%s'", + target_name(target), mode_str, CMD_NAME); + bool ebreak_ctl; + COMMAND_PARSE_ON_OFF(CMD_ARGV[0], ebreak_ctl); + pc->dcsr_ebreak_ctls[mode] = ebreak_ctl; + switch (mode) { + case RISCV_DCSR_EBREAK_S_CTL: + pc->dcsr_ebreak_ctls[RISCV_DCSR_EBREAK_VS_CTL] = ebreak_ctl; + break; + case RISCV_DCSR_EBREAK_U_CTL: + pc->dcsr_ebreak_ctls[RISCV_DCSR_EBREAK_VU_CTL] = ebreak_ctl; + break; + default: + break; + } + return ERROR_OK; +} - return ERROR_COMMAND_SYNTAX_ERROR; +COMMAND_HANDLER(riscv_set_ebreakm) +{ + return CALL_COMMAND_HANDLER(ebreakx_deprecation_helper, + RISCV_DCSR_EBREAK_M_CTL); } COMMAND_HANDLER(riscv_set_ebreaks) { - struct target *target = get_current_target(CMD_CTX); - RISCV_INFO(r); - - if (CMD_ARGC == 0) { - command_print(CMD, "riscv_ebreaks enabled: %s", r->riscv_ebreaks ? "on" : "off"); - return ERROR_OK; - } else if (CMD_ARGC == 1) { - COMMAND_PARSE_ON_OFF(CMD_ARGV[0], r->riscv_ebreaks); - return ERROR_OK; - } - - return ERROR_COMMAND_SYNTAX_ERROR; + return CALL_COMMAND_HANDLER(ebreakx_deprecation_helper, + RISCV_DCSR_EBREAK_S_CTL); } COMMAND_HANDLER(riscv_set_ebreaku) { - struct target *target = get_current_target(CMD_CTX); - RISCV_INFO(r); - - if (CMD_ARGC == 0) { - command_print(CMD, "riscv_ebreaku enabled: %s", r->riscv_ebreaku ? "on" : "off"); - return ERROR_OK; - } else if (CMD_ARGC == 1) { - COMMAND_PARSE_ON_OFF(CMD_ARGV[0], r->riscv_ebreaku); - return ERROR_OK; - } - - return ERROR_COMMAND_SYNTAX_ERROR; + return CALL_COMMAND_HANDLER(ebreakx_deprecation_helper, + RISCV_DCSR_EBREAK_U_CTL); } COMMAND_HELPER(riscv_clear_trigger, int trigger_id, const char *name) @@ -5411,24 +5577,21 @@ static const struct command_registration riscv_exec_command_handlers[] = { .handler = riscv_set_ebreakm, .mode = COMMAND_ANY, .usage = "[on|off]", - .help = "Control dcsr.ebreakm. When off, M-mode ebreak instructions " - "don't trap to OpenOCD. Defaults to on." + .help = "DEPRECATED! use ' configure/cget -ebreak'" }, { .name = "set_ebreaks", .handler = riscv_set_ebreaks, .mode = COMMAND_ANY, .usage = "[on|off]", - .help = "Control dcsr.ebreaks. When off, S-mode ebreak instructions " - "don't trap to OpenOCD. Defaults to on." + .help = "DEPRECATED! use ' configure/cget -ebreak'" }, { .name = "set_ebreaku", .handler = riscv_set_ebreaku, .mode = COMMAND_ANY, .usage = "[on|off]", - .help = "Control dcsr.ebreaku. When off, U-mode ebreak instructions " - "don't trap to OpenOCD. Defaults to on." + .help = "DEPRECATED! use ' configure/cget -ebreak'" }, { .name = "etrigger", @@ -5548,6 +5711,7 @@ struct target_type riscv_target = { .name = "riscv", .target_create = riscv_create_target, + .target_jim_configure = riscv_jim_configure, .init_target = riscv_init_target, .deinit_target = riscv_deinit_target, .examine = riscv_examine, @@ -5627,10 +5791,6 @@ static void riscv_info_init(struct target *target, struct riscv_info *r) r->vsew64_supported = YNM_MAYBE; - r->riscv_ebreakm = true; - r->riscv_ebreaks = true; - r->riscv_ebreaku = true; - r->wp_allow_equality_match_trigger = true; r->wp_allow_ge_lt_trigger = true; r->wp_allow_napot_trigger = true; diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 3c02174ec..deefb9095 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -326,10 +326,6 @@ struct riscv_info { bool range_trigger_fallback_encountered; - bool riscv_ebreakm; - bool riscv_ebreaks; - bool riscv_ebreaku; - bool wp_allow_equality_match_trigger; bool wp_allow_napot_trigger; bool wp_allow_ge_lt_trigger; @@ -337,6 +333,26 @@ struct riscv_info { bool autofence; }; +enum riscv_ebreak_ctl { + RISCV_DCSR_EBREAK_M_CTL, + RISCV_DCSR_EBREAK_S_CTL, + RISCV_DCSR_EBREAK_U_CTL, + RISCV_DCSR_EBREAK_VS_CTL, + RISCV_DCSR_EBREAK_VU_CTL, + N_RISCV_DCSR_EBREAK_CTL +}; + +struct riscv_private_config { + bool dcsr_ebreak_ctls[N_RISCV_DCSR_EBREAK_CTL]; +}; + +static inline struct riscv_private_config +*riscv_private_config(const struct target *target) +{ + assert(target->private_config); + return target->private_config; +} + COMMAND_HELPER(riscv_print_info_line, const char *section, const char *key, unsigned int value); diff --git a/src/target/riscv/startup.tcl b/src/target/riscv/startup.tcl index 31e5059e2..28e03a407 100644 --- a/src/target/riscv/startup.tcl +++ b/src/target/riscv/startup.tcl @@ -46,6 +46,10 @@ proc {riscv set_enable_virt2phys} on_off { return {} } +foreach mode {m s u} { + lappend _telnet_autocomplete_skip "riscv set_ebreak$mode" +} + proc riscv {cmd args} { tailcall "riscv $cmd" {*}$args }