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 <evgeniy.naydanov@syntacore.com>

doc/openocd.texi

@@ -11225,6 +11225,25 @@ follows:
 </feature>
 @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.
 

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;

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)

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 '<target_name> 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 '<target_name> 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 '<target_name> 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;

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);
 

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
 }