Merge pull request #909 from en-sc/en-sc/cleanup-enumerate-triggers

target/riscv: cleanup riscv_enumerate_triggers()

2 files changed, 125 insertions, 75 deletions

diff --git a/doc/openocd.texi b/doc/openocd.texi
index 0fe5a6f..5d22707 100644
--- a/doc/openocd.texi
+++ b/doc/openocd.texi
@@ -10734,7 +10734,9 @@ address, or to sample a changing value in a memory-mapped device.
 @end deffn
 
 @deffn {Command} {riscv info}
-Displays some information OpenOCD detected about the target.
+Displays some information OpenOCD detected about the target. Output's format
+allows to use it directly with TCL's `array set` function. In case obtaining an
+info point failed, the corresponding value is displayed as "unavailable".
 @end deffn
 
 @deffn {Command} {riscv reset_delays} [wait]
diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c
index c0742c7..4b33eb7 100644
--- a/src/target/riscv/riscv.c
+++ b/src/target/riscv/riscv.c
@@ -4164,15 +4164,25 @@ error:
 	return result;
 }
 
-COMMAND_HELPER(riscv_print_info_line, const char *section, const char *key,
-			   unsigned int value)
+static COMMAND_HELPER(riscv_print_info_line_if_available, const char *section,
+		const char *key, unsigned int value, bool is_available)
 {
 	char full_key[80];
 	snprintf(full_key, sizeof(full_key), "%s.%s", section, key);
-	command_print(CMD, "%-21s %3d", full_key, value);
+	if (is_available)
+		command_print(CMD, "%-21s %3d", full_key, value);
+	else
+		command_print(CMD, "%-21s unavailable", full_key);
 	return 0;
 }
 
+COMMAND_HELPER(riscv_print_info_line, const char *section, const char *key,
+			   unsigned int value)
+{
+	return CALL_COMMAND_HANDLER(riscv_print_info_line_if_available, section,
+			key, value, /*is_available*/ true);
+}
+
 COMMAND_HANDLER(handle_info)
 {
 	struct target *target = get_current_target(CMD_CTX);
@@ -4181,10 +4191,11 @@ COMMAND_HANDLER(handle_info)
 	/* This output format can be fed directly into TCL's "array set". */
 
 	riscv_print_info_line(CMD, "hart", "xlen", riscv_xlen(target));
-	riscv_enumerate_triggers(target);
-	riscv_print_info_line(CMD, "hart", "trigger_count",
-						  r->trigger_count);
 
+	const bool trigger_count_available =
+		riscv_enumerate_triggers(target) == ERROR_OK;
+	riscv_print_info_line_if_available(CMD, "hart", "trigger_count",
+				r->trigger_count, trigger_count_available);
 	if (r->print_info)
 		return CALL_COMMAND_HANDLER(r->print_info, target);
 
@@ -5034,6 +5045,78 @@ int riscv_dmi_write_u64_bits(struct target *target)
 	return r->dmi_write_u64_bits(target);
 }
 
+static int check_if_trigger_exists(struct target *target, unsigned int index)
+{
+	/* If we can't write tselect, then this hart does not support triggers. */
+	if (riscv_set_register(target, GDB_REGNO_TSELECT, index) != ERROR_OK)
+		return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
+	riscv_reg_t tselect_rb;
+	if (riscv_get_register(target, &tselect_rb, GDB_REGNO_TSELECT) != ERROR_OK)
+		return ERROR_FAIL;
+	/* Mask off the top bit, which is used as tdrmode in legacy RISC-V Debug Spec
+	 * (old revisions of v0.11 spec). */
+	tselect_rb &= ~(1ULL << (riscv_xlen(target) - 1));
+	if (tselect_rb != index)
+		return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
+	return ERROR_OK;
+}
+
+/**
+ * This function reads `tinfo` or `tdata1`, when reading `tinfo` fails,
+ * to determine trigger types supported by a trigger.
+ * It is assumed that the trigger is already selected via writing `tselect`.
+ */
+static int get_trigger_types(struct target *target, unsigned int *trigger_tinfo,
+		riscv_reg_t tdata1)
+{
+	assert(trigger_tinfo);
+	riscv_reg_t tinfo;
+	if (riscv_get_register(target, &tinfo, GDB_REGNO_TINFO) == ERROR_OK) {
+		/* tinfo.INFO == 1: trigger doesn’t exist
+		 * tinfo == 0 or tinfo.INFO != 1 and tinfo LSB is set: invalid tinfo */
+		if (tinfo == 0 || tinfo & 0x1)
+			return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
+		*trigger_tinfo = tinfo;
+		return ERROR_OK;
+	}
+	const unsigned int type = get_field(tdata1, CSR_TDATA1_TYPE(riscv_xlen(target)));
+	if (type == 0)
+		return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
+	*trigger_tinfo = 1 << type;
+	return ERROR_OK;
+}
+
+static int disable_trigger_if_dmode(struct target *target, riscv_reg_t tdata1)
+{
+	bool dmode_is_set = false;
+	switch (get_field(tdata1, CSR_TDATA1_TYPE(riscv_xlen(target)))) {
+		case CSR_TDATA1_TYPE_LEGACY:
+			/* On these older cores we don't support software using
+			 * triggers. */
+			dmode_is_set = true;
+			break;
+		case CSR_TDATA1_TYPE_MCONTROL:
+			dmode_is_set = tdata1 & CSR_MCONTROL_DMODE(riscv_xlen(target));
+			break;
+		case CSR_TDATA1_TYPE_MCONTROL6:
+			dmode_is_set = tdata1 & CSR_MCONTROL6_DMODE(riscv_xlen(target));
+			break;
+		case CSR_TDATA1_TYPE_ICOUNT:
+			dmode_is_set = tdata1 & CSR_ICOUNT_DMODE(riscv_xlen(target));
+			break;
+		case CSR_TDATA1_TYPE_ITRIGGER:
+			dmode_is_set = tdata1 & CSR_ITRIGGER_DMODE(riscv_xlen(target));
+			break;
+		case CSR_TDATA1_TYPE_ETRIGGER:
+			dmode_is_set = tdata1 & CSR_ETRIGGER_DMODE(riscv_xlen(target));
+			break;
+	}
+	if (!dmode_is_set)
+		/* Nothing to do */
+		return ERROR_OK;
+	return riscv_set_register(target, GDB_REGNO_TDATA1, 0);
+}
+
 /**
  * Count triggers, and initialize trigger_count for each hart.
  * trigger_count is initialized even if this function fails to discover
@@ -5048,89 +5131,54 @@ int riscv_enumerate_triggers(struct target *target)
 	if (r->triggers_enumerated)
 		return ERROR_OK;
 
-	r->triggers_enumerated = true;	/* At the very least we tried. */
+	if (target->state != TARGET_HALTED) {
+		LOG_TARGET_ERROR(target, "Unable to enumerate triggers: target not halted.");
+		return ERROR_FAIL;
+	}
 
-	riscv_reg_t tselect;
-	int result = riscv_get_register(target, &tselect, GDB_REGNO_TSELECT);
+	riscv_reg_t orig_tselect;
+	int result = riscv_get_register(target, &orig_tselect, GDB_REGNO_TSELECT);
 	/* If tselect is not readable, the trigger module is likely not
-		* implemented. There are no triggers to enumerate then and no error
-		* should be thrown. */
+	 * implemented. */
 	if (result != ERROR_OK) {
-		LOG_TARGET_DEBUG(target, "Cannot access tselect register. "
+		LOG_TARGET_INFO(target, "Cannot access tselect register. "
 				"Assuming that triggers are not implemented.");
+		r->triggers_enumerated = true;
 		r->trigger_count = 0;
 		return ERROR_OK;
 	}
 
-	for (unsigned int t = 0; t < RISCV_MAX_TRIGGERS; ++t) {
-		r->trigger_count = t;
-
-		/* If we can't write tselect, then this hart does not support triggers. */
-		if (riscv_set_register(target, GDB_REGNO_TSELECT, t) != ERROR_OK)
+	unsigned int t = 0;
+	for (; t < ARRAY_SIZE(r->trigger_tinfo); ++t) {
+		result = check_if_trigger_exists(target, t);
+		if (result == ERROR_FAIL)
+			return ERROR_FAIL;
+		if (result == ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
 			break;
-		uint64_t tselect_rb;
-		result = riscv_get_register(target, &tselect_rb, GDB_REGNO_TSELECT);
-		if (result != ERROR_OK)
-			return result;
-		/* Mask off the top bit, which is used as tdrmode in old
-			* implementations. */
-		tselect_rb &= ~(1ULL << (riscv_xlen(target) - 1));
-		if (tselect_rb != t)
+
+		riscv_reg_t tdata1;
+		if (riscv_get_register(target, &tdata1, GDB_REGNO_TDATA1) != ERROR_OK)
+			return ERROR_FAIL;
+
+		result = get_trigger_types(target, &r->trigger_tinfo[t], tdata1);
+		if (result == ERROR_FAIL)
+			return ERROR_FAIL;
+		if (result == ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
 			break;
 
-		uint64_t tinfo;
-		result = riscv_get_register(target, &tinfo, GDB_REGNO_TINFO);
-		if (result == ERROR_OK) {
-			/* tinfo == 0 invalid tinfo
-			 * tinfo == 1 trigger doesn’t exist */
-			if (tinfo == 0 || tinfo == 1)
-				break;
-			r->trigger_tinfo[t] = tinfo;
-		} else {
-			uint64_t tdata1;
-			result = riscv_get_register(target, &tdata1, GDB_REGNO_TDATA1);
-			if (result != ERROR_OK)
-				return result;
+		LOG_TARGET_DEBUG(target, "Trigger %u: supported types (mask) = 0x%08x",
+				t, r->trigger_tinfo[t]);
 
-			int type = get_field(tdata1, CSR_TDATA1_TYPE(riscv_xlen(target)));
-			if (type == 0)
-				break;
-			switch (type) {
-				case CSR_TDATA1_TYPE_LEGACY:
-					/* On these older cores we don't support software using
-						* triggers. */
-					riscv_set_register(target, GDB_REGNO_TDATA1, 0);
-					break;
-				case CSR_TDATA1_TYPE_MCONTROL:
-					if (tdata1 & CSR_MCONTROL_DMODE(riscv_xlen(target)))
-						riscv_set_register(target, GDB_REGNO_TDATA1, 0);
-					break;
-				case CSR_TDATA1_TYPE_MCONTROL6:
-					if (tdata1 & CSR_MCONTROL6_DMODE(riscv_xlen(target)))
-						riscv_set_register(target, GDB_REGNO_TDATA1, 0);
-					break;
-				case CSR_TDATA1_TYPE_ICOUNT:
-					if (tdata1 & CSR_ICOUNT_DMODE(riscv_xlen(target)))
-						riscv_set_register(target, GDB_REGNO_TDATA1, 0);
-					break;
-				case CSR_TDATA1_TYPE_ITRIGGER:
-					if (tdata1 & CSR_ITRIGGER_DMODE(riscv_xlen(target)))
-						riscv_set_register(target, GDB_REGNO_TDATA1, 0);
-					break;
-				case CSR_TDATA1_TYPE_ETRIGGER:
-					if (tdata1 & CSR_ETRIGGER_DMODE(riscv_xlen(target)))
-						riscv_set_register(target, GDB_REGNO_TDATA1, 0);
-					break;
-			}
-			r->trigger_tinfo[t] = 1 << type;
-		}
-		LOG_TARGET_DEBUG(target, "Trigger %u: supported types (mask) = 0x%08x", t, r->trigger_tinfo[t]);
+		if (disable_trigger_if_dmode(target, tdata1) != ERROR_OK)
+			return ERROR_FAIL;
 	}
 
-	riscv_set_register(target, GDB_REGNO_TSELECT, tselect);
-
-	LOG_TARGET_INFO(target, "Found %d triggers.", r->trigger_count);
+	if (riscv_set_register(target, GDB_REGNO_TSELECT, orig_tselect) != ERROR_OK)
+		return ERROR_FAIL;
 
+	r->triggers_enumerated = true;
+	r->trigger_count = t;
+	LOG_TARGET_INFO(target, "Found %d triggers", r->trigger_count);
 	return ERROR_OK;
 }