Re: [PATCH] hw/arm/smmu: add memory regions as property for an SMMU instance

Fri, 12 Dec 2025 09:32:58 -0800 

On 12/12/25 8:45 AM, Tao Tang wrote:

Hi Pierrick,

On 2025/12/12 06:17, Pierrick Bouvier wrote:

This will be used to access non-secure and secure memory. Secure support
and Granule Protection Check (for RME) for SMMU need to access secure
memory.

As well, it allows to remove usage of global address_space_memory,
allowing different SMMU instances to have a specific view of memory.



Thanks for the patch.

I have to admit that in my earlier series I didn’t find a clean way to
model SMMU’s AddressSpace handling properly, so I ended up using weak
globals as a pragmatic workaround. Your approach is much cleaner and
fits QEMU’s usual memory model.



Signed-off-by: Pierrick Bouvier <[email protected]>
---
   include/hw/arm/smmu-common.h |  4 ++++
   hw/arm/sbsa-ref.c            | 16 ++++++++++++----
   hw/arm/smmu-common.c         | 24 ++++++++++++++++++++++++
   hw/arm/virt.c                | 16 +++++++++++-----
   4 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index a6bdb67a983..0f08ae080c9 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -227,6 +227,10 @@ struct SMMUState {
       uint8_t bus_num;
       PCIBus *primary_bus;
       bool smmu_per_bus; /* SMMU is specific to the primary_bus */
+    MemoryRegion *memory;
+    AddressSpace as_memory;
+    MemoryRegion *secure_memory;
+    AddressSpace as_secure_memory;
   };
struct SMMUBaseClass { 
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 45d2e3e946d..840b1a216f4 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -616,7 +616,9 @@ static void create_xhci(const SBSAMachineState *sms)
       sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, qdev_get_gpio_in(sms->gic, 
irq));
   }
-static void create_smmu(const SBSAMachineState *sms, PCIBus *bus) 
+static void create_smmu(const SBSAMachineState *sms, PCIBus *bus,
+                        MemoryRegion *sysmem,
+                        MemoryRegion *secure_sysmem)
   {
       hwaddr base = sbsa_ref_memmap[SBSA_SMMU].base;
       int irq =  sbsa_ref_irqmap[SBSA_SMMU];
@@ -628,6 +630,10 @@ static void create_smmu(const SBSAMachineState *sms, 
PCIBus *bus)
       object_property_set_str(OBJECT(dev), "stage", "nested", &error_abort);
       object_property_set_link(OBJECT(dev), "primary-bus", OBJECT(bus),
                                &error_abort);
+    object_property_set_link(OBJECT(dev), "memory", OBJECT(sysmem),
+                             &error_abort);
+    object_property_set_link(OBJECT(dev), "secure-memory", 
OBJECT(secure_sysmem),
+                             &error_abort);
       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
       for (i = 0; i < NUM_SMMU_IRQS; i++) {
@@ -636,7 +642,9 @@ static void create_smmu(const SBSAMachineState *sms, PCIBus 
*bus)
       }
   }
-static void create_pcie(SBSAMachineState *sms) 
+static void create_pcie(SBSAMachineState *sms,
+                        MemoryRegion *sysmem,
+                        MemoryRegion *secure_sysmem)
   {
       hwaddr base_ecam = sbsa_ref_memmap[SBSA_PCIE_ECAM].base;
       hwaddr size_ecam = sbsa_ref_memmap[SBSA_PCIE_ECAM].size;
@@ -692,7 +700,7 @@ static void create_pcie(SBSAMachineState *sms)
pci_create_simple(pci->bus, -1, "bochs-display"); - create_smmu(sms, pci->bus); 
+    create_smmu(sms, pci->bus, sysmem, secure_sysmem);
   }
static void *sbsa_ref_dtb(const struct arm_boot_info *binfo, int *fdt_size) 
@@ -831,7 +839,7 @@ static void sbsa_ref_init(MachineState *machine)
create_xhci(sms); - create_pcie(sms); 
+    create_pcie(sms, sysmem, secure_sysmem);
create_secure_ec(secure_sysmem); diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c 
index 66367adc2a4..5fbfe825fd0 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -1171,6 +1171,12 @@ static void smmu_base_realize(DeviceState *dev, Error 
**errp)
           return;
       }
+ g_assert(s->memory); 
+    address_space_init(&s->as_memory, s->memory, "memory");
+    if (s->secure_memory) {
+        address_space_init(&s->as_secure_memory, s->secure_memory, 
"secure-memory");
+    }
+
       /*
        * We only allow default PCIe Root Complex(pcie.0) or pxb-pcie based 
extra
        * root complexes to be associated with SMMU.
@@ -1235,10 +1241,28 @@ static void smmu_base_class_init(ObjectClass *klass, 
const void *data)
       rc->phases.exit = smmu_base_reset_exit;
   }
+static void smmu_base_instance_init(Object *obj) 
+{
+    SMMUState *s = ARM_SMMU(obj);
+
+    object_property_add_link(obj, "memory",
+                             TYPE_MEMORY_REGION,
+                             (Object **)&s->memory,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_STRONG);
+
+    object_property_add_link(obj, "secure-memory",
+                             TYPE_MEMORY_REGION,
+                             (Object **)&s->secure_memory,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_STRONG);
+}
+
   static const TypeInfo smmu_base_info = {
       .name          = TYPE_ARM_SMMU,
       .parent        = TYPE_SYS_BUS_DEVICE,
       .instance_size = sizeof(SMMUState),
+    .instance_init = smmu_base_instance_init,
       .class_data    = NULL,
       .class_size    = sizeof(SMMUBaseClass),
       .class_init    = smmu_base_class_init,
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 5d205eff3a1..d446c3349e9 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1514,8 +1514,9 @@ static void create_smmuv3_dev_dtb(VirtMachineState *vms,
                              0x0, vms->iommu_phandle, 0x0, 0x10000);
   }
-static void create_smmu(const VirtMachineState *vms, 
-                        PCIBus *bus)
+static void create_smmu(const VirtMachineState *vms, PCIBus *bus,
+                        MemoryRegion *sysmem,
+                        MemoryRegion *secure_sysmem)
   {
       VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
       int irq =  vms->irqmap[VIRT_SMMU];
@@ -1549,6 +1550,10 @@ static void create_smmu(const VirtMachineState *vms,
       object_property_set_str(OBJECT(dev), "stage", stage, &error_fatal);


This unchanged line doesn't seem to match the current master branch
code. It was actually modified last year by Peter in commit 8a934f1c4a
which changed the virt board's default stage. So I failed to apply this
patch. Anyway this discrepancy doesn't affect the core functionality of
the patch.
Sorry about it, it's a local (and personal) patch I have to override the default stage, so it didn't apply cleanly on upstream/master. I'll fix the conflict in next version.
Applying: hw/arm/smmu: add memory regions as property for an SMMU instance 
error: patch failed: hw/arm/virt.c:1549
error: hw/arm/virt.c: patch does not apply
Patch failed at 0001 hw/arm/smmu: add memory regions as property for an
SMMU instance
hint: Use 'git am --show-current-patch=diff' to see the failed patch



       object_property_set_link(OBJECT(dev), "primary-bus", OBJECT(bus),
                                &error_abort);
+    object_property_set_link(OBJECT(dev), "memory", OBJECT(sysmem),
+                             &error_abort);


I noticed when using info mtree: after applying the patch, I see two
address-space: memory which may confuse others. Is it necessary to
distinguish the names here?


address-space: cpu-memory-0
address-space: gicv3-its-sysmem
address-space: memory
address-space: memory
    0000000000000000-ffffffffffffffff (prio 0, i/o): system
      0000000000000000-0000000003ffffff (prio 0, romd): virt.flash0
      0000000004000000-0000000007ffffff (prio 0, romd): virt.flash1
      0000000008000000-000000000800ffff (prio 0, i/o): gicv3_dist
I thought about naming it smmu-memory, smmu-secure-memory, so your remark tends to confirm it. 
@Peter, which name would be the best for you?



For the functionality, based on this patch plus attachment #2 from the
earlier thread [1], the NS/S SMMU code path in smmu_get_address_space
works fine in my setup.


[1]
https://lore.kernel.org/qemu-devel/[email protected]/


Thanks,

Tao

Reply via email to