bucaps/marss-riscv

ISA compatibility issue?

weedmank opened this issue · 1 comments

in the src/riscv_cpu.c file
case 0xc00: /* ucycle /
case 0xc02: /
uinstret */
{
uint32_t counteren;
if (s->priv < PRV_M) {
if (s->priv < PRV_S)
counteren = s->scounteren;
else
counteren = s->mcounteren;
if (((counteren >> (csr & 0x1f)) & 1) == 0)
goto invalid_csr;
}
}
val = (int64_t)s->insn_counter;
break;

Assume s->priv is User mode and extension S is not used. Code above would select
counteren = s->scounteren;
which should not exist because there is no extension S
from p 34 of riscv-priveleged.pdf, says

"When the CY, TM, IR, or HPMn bit in the mcounteren register is clear, attempts to read the
cycle, time, instret, or hpmcountern register while executing in S-mode or U-mode will cause
an illegal instruction exception. When one of these bits is set, access to the corresponding register
is permitted in the next implemented privilege mode (S-mode if implemented, otherwise U-mode).

If S-mode is implemented, the same bit positions in the scounteren register analogously control
access to these registers while executing in U-mode. If S-mode is permitted to access a counter
register and the corresponding bit is set in scounteren, then U-mode is also permitted to access
that register."

The concern here is what to select while in user mode, when only M and U extensions exist, and let's say CY bit is 1 in the Cycle register. It seems that mcounteren would be selected

gktrk commented

Your analysis appear to be correct. However, when the spec says "S-mode if implemented", it doesn't refer to whether the software stack makes use of the S-mode or not. It specifies whether the hardware has support for it or not. The support for S and U modes can be checked through the Extensions field of misa (Figure 3.1 and Table 3.2 of RISC-V Privileged ISA spec version "20190608-Priv-MSU-Ratified"). You can see in https://github.com/bucaps/marss-riscv/blob/master/src/riscv_cpu.c#L1421 that marss-riscv unconditionally sets the bits for U-mode and S-mode support:

    s->misa |= MCPUID_SUPER | MCPUID_USER | MCPUID_I | MCPUID_M | MCPUID_A;

The implication is that there is always support for supervisor and user modes. Unfortunately, there is no way to disable support for S and U modes currently. I'm not aware of any efforts by our team to implement it. Patches are always welcome ;)

Hope this clears the confusion a bit.