riscv-software-src / riscv-isa-sim

Spike, a RISC-V ISA Simulator
Other
2.45k stars 861 forks source link

PMP backward-compatible defaults could be misleading #1850

Open caizixian opened 3 weeks ago

caizixian commented 3 weeks ago

According to the privileged spec (Section 3.7.1.3 of version 20240411)

If at least one PMP entry is implemented, but all PMP entries' A fields are set to OFF, then all S-mode and U-mode memory accesses will fail.

Currently Spike initializes PMP permissively. https://github.com/riscv-software-src/riscv-isa-sim/blob/fd0a927d5bb799a278a47980a7f6f984b1071035/riscv/processor.cc#L196

This could lead to software developers debugging against Spike ignoring the need to program PMP (I made that mistake today, and the same executable that works on Spike doesn't on Rocket Chip).

aswaterman commented 3 weeks ago

As a reminder, Spike's behavior is explicitly legal, because the Reset section in the Machine Mode chapter allows for platform-specific reset values.

I acknowledge that this property caused you to waste some time, but I can assure you that reversing this property will result in quite a bit more friction. Still, we should hear what others have to say. cc @jerryz123

caizixian commented 3 weeks ago

Thanks for the clarification @aswaterman . I actually had a similar problem shortly after: Rocket Chip uses Reg for mideleg and medeleg, and I got some nondeterministic U-mode ecall behavior on Verilator, when sometimes user ecall is delegated to uninitialized bogus stvec. You are right that the Reset section permits such behaviors.