opentitan: Add ROM_EXT-compatible ePMP setup. #4034
Conversation
The existing ePMP code was developed and tested in environments that use a test ROM. On the real silicon, we instead use a ROM_EXT, which passes a different ePMP config to Tock than the test ROM. In particular, it: 1. Sometimes configures and locks entries 0 and 1 (depends on which ROM_EXT is in use). 2. Sets up several entries in a manner that works for Tock, but without the lock bit set. 3. Disables the Rule Lock Bypass bit (this breaks the dance the current constructor does with the MMIO region). This ePMP config is documented at https://opentitan.org/book/sw/device/silicon_creator/rom_ext/doc/si_val.html#epmp. This PR renames the existing EarlGreyEPMP constructor to `new_test_rom`, and introduces a new constructor for use with a ROM_EXT, called simply `new`. This constructor makes use of the ROM_EXT's existing layout to make ePMP configuration much simpler; it pretty much just locks down the ROM_EXT's config in place. This has been tested and verified to work on OpenTitan silicon.
There was a problem hiding this comment.
This looks great to me, generally. Just one minor nit with the mseccfg documentation.
Also, I think that we should update the constructor of new_test_rom to explicitly lock down regions 2 & 3, now that they're unused (in order to be compatible to the regular new), like so:
// Set the appropriate `pmpcfg0` register value:
//
// 0x80 = 0b10000000, for start address of the kernel .text TOR entry
// setting L(7) = 1, A(4-3) = OFF, X(2) = 0, W(1) = 0, R(0) = 0
//
// 0x8d = 0b10001101, for kernel .text TOR region
// setting L(7) = 1, A(4-3) = TOR, X(2) = 1, W(1) = 0, R(0) = 1
+ //
+ // 0x80 = 0b10000000, for disabled padding regions 2 & 3 (to be
+ // compatible with the non test-rom constructor)
+ // setting L(7) = 1, A(4-3) = OFF, X(2) = 0, W(1) = 0, R(0) = 0
- csr::CSR.pmpcfg0.set(0x00008d80);
+ csr::CSR.pmpcfg0.set(0x80808d80);
chips/earlgrey/src/epmp.rs
Outdated
| // ---------- PMP machine CSRs configured, lock down the system | ||
|
|
||
| // Finally, unset the rule-lock bypass (RLB) bit. If we don't have a | ||
| // debug memory region provided, further set machine-mode lockdown (we | ||
| // can't enable MML and also have a R/W/X region). We also set MMWP for | ||
| 8000 | // good measure, but that shouldn't make a difference -- it can't be | |
| // cleared anyways as it is a sticky bit. | ||
| // | ||
| // Unsetting RLB with at least one locked region will mean that we can't | ||
| // set it again, thus actually enforcing the region lock bits. | ||
| // | ||
| // Set RLB(2) = 0, MMWP(1) = 1, MML(0) = 1 |
There was a problem hiding this comment.
From what I understand, the new ROM_EXT will already have this exact(?) mseccfg CSR value set for us. If that is the case, I'm by no means opposed to keeping this write in to make this explicit, but we might want to update the comment documenting this expectation.
There was a problem hiding this comment.
The ROM_EXT I'm working with actually sets 0x2 (MMWP, but not MML or RLB). I've updated the documentation to just say we're enabling MML.
|
We'll probably also want to update the in-tree board to use the |
…be compatible with the rom_ext constructor, and reviewer-suggested doc fixes.
Done.
It currently uses |
|
Question: does this (and if so how) impact the qemu virtualized opentitan? |
I renamed The only change to |
|
@alevy One other insight is that (upstream) QEMU does not faithfully emulate any of OpenTitan's ROMs right now. So on QEMU, the default constructors that we have will fail their pre-configuration checks, unless we pass an option that explicitly disables these (at risk of ending up with a configuration that's not secure or soft-bricks the chip, depending on the exact hardware configuration that we start with). |
The existing ePMP code was developed and tested in environments that use a test ROM. On the real silicon, we instead use a ROM_EXT, which passes a different ePMP config to Tock than the test ROM. In particular, it:
This ePMP config is documented at https://opentitan.org/book/sw/device/silicon_creator/rom_ext/doc/si_val.html#epmp.
This PR renames the existing EarlGreyEPMP constructor to
new_test_rom, and introduces a new constructor for use with a ROM_EXT, called simplynew. This constructor makes use of the ROM_EXT's existing layout to make ePMP configuration much simpler; it pretty much just locks down the ROM_EXT's config in place.This has been tested and verified to work on OpenTitan silicon.