Skip to content

Commit

Permalink
target/riscv: check misa value before reporting
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
en-sc committed Sep 5, 2024
1 parent 909bbb8 commit 6c021da
Showing 1 changed file with 90 additions and 8 deletions.
98 changes: 90 additions & 8 deletions src/target/riscv/riscv-013_reg.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#endif

#include "riscv-013_reg.h"
#include "field_helpers.h"

#include "riscv_reg.h"
#include "riscv_reg_impl.h"
Expand Down Expand Up @@ -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);
Expand All @@ -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)
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit 6c021da

Please sign in to comment.