Merge pull request #1168 from en-sc/en-sc/ebreak

target/riscv: new `ebreak` controls

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 config = riscv_private_config(target);
+	dcsr = set_field(dcsr, DCSR_EBREAKM, config->dcsr_ebreak_fields[RISCV_MODE_M]);
+	dcsr = set_field(dcsr, DCSR_EBREAKS, config->dcsr_ebreak_fields[RISCV_MODE_S]);
+	dcsr = set_field(dcsr, DCSR_EBREAKU, config->dcsr_ebreak_fields[RISCV_MODE_U]);
+	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_ebreakx_fields(info->dcsr, target);
-	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 &= ~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_ebreakx_fields(info->dcsr, target);
-	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 |= DCSR_HALT;
 	if (target->reset_halt)
 		info->dcsr |= DCSR_NDRESET;

src/target/riscv/riscv-013.c

@@ -1611,7 +1611,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. */
@@ -1619,11 +1618,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);
+	const struct riscv_private_config * const config = riscv_private_config(target);
-	dcsr = set_field(dcsr, CSR_DCSR_EBREAKS, r->riscv_ebreaks && riscv_supports_extension(target, 'S'));
+	dcsr = set_field(dcsr, CSR_DCSR_EBREAKM, config->dcsr_ebreak_fields[RISCV_MODE_M]);
-	dcsr = set_field(dcsr, CSR_DCSR_EBREAKU, r->riscv_ebreaku && riscv_supports_extension(target, 'U'));
+	dcsr = set_field(dcsr, CSR_DCSR_EBREAKS, config->dcsr_ebreak_fields[RISCV_MODE_S]);
-	dcsr = set_field(dcsr, CSR_DCSR_EBREAKVS, r->riscv_ebreaku && riscv_supports_extension(target, 'H'));
+	dcsr = set_field(dcsr, CSR_DCSR_EBREAKU, config->dcsr_ebreak_fields[RISCV_MODE_U]);
-	dcsr = set_field(dcsr, CSR_DCSR_EBREAKVU, r->riscv_ebreaku && riscv_supports_extension(target, 'H'));
+	dcsr = set_field(dcsr, CSR_DCSR_EBREAKVS, config->dcsr_ebreak_fields[RISCV_MODE_VS]);
+	dcsr = set_field(dcsr, CSR_DCSR_EBREAKVU, config->dcsr_ebreak_fields[RISCV_MODE_VU]);
 	if (dcsr != original_dcsr &&
 			riscv_reg_set(target, GDB_REGNO_DCSR, dcsr) != ERROR_OK)
 		return ERROR_FAIL;
@@ -2858,8 +2858,11 @@ static int assert_reset(struct target *target)
 
 static bool dcsr_ebreak_config_equals_reset_value(const struct target *target)
 {
-	RISCV_INFO(r);
+	const struct riscv_private_config * const config = riscv_private_config(target);
-	return !(r->riscv_ebreakm || r->riscv_ebreaks || r->riscv_ebreaku);
+	for (int i = 0; i < N_RISCV_MODE; ++i)
+		if (config->dcsr_ebreak_fields[i])
+			return false;
+	return true;
 }
 
 static int deassert_reset(struct target *target)

src/target/riscv/riscv.c

@@ -467,9 +467,30 @@ static struct target_type *get_target_type(struct target *target)
 	}
 }
 
+static struct riscv_private_config *alloc_default_riscv_private_config(void)
+{
+	struct riscv_private_config * const config = malloc(sizeof(*config));
+	if (!config) {
+		LOG_ERROR("Out of memory!");
+		return NULL;
+	}
+
+	for (unsigned int i = 0; i < ARRAY_SIZE(config->dcsr_ebreak_fields); ++i)
+		config->dcsr_ebreak_fields[i] = true;
+
+	return config;
+}
+
 static int riscv_create_target(struct target *target, Jim_Interp *interp)
 {
 	LOG_TARGET_DEBUG(target, "riscv_create_target()");
+	struct riscv_private_config *config = target->private_config;
+	if (!config) {
+		config = alloc_default_riscv_private_config();
+		if (!config)
+			return ERROR_FAIL;
+		target->private_config = config;
+	}
 	target->arch_info = calloc(1, sizeof(struct riscv_info));
 	if (!target->arch_info) {
 		LOG_TARGET_ERROR(target, "Failed to allocate RISC-V target structure.");
@@ -479,6 +500,155 @@ 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_MODE_M },
+	{ .name = "s", .value = RISCV_MODE_S },
+	{ .name = "u", .value = RISCV_MODE_U },
+	{ .name = "vs", .value = RISCV_MODE_VS },
+	{ .name = "vu", .value = RISCV_MODE_VU },
+	{ .name = NULL, .value = N_RISCV_MODE }
+};
+
+#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 riscv_private_config *config, struct jim_getopt_info *goi)
+{
+	if (goi->argc == 0) {
+		Jim_WrongNumArgs(goi->interp, 1, goi->argv - 1,
+				"[?execution_mode?] ?ebreak_action?");
+		return JIM_ERR;
+	}
+	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_MODE; ++ebreak_ctl_i)
+			config->dcsr_ebreak_fields[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"
+	 */
+	if (goi->argc < 2) {
+		Jim_WrongNumArgs(goi->interp, 2, goi->argv - 2,
+				"?ebreak_action?");
+		return JIM_ERR;
+	}
+	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;
+	}
+	config->dcsr_ebreak_fields[ctrl_nvp->value] = mode_nvp->value;
+	return JIM_OK;
+}
+
+/**
+ * Obtain dcsr.ebreak* configuration as a Tcl dictionary.
+ * Print the resulting string to the "buffer" and return the string length.
+ * The "buffer" can be NULL, in which case only the length is computed but
+ * nothing is written.
+ */
+static int ebreak_config_to_tcl_dict(const struct riscv_private_config *config,
+		char *buffer)
+{
+	int len = 0;
+	const char *separator = "";
+	for (int ebreak_ctl_i = 0; ebreak_ctl_i < N_RISCV_MODE;
+			++ebreak_ctl_i) {
+		const char * const format = "%s%s %s";
+		const char * const priv_mode =
+			jim_nvp_value2name_simple(nvp_ebreak_config_opts, ebreak_ctl_i)->name;
+		const char * const mode = jim_nvp_value2name_simple(nvp_ebreak_mode_opts,
+				config->dcsr_ebreak_fields[ebreak_ctl_i])->name;
+		if (!buffer)
+			len += snprintf(NULL, 0, format, separator, priv_mode, mode);
+		else
+			len += sprintf(buffer + len, format, separator, priv_mode, mode);
+
+		separator = "\n";
+	}
+	return len;
+}
+
+static int jim_report_ebreak_config(const struct riscv_private_config *config,
+		Jim_Interp *interp)
+{
+	const int len = ebreak_config_to_tcl_dict(config, NULL);
+	char *str = malloc(len + 1);
+	if (!str) {
+		LOG_ERROR("Unable to allocate a string of %d bytes.", len + 1);
+		return JIM_ERR;
+	}
+	ebreak_config_to_tcl_dict(config, 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 int riscv_jim_configure(struct target *target,
+		struct jim_getopt_info *goi)
+{
+	struct riscv_private_config *config = target->private_config;
+	if (!config) {
+		config = alloc_default_riscv_private_config();
+		if (!config)
+			return JIM_ERR;
+		target->private_config = config;
+	}
+	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;
+
+	if (!goi->is_configure && goi->argc > 0) {
+		/* Expecting no arguments */
+		Jim_WrongNumArgs(goi->interp, 2, goi->argv - 2, "");
+		return JIM_ERR;
+	}
+	switch (n->value) {
+	case RISCV_CFG_EBREAK:
+		return goi->is_configure
+			? jim_configure_ebreak(config, goi)
+			: jim_report_ebreak_config(config, 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)
 {
@@ -536,6 +706,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)
@@ -4697,52 +4869,69 @@ COMMAND_HANDLER(riscv_set_autofence)
 	return ERROR_COMMAND_SYNTAX_ERROR;
 }
 
-COMMAND_HANDLER(riscv_set_ebreakm)
+COMMAND_HELPER(ebreakx_deprecation_helper, enum riscv_priv_mode mode)
 {
-	struct target *target = get_current_target(CMD_CTX);
+	struct target * const target = get_current_target(CMD_CTX);
-	RISCV_INFO(r);
+	struct riscv_private_config * const config = riscv_private_config(target);
-
+	const char *mode_str;
+	switch (mode) {
+	case RISCV_MODE_M:
+		mode_str = "m";
+		break;
+	case RISCV_MODE_S:
+		mode_str = "s";
+		break;
+	case RISCV_MODE_U:
+		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");
+		LOG_WARNING("DEPRECATED! use '%s cget -ebreak' not '%s'",
-		return ERROR_OK;
+				target_name(target), CMD_NAME);
-	} else if (CMD_ARGC == 1) {
+		command_print(CMD, "riscv_ebreak%s enabled: %s", mode_str,
-		COMMAND_PARSE_ON_OFF(CMD_ARGV[0], r->riscv_ebreakm);
+				config->dcsr_ebreak_fields[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);
+	config->dcsr_ebreak_fields[mode] = ebreak_ctl;
+	switch (mode) {
+	case RISCV_MODE_S:
+		config->dcsr_ebreak_fields[RISCV_MODE_VS] = ebreak_ctl;
+		break;
+	case RISCV_MODE_U:
+		config->dcsr_ebreak_fields[RISCV_MODE_VU] = 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_MODE_M);
 }
 
 COMMAND_HANDLER(riscv_set_ebreaks)
 {
-	struct target *target = get_current_target(CMD_CTX);
+	return CALL_COMMAND_HANDLER(ebreakx_deprecation_helper,
-	RISCV_INFO(r);
+			RISCV_MODE_S);
-
-	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;
 }
 
 COMMAND_HANDLER(riscv_set_ebreaku)
 {
-	struct target *target = get_current_target(CMD_CTX);
+	return CALL_COMMAND_HANDLER(ebreakx_deprecation_helper,
-	RISCV_INFO(r);
+			RISCV_MODE_U);
-
-	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;
 }
 
 COMMAND_HELPER(riscv_clear_trigger, int trigger_id, const char *name)
@@ -5517,24 +5706,24 @@ 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 "
+		.help = "DEPRECATED! use '<target_name> configure -ebreak' or "
-			"don't trap to OpenOCD. Defaults to on."
+			"'<target_name> 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 "
+		.help = "DEPRECATED! use '<target_name> configure -ebreak' or "
-			"don't trap to OpenOCD. Defaults to on."
+			"'<target_name> 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 "
+		.help = "DEPRECATED! use '<target_name> configure -ebreak' or "
-			"don't trap to OpenOCD. Defaults to on."
+			"'<target_name> cget -ebreak'"
 	},
 	{
 		.name = "etrigger",
@@ -5654,6 +5843,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,
@@ -5733,10 +5923,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

@@ -358,10 +358,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;
@@ -369,6 +365,26 @@ struct riscv_info {
 	bool autofence;
 };
 
+enum riscv_priv_mode {
+	RISCV_MODE_M,
+	RISCV_MODE_S,
+	RISCV_MODE_U,
+	RISCV_MODE_VS,
+	RISCV_MODE_VU,
+	N_RISCV_MODE
+};
+
+struct riscv_private_config {
+	bool dcsr_ebreak_fields[N_RISCV_MODE];
+};
+
+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
 }