Hi Eric,
On 2025/12/4 22:33, Eric Auger wrote:
On 10/12/25 5:13 PM, Tao Tang wrote:
Implement read/write handlers for the SMMU_S_INIT secure-only
register.
Writing to this register provides a mechanism for software to perform a
global invalidation of ALL caches within the SMMU. This includes the
IOTLBs and Configuration Caches across all security states.
This feature is critical for secure hypervisors like Hafnium, which
use it as a final step in their SMMU initialization sequence. It
provides a reliable, architecturally defined method to ensure a clean
and known-good cache state before enabling translations.
Signed-off-by: Tao Tang <[email protected]>
---
hw/arm/smmuv3.c | 33 +++++++++++++++++++++++++++++++++
hw/arm/trace-events | 1 +
2 files changed, 34 insertions(+)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 100caeeb35..432de88610 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -354,6 +354,21 @@ static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr,
STE *buf,
}
+static void smmuv3_invalidate_all_caches(SMMUv3State *s)
+{
+ trace_smmuv3_invalidate_all_caches();
+ SMMUState *bs = &s->smmu_state;
+
+ /* Clear all cached configs including STE and CD */
+ if (bs->configs) {
+ g_hash_table_remove_all(bs->configs);
+ }
+
+ /* Invalidate all SMMU IOTLB entries */
+ smmu_inv_notifiers_all(&s->smmu_state);
+ smmu_iotlb_inv_all(bs);
+}
+
static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
SMMUTransCfg *cfg,
SMMUEventInfo *event,
@@ -1969,6 +1984,21 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr
offset,
bank->eventq_irq_cfg2 = data;
return MEMTX_OK;
+ case (A_S_INIT & 0xfff):
why do we apply & 0xfff ?
Let me clarify what I was trying to do with the `(A_S_INIT & 0xfff)`
case, and then ask your opinion on how to clean this up.
According to the SMMUv3 spec , registers with the same function across
different security states (Non-secure, Realm, Root) share the same
*relative* offset within their 4KB bank window. In the MMIO handlers we
first decode the bank from the high bits of the offset (to get
`reg_sec_sid`), and then do:
uint32_t reg_offset = offset & 0xfff;
switch (reg_offset) {
...
}
So the `switch` is really operating on the per-bank 4KB-relative offset,
and the banked registers can share the same `case` logic while
`reg_sec_sid` selects which bank structure we actually touch.
Most of the `A_*` macros (e.g. `A_CR0`) are already defined as these
relative offsets (just the low 12 bits), so they fit naturally in this
scheme and we can simply write `case A_CR0:`.
`A_S_INIT`, however, is still defined as an *absolute* offset that
already includes the secure-bank base. Since the `switch` is matching on
`reg_offset = offset & 0xfff`, I ended up writing:
case (A_S_INIT & 0xfff):
as a shortcut to adapt an absolute macro to the “relative” decode.
S_INIT is also a secure-only register, so there is no NS twin that I
could reuse as the shared low-12-bit macro. In practice that makes
S_INIT a one-off special case in the current code, which is why the `&
0xfff` sticks out.
I agree this looks more like a hack than a clean design, so I’d like to
rework it in the next version. I see a couple of possible directions and
would appreciate your view on which one is preferable:
1) Stop using NS macros as the canonical case labels and list all
bank-specific variants explicitly.
Instead of relying on a single “NS” macro for the shared low-12-bit
offset, we could have:
switch (reg_offset) {
case A_STRTAB_BASE_CFG:
case A_S_STRTAB_BASE_CFG:
...
break;
}
This makes the banked nature explicit in the `switch` and avoids
relying on NS as the “reference” definition. The bank selection would
still be driven by `reg_sec_sid`, but the case labels themselves would
be per-bank macros where that makes sense.
2) Go back to the v2 [1] idea and introduce a dedicated S_INIT-relative
alias macro.
As I had in v2, we could define a separate macro that encodes only
the relative offset, e.g. something like:
REG32(S_INIT, 0x803c)
FIELD(S_INIT, INV_ALL, 0, 1)
/* Alias for the S_INIT offset to match in the dispatcher switch */
#define A_S_INIT_ALIAS 0x3c
and then the decode would simply use:
switch (reg_offset) {
case A_S_INIT_ALIAS:
...
break;
}
This keeps the “absolute” constant for the secure MMIO layout (if we
still find that useful elsewhere), but makes the decode logic work
entirely in terms of relative offsets and removes the inline.
3) Keep the current implementation and add some comment about it.
Which option would you prefer? Any other thoughts or suggestions would
be greatly appreciated.
[1]
https://lore.kernel.org/qemu-devel/[email protected]/
+ if (data & R_S_INIT_INV_ALL_MASK) {
+ int cr0_smmuen = smmu_enabled(s, reg_sec_sid);
+ int s_cr0_smmuen = smmuv3_get_cr0ack_smmuen(s, reg_sec_sid);
+ if (cr0_smmuen || s_cr0_smmuen) {
use smmuv3_is_smmu_enabled()?
Yes using smmuv3_is_smmu_enabled is a better choice. And I'll check both
CR0.SMMUEN and S_CR0.SMMUEN in V4. As I found *any SMMU_(*_)CR0.SMMUEN
== 1* descriptions in 6.3.62 SMMU_S_INIT ARM IHI 0070 G.b:
> If SMMU_ROOT_CR0.GPCEN == 0, a write of 1 to INV_ALL when any
SMMU_(*_)CR0.SMMUEN == 1,
> or an Update of any SMMUEN to 1 is in progress ........ , is
CONSTRAINED UNPREDICTABLE......
Thanks for your review.
Yours,
Tao
+ /* CONSTRAINED UNPREDICTABLE behavior: Ignore this write */
+ qemu_log_mask(LOG_GUEST_ERROR, "S_INIT write ignored: "
+ "CR0.SMMUEN=%d or S_CR0.SMMUEN=%d is set\n",
+ cr0_smmuen, s_cr0_smmuen);
+ return MEMTX_OK;
+ }
+ smmuv3_invalidate_all_caches(s);
+ }
+ /* Synchronous emulation: invalidation completed instantly. */
+ return MEMTX_OK;
default:
qemu_log_mask(LOG_UNIMP,
"%s Unexpected 32-bit access to 0x%"PRIx64" (WI)\n",
@@ -2172,6 +2202,9 @@ static MemTxResult smmu_readl(SMMUv3State *s, hwaddr
offset,
case A_EVENTQ_CONS:
*data = bank->eventq.cons;
return MEMTX_OK;
+ case (A_S_INIT & 0xfff):
+ *data = 0;
+ return MEMTX_OK;
default:
*data = 0;
qemu_log_mask(LOG_UNIMP,
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 434d6abfc2..0e7ad8fee3 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -64,6 +64,7 @@ smmuv3_cmdq_tlbi_s12_vmid(int vmid) "vmid=%d"
smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu
mr=%s"
smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu
mr=%s"
smmuv3_inv_notifiers_iova(const char *name, int asid, int vmid, uint64_t iova, uint8_t tg, uint64_t
num_pages, int stage) "iommu mr=%s asid=%d vmid=%d iova=0x%"PRIx64" tg=%d
num_pages=0x%"PRIx64" stage=%d"
+smmuv3_invalidate_all_caches(void) "Invalidate all SMMU caches and TLBs"
smmu_reset_exit(void) ""
# strongarm.c
Thanks
Eric
