From 6c021da8cc53b43a9fb9a43af57027192a89c823 Mon Sep 17 00:00:00 2001
From: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
Date: Fri, 9 Aug 2024 16:13:39 +0300
Subject: [PATCH] target/riscv: check `misa` value before reporting

Currently, during register file examination:
1. A read of an XPR is attempted via 64-bit abstract access.
2. If such a read fails (e.g. connection unstable) XLEN is assumed to be
   32.
3. Then `misa` is read. Since `misa` is a CSR and it may be only
   readable via program buffer, `s0` should be readable beforehand (at
   least some assumption about `xlen` should be made).
4. Before the commit, the `misa.mxl` field was not checked against
   `xlen`, therefore erroneous info may have been reported to the user.
   Moreover, the `examine()` would pass indicating no error at all.
5. After the commit, `misa.mxl` is checked against `xlen` value.

Change-Id: I3fe5bd6742e564e6de782aad9ed10e65c0728923
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
---
 src/target/riscv/riscv-013_reg.c | 98 +++++++++++++++++++++++++++++---
 1 file changed, 90 insertions(+), 8 deletions(-)

diff --git a/src/target/riscv/riscv-013_reg.c b/src/target/riscv/riscv-013_reg.c
index ad1130d63..2d5229d0c 100644
--- a/src/target/riscv/riscv-013_reg.c
+++ b/src/target/riscv/riscv-013_reg.c
@@ -5,6 +5,7 @@
 #endif
 
 #include "riscv-013_reg.h"
+#include "field_helpers.h"
 
 #include "riscv_reg.h"
 #include "riscv_reg_impl.h"
@@ -173,6 +174,94 @@ static int examine_vlenb(struct target *target)
 	return ERROR_OK;
 }
 
+enum misa_mxl {
+	MISA_MXL_INVALID = 0,
+	MISA_MXL_32 = 1,
+	MISA_MXL_64 = 2,
+	MISA_MXL_128 = 3
+};
+
+unsigned int mxl_to_xlen(enum misa_mxl mxl)
+{
+	switch (mxl) {
+	case MISA_MXL_32:
+		return 32;
+	case MISA_MXL_64:
+		return 64;
+	case MISA_MXL_128:
+		return 128;
+	case MISA_MXL_INVALID:
+		assert(0);
+	}
+	return 0;
+}
+
+static int check_misa_mxl(const struct target *target)
+{
+	RISCV_INFO(r);
+
+	if (r->misa == 0) {
+		LOG_TARGET_WARNING(target, "'misa' register is read as zero."
+				"OpenOCD will not be able to determine some hart's capabilities.");
+		return ERROR_OK;
+	}
+	const unsigned int dxlen = riscv_xlen(target);
+	assert(dxlen <= sizeof(riscv_reg_t) * CHAR_BIT);
+	assert(dxlen >= 2);
+	const riscv_reg_t misa_mxl_mask = (riscv_reg_t)0x3 << (dxlen - 2);
+	const unsigned int mxl = get_field(r->misa, misa_mxl_mask);
+	if (mxl == MISA_MXL_INVALID) {
+		/* This is not an error!
+		 * Imagine the platform that:
+		 * - Has no abstract access to CSRs, so that CSRs are read
+		 *   through Program Buffer via "csrr" instruction.
+		 * - Complies to v1.10 of the Priveleged Spec, so that misa.mxl
+		 *   is WARL and MXLEN may be chainged.
+		 *   https://github.com/riscv/riscv-isa-manual/commit/9a7dd2fe29011587954560b5dcf1875477b27ad8
+		 * - DXLEN == MXLEN on reset == 64.
+		 * In a following scenario:
+		 * - misa.mxl was written, so that MXLEN is 32.
+		 * - Debugger connects to the target.
+		 * - Debugger observes DXLEN == 64.
+		 * - Debugger reads misa:
+		 *   - Abstract access fails with "cmderr == not supported".
+		 *   - Access via Program Buffer involves reading "misa" to an
+		 *     "xreg" via "csrr", so that the "xreg" is filled with
+		 *     zero-extended value of "misa" (since "misa" is
+		 *     MXLEN-wide).
+		 * - Debugger derives "misa.mxl" assumig "misa" is DXLEN-bit
+		 *   wide (64) while MXLEN is 32 and therefore erroneously
+		 *   assumes "misa.mxl" to be zero (invalid).
+		 */
+		LOG_TARGET_WARNING(target, "Detected DXLEN (%u) does not match "
+				"MXLEN: misa.mxl == 0, misa == 0x%" PRIx64 ".",
+				dxlen, r->misa);
+		return ERROR_OK;
+	}
+	const unsigned int mxlen = mxl_to_xlen(mxl);
+	if (dxlen < mxlen) {
+		LOG_TARGET_ERROR(target,
+				"MXLEN (%u) reported in misa.mxl field exceeds "
+				"the detected DXLEN (%u)",
+				mxlen, dxlen);
+		return ERROR_FAIL;
+	}
+	/* NOTE:
+	 * The value of "misa.mxl" may stil not coincide with "xlen".
+	 * "misa[26:XLEN-3]" bits are marked as WIRI in at least version 1.10
+	 * of RISC-V Priveleged Spec. Therefore, if "xlen" is erroneously
+	 * assumed to be 32 when it actually is 64, "mxl" will be read from
+	 * this WIRI field and may be equal to "MISA_MXL_32" by coincidence.
+	 * This is not an issue though from the version 1.11 onward, since
+	 * "misa[26:XLEN-3]" became WARL and equal to 0.
+	 */
+
+	/* Display this as early as possible to help people who are using
+	 * really slow simulators. */
+	LOG_TARGET_DEBUG(target, " XLEN=%d, misa=0x%" PRIx64, riscv_xlen(target), r->misa);
+	return ERROR_OK;
+}
+
 static int examine_misa(struct target *target)
 {
 	RISCV_INFO(r);
@@ -184,8 +273,7 @@ static int examine_misa(struct target *target)
 	res = riscv_reg_get(target, &r->misa, GDB_REGNO_MISA);
 	if (res != ERROR_OK)
 		return res;
-
-	return ERROR_OK;
+	return check_misa_mxl(target);
 }
 
 static int examine_mtopi(struct target *target)
@@ -226,8 +314,6 @@ static int examine_mtopi(struct target *target)
  */
 int riscv013_reg_examine_all(struct target *target)
 {
-	RISCV_INFO(r);
-
 	int res = riscv_reg_impl_init_cache(target);
 	if (res != ERROR_OK)
 		return res;
@@ -253,10 +339,6 @@ int riscv013_reg_examine_all(struct target *target)
 	if (res != ERROR_OK)
 		return res;
 
-	/* Display this as early as possible to help people who are using
-	 * really slow simulators. */
-	LOG_TARGET_DEBUG(target, " XLEN=%d, misa=0x%" PRIx64, riscv_xlen(target), r->misa);
-
 	res = examine_vlenb(target);
 	if (res != ERROR_OK)
 		return res;