how to change pmp granularity

[neelgala]

I wanted the pmps to have an 8-byte granularity. I have made the following changes but it still picks up 4 byte granularity:

Changed 4 to 8 below: https://github.com/riscv-software-src/riscv-isa-sim/blob/82c5d885d14fbbb81a856752aa4668333ce8094c/riscv/dts.cc#L60

Change 2 to 3 below: https://github.com/riscv-software-src/riscv-isa-sim/blob/82c5d885d14fbbb81a856752aa4668333ce8094c/riscv/encoding.h#L188

My expectation from the above is that if I write a value of 0x003fffffffffffff to pmpaddr the value stored should be 0x003ffffffffffffe but what I get is 0x001fffffffffffff

What more should I do ?

[neelgala]

I could be wrong but shouldn't the following : https://github.com/riscv-software-src/riscv-isa-sim/blob/82c5d885d14fbbb81a856752aa4668333ce8094c/riscv/csrs.cc#L122

be something like:

this->val = val & ((reg_t(1) << (MAX_PADDR_BITS)) - 1) & ( -1 << (PMP_SHIFT-2));

[aswaterman]

The current version looks correct to me, since the addresses are right-shifted in the CSR.

[aswaterman]

You should never change PMP_SHIFT. Even coarse-grained PMPs always use a right-shift of 2.

[neelgala]

The sections 3.7.1 of the privilege spec has the following commentary:

Software may determine the PMP granularity by writing zero to pmp0cfg, then writing all ones
to pmpaddr0, then reading back pmpaddr0. If G is the index of the least-significant bit set, the
PMP granularity is 2 G+2 bytes.

So when the pmpaddr is read by software it should return 0x003ffffffffffffe for the case I described above. so even if pmpaddr stores 0x003fffffffffffff (with PMP_SHIFT==2) there should be some sanitization done on the read-csr path.. which isn't there at this point.

Opensbi uses this software approach to identify granularity as well : https://github.com/riscv-software-src/opensbi/blob/master/lib/sbi/sbi_hart.c#L446

[scottj97]

@neelgala I have successfully changed the PMP granularity (to 4KB) by changing only the riscv,pmpgranularity number in dts.cc.

I tried a value of 8 just now, and I see the expected result in pmpaddr0 of 0x003ffffffffffffe.

[scottj97]

There is sanitization on the read-csr path: pmp_tor_mask

[aswaterman]

@neelgala I am not disputing what the ISA spec says. I am merely saying that PMP_SHIFT is not the correct way to control PMP granularity in Spike. As Scott says, changing riscv,pmpgranularity but leaving PMP_SHIFT constant is the correct approach.

[neelgala]

This is wierd.. I started from scratch and changed just the riscv,pmpgranularity to 8 in dts.cc as scott suggested and I still see the same error. Is the reset of value of pmpcfg0 0x0? Because opensbi directly reads the pmpaddr0 without setting pmpcfg0. would that be an issue ?

@scottj97 did you setup the pmpcfgs differently to read 0x003ffffffffffffe in pmpaddr0 ?

[neelgala]

Got it.. so the reset value of all pmpcfgs is NAPOT has done here. And I think opensbi assumes the reset value of pmpcfgs to be 0x0 and directly probes pmpaddr csrs hence the issue.

And I also realized from the spec that reset values for PMP can be implementation defined. Thanks for the help.

[scottj97]

And I think opensbi assumes the reset value of pmpcfgs to be 0x0 and directly probes pmpaddr csrs hence the issue.

Sounds like a bug in OpenSBI!

Glad you solved the issue.