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

Fri, 12 Dec 2025 08:47:39 -0800 

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. 


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
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