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 <evgeniy.naydanov@syntacore.com>
  • Loading branch information
en-sc committed Aug 9, 2024
1 parent f7e415f commit 0cbe4dc
Showing 1 changed file with 60 additions and 8 deletions.
68 changes: 60 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,64 @@ 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
};

enum misa_mxl xlen_to_mxl(unsigned int xlen)
{
switch (xlen) {
case 32:
return MISA_MXL_32;
case 64:
return MISA_MXL_64;
case 128:
return MISA_MXL_128;
}
return MISA_MXL_INVALID;
}

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 xlen = riscv_xlen(target);
assert(xlen <= sizeof(riscv_reg_t) * CHAR_BIT);
assert(xlen >= 2);
riscv_reg_t misa_mxl_field = (riscv_reg_t)0x3 << (xlen - 2);
const unsigned int mxl = get_field(r->misa, misa_mxl_field);
/* RISC-V Debug Spec. requires "DXLEN" to be the widest. */
if (mxl == MISA_MXL_INVALID || xlen_to_mxl(xlen) < mxl) {
LOG_TARGET_ERROR(target,
"'misa.mxl' field's value of %u does not "
" coincide with 'xlen' of %u.",
mxl, xlen);
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, "actual_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 +243,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 +284,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 +309,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 0cbe4dc

Please sign in to comment.