Re: [PATCH 0/1] SBI: Introduce Physical Memory Protection Extension
mick@...
Hello all,
The assumption that there'll always be a PMP rule for marking the firmware's memory isn't always true. M-mode can access any region without a matching PMP rule so basically the firmware can be anywhere and work fine, without any PMP rule marking its region. With the current PMP spec this is probably the best approach since it will also prevent S-mode from ever accessing this region (S-mode will fail if there is no matching PMP rule). Why waste a rule for the firmware's region to deny any access from S-mode, when you can have the same result by not putting that rule on PMP ? Even if that assumption was always true, what about systems without PMP, systems with a different number of PMP regions, or systems with custom PMP-equivalent solutions ? Trap-n-emulate is not that generic (e.g. in the case of different number of PMP regions). Any interface from S-mode to PMP or a PMP-equivalent mechanism, should be more abstract than this IMHO.
We should also consider the security implications of exposing the PMP configuration to a less privileged mode. It's one thing to let S-mode know about the DRAM regions that it can't touch because the firmware is there, and another to reveal the full PMP configuration. PMP may contain settings that S-mode doesn't even know about, devices that are not on the device tree for example or DRAM regions that are outside the device tree's /memory node range (so S-mode won't touch them anyway). I'm not a fan of security through obscurity but there are good reasons to keep PMP configuration accessible only to M-mode (in addition to having a more abstract API with S-mode to be able to handle the various system configurations / implementations). We can have S-mode request M-mode to protect specific physical regions that S-mode manages through PMP (or a vendor-specific PMP-equivalent mechanism) but anything more than that doesn't make much sense to me.
If we just want to prevent S-mode from touching firmware's memory we can either exclude its region from the /memory node on the device tree, or let the firmware modify the device tree it passes on to the next-stage boot loader and add a reserved-memory node. I think that's the best approach since it's the most generic one and it's standards-compliant, plus the device-tree is platform-specific anyway, even if it's included on the kernel blob directly without passing through OpenSBI or whatever boot loader is there, we can always add there a reserved region, or modify the /memory node manually.
Regards,
Nick
Let's continue the discussion here as it has a wider audience than
github PR. I have also cc'd all the possible stakeholders.
The other alternatives propsed so far
1. Just parse the deveice tree node in S-mode software.
Pros: No additional SBI extensions are required.
Cons: U-Boot is responsible for copying the reserved-memory node from
the DT passed by OpenSBI and set it to the final DT if it is a
different one. OpenSBI also need to provide the DT where the previous
stage (FSBL/U-Boot SPL) doesn't provide a DT.
IMHO, this is not a very difficult problem to solve. The U-Boot
implementation is available here (just ~40 lines of code).
https://patchwork.ozlabs.org/patch/1254740/
2. Trap-n-Emulate PMP CSR reads from S-mode. The details are available
here.
https://github.com/riscv/riscv-sbi-doc/pull/37#issuecomment-596944084
Pros: No SBI extension or prior DTB fixup required in U-Boot.
Cons: As PMP extensions proposals are already in place, this may need
to be extended. Privilege spec may need to be modified to explicitly
say that M-mode can provide PMP csr emulation.
Any thoughts ?
The assumption that there'll always be a PMP rule for marking the firmware's memory isn't always true. M-mode can access any region without a matching PMP rule so basically the firmware can be anywhere and work fine, without any PMP rule marking its region. With the current PMP spec this is probably the best approach since it will also prevent S-mode from ever accessing this region (S-mode will fail if there is no matching PMP rule). Why waste a rule for the firmware's region to deny any access from S-mode, when you can have the same result by not putting that rule on PMP ? Even if that assumption was always true, what about systems without PMP, systems with a different number of PMP regions, or systems with custom PMP-equivalent solutions ? Trap-n-emulate is not that generic (e.g. in the case of different number of PMP regions). Any interface from S-mode to PMP or a PMP-equivalent mechanism, should be more abstract than this IMHO.
We should also consider the security implications of exposing the PMP configuration to a less privileged mode. It's one thing to let S-mode know about the DRAM regions that it can't touch because the firmware is there, and another to reveal the full PMP configuration. PMP may contain settings that S-mode doesn't even know about, devices that are not on the device tree for example or DRAM regions that are outside the device tree's /memory node range (so S-mode won't touch them anyway). I'm not a fan of security through obscurity but there are good reasons to keep PMP configuration accessible only to M-mode (in addition to having a more abstract API with S-mode to be able to handle the various system configurations / implementations). We can have S-mode request M-mode to protect specific physical regions that S-mode manages through PMP (or a vendor-specific PMP-equivalent mechanism) but anything more than that doesn't make much sense to me.
If we just want to prevent S-mode from touching firmware's memory we can either exclude its region from the /memory node on the device tree, or let the firmware modify the device tree it passes on to the next-stage boot loader and add a reserved-memory node. I think that's the best approach since it's the most generic one and it's standards-compliant, plus the device-tree is platform-specific anyway, even if it's included on the kernel blob directly without passing through OpenSBI or whatever boot loader is there, we can always add there a reserved region, or modify the /memory node manually.
Regards,
Nick