seed CSR: illegal-instruction raised for legal CSRRS/CSRRC/CSRRSI/CSRRCI access forms (regression from #218)

[SolAstrius]

Summary

The seed CSR (Zkr entropy source, addr 0x015) over-restricts which CSR instructions may access it. The handler raises an illegal-instruction exception for every access form except CSRRW/CSRRWI, but the spec only forbids the read-only set/clear forms. This is a conformance regression introduced in #218.

Where

src/cpu/riscv_csr.c, riscv_csr_seed() (added in #218):

static inline bool riscv_csr_seed(rvvm_hart_t* vm, rvvm_uxlen_t* dest, uint8_t op)
{
    // Zkr: SEED must be accessed via CSR read-write instruction form
    if (op != CSR_SWAP) {
        return false;   // -> riscv_illegal_insn()
    }
    ...
}

What the spec actually says

RISC-V Unprivileged ISA, §11.1.4.1 "The seed CSR":

Attempts to access the seed CSR using a read-only CSR-access instruction (CSRRS/CSRRC with rs1=x0 or CSRRSI/CSRRCI with uimm=0) raise an illegal-instruction exception; any other CSR-access instruction may be used to access seed. The write value (in rs1 or uimm) must be ignored by implementations.

So the only illegal forms are:

• CSRRS/CSRRC with rs1 == x0
• CSRRSI/CSRRCI with uimm == 0

CSRRS/CSRRC/CSRRSI/CSRRCI with a nonzero operand are explicitly legal — they perform a write (the value is just ignored), so they may access seed.

The bug

op != CSR_SWAP rejects all of CSRRS/CSRRC/CSRRSI/CSRRCI, including the legal nonzero forms. Concretely, with t1 != 0:

csrrs t0, seed, t1   # legal per spec, returns entropy in t0, write value ignored

currently traps as an illegal instruction. #218 traded the pre-existing under-restriction (read-only forms used to succeed) for an over-restriction (legal writing forms now fail).

Why op alone can't fix it

The dispatcher decodes CSRRS/CSRRC to CSR_SETBITS/CSR_CLEARBITS with val = registers[rs1] and never tells the handler whether rs1 == x0:

case 0x02: { // csrrs
    rvvm_uxlen_t val = vm->registers[rs1];
    if (riscv_csr_op(vm, csr, &val, CSR_SETBITS)) { ... }
}

So from op alone the handler cannot distinguish the read-only csrrs seed, x0 (which should trap) from csrrs seed, x5 (which should not). The "is this a read-only access" signal has to come from the decode site (rs1 == 0 for set/clear, uimm == 0 for the immediate forms), not from a check on op.

Suggested fix

Plumb a read-only flag from the decode site and gate on that instead of on op:

• CSRRW/CSRRWI: always a write -> legal.
• CSRRS/CSRRC with rs1 == x0, CSRRSI/CSRRCI with uimm == 0: read-only -> illegal.
• everything else: legal.

Severity

Low. seed is Zkr-only and the overwhelmingly common access is CSRRW rd, seed, x0, so real guests rarely hit the affected forms. It is still a spec violation: any code that reads seed via a nonzero CSRRS/CSRRC/CSRRSI/CSRRCI will now take an unexpected illegal-instruction trap.