microsoft / cheriot-ibex

cheriot-ibex is a RTL implementation of CHERIoT ISA based on LowRISC's Ibex core.
Apache License 2.0
73 stars 14 forks source link

tvec_addr base alignment #15

Closed mndstrmr closed 5 months ago

mndstrmr commented 11 months ago

In the instruction fetch stage of CherIoT-ibex the trap address for exceptions (for example) is calculated like so:

exc_pc = csr_mtvec_i[0] ? { csr_mtvec_i[31:8], 8'h00 } : {csr_mtvec_i[31:2], 2'b00};

According to the Sail however the trap vector address should be base (i.e. just {csr_mtvec_i[31:2], 2'b00}) for both TV_Direct and TV_Vector (i.e. regardless of csr_mtvec_i[0]).

The same issue is present for interrupts.

The RISC-V spec suggests that stricter alignment requirements are allowed, so in this case I would suggest that CherIoT-ibex is correct, and the Sail needs updating to reflect that that is OK.

A correct implementation of the Sail function tvec_addr would be

function tvec_addr(m : Mtvec, c : Mcause) -> option(xlenbits) = {
  let base : xlenbits = m.Base() @ 0b00;
  match (trapVectorMode_of_bits(m.Mode())) {
    TV_Direct => Some(base),
    TV_Vector => if   c.IsInterrupt() == 0b1
                 then Some(base[sizeof(xlen) - 1 .. 8] @ 0b0 @ c.Cause()[4..0] @ 0b00)
                 else Some(base[sizeof(xlen) - 1 .. 8] @ 0x00),
    TV_Reserved => None()
  }
}
rmn30 commented 11 months ago

Hmmm, so this is also an issue in the upstream RISC-V Sail model? Might be worth reporting there if so.

As per #10 there's still the issue of potentially creating an unrepresentable PCC which is a bit awkward. Seems like another reason not to support vectored interrupts...

kliuMsft commented 6 months ago

See comments in issue #10. We now only allow mtvec[1:0] == 2'b00, otherwise tag will be cleared at legalization (cspecialw) time.

mndstrmr commented 5 months ago

Thanks, looks good to me.