sail-riscv
sail-riscv copied to clipboard
Check the pmpcfg_n cause segmentation fault
When I rebased #601 to the latest version, I found that checking if the L field in the Pmpcfg register was set caused a segmentation fault.
$ ./build/c_emulator/sail_riscv_sim build/test/2025-06-22/riscv-tests/rv64mi-p-ma_addr
Running file build/test/2025-06-22/riscv-tests/rv64mi-p-ma_addr.
Segmentation fault (core dumped) ./build/c_emulator/sail_riscv_sim build/test/2025-06-22/riscv-tests/rv64mi-p-ma_addr
These are the functions I checked in model/riscv_pmp_regs:
function checkPmpNcfgL() -> bool = {
foreach (i from 0 to 63) {
let test = pmpcfg_n[i];
if pmpLocked(test) then return true
};
false
}
I call this function in model/riscv_sys_regs as follows:
val checkPmpNcfgL : unit -> bool
function legalize_mseccfg(o : Seccfg, v : bits(64)) -> Seccfg = {
let sseed_read_only_zero =
(config extensions.Zkr.sseed_read_only_zero : bool)
| not(currentlyEnabled(Ext_S))
| not(currentlyEnabled(Ext_Zkr));
let useed_read_only_zero =
(config extensions.Zkr.useed_read_only_zero : bool)
| not(currentlyEnabled(Ext_U))
| not(currentlyEnabled(Ext_Zkr));
let rlb_read_only_zero =
not(currentlyEnabled(Ext_Smepmp))
| (o[RLB] == 0b0 & checkPmpNcfgL());
let mml_unable_to_set =
not(currentlyEnabled(Ext_Smepmp))
| (o[MML] == 0b1);
let mmwp_unable_to_set =
not(currentlyEnabled(Ext_Smepmp))
| (o[MMWP] == 0b1);
let v = Mk_Seccfg(v);
[o with
SSEED = if sseed_read_only_zero then 0b0 else v[SSEED],
USEED = if useed_read_only_zero then 0b0 else v[USEED],
RLB = if rlb_read_only_zero then 0b0 else v[RLB],
MMWP = if mmwp_unable_to_set then o[MMWP] else v[MMWP],
MML = if mml_unable_to_set then o[MML] else v[MML],
]
}
Can anyone share similar experiences to help me out?
Nothing you do in Sail should trigger a segmentation fault. Did you push the code that triggers this to https://github.com/riscv/sail-riscv/pull/601 already? I can try to reproduce it if so.
Otherwise it might be useful to build with debug info, and try using address sanitiser or valgrind to get more information.
yes, then the CI told me that every test false because segmentation fault and I have found that it's such code that cause the fault.
I can reproduce this. The problem is the code is this:
register mseccfg : Seccfg = legalize_mseccfg(Mk_Seccfg(zeros()), zeros())
In legalize_mseccfg() it calls checkPmpNcfgL(63) which eventually reads pmpcfg_n.
However if you look at model_init then the call to initialise mseccfg is before pmpcfg_n is initialised:
void model_init() {
...
zmseccfg = zlegalizze_mseccfg(z3zE1752, UINT64_C(0x0000000000000000));
...
zinitializze_registers(UNIT);
}
I'm kind of surprised we didn't run into this before. It seems to only happen if the register is a vector of bitfields; if it's just a vector of bits(..) then it gets initialised directly in model_init() rather than in zinitializze_registers().
Here's a smaller reproducer:
bitfield Pmpcfg_ent : bits(8) = {
L : 7,
}
register pmpcfg_n : vector(64, Pmpcfg_ent)
register foo : bits(8) = pmpcfg_n[63].bits
function main() -> unit = {
print_endline(bits_str(foo));
}
If you initialise the register then it works:
register pmpcfg_n : vector(64, Pmpcfg_ent) = vector_init(Mk_Pmpcfg_ent(zeros()))
Ok, I see the problem. Looks like we need to initialise registers in the correct dependency order? Probably a good time to re-factor the register initialisation logic anyway.
On the other hand here it is revealing an underlying bug, because I assume it isn't intended to use an undefined pmpcfg_n to determine the initial state of mseccfg.
Yeah maybe time to just require all registers to be initialised full stop. Still you'll probably need to deal with the order anyway I would have thought?
Not sure what you mean by register initialization, but there are very few bits that need iniitialization (e.g. PMP lock bits, mode bits, and mseccfg bits). but they should all be initialized before the model is out of reset and anything is being fetched. If that's not working, I'd expect some odd behavior - but not a segfault.
On Thu, Jul 24, 2025 at 2:16 PM Tim Hutt @.***> wrote:
Timmmm left a comment (riscv/sail-riscv#1162) https://github.com/riscv/sail-riscv/issues/1162#issuecomment-3114984272
Yeah maybe time to just require all registers to be initialised full stop. Still you'll probably need to deal with the order anyway I would have thought?
— Reply to this email directly, view it on GitHub https://github.com/riscv/sail-riscv/issues/1162#issuecomment-3114984272, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJV5X4HYZLTVU4I3QWT3KFEJPAVCNFSM6AAAAACCGDVQRWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCMJUHE4DIMRXGI . You are receiving this because you are subscribed to this thread.Message ID: @.***>