On 12/11/25 10:16 PM, Philippe Mathieu-Daudé wrote:
Hi Pierrick,On 11/12/25 23: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.Sorry yesterday it was late for me and I forgot to post the similar patch :/As well, it allows to remove usage of global address_space_memory, allowing different SMMU instances to have a specific view of memory. 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;Has SMMU concept of "secure memory"? My understanding is just a different memory to access GPT, so I'd name it "gpt_memory".
Yes, it has the concept of secure state, secure devices that can make transactions that target secure memory. The fact GPT is kept there is a detail of implementation, so secure_memory is the right term to use here, and match how we call it for cpu implementation.
};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);As a preliminary cleanup create_pcie() returns the PCI bus, so the machine_init() can call create_smmu() itself. I'll post later.}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.cindex 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"); + }Allocating AddressSpaces on the heap: else { s->as_secure_memory = s->as_memory; }
What's the benefit? SMMUState is already heap allocated.
/* * 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); 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++) { @@ -1587,7 +1592,8 @@ static void create_virtio_iommu_dt_bindings(VirtMachineState *vms) } }-static void create_pcie(VirtMachineState *vms)+static void create_pcie(VirtMachineState *vms, + MemoryRegion *sysmem, MemoryRegion *secure_sysmem) { hwaddr base_mmio = vms->memmap[VIRT_PCIE_MMIO].base; hwaddr size_mmio = vms->memmap[VIRT_PCIE_MMIO].size; @@ -1706,7 +1712,7 @@ static void create_pcie(VirtMachineState *vms)switch (vms->iommu) {case VIRT_IOMMU_SMMUV3: - create_smmu(vms, vms->bus); + create_smmu(vms, vms->bus, sysmem, secure_sysmem); if (!vms->default_bus_bypass_iommu) { qemu_fdt_setprop_cells(ms->fdt, nodename, "iommu-map", 0x0, vms->iommu_phandle, 0x0, 0x10000); @@ -2520,7 +2526,7 @@ static void machvirt_init(MachineState *machine)create_rtc(vms); - create_pcie(vms);+ create_pcie(vms, sysmem, secure_sysmem); create_cxl_host_reg_region(vms);if (aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) {
