Merge pull request #977 from kr-sc/kr-sc/improve-riscv-controls

target/riscv: Improve riscv controls that manage the set of available triggers for watchpoints

doc/openocd.texi

@@ -11379,16 +11379,28 @@ OpenOCD. When off, they generate a breakpoint exception handled internally.
 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.
 
-@deffn {Command} {riscv set_enable_eq_match_trigger} [on|off]
+@deffn {Command} {riscv set_enable_trigger_feature} [(@option{eq}|@option{napot}|@option{ge_lt}|@option{all}) (@option{wp}|@option{none})]
-When on (default), allow OpenOCD to use exact-match triggers in watchpoints.
+Control which RISC-V trigger features can be used by OpenOCD placing watchpoints.
-@end deffn
+All trigger features are allowed by default. Only new watchpoints, inserted after this command,
+are affected (watchpoints that were already placed before are not changed).
 
-@deffn {Command} {riscv set_enable_napot_trigger} [on|off]
+The first argument selects one of the configurable RISC-V trigger features:
-When on (default), allow OpenOCD to use NAPOT trigger in watchpoints.
-@end deffn
 
-@deffn {Command} {riscv set_enable_ge_lt_trigger} [on|off]
+@itemize @minus
-When on (default), allow OpenOCD to use a pair of chained less-than & greater-than triggers in watchpoints.
+@item @option{eq}: Equality match trigger
+@item @option{napot}: NAPOT trigger
+@item @option{ge_lt}: Chained pair of `greater-equal` and `less-than` triggers
+@item @option{all}: All trigger features which were described above
+@end itemize
+
+The second argument configures how OpenOCD should use the selected trigger feature:
+
+@itemize @minus
+@item @option{wp}: Enable this trigger feature for watchpoints - allow OpenOCD to use it. (Default.)
+@item @option{none}: Disable the use of this trigger feature. OpenOCD will not attempt to use it.
+@end itemize
+
+With no parameters, prints current trigger features configuration.
 @end deffn
 
 @subsection RISC-V Authentication Commands

src/target/riscv/riscv.c

@@ -1011,12 +1011,12 @@ static int maybe_add_trigger_t2_t6_for_wp(struct target *target,
 		struct trigger *trigger, struct match_triggers_tdata1_fields fields)
 {
 	RISCV_INFO(r);
-	int ret = ERROR_FAIL;
+	int ret = ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
 
 	if (trigger->length > 0) {
 		/* Setting a load/store trigger ("watchpoint") on a range of addresses */
-
+		if (can_use_napot_match(trigger)) {
-		if (r->enable_napot_trigger && can_use_napot_match(trigger)) {
+			if (r->wp_allow_napot_trigger) {
 				LOG_TARGET_DEBUG(target, "trying to setup NAPOT match trigger");
 				struct trigger_request_info napot = {
 					.tdata1 = fields.common | fields.size.any |
@@ -1027,9 +1027,13 @@ static int maybe_add_trigger_t2_t6_for_wp(struct target *target,
 				ret = try_setup_single_match_trigger(target, trigger, napot);
 				if (ret != ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
 					return ret;
+			} else {
+				LOG_TARGET_DEBUG(target, "NAPOT match triggers are disabled for watchpoints. "
+							 "Use 'riscv set_enable_trigger_feature napot wp' to enable it.");
+			}
 		}
 
-		if (r->enable_ge_lt_trigger) {
+		if (r->wp_allow_ge_lt_trigger) {
 			LOG_TARGET_DEBUG(target, "trying to setup GE+LT chained match trigger pair");
 			struct trigger_request_info ge_1 = {
 				.tdata1 = fields.common | fields.size.any | fields.chain.enable |
@@ -1063,10 +1067,13 @@ static int maybe_add_trigger_t2_t6_for_wp(struct target *target,
 			ret = try_setup_chained_match_triggers(target, trigger, lt_1, ge_2);
 			if (ret != ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
 				return ret;
+		} else {
+			LOG_TARGET_DEBUG(target, "LT+GE chained match triggers are disabled for watchpoints. "
+						 "Use 'riscv set_enable_trigger_feature ge_lt wp' to enable it.");
 		}
 	}
 
-	if (r->enable_equality_match_trigger) {
+	if (r->wp_allow_equality_match_trigger) {
 		LOG_TARGET_DEBUG(target, "trying to setup equality match trigger");
 		struct trigger_request_info eq = {
 			.tdata1 = fields.common | fields.size.any | fields.chain.disable |
@@ -1077,6 +1084,9 @@ static int maybe_add_trigger_t2_t6_for_wp(struct target *target,
 		ret = try_setup_single_match_trigger(target, trigger, eq);
 		if (ret != ERROR_OK)
 			return ret;
+	} else {
+		LOG_TARGET_DEBUG(target, "equality match triggers are disabled for watchpoints. "
+					 "Use 'riscv set_enable_trigger_feature eq wp' to enable it.");
 	}
 
 	if (ret == ERROR_OK && trigger->length > 1) {
@@ -4505,55 +4515,47 @@ COMMAND_HANDLER(riscv_exec_progbuf)
 	return ERROR_OK;
 }
 
-COMMAND_HANDLER(riscv_set_enable_eq_match_trigger)
+COMMAND_HANDLER(riscv_set_enable_trigger_feature)
 {
 	struct target *target = get_current_target(CMD_CTX);
 	RISCV_INFO(r);
 
-	if (CMD_ARGC == 0) {
+	if (CMD_ARGC == 2) {
-		command_print(CMD, "equality match trigger enabled: %s", r->enable_equality_match_trigger ? "on" : "off");
+		bool enable_for_wp = true;
-		return ERROR_OK;
+
-	} else if (CMD_ARGC == 1) {
+		if (!strcmp(CMD_ARGV[1], "wp"))
-		COMMAND_PARSE_ON_OFF(CMD_ARGV[0], r->enable_equality_match_trigger);
+			enable_for_wp = true;
-		return ERROR_OK;
+		else if (!strcmp(CMD_ARGV[1], "none"))
+			enable_for_wp = false;
+		else
+			return ERROR_COMMAND_SYNTAX_ERROR;
+
+		if (!strcmp(CMD_ARGV[0], "all")) {
+			r->wp_allow_equality_match_trigger = enable_for_wp;
+			r->wp_allow_napot_trigger = enable_for_wp;
+			r->wp_allow_ge_lt_trigger = enable_for_wp;
+		} else if (!strcmp(CMD_ARGV[0], "eq")) {
+			r->wp_allow_equality_match_trigger = enable_for_wp;
+		} else if (!strcmp(CMD_ARGV[0], "napot")) {
+			r->wp_allow_napot_trigger = enable_for_wp;
+		} else if (!strcmp(CMD_ARGV[0], "ge_lt")) {
+			r->wp_allow_ge_lt_trigger = enable_for_wp;
+		} else {
+			return ERROR_COMMAND_SYNTAX_ERROR;
+		}
+	} else if (CMD_ARGC != 0) {
+		return ERROR_COMMAND_SYNTAX_ERROR;
 	}
 
-	LOG_ERROR("Command takes 0 or 1 parameters");
+	command_print(CMD, "Triggers feature configuration:\n"
-	return ERROR_COMMAND_SYNTAX_ERROR;
+			   "Equality match trigger: for wp (%s)\n"
-}
+			   "NAPOT trigger: for wp (%s)\n"
+			   "ge-lt chained triggers: for wp (%s)",
+			   r->wp_allow_equality_match_trigger ? "enabled" : "disabled",
+			   r->wp_allow_napot_trigger ? "enabled" : "disabled",
+			   r->wp_allow_ge_lt_trigger ? "enabled" : "disabled");
 
-COMMAND_HANDLER(riscv_set_enable_napot_trigger)
-{
-	struct target *target = get_current_target(CMD_CTX);
-	RISCV_INFO(r);
-
-	if (CMD_ARGC == 0) {
-		command_print(CMD, "NAPOT trigger enabled: %s", r->enable_napot_trigger ? "on" : "off");
 	return ERROR_OK;
-	} else if (CMD_ARGC == 1) {
-		COMMAND_PARSE_ON_OFF(CMD_ARGV[0], r->enable_napot_trigger);
-		return ERROR_OK;
-	}
-
-	LOG_ERROR("Command takes 0 or 1 parameters");
-	return ERROR_COMMAND_SYNTAX_ERROR;
-}
-
-COMMAND_HANDLER(riscv_set_enable_ge_lt_trigger)
-{
-	struct target *target = get_current_target(CMD_CTX);
-	RISCV_INFO(r);
-
-	if (CMD_ARGC == 0) {
-		command_print(CMD, "ge-lt triggers enabled: %s", r->enable_ge_lt_trigger ? "on" : "off");
-		return ERROR_OK;
-	} else if (CMD_ARGC == 1) {
-		COMMAND_PARSE_ON_OFF(CMD_ARGV[0], r->enable_ge_lt_trigger);
-		return ERROR_OK;
-	}
-
-	LOG_ERROR("Command takes 0 or 1 parameters");
-	return ERROR_COMMAND_SYNTAX_ERROR;
 }
 
 static const struct command_registration riscv_exec_command_handlers[] = {
@@ -4806,25 +4808,11 @@ static const struct command_registration riscv_exec_command_handlers[] = {
 			"The final ebreak instruction is added automatically, if needed."
 	},
 	{
-		.name = "set_enable_eq_match_trigger",
+		.name = "set_enable_trigger_feature",
-		.handler = riscv_set_enable_eq_match_trigger,
+		.handler = riscv_set_enable_trigger_feature,
-		.mode = COMMAND_CONFIG,
+		.mode = COMMAND_ANY,
-		.usage = "[on|off]",
+		.usage = "[('eq'|'napot'|'ge_lt'|'all') ('wp'|'none')]",
-		.help = "When on, allow OpenOCD to use equality match trigger in wp."
+		.help = "Control whether OpenOCD is allowed to use certain RISC-V trigger features for watchpoints."
-	},
-	{
-		.name = "set_enable_napot_trigger",
-		.handler = riscv_set_enable_napot_trigger,
-		.mode = COMMAND_CONFIG,
-		.usage = "[on|off]",
-		.help = "When on, allow OpenOCD to use NAPOT trigger in wp."
-	},
-	{
-		.name = "set_enable_ge_lt_trigger",
-		.handler = riscv_set_enable_ge_lt_trigger,
-		.mode = COMMAND_CONFIG,
-		.usage = "[on|off]",
-		.help = "When on, allow OpenOCD to use GE/LT triggers in wp."
 	},
 	COMMAND_REGISTRATION_DONE
 };
@@ -4962,9 +4950,9 @@ static void riscv_info_init(struct target *target, struct riscv_info *r)
 	r->riscv_ebreaks = true;
 	r->riscv_ebreaku = true;
 
-	r->enable_equality_match_trigger = true;
+	r->wp_allow_equality_match_trigger = true;
-	r->enable_ge_lt_trigger = true;
+	r->wp_allow_ge_lt_trigger = true;
-	r->enable_napot_trigger = true;
+	r->wp_allow_napot_trigger = true;
 }
 
 static int riscv_resume_go_all_harts(struct target *target)

src/target/riscv/riscv.h

@@ -314,9 +314,9 @@ struct riscv_info {
 	bool riscv_ebreaks;
 	bool riscv_ebreaku;
 
-	bool enable_equality_match_trigger;
+	bool wp_allow_equality_match_trigger;
-	bool enable_napot_trigger;
+	bool wp_allow_napot_trigger;
-	bool enable_ge_lt_trigger;
+	bool wp_allow_ge_lt_trigger;
 };
 
 COMMAND_HELPER(riscv_print_info_line, const char *section, const char *key,