On 10/12/25 5:13 PM, Tao Tang wrote:
Modify the main MMIO handlers (smmu_write_mmio, smmu_read_mmio)
to determine the security state of the target register based on its
memory-mapped offset.
By checking if the offset is within the secure register space (>=
SMMU_SECURE_REG_START), the handlers can deduce the register's
SEC_SID (reg_sec_sid). This SID is then passed down to the register
access helper functions (smmu_writel, smmu_readl, etc.).
Inside these helpers, the switch statement now operates on a masked,
relative offset:
uint32_t reg_offset = offset & 0xfff;
switch (reg_offset) {
...
}
This design leverages a key feature of the SMMU specification: registers
with the same function across different security states
(Non-secure, Realm, Root) share the same relative offset. This avoids
significant code duplication. The reg_sec_sid passed from the MMIO
handler determines which security bank to operate on, while the masked
offset identifies the specific register within that bank.
It is important to distinguish between the security state of the
register itself and the security state of the access. A
higher-privilege security state is permitted to access registers
belonging to a lower-privilege state, but the reverse is not allowed.
This patch lays the groundwork for enforcing such rules.
For future compatibility with Realm and Root states, the logic in the
else block corresponding to the secure offset check:
if (offset >= SMMU_SECURE_REG_START) {
reg_sec_sid = SMMU_SEC_SID_S;
} else {
/* Future Realm/Root handling */
}
will need to be expanded. This will be necessary to differentiate
between the Root Control Page and Realm Register Pages, especially since
the Root Control Page is IMPLEMENTATION DEFINED.
Signed-off-by: Tao Tang <[email protected]>
---
hw/arm/smmuv3.c | 61 ++++++++++++++++++++++++++++++++++++-------------
1 file changed, 45 insertions(+), 16 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index f161dd3eff..100caeeb35 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
------------------------------<snip>------------------------------
------------------------------<snip>------------------------------
bank->cr[0] = data;
bank->cr0ack = data & ~SMMU_CR0_RESERVED;
@@ -1985,12 +1987,33 @@ static MemTxResult smmu_write_mmio(void *opaque, hwaddr
offset, uint64_t data,
/* CONSTRAINED UNPREDICTABLE choice to have page0/1 be exact aliases */
offset &= ~0x10000;
+ SMMUSecSID reg_sec_sid = SMMU_SEC_SID_NS;
+ /*
+ * The security state of the access (MemTxAttrs attrs) may differ from the
+ * security state to which the register belongs. Future support will
include
+ * Realm and Root security states.
+ *
+ * The SMMU architecture specifies that Realm, Root, and Non-secure
+ * registers share the same offset layout within the last 4 hexadecimal
+ * digits (16 bits) of the address. Only Secure state registers are
+ * mapped to a higher address space, starting from
+ * SMMU_SECURE_REG_START (0x8000).
+ *
+ * Therefore, we can directly use the offset to distinguish Secure
+ * registers. When Realm and Root support is needed in the future, we
+ * only need to enhance the 'else' block of the corresponding 'if'
+ * statement to handle those specific security states.
I wouldn't add that many references to the Realm and Root support in the
very code. Maybe move that in the commit desc.
------------------------------<snip>------------------------------
------------------------------<snip>------------------------------
static MemTxResult smmu_readl(SMMUv3State *s, hwaddr offset,
- uint64_t *data, MemTxAttrs attrs)
+ uint64_t *data, MemTxAttrs attrs,
+ SMMUSecSID reg_sec_sid)
{
- SMMUSecSID reg_sec_sid = SMMU_SEC_SID_NS;
SMMUv3RegBank *bank = smmuv3_bank(s, reg_sec_sid);
+ uint32_t reg_offset = offset & 0xfff;
- switch (offset) {
+ switch (reg_offset) {
case A_IDREGS ... A_IDREGS + 0x2f:
*data = smmuv3_idreg(offset - A_IDREGS);
return MEMTX_OK;
@@ -2165,13 +2190,17 @@ static MemTxResult smmu_read_mmio(void *opaque, hwaddr
offset, uint64_t *data,
/* CONSTRAINED UNPREDICTABLE choice to have page0/1 be exact aliases */
offset &= ~0x10000;
+ SMMUSecSID reg_sec_sid = SMMU_SEC_SID_NS;
declaration should be at the beginning of the function, ie before the
offset setting
