The SMMUv3 model was missing checks for register accessibility under
certain conditions. This allowed guest software to write to registers
like STRTAB_BASE when they should be read-only, or read from
GERROR_IRQ_CFG registers when they should be RES0.
This patch fixes this by introducing helper functions, such as the
smmu_(reg_name)_writable pattern, to encapsulate the architectural
access rules. In addition, writes to registers like STRTAB_BASE,
queue bases, and IRQ configurations are now masked to correctly
handle reserved bits.
The MMIO handlers are updated to call these functions before accessing
registers. To purely fix the missing checks without introducing new
functionality, the security context in the MMIO handlers is explicitly
fixed to Non-secure. This ensures that the scope of this patch is
limited to fixing existing Non-secure logic.
If a register is not accessible, the access is now correctly handled
and a guest error is logged, bringing the model's behavior in line with
the specification.
Fixes: fae4be38b35d ("hw/arm/smmuv3: Implement MMIO write operations")
Fixes: 10a83cb9887e ("hw/arm/smmuv3: Skeleton")
Signed-off-by: Tao Tang <tangtao1...@phytium.com.cn>
---
hw/arm/smmuv3.c | 304 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 298 insertions(+), 6 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index f9395c3821..f161dd3eff 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1321,6 +1321,127 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd,
SMMUStage stage,
}
}
+static inline int smmuv3_get_cr0ack_smmuen(SMMUv3State *s, SMMUSecSID sec_sid)
+{
+ return FIELD_EX32(s->bank[sec_sid].cr0ack, CR0, SMMUEN);
+}
+
+static inline bool smmuv3_is_smmu_enabled(SMMUv3State *s, SMMUSecSID sec_sid)
+{
+ int cr0_smmuen = smmu_enabled(s, sec_sid);
+ int cr0ack_smmuen = smmuv3_get_cr0ack_smmuen(s, sec_sid);
+ return (cr0_smmuen == 0 && cr0ack_smmuen == 0);
+}
+
+/* Check if STRTAB_BASE register is writable */
+static bool smmu_strtab_base_writable(SMMUv3State *s, SMMUSecSID sec_sid)
+{
+ /* Check TABLES_PRESET - use NS bank as it's the global setting */
+ if (FIELD_EX32(s->bank[sec_sid].idr[1], IDR1, TABLES_PRESET)) {
+ return false;
+ }
+
+ /* Check SMMUEN conditions for the specific security domain */
+ return smmuv3_is_smmu_enabled(s, sec_sid);
+}
+
+static inline int smmuv3_get_cr0_cmdqen(SMMUv3State *s, SMMUSecSID sec_sid)
+{
+ return FIELD_EX32(s->bank[sec_sid].cr[0], CR0, CMDQEN);
+}
+
+static inline int smmuv3_get_cr0ack_cmdqen(SMMUv3State *s, SMMUSecSID sec_sid)
+{
+ return FIELD_EX32(s->bank[sec_sid].cr0ack, CR0, CMDQEN);
+}
+
+static inline int smmuv3_get_cr0_eventqen(SMMUv3State *s, SMMUSecSID sec_sid)
+{
+ return FIELD_EX32(s->bank[sec_sid].cr[0], CR0, EVENTQEN);
+}
+
+static inline int smmuv3_get_cr0ack_eventqen(SMMUv3State *s, SMMUSecSID
sec_sid)
+{
+ return FIELD_EX32(s->bank[sec_sid].cr0ack, CR0, EVENTQEN);
+}
+
+/* Check if MSI is supported */
+static inline bool smmu_msi_supported(SMMUv3State *s, SMMUSecSID sec_sid)
+{
+ return FIELD_EX32(s->bank[sec_sid].idr[0], IDR0, MSI);
+}
+
+/* Check if secure GERROR_IRQ_CFGx registers are writable */
+static inline bool smmu_gerror_irq_cfg_writable(SMMUv3State *s,
+ SMMUSecSID sec_sid)
+{
+ if (!smmu_msi_supported(s, sec_sid)) {
+ return false;
+ }
+
+ bool ctrl_en = FIELD_EX32(s->bank[sec_sid].irq_ctrl,
+ IRQ_CTRL, GERROR_IRQEN);
+ return !ctrl_en;
+}
+
+/* Check if CMDQEN is disabled */
+static bool smmu_cmdqen_disabled(SMMUv3State *s, SMMUSecSID sec_sid)
+{
+ int cr0_cmdqen = smmuv3_get_cr0_cmdqen(s, sec_sid);
+ int cr0ack_cmdqen = smmuv3_get_cr0ack_cmdqen(s, sec_sid);
+ return (cr0_cmdqen == 0 && cr0ack_cmdqen == 0);
+}
+
+/* Check if CMDQ_BASE register is writable */
+static bool smmu_cmdq_base_writable(SMMUv3State *s, SMMUSecSID sec_sid)
+{
+ /* Check TABLES_PRESET - use NS bank as it's the global setting */
+ if (FIELD_EX32(s->bank[sec_sid].idr[1], IDR1, QUEUES_PRESET)) {
+ return false;
+ }
+
+ return smmu_cmdqen_disabled(s, sec_sid);
+}
+
+/* Check if EVENTQEN is disabled */
+static bool smmu_eventqen_disabled(SMMUv3State *s, SMMUSecSID sec_sid)
+{
+ int cr0_eventqen = smmuv3_get_cr0_eventqen(s, sec_sid);
+ int cr0ack_eventqen = smmuv3_get_cr0ack_eventqen(s, sec_sid);
+ return (cr0_eventqen == 0 && cr0ack_eventqen == 0);
+}
+
+static bool smmu_idr1_queue_preset(SMMUv3State *s, SMMUSecSID sec_sid)
+{
+ return FIELD_EX32(s->bank[sec_sid].idr[1], IDR1, QUEUES_PRESET);
+}
+
+/* Check if EVENTQ_BASE register is writable */
+static bool smmu_eventq_base_writable(SMMUv3State *s, SMMUSecSID sec_sid)
+{
+ /* Check TABLES_PRESET - use NS bank as it's the global setting */
+ if (smmu_idr1_queue_preset(s, sec_sid)) {
+ return false;
+ }
+
+ return smmu_eventqen_disabled(s, sec_sid);
+}
+
+static bool smmu_irq_ctl_evtq_irqen_disabled(SMMUv3State *s, SMMUSecSID
sec_sid)
+{
+ return FIELD_EX32(s->bank[sec_sid].irq_ctrl, IRQ_CTRL, EVENTQ_IRQEN);
+}
+
+/* Check if EVENTQ_IRQ_CFGx is writable */
+static bool smmu_eventq_irq_cfg_writable(SMMUv3State *s, SMMUSecSID sec_sid)
+{
+ if (smmu_msi_supported(s, sec_sid)) {
+ return false;
+ }
+
+ return smmu_irq_ctl_evtq_irqen_disabled(s, sec_sid);
+}
+
static int smmuv3_cmdq_consume(SMMUv3State *s)
{
SMMUState *bs = ARM_SMMU(s);
@@ -1561,27 +1682,59 @@ static MemTxResult smmu_writell(SMMUv3State *s, hwaddr
offset,
switch (offset) {
case A_GERROR_IRQ_CFG0:
- bank->gerror_irq_cfg0 = data;
+ if (!smmu_gerror_irq_cfg_writable(s, reg_sec_sid)) {
+ /* SMMU_(*_)_IRQ_CTRL.GERROR_IRQEN == 1: IGNORED this write */
+ qemu_log_mask(LOG_GUEST_ERROR, "GERROR_IRQ_CFG0 write ignored: "
+ "register is RO when IRQ enabled\n");
+ return MEMTX_OK;
+ }
+
+ bank->gerror_irq_cfg0 = data & SMMU_GERROR_IRQ_CFG0_RESERVED;
return MEMTX_OK;
case A_STRTAB_BASE:
- bank->strtab_base = data;
+ if (!smmu_strtab_base_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "STRTAB_BASE write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
+ /* Clear reserved bits according to spec */
+ bank->strtab_base = data & SMMU_STRTAB_BASE_RESERVED;
return MEMTX_OK;
case A_CMDQ_BASE:
- bank->cmdq.base = data;
+ if (!smmu_cmdq_base_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "CMDQ_BASE write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
+ bank->cmdq.base = data & SMMU_QUEUE_BASE_RESERVED;
bank->cmdq.log2size = extract64(bank->cmdq.base, 0, 5);
if (bank->cmdq.log2size > SMMU_CMDQS) {
bank->cmdq.log2size = SMMU_CMDQS;
}
return MEMTX_OK;
case A_EVENTQ_BASE:
- bank->eventq.base = data;
+ if (!smmu_eventq_base_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "EVENTQ_BASE write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
+ bank->eventq.base = data & SMMU_QUEUE_BASE_RESERVED;
bank->eventq.log2size = extract64(bank->eventq.base, 0, 5);
if (bank->eventq.log2size > SMMU_EVENTQS) {
bank->eventq.log2size = SMMU_EVENTQS;
}
return MEMTX_OK;
case A_EVENTQ_IRQ_CFG0:
- bank->eventq_irq_cfg0 = data;
+ if (!smmu_eventq_irq_cfg_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "EVENTQ_IRQ_CFG0 write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
+ bank->eventq_irq_cfg0 = data & SMMU_EVENTQ_IRQ_CFG0_RESERVED;
return MEMTX_OK;
default:
qemu_log_mask(LOG_UNIMP,
@@ -1608,7 +1761,15 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr
offset,
bank->cr[1] = data;
return MEMTX_OK;
case A_CR2:
- bank->cr[2] = data;
+ if (smmuv3_is_smmu_enabled(s, reg_sec_sid)) {
+ /* Allow write: SMMUEN is 0 in both CR0 and CR0ACK */
+ bank->cr[2] = data;
+ } else {
+ /* CONSTRAINED UNPREDICTABLE behavior: Ignore this write */
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "CR2 write ignored: register is read-only when "
+ "CR0.SMMUEN or CR0ACK.SMMUEN is set\n");
+ }
return MEMTX_OK;
case A_IRQ_CTRL:
bank->irq_ctrl = data;
@@ -1622,12 +1783,31 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr
offset,
smmuv3_cmdq_consume(s);
return MEMTX_OK;
case A_GERROR_IRQ_CFG0: /* 64b */
+ if (!smmu_gerror_irq_cfg_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "GERROR_IRQ_CFG0 write ignored: "
+ "register is RO when IRQ enabled\n");
+ return MEMTX_OK;
+ }
+
+ data &= SMMU_GERROR_IRQ_CFG0_RESERVED;
bank->gerror_irq_cfg0 = deposit64(bank->gerror_irq_cfg0, 0, 32, data);
return MEMTX_OK;
case A_GERROR_IRQ_CFG0 + 4:
+ if (!smmu_gerror_irq_cfg_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "GERROR_IRQ_CFG0 + 4 write ignored:
"
+ "register is RO when IRQ enabled\n");
+ return MEMTX_OK;
+ }
+
bank->gerror_irq_cfg0 = deposit64(bank->gerror_irq_cfg0, 32, 32, data);
return MEMTX_OK;
case A_GERROR_IRQ_CFG1:
+ if (!smmu_gerror_irq_cfg_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "GERROR_IRQ_CFG1 write ignored: "
+ "register is RO when IRQ enabled\n");
+ return MEMTX_OK;
+ }
+
bank->gerror_irq_cfg1 = data;
return MEMTX_OK;
case A_GERROR_IRQ_CFG2:
@@ -1644,12 +1824,32 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr
offset,
}
return MEMTX_OK;
case A_STRTAB_BASE: /* 64b */
+ if (!smmu_strtab_base_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "STRTAB_BASE write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
+ data &= SMMU_STRTAB_BASE_RESERVED;
bank->strtab_base = deposit64(bank->strtab_base, 0, 32, data);
return MEMTX_OK;
case A_STRTAB_BASE + 4:
+ if (!smmu_strtab_base_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "STRTAB_BASE + 4 write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
+ data &= SMMU_STRTAB_BASE_RESERVED;
bank->strtab_base = deposit64(bank->strtab_base, 32, 32, data);
return MEMTX_OK;
case A_STRTAB_BASE_CFG:
+ if (!smmu_strtab_base_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "STRTAB_BASE_CFG write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
bank->strtab_base_cfg = data;
if (FIELD_EX32(data, STRTAB_BASE_CFG, FMT) == 1) {
bank->sid_split = FIELD_EX32(data, STRTAB_BASE_CFG, SPLIT);
@@ -1657,6 +1857,13 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr
offset,
}
return MEMTX_OK;
case A_CMDQ_BASE: /* 64b */
+ if (!smmu_cmdq_base_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "CMDQ_BASE write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
+ data &= SMMU_QUEUE_BASE_RESERVED;
bank->cmdq.base = deposit64(bank->cmdq.base, 0, 32, data);
bank->cmdq.log2size = extract64(bank->cmdq.base, 0, 5);
if (bank->cmdq.log2size > SMMU_CMDQS) {
@@ -1664,6 +1871,13 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr
offset,
}
return MEMTX_OK;
case A_CMDQ_BASE + 4: /* 64b */
+ if (!smmu_cmdq_base_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "CMDQ_BASE + 4 write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
+ data &= SMMU_QUEUE_BASE_RESERVED;
bank->cmdq.base = deposit64(bank->cmdq.base, 32, 32, data);
return MEMTX_OK;
case A_CMDQ_PROD:
@@ -1671,9 +1885,22 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr
offset,
smmuv3_cmdq_consume(s);
return MEMTX_OK;
case A_CMDQ_CONS:
+ if (!smmu_cmdqen_disabled(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "CMDQ_CONS write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
bank->cmdq.cons = data;
return MEMTX_OK;
case A_EVENTQ_BASE: /* 64b */
+ if (!smmu_eventq_base_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "EVENTQ_BASE write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
+ data &= SMMU_QUEUE_BASE_RESERVED;
bank->eventq.base = deposit64(bank->eventq.base, 0, 32, data);
bank->eventq.log2size = extract64(bank->eventq.base, 0, 5);
if (bank->eventq.log2size > SMMU_EVENTQS) {
@@ -1681,24 +1908,63 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr
offset,
}
return MEMTX_OK;
case A_EVENTQ_BASE + 4:
+ if (!smmu_eventq_base_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "EVENTQ_BASE + 4 write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
+ data &= SMMU_QUEUE_BASE_RESERVED;
bank->eventq.base = deposit64(bank->eventq.base, 32, 32, data);
return MEMTX_OK;
case A_EVENTQ_PROD:
+ if (!smmu_eventqen_disabled(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "EVENTQ_PROD write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
bank->eventq.prod = data;
return MEMTX_OK;
case A_EVENTQ_CONS:
bank->eventq.cons = data;
return MEMTX_OK;
case A_EVENTQ_IRQ_CFG0: /* 64b */
+ if (!smmu_eventq_irq_cfg_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "EVENTQ_IRQ_CFG0 write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
+ data &= SMMU_EVENTQ_IRQ_CFG0_RESERVED;
bank->eventq_irq_cfg0 = deposit64(bank->eventq_irq_cfg0, 0, 32, data);
return MEMTX_OK;
case A_EVENTQ_IRQ_CFG0 + 4:
+ if (!smmu_eventq_irq_cfg_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "EVENTQ_IRQ_CFG0+4 write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
+ data &= SMMU_EVENTQ_IRQ_CFG0_RESERVED;
bank->eventq_irq_cfg0 = deposit64(bank->eventq_irq_cfg0, 32, 32, data);
return MEMTX_OK;
case A_EVENTQ_IRQ_CFG1:
+ if (!smmu_eventq_irq_cfg_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "EVENTQ_IRQ_CFG1 write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
bank->eventq_irq_cfg1 = data;
return MEMTX_OK;
case A_EVENTQ_IRQ_CFG2:
+ if (!smmu_eventq_irq_cfg_writable(s, reg_sec_sid)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "EVENTQ_IRQ_CFG2 write ignored: register is RO\n");
+ return MEMTX_OK;
+ }
+
bank->eventq_irq_cfg2 = data;
return MEMTX_OK;
default:
@@ -1743,6 +2009,12 @@ static MemTxResult smmu_readll(SMMUv3State *s, hwaddr
offset,
switch (offset) {
case A_GERROR_IRQ_CFG0:
+ /* SMMU_(*_)GERROR_IRQ_CFG0 BOTH check SMMU_IDR0.MSI */
+ if (!smmu_msi_supported(s, reg_sec_sid)) {
+ *data = 0; /* RES0 */
+ return MEMTX_OK;
+ }
+
*data = bank->gerror_irq_cfg0;
return MEMTX_OK;
case A_STRTAB_BASE:
@@ -1811,15 +2083,35 @@ static MemTxResult smmu_readl(SMMUv3State *s, hwaddr
offset,
*data = bank->gerrorn;
return MEMTX_OK;
case A_GERROR_IRQ_CFG0: /* 64b */
+ if (!smmu_msi_supported(s, reg_sec_sid)) {
+ *data = 0; /* RES0 */
+ return MEMTX_OK;
+ }
+
*data = extract64(bank->gerror_irq_cfg0, 0, 32);
return MEMTX_OK;
case A_GERROR_IRQ_CFG0 + 4:
+ if (!smmu_msi_supported(s, reg_sec_sid)) {
+ *data = 0; /* RES0 */
+ return MEMTX_OK;
+ }
+
*data = extract64(bank->gerror_irq_cfg0, 32, 32);
return MEMTX_OK;
case A_GERROR_IRQ_CFG1:
+ if (!smmu_msi_supported(s, reg_sec_sid)) {
+ *data = 0; /* RES0 */
+ return MEMTX_OK;
+ }
+
*data = bank->gerror_irq_cfg1;
return MEMTX_OK;
case A_GERROR_IRQ_CFG2:
+ if (!smmu_msi_supported(s, reg_sec_sid)) {
+ *data = 0; /* RES0 */
+ return MEMTX_OK;
+ }
+
*data = bank->gerror_irq_cfg2;
return MEMTX_OK;
case A_STRTAB_BASE: /* 64b */
--
2.34.1
