PmpPluginOld: fix NAPOT address calculation overflow issue

Because pmpaddrX registers are defined to encode the address'
[XLEN + 2 downto 2] bits, the length of a NAPOT region is defined
through the most significant 0 bit in a pmpaddrX register (which in
the case of ~0 is the 33rd non-existant "virtual" bit), and the
VexRiscv PmpOld plugin represents the addresses covered by a region as
[start; end) (bounded inclusively below and exclusively above), the
start and end address registers need to be XLEN + 4 bit wide to avoid
overflows.

If such an overflow occurs, it may be that the region does not cover
any address, an issue uncovered in the Tock LiteX + VexRiscv CI during
a PMP infrastructure redesign in the Tock OS [1].

This commit has been tested on Tock's redesigned PMP infrastructure,
and by inspecting all of the intermediate signals in the PMP address
calculation through a Verilator trace file. It works correctly for
various NAPOT and TOR addresses, and I made sure that the edge cases
of pmpaddrX = [0x00000000, 0x7FFFFFFF, 0xFFFFFFFF] are all handled.

[1]: https://github.com/tock/tock/pull/3597
This commit is contained in:
Leon Schuermann 2023-11-03 09:11:42 -04:00
parent b6f6120ec6
commit 9baba6d11f
1 changed files with 30 additions and 5 deletions

View File

@ -98,7 +98,29 @@ case class PmpRegister(previous : PmpRegister) extends Area {
// Computed PMP region bounds // Computed PMP region bounds
val region = new Area { val region = new Area {
val valid, locked = Bool val valid, locked = Bool
val start, end = UInt(32 bits)
// The calculated start & end addresses can overflow xlen by 4 bit:
//
// - 2 bit, as the pmpaddrX registers are defined as to encode
// [XLEN + 2 downto 2] addresses.
//
// - 2 bit, as for NAPOT the most significant 0 bit encodes the region
// length, with this bit included in the range!
//
// This means that (for xlen == 32 bit)
//
// pmpcfg(X / 4)(X % 4) = NAPOT
// pmpaddrX = 0xFFFFFFFF
//
// will expand to
//
// start (inclusive): 0x000000000 << 2
// end (exclusive): 0x200000000 << 2
//
// hence requiring xlen + 2 + 2 bit to represent the exclusive end
// address. This could be optimized by using a saturating add, or making the
// end address exclusive.
val start, end = UInt(36 bits)
} }
when(~state.l) { when(~state.l) {
@ -114,9 +136,12 @@ case class PmpRegister(previous : PmpRegister) extends Area {
} }
} }
val shifted = state.addr |<< 2 // Extend state.addr to 36 bits, to avoid these computations overflowing (as
val mask = state.addr & ~(state.addr + 1) // explained above):
val masked = (state.addr & ~mask) |<< 2 val extended_addr = (B"00" ## state.addr.asBits).asUInt
val shifted = extended_addr << 2
val mask = extended_addr ^ (extended_addr + 1)
val masked = (extended_addr & ~mask) << 2
// PMP changes take effect two clock cycles after the initial CSR write (i.e., // PMP changes take effect two clock cycles after the initial CSR write (i.e.,
// settings propagate from csr -> state -> region). // settings propagate from csr -> state -> region).
@ -135,7 +160,7 @@ case class PmpRegister(previous : PmpRegister) extends Area {
} }
is(NAPOT) { is(NAPOT) {
region.start := masked region.start := masked
region.end := masked + ((mask + 1) |<< 3) region.end := masked + ((mask + 1) << 2)
} }
default { default {
region.start := 0 region.start := 0