[PATCH 0/5] Add support for user creatable SMMUv3 device

[PATCH 0/5] Add support for user creatable SMMUv3 device

[Shameer Kolothum via]

Hi All,

This patch series introduces support for a user-creatable SMMUv3 device
(-device arm-smmuv3-dev) in QEMU.

The implementation is based on feedback received from the RFCv2[0]:
"hw/arm/virt: Add support for user-creatable accelerated SMMUv3"

Currently, QEMU's SMMUv3 emulation (iommu=smmuv3) is tied to the machine
and does not support instantiating multiple SMMUv3 devices—each associated
with a separate PCIe root complex. In contrast, real-world ARM systems
often include multiple SMMUv3 instances, each bound to a different PCIe
root complex.

This also lays the groundwork for supporting accelerated SMMUv3, as
proposed in the aforementioned RFC. Please note, the accelerated SMMUv3
support is not part of this series and will be sent out as a separate
series later on top of this one.

Summary of changes:

 -Introduces support for multiple -device arm-smmuv3-dev,bus=pcie.x
  instances.

  Example usage:

  -device arm-smmuv3-dev,bus=pcie.0
  -device virtio-net-pci,bus=pcie.0
  -device pxb-pcie,id=pcie.1,bus_nr=2
  -device arm-smmuv3-dev,bus=pcie.1
  -device pcie-root-port,id=pcie.port1,bus=pcie.1
  -device virtio-net-pci,bus=pcie.port1

  -Supports either the legacy iommu=smmuv3 option or the new
  "-device arm-smmuv3-dev" model.

  -Adds device tree bindings for the new SMMUv3 device on the arm/virt
   machine only, and only for the default pcie.0 root complex.
   (Note: pxb-pcie root complexes are currently not supported with the
    device tree due to known limitations[1].)
  -Restricts usage of the new SMMUv3 device to virt machine versions > 9.2.
   This is to avoid creating new smmuv3 device for old virt machine versions
   and accidently breaking the migration support.

Please take a look and let me know your feedback.

Thanks,
Shameer
[0]:https://lore.kernel.org/qemu-devel/20250311141045.66620-1-shameerali.kolothum.thodi@huawei.com/
[1]:https://lore.kernel.org/qemu-devel/20230421165037.2506-1-Jonathan.Cameron@huawei.com/

Shameer Kolothum (5):
  hw/arm/smmuv3: Introduce SMMUv3 device
  hw/arm/virt-acpi-build: Update IORT for multiple smmuv3 devices
  hw/arm/virt: Factor out common SMMUV3 dt bindings code
  hw/arm/virt: Add support for smmuv3 device
  hw/arm/smmuv3: Enable smmuv3 device creation

 hw/arm/smmuv3.c          |  55 ++++++++++++++++++
 hw/arm/virt-acpi-build.c | 119 +++++++++++++++++++++++++++++++++++----
 hw/arm/virt.c            | 109 ++++++++++++++++++++++++++---------
 hw/core/sysbus-fdt.c     |   3 +
 include/hw/arm/smmuv3.h  |  16 ++++++
 include/hw/arm/virt.h    |   2 +
 6 files changed, 266 insertions(+), 38 deletions(-)

-- 
2.34.1

[PATCH 1/5] hw/arm/smmuv3: Introduce SMMUv3 device

[Shameer Kolothum via]

Initial support to have a user-creatable SMMUv3 device associated
with a PCIe root complex,

-device arm-smmuv3-dev,bus=pcie.x

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 hw/arm/smmuv3.c         | 54 +++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/smmuv3.h | 16 ++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 1a96287ba9..e3b8e13ca3 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -24,6 +24,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/qdev-core.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
 #include "cpu.h"
 #include "trace.h"
 #include "qemu/log.h"
@@ -1873,6 +1874,38 @@ static void smmu_reset_exit(Object *obj, ResetType type)
     smmuv3_init_regs(s);
 }
 
+static int smmuv3_dev_pcie_bus(Object *obj, void *opaque)
+{
+    DeviceState *d = opaque;
+    PCIBus *bus;
+
+    if (!object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) {
+        return 0;
+    }
+
+    bus = PCI_HOST_BRIDGE(obj)->bus;
+    if (d->parent_bus && !strcmp(bus->qbus.name, d->parent_bus->name)) {
+        object_property_set_link(OBJECT(d), "primary-bus", OBJECT(bus),
+                                 &error_abort);
+        return 1;
+    }
+    return 0;
+}
+
+static void smmuv3_dev_realize(DeviceState *d, Error **errp)
+{
+    SMMUv3DevState *s_nested = ARM_SMMUV3_DEV(d);
+    SMMUv3DevClass *c = ARM_SMMUV3_DEV_GET_CLASS(s_nested);
+    Error *local_err = NULL;
+
+    object_child_foreach_recursive(object_get_root(), smmuv3_dev_pcie_bus, d);
+    c->parent_realize(d, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+}
+
 static void smmu_realize(DeviceState *d, Error **errp)
 {
     SMMUState *sys = ARM_SMMU(d);
@@ -1983,6 +2016,18 @@ static void smmuv3_instance_init(Object *obj)
     /* Nothing much to do here as of now */
 }
 
+static void smmuv3_dev_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SMMUv3DevClass *c = ARM_SMMUV3_DEV_CLASS(klass);
+
+    dc->vmsd = &vmstate_smmuv3;
+    device_class_set_parent_realize(dc, smmuv3_dev_realize,
+                                    &c->parent_realize);
+    dc->hotpluggable = false;
+    dc->bus_type = TYPE_PCIE_BUS;
+}
+
 static void smmuv3_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -2038,6 +2083,14 @@ static void smmuv3_iommu_memory_region_class_init(ObjectClass *klass,
     imrc->notify_flag_changed = smmuv3_notify_flag_changed;
 }
 
+static const TypeInfo smmuv3_dev_type_info = {
+    .name          = TYPE_ARM_SMMUV3_DEV,
+    .parent        = TYPE_ARM_SMMUV3,
+    .instance_size = sizeof(SMMUv3DevState),
+    .class_size    = sizeof(SMMUv3DevClass),
+    .class_init    = smmuv3_dev_class_init,
+};
+
 static const TypeInfo smmuv3_type_info = {
     .name          = TYPE_ARM_SMMUV3,
     .parent        = TYPE_ARM_SMMU,
@@ -2056,6 +2109,7 @@ static const TypeInfo smmuv3_iommu_memory_region_info = {
 static void smmuv3_register_types(void)
 {
     type_register_static(&smmuv3_type_info);
+    type_register_static(&smmuv3_dev_type_info);
     type_register_static(&smmuv3_iommu_memory_region_info);
 }
 
diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index d183a62766..7d3846ac40 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -87,4 +87,20 @@ OBJECT_DECLARE_TYPE(SMMUv3State, SMMUv3Class, ARM_SMMUV3)
 #define STAGE1_SUPPORTED(s)      FIELD_EX32(s->idr[0], IDR0, S1P)
 #define STAGE2_SUPPORTED(s)      FIELD_EX32(s->idr[0], IDR0, S2P)
 
+struct SMMUv3DevState {
+    SMMUv3State smmuv3_state;
+};
+
+struct SMMUv3DevClass {
+    /*< private >*/
+    SMMUv3Class smmuv3_class;
+    /*< public >*/
+
+    DeviceRealize parent_realize;
+};
+
+/* User creatable smmuv3 device */
+#define TYPE_ARM_SMMUV3_DEV   "arm-smmuv3-dev"
+OBJECT_DECLARE_TYPE(SMMUv3DevState, SMMUv3DevClass, ARM_SMMUV3_DEV)
+
 #endif
-- 
2.34.1

[PATCH 2/5] hw/arm/virt-acpi-build: Update IORT for multiple smmuv3 devices

[Shameer Kolothum via]

With the soon to be introduced user-creatable SMMUv3 devices for
virt, it is possible to have multiple SMMUv3 devices associated
with different PCIe root complexes.

Update IORT nodes accordingly.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 hw/arm/virt-acpi-build.c | 119 +++++++++++++++++++++++++++++++++++----
 include/hw/arm/virt.h    |   1 +
 2 files changed, 108 insertions(+), 12 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 3ac8f8e178..d1e083ee31 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -43,6 +43,7 @@
 #include "hw/acpi/generic_event_device.h"
 #include "hw/acpi/tpm.h"
 #include "hw/acpi/hmat.h"
+#include "hw/arm/smmuv3.h"
 #include "hw/pci/pcie_host.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_bus.h"
@@ -266,6 +267,60 @@ static int iort_idmap_compare(gconstpointer a, gconstpointer b)
     return idmap_a->input_base - idmap_b->input_base;
 }
 
+struct SMMUv3Device {
+    int irq;
+    hwaddr base;
+    AcpiIortIdMapping smmu_idmap;
+};
+typedef struct SMMUv3Device SMMUv3Device;
+
+static int smmuv3_dev_idmap_compare(gconstpointer a, gconstpointer b)
+{
+    SMMUv3Device *sdev_a = (SMMUv3Device *)a;
+    SMMUv3Device *sdev_b = (SMMUv3Device *)b;
+
+    return sdev_a->smmu_idmap.input_base - sdev_b->smmu_idmap.input_base;
+}
+
+static int get_smmuv3_device(Object *obj, void *opaque)
+{
+    GArray *sdev_blob = opaque;
+    int min_bus, max_bus;
+    VirtMachineState *vms;
+    PlatformBusDevice *pbus;
+    SysBusDevice *sbdev;
+    SMMUv3Device sdev;
+    hwaddr base;
+    int irq;
+    PCIBus *bus;
+
+    if (!object_dynamic_cast(obj, TYPE_ARM_SMMUV3_DEV)) {
+        return 0;
+    }
+
+    bus = PCI_BUS(object_property_get_link(obj, "primary-bus", &error_abort));
+    if (!bus || pci_bus_bypass_iommu(bus)) {
+        return 0;
+    }
+
+    vms = VIRT_MACHINE(qdev_get_machine());
+    pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
+    sbdev = SYS_BUS_DEVICE(obj);
+    base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
+    irq = platform_bus_get_irqn(pbus, sbdev, 0);
+
+    base += vms->memmap[VIRT_PLATFORM_BUS].base;
+    irq += vms->irqmap[VIRT_PLATFORM_BUS];
+
+    pci_bus_range(bus, &min_bus, &max_bus);
+    sdev.smmu_idmap.input_base = min_bus << 8;
+    sdev.smmu_idmap.id_count = (max_bus - min_bus + 1) << 8;
+    sdev.base = base;
+    sdev.irq = irq + ARM_SPI_BASE;
+    g_array_append_val(sdev_blob, sdev);
+    return 0;
+}
+
 /*
  * Input Output Remapping Table (IORT)
  * Conforms to "IO Remapping Table System Software on ARM Platforms",
@@ -275,25 +330,42 @@ static void
 build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 {
     int i, nb_nodes, rc_mapping_count;
-    size_t node_size, smmu_offset = 0;
+    size_t node_size, *smmu_offset = NULL;
     AcpiIortIdMapping *idmap;
+    int num_smmus = 0;
     uint32_t id = 0;
     GArray *smmu_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
     GArray *its_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
+    GArray *smmuv3_devices = g_array_new(false, true, sizeof(SMMUv3Device));
 
     AcpiTable table = { .sig = "IORT", .rev = 3, .oem_id = vms->oem_id,
                         .oem_table_id = vms->oem_table_id };
     /* Table 2 The IORT */
     acpi_table_begin(&table, table_data);
 
-    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
-        AcpiIortIdMapping next_range = {0};
-
+    nb_nodes = 2; /* RC, ITS */
+    if (vms->iommu == VIRT_IOMMU_SMMUV3_DEV) {
+        object_child_foreach_recursive(object_get_root(),
+                                       get_smmuv3_device, smmuv3_devices);
+         /* Sort the smmuv3-devices by smmu idmap input_base */
+        g_array_sort(smmuv3_devices, smmuv3_dev_idmap_compare);
+        /*  Fill smmu idmap from sorted smmuv3 devices array */
+        for (i = 0; i < smmuv3_devices->len; i++) {
+            SMMUv3Device *s = &g_array_index(smmuv3_devices, SMMUv3Device, i);
+            g_array_append_val(smmu_idmaps, s->smmu_idmap);
+        }
+        num_smmus = smmuv3_devices->len;
+    } else if (vms->iommu == VIRT_IOMMU_SMMUV3) {
         object_child_foreach_recursive(object_get_root(),
                                        iort_host_bridges, smmu_idmaps);
 
         /* Sort the smmu idmap by input_base */
         g_array_sort(smmu_idmaps, iort_idmap_compare);
+        num_smmus = 1;
+    }
+
+    if (num_smmus) {
+        AcpiIortIdMapping next_range = {0};
 
         /*
          * Split the whole RIDs by mapping from RC to SMMU,
@@ -316,10 +388,10 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
             g_array_append_val(its_idmaps, next_range);
         }
 
-        nb_nodes = 3; /* RC, ITS, SMMUv3 */
+        smmu_offset = g_new0(size_t, num_smmus);
+        nb_nodes += num_smmus;
         rc_mapping_count = smmu_idmaps->len + its_idmaps->len;
     } else {
-        nb_nodes = 2; /* RC, ITS */
         rc_mapping_count = 1;
     }
     /* Number of IORT Nodes */
@@ -341,10 +413,20 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     /* GIC ITS Identifier Array */
     build_append_int_noprefix(table_data, 0 /* MADT translation_id */, 4);
 
-    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
-        int irq =  vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
+    for (i = 0; i < num_smmus; i++) {
+        hwaddr base;
+        int irq;
+
+        if (vms->iommu == VIRT_IOMMU_SMMUV3_DEV) {
+            SMMUv3Device *s = &g_array_index(smmuv3_devices, SMMUv3Device, i);
+            base = s->base;
+            irq = s->irq;
+        } else {
+            base = vms->memmap[VIRT_SMMU].base;
+            irq = vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
+        }
 
-        smmu_offset = table_data->len - table.table_offset;
+        smmu_offset[i] = table_data->len - table.table_offset;
         /* Table 9 SMMUv3 Format */
         build_append_int_noprefix(table_data, 4 /* SMMUv3 */, 1); /* Type */
         node_size =  SMMU_V3_ENTRY_SIZE + ID_MAPPING_ENTRY_SIZE;
@@ -355,7 +437,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
         /* Reference to ID Array */
         build_append_int_noprefix(table_data, SMMU_V3_ENTRY_SIZE, 4);
         /* Base address */
-        build_append_int_noprefix(table_data, vms->memmap[VIRT_SMMU].base, 8);
+        build_append_int_noprefix(table_data, base, 8);
         /* Flags */
         build_append_int_noprefix(table_data, 1 /* COHACC Override */, 4);
         build_append_int_noprefix(table_data, 0, 4); /* Reserved */
@@ -404,15 +486,26 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     build_append_int_noprefix(table_data, 0, 3); /* Reserved */
 
     /* Output Reference */
-    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
+    if (num_smmus) {
         AcpiIortIdMapping *range;
 
         /* translated RIDs connect to SMMUv3 node: RC -> SMMUv3 -> ITS */
         for (i = 0; i < smmu_idmaps->len; i++) {
+            size_t offset;
+            if (vms->iommu == VIRT_IOMMU_SMMUV3_DEV) {
+                offset = smmu_offset[i];
+            } else {
+                /*
+                 * For legacy VIRT_IOMMU_SMMUV3 case, one machine wide
+                 * smmuv3 may have multiple smmu_idmaps.
+                 */
+                offset = smmu_offset[0];
+            }
+
             range = &g_array_index(smmu_idmaps, AcpiIortIdMapping, i);
             /* output IORT node is the smmuv3 node */
             build_iort_id_mapping(table_data, range->input_base,
-                                  range->id_count, smmu_offset);
+                                  range->id_count, offset);
         }
 
         /* bypassed RIDs connect to ITS group node directly: RC -> ITS */
@@ -430,6 +523,8 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     acpi_table_end(linker, &table);
     g_array_free(smmu_idmaps, true);
     g_array_free(its_idmaps, true);
+    g_free(smmu_offset);
+    g_array_free(smmuv3_devices, true);
 }
 
 /*
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index c8e94e6aed..12395c7594 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -92,6 +92,7 @@ enum {
 typedef enum VirtIOMMUType {
     VIRT_IOMMU_NONE,
     VIRT_IOMMU_SMMUV3,
+    VIRT_IOMMU_SMMUV3_DEV,
     VIRT_IOMMU_VIRTIO,
 } VirtIOMMUType;
 
-- 
2.34.1

[PATCH 3/5] hw/arm/virt: Factor out common SMMUV3 dt bindings code

[Shameer Kolothum via]

No functional changes intended. This will be useful when we
add support for user-creatable smmuv3 device.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 hw/arm/virt.c | 55 +++++++++++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a96452f17a..729f192558 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1417,19 +1417,42 @@ static void create_pcie_irq_map(const MachineState *ms,
                            0x7           /* PCI irq */);
 }
 
+static void create_smmuv3_dt_bindings(const VirtMachineState *vms, hwaddr base,
+                                      int irq)
+{
+    char *node;
+    const char compat[] = "arm,smmu-v3";
+    const char irq_names[] = "eventq\0priq\0cmdq-sync\0gerror";
+    MachineState *ms = MACHINE(vms);
+
+    node = g_strdup_printf("/smmuv3@%" PRIx64, base);
+    qemu_fdt_add_subnode(ms->fdt, node);
+    qemu_fdt_setprop(ms->fdt, node, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_sized_cells(ms->fdt, node, "reg", 2, base, 2, 0x20000);
+
+    qemu_fdt_setprop_cells(ms->fdt, node, "interrupts",
+            GIC_FDT_IRQ_TYPE_SPI, irq    , GIC_FDT_IRQ_FLAGS_EDGE_LO_HI,
+            GIC_FDT_IRQ_TYPE_SPI, irq + 1, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI,
+            GIC_FDT_IRQ_TYPE_SPI, irq + 2, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI,
+            GIC_FDT_IRQ_TYPE_SPI, irq + 3, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
+
+    qemu_fdt_setprop(ms->fdt, node, "interrupt-names", irq_names,
+                     sizeof(irq_names));
+
+    qemu_fdt_setprop(ms->fdt, node, "dma-coherent", NULL, 0);
+    qemu_fdt_setprop_cell(ms->fdt, node, "#iommu-cells", 1);
+    qemu_fdt_setprop_cell(ms->fdt, node, "phandle", vms->iommu_phandle);
+    g_free(node);
+}
+
 static void create_smmu(const VirtMachineState *vms,
                         PCIBus *bus)
 {
     VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
-    char *node;
-    const char compat[] = "arm,smmu-v3";
     int irq =  vms->irqmap[VIRT_SMMU];
     int i;
     hwaddr base = vms->memmap[VIRT_SMMU].base;
-    hwaddr size = vms->memmap[VIRT_SMMU].size;
-    const char irq_names[] = "eventq\0priq\0cmdq-sync\0gerror";
     DeviceState *dev;
-    MachineState *ms = MACHINE(vms);
 
     if (vms->iommu != VIRT_IOMMU_SMMUV3 || !vms->iommu_phandle) {
         return;
@@ -1448,27 +1471,7 @@ static void create_smmu(const VirtMachineState *vms,
         sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
                            qdev_get_gpio_in(vms->gic, irq + i));
     }
-
-    node = g_strdup_printf("/smmuv3@%" PRIx64, base);
-    qemu_fdt_add_subnode(ms->fdt, node);
-    qemu_fdt_setprop(ms->fdt, node, "compatible", compat, sizeof(compat));
-    qemu_fdt_setprop_sized_cells(ms->fdt, node, "reg", 2, base, 2, size);
-
-    qemu_fdt_setprop_cells(ms->fdt, node, "interrupts",
-            GIC_FDT_IRQ_TYPE_SPI, irq    , GIC_FDT_IRQ_FLAGS_EDGE_LO_HI,
-            GIC_FDT_IRQ_TYPE_SPI, irq + 1, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI,
-            GIC_FDT_IRQ_TYPE_SPI, irq + 2, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI,
-            GIC_FDT_IRQ_TYPE_SPI, irq + 3, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
-
-    qemu_fdt_setprop(ms->fdt, node, "interrupt-names", irq_names,
-                     sizeof(irq_names));
-
-    qemu_fdt_setprop(ms->fdt, node, "dma-coherent", NULL, 0);
-
-    qemu_fdt_setprop_cell(ms->fdt, node, "#iommu-cells", 1);
-
-    qemu_fdt_setprop_cell(ms->fdt, node, "phandle", vms->iommu_phandle);
-    g_free(node);
+    create_smmuv3_dt_bindings(vms, base, irq);
 }
 
 static void create_virtio_iommu_dt_bindings(VirtMachineState *vms)
-- 
2.34.1

[PATCH 4/5] hw/arm/virt: Add support for smmuv3 device

[Shameer Kolothum via]

Allow cold-plug of smmuv3 device to virt If the machine wide smmuv3
or a virtio-iommu is not specified.

Also restrict the usage if virt <= 9.2. This will prevent accidently
creating a SMMUv3 device on machines prior to 9.2 and cause failure
on migrating to machines with same version but has a legacy smmuv3
device.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
ToDo: probably need to change virt <= 9.2 to 10.0 considering the 
Qemu cycle we are at now.
---
 hw/arm/virt.c         | 54 +++++++++++++++++++++++++++++++++++++++++++
 hw/core/sysbus-fdt.c  |  3 +++
 include/hw/arm/virt.h |  1 +
 3 files changed, 58 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 729f192558..8d0ae79f4d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -56,6 +56,7 @@
 #include "qemu/cutils.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
+#include "hw/pci/pci_bus.h"
 #include "hw/pci-host/gpex.h"
 #include "hw/virtio/virtio-pci.h"
 #include "hw/core/sysbus-fdt.h"
@@ -1445,6 +1446,31 @@ static void create_smmuv3_dt_bindings(const VirtMachineState *vms, hwaddr base,
     g_free(node);
 }
 
+static void create_smmuv3_dev_dtb(VirtMachineState *vms,
+                                  DeviceState *dev)
+{
+    PlatformBusDevice *pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
+    SysBusDevice *sbdev = SYS_BUS_DEVICE(dev);
+    int irq = platform_bus_get_irqn(pbus, sbdev, 0);
+    hwaddr base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
+    MachineState *ms = MACHINE(vms);
+    PCIBus *bus;
+
+    bus = PCI_BUS(object_property_get_link(OBJECT(dev), "primary-bus",
+                                           &error_abort));
+    if (strcmp("pcie.0", bus->qbus.name)) {
+        warn_report("SMMUv3 device only supported with pcie.0 for DT");
+        return;
+    }
+    base += vms->memmap[VIRT_PLATFORM_BUS].base;
+    irq += vms->irqmap[VIRT_PLATFORM_BUS];
+
+    vms->iommu_phandle = qemu_fdt_alloc_phandle(ms->fdt);
+    create_smmuv3_dt_bindings(vms, base, irq);
+    qemu_fdt_setprop_cells(ms->fdt, vms->pciehb_nodename, "iommu-map",
+                           0x0, vms->iommu_phandle, 0x0, 0x10000);
+}
+
 static void create_smmu(const VirtMachineState *vms,
                         PCIBus *bus)
 {
@@ -2944,6 +2970,18 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
         qlist_append_str(reserved_regions, resv_prop_str);
         qdev_prop_set_array(dev, "reserved-regions", reserved_regions);
         g_free(resv_prop_str);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_ARM_SMMUV3_DEV)) {
+        VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
+
+        if (vmc->no_smmuv3_device) {
+            error_setg(errp, "virt machine does not support arm-smmuv3-device");
+        } else if ((vms->iommu == VIRT_IOMMU_VIRTIO) ||
+                   (vms->iommu == VIRT_IOMMU_SMMUV3)) {
+            error_setg(errp, "virt machine already has %s set."
+                       "Doesn't support multiple incompatible iommus",
+                       (vms->iommu == VIRT_IOMMU_VIRTIO) ?
+                       "virtio-iommu" : "iommu=smmuv3");
+        }
     }
 }
 
@@ -2967,6 +3005,19 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
         virtio_md_pci_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
     }
 
+    if (object_dynamic_cast(OBJECT(dev), TYPE_ARM_SMMUV3_DEV)) {
+        VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
+
+        create_smmuv3_dev_dtb(vms, dev);
+        if (vms->iommu != VIRT_IOMMU_SMMUV3_DEV) {
+            vms->iommu = VIRT_IOMMU_SMMUV3_DEV;
+        }
+        if (!vmc->no_nested_smmu) {
+            object_property_set_str(OBJECT(dev), "stage", "nested",
+                                    &error_fatal);
+        }
+    }
+
     if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
         PCIDevice *pdev = PCI_DEVICE(dev);
 
@@ -3169,6 +3220,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_UEFI_VARS_SYSBUS);
+    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_ARM_SMMUV3_DEV);
 #ifdef CONFIG_TPM
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
 #endif
@@ -3418,8 +3470,10 @@ DEFINE_VIRT_MACHINE_AS_LATEST(10, 0)
 
 static void virt_machine_9_2_options(MachineClass *mc)
 {
+    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
     virt_machine_10_0_options(mc);
     compat_props_add(mc->compat_props, hw_compat_9_2, hw_compat_9_2_len);
+    vmc->no_smmuv3_device = true;
 }
 DEFINE_VIRT_MACHINE(9, 2)
 
diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
index e85066b905..ec90ed2c14 100644
--- a/hw/core/sysbus-fdt.c
+++ b/hw/core/sysbus-fdt.c
@@ -31,6 +31,7 @@
 #include "qemu/error-report.h"
 #include "system/device_tree.h"
 #include "system/tpm.h"
+#include "hw/arm/smmuv3.h"
 #include "hw/platform-bus.h"
 #include "hw/vfio/vfio-platform.h"
 #include "hw/vfio/vfio-calxeda-xgmac.h"
@@ -512,6 +513,8 @@ static const BindingEntry bindings[] = {
 #ifdef CONFIG_LINUX
     TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node),
     TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node),
+    /* No generic DT support for smmuv3 dev. Support added for arm virt only */
+    TYPE_BINDING(TYPE_ARM_SMMUV3_DEV, no_fdt_node),
     VFIO_PLATFORM_BINDING("amd,xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
 #endif
 #ifdef CONFIG_TPM
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 12395c7594..9f98345c92 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -136,6 +136,7 @@ struct VirtMachineClass {
     bool no_tcg_lpa2;
     bool no_ns_el2_virt_timer_irq;
     bool no_nested_smmu;
+    bool no_smmuv3_device;
 };
 
 struct VirtMachineState {
-- 
2.34.1

[PATCH 5/5] hw/arm/smmuv3: Enable smmuv3 device creation

[Shameer Kolothum via]

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 hw/arm/smmuv3.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index e3b8e13ca3..572119e472 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -2026,6 +2026,7 @@ static void smmuv3_dev_class_init(ObjectClass *klass, void *data)
                                     &c->parent_realize);
     dc->hotpluggable = false;
     dc->bus_type = TYPE_PCIE_BUS;
+    dc->user_creatable = true;
 }
 
 static void smmuv3_class_init(ObjectClass *klass, void *data)
-- 
2.34.1

Re: [PATCH 4/5] hw/arm/virt: Add support for smmuv3 device

[Jonathan Cameron via]

On Tue, 15 Apr 2025 09:11:03 +0100
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:

> Allow cold-plug of smmuv3 device to virt If the machine wide smmuv3
> or a virtio-iommu is not specified.
> 
> Also restrict the usage if virt <= 9.2. This will prevent accidently
> creating a SMMUv3 device on machines prior to 9.2 and cause failure
> on migrating to machines with same version but has a legacy smmuv3
> device.

Hi,

As we discussed internally I'm not convinced we need to prevent this particular
way for a user to shoot themselves in the foot.

To be a problem they have to specifically request an old machine + the
device that didn't exist for that machine, then migrate to a real old
version of QEMU.  Agreed it is possible but I'm not sure we need to
prevent that particular crazy.

Jonathan


> 
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
> ToDo: probably need to change virt <= 9.2 to 10.0 considering the 
> Qemu cycle we are at now.
> ---
>  hw/arm/virt.c         | 54 +++++++++++++++++++++++++++++++++++++++++++
>  hw/core/sysbus-fdt.c  |  3 +++
>  include/hw/arm/virt.h |  1 +
>  3 files changed, 58 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 729f192558..8d0ae79f4d 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -56,6 +56,7 @@
>  #include "qemu/cutils.h"
>  #include "qemu/error-report.h"
>  #include "qemu/module.h"
> +#include "hw/pci/pci_bus.h"
>  #include "hw/pci-host/gpex.h"
>  #include "hw/virtio/virtio-pci.h"
>  #include "hw/core/sysbus-fdt.h"
> @@ -1445,6 +1446,31 @@ static void create_smmuv3_dt_bindings(const VirtMachineState *vms, hwaddr base,
>      g_free(node);
>  }
>  
> +static void create_smmuv3_dev_dtb(VirtMachineState *vms,
> +                                  DeviceState *dev)
> +{
> +    PlatformBusDevice *pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
> +    SysBusDevice *sbdev = SYS_BUS_DEVICE(dev);
> +    int irq = platform_bus_get_irqn(pbus, sbdev, 0);
> +    hwaddr base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
> +    MachineState *ms = MACHINE(vms);
> +    PCIBus *bus;
> +
> +    bus = PCI_BUS(object_property_get_link(OBJECT(dev), "primary-bus",
> +                                           &error_abort));
> +    if (strcmp("pcie.0", bus->qbus.name)) {
> +        warn_report("SMMUv3 device only supported with pcie.0 for DT");
> +        return;
> +    }
> +    base += vms->memmap[VIRT_PLATFORM_BUS].base;
> +    irq += vms->irqmap[VIRT_PLATFORM_BUS];
> +
> +    vms->iommu_phandle = qemu_fdt_alloc_phandle(ms->fdt);
> +    create_smmuv3_dt_bindings(vms, base, irq);
> +    qemu_fdt_setprop_cells(ms->fdt, vms->pciehb_nodename, "iommu-map",
> +                           0x0, vms->iommu_phandle, 0x0, 0x10000);
> +}
> +
>  static void create_smmu(const VirtMachineState *vms,
>                          PCIBus *bus)
>  {
> @@ -2944,6 +2970,18 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>          qlist_append_str(reserved_regions, resv_prop_str);
>          qdev_prop_set_array(dev, "reserved-regions", reserved_regions);
>          g_free(resv_prop_str);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_ARM_SMMUV3_DEV)) {
> +        VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
> +
> +        if (vmc->no_smmuv3_device) {
> +            error_setg(errp, "virt machine does not support arm-smmuv3-device");
> +        } else if ((vms->iommu == VIRT_IOMMU_VIRTIO) ||
> +                   (vms->iommu == VIRT_IOMMU_SMMUV3)) {
> +            error_setg(errp, "virt machine already has %s set."
> +                       "Doesn't support multiple incompatible iommus",
> +                       (vms->iommu == VIRT_IOMMU_VIRTIO) ?
> +                       "virtio-iommu" : "iommu=smmuv3");
> +        }
>      }
>  }
>  
> @@ -2967,6 +3005,19 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>          virtio_md_pci_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
>      }
>  
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_ARM_SMMUV3_DEV)) {
> +        VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
> +
> +        create_smmuv3_dev_dtb(vms, dev);
> +        if (vms->iommu != VIRT_IOMMU_SMMUV3_DEV) {
> +            vms->iommu = VIRT_IOMMU_SMMUV3_DEV;
> +        }
> +        if (!vmc->no_nested_smmu) {
> +            object_property_set_str(OBJECT(dev), "stage", "nested",
> +                                    &error_fatal);
> +        }
> +    }
> +
>      if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>          PCIDevice *pdev = PCI_DEVICE(dev);
>  
> @@ -3169,6 +3220,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_UEFI_VARS_SYSBUS);
> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_ARM_SMMUV3_DEV);
>  #ifdef CONFIG_TPM
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
>  #endif
> @@ -3418,8 +3470,10 @@ DEFINE_VIRT_MACHINE_AS_LATEST(10, 0)
>  
>  static void virt_machine_9_2_options(MachineClass *mc)
>  {
> +    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
>      virt_machine_10_0_options(mc);
>      compat_props_add(mc->compat_props, hw_compat_9_2, hw_compat_9_2_len);
> +    vmc->no_smmuv3_device = true;
>  }
>  DEFINE_VIRT_MACHINE(9, 2)
>  
> diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
> index e85066b905..ec90ed2c14 100644
> --- a/hw/core/sysbus-fdt.c
> +++ b/hw/core/sysbus-fdt.c
> @@ -31,6 +31,7 @@
>  #include "qemu/error-report.h"
>  #include "system/device_tree.h"
>  #include "system/tpm.h"
> +#include "hw/arm/smmuv3.h"
>  #include "hw/platform-bus.h"
>  #include "hw/vfio/vfio-platform.h"
>  #include "hw/vfio/vfio-calxeda-xgmac.h"
> @@ -512,6 +513,8 @@ static const BindingEntry bindings[] = {
>  #ifdef CONFIG_LINUX
>      TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node),
>      TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node),
> +    /* No generic DT support for smmuv3 dev. Support added for arm virt only */
> +    TYPE_BINDING(TYPE_ARM_SMMUV3_DEV, no_fdt_node),
>      VFIO_PLATFORM_BINDING("amd,xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
>  #endif
>  #ifdef CONFIG_TPM
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 12395c7594..9f98345c92 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -136,6 +136,7 @@ struct VirtMachineClass {
>      bool no_tcg_lpa2;
>      bool no_ns_el2_virt_timer_irq;
>      bool no_nested_smmu;
> +    bool no_smmuv3_device;
>  };
>  
>  struct VirtMachineState {

RE: [PATCH 4/5] hw/arm/virt: Add support for smmuv3 device

[Shameerali Kolothum Thodi via]

> -----Original Message-----
> From: Jonathan Cameron <jonathan.cameron@huawei.com>
> Sent: Tuesday, April 15, 2025 10:26 AM
> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; Linuxarm
> <linuxarm@huawei.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com;
> nathanc@nvidia.com; mochs@nvidia.com; smostafa@google.com;
> Wangzhou (B) <wangzhou1@hisilicon.com>; jiangkunkun
> <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [PATCH 4/5] hw/arm/virt: Add support for smmuv3 device
> 
> On Tue, 15 Apr 2025 09:11:03 +0100
> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> 
> > Allow cold-plug of smmuv3 device to virt If the machine wide smmuv3
> > or a virtio-iommu is not specified.
> >
> > Also restrict the usage if virt <= 9.2. This will prevent accidently
> > creating a SMMUv3 device on machines prior to 9.2 and cause failure
> > on migrating to machines with same version but has a legacy smmuv3
> > device.
> 
> Hi,
> 
> As we discussed internally I'm not convinced we need to prevent this
> particular
> way for a user to shoot themselves in the foot.
> 
> To be a problem they have to specifically request an old machine + the
> device that didn't exist for that machine, then migrate to a real old
> version of QEMU.  Agreed it is possible but I'm not sure we need to
> prevent that particular crazy.

Agree. If there is no precedence or requirement for blocking such a use case
I will remove that check.

Thanks,
Shameer

Re: [PATCH 4/5] hw/arm/virt: Add support for smmuv3 device

[Daniel P. Berrangé]

On Tue, Apr 15, 2025 at 10:25:42AM +0100, Jonathan Cameron wrote:
> On Tue, 15 Apr 2025 09:11:03 +0100
> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> 
> > Allow cold-plug of smmuv3 device to virt If the machine wide smmuv3
> > or a virtio-iommu is not specified.
> > 
> > Also restrict the usage if virt <= 9.2. This will prevent accidently
> > creating a SMMUv3 device on machines prior to 9.2 and cause failure
> > on migrating to machines with same version but has a legacy smmuv3
> > device.
> 
> Hi,
> 
> As we discussed internally I'm not convinced we need to prevent this particular
> way for a user to shoot themselves in the foot.
> 
> To be a problem they have to specifically request an old machine + the
> device that didn't exist for that machine, then migrate to a real old
> version of QEMU.  Agreed it is possible but I'm not sure we need to
> prevent that particular crazy.

Agreed, there are a million ways to screw up using old machine types
with new features, so there's no compelling need to single out smmu for
special protection in this respect.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

RE: [PATCH 1/5] hw/arm/smmuv3: Introduce SMMUv3 device

[Shameerali Kolothum Thodi via]

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Wednesday, April 16, 2025 4:42 AM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
> mochs@nvidia.com; smostafa@google.com; Linuxarm
> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [PATCH 1/5] hw/arm/smmuv3: Introduce SMMUv3 device
> 
> On Tue, Apr 15, 2025 at 09:11:00AM +0100, Shameer Kolothum wrote:
> > +static int smmuv3_dev_pcie_bus(Object *obj, void *opaque)
> > +{
> > +    DeviceState *d = opaque;
> > +    PCIBus *bus;
> > +
> > +    if (!object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) {
> > +        return 0;
> > +    }
> > +
> > +    bus = PCI_HOST_BRIDGE(obj)->bus;
> > +    if (d->parent_bus && !strcmp(bus->qbus.name, d->parent_bus-
> >name)) {
> > +        object_property_set_link(OBJECT(d), "primary-bus", OBJECT(bus),
> > +                                 &error_abort);
> > +        return 1;
> 
> Return 1 would break the caller's foreach. And I guess that's what
> we want since "primary-bus" should only be set once?

Yes. It will only set once and once you get it no need to iterate again.
> 
> Maybe better to add a line of comments to explain that a bit?
Ok.

> 
> > +    }
> > +    return 0;
> > +}
> > +
> > +static void smmuv3_dev_realize(DeviceState *d, Error **errp)
> > +{
> > +    SMMUv3DevState *s_nested = ARM_SMMUV3_DEV(d);
> > +    SMMUv3DevClass *c = ARM_SMMUV3_DEV_GET_CLASS(s_nested);
> 
Oops...that's probably from a prototype code I had earlier!. 

Thanks,
Shameer

RE: [PATCH 2/5] hw/arm/virt-acpi-build: Update IORT for multiple smmuv3 devices

[Shameerali Kolothum Thodi via]

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Wednesday, April 16, 2025 5:19 AM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
> mochs@nvidia.com; smostafa@google.com; Linuxarm
> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [PATCH 2/5] hw/arm/virt-acpi-build: Update IORT for multiple
> smmuv3 devices
> 
> On Tue, Apr 15, 2025 at 09:11:01AM +0100, Shameer Kolothum wrote:
> > +static int get_smmuv3_device(Object *obj, void *opaque)
> > +{
> > +    GArray *sdev_blob = opaque;
> > +    int min_bus, max_bus;
> > +    VirtMachineState *vms;
> > +    PlatformBusDevice *pbus;
> > +    SysBusDevice *sbdev;
> > +    SMMUv3Device sdev;
> > +    hwaddr base;
> > +    int irq;
> > +    PCIBus *bus;
> 
> In a reverse christmas tree order? Or we could at least group
> those similar types together.

Yeah. Reverse will look better I guess.
> 
> > +    vms = VIRT_MACHINE(qdev_get_machine());
> > +    pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
> > +    sbdev = SYS_BUS_DEVICE(obj);
> > +    base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
> > +    irq = platform_bus_get_irqn(pbus, sbdev, 0);
> > +
> > +    base += vms->memmap[VIRT_PLATFORM_BUS].base;
> > +    irq += vms->irqmap[VIRT_PLATFORM_BUS];
> > +
> > +    pci_bus_range(bus, &min_bus, &max_bus);
> > +    sdev.smmu_idmap.input_base = min_bus << 8;
> > +    sdev.smmu_idmap.id_count = (max_bus - min_bus + 1) << 8;
> > +    sdev.base = base;
> > +    sdev.irq = irq + ARM_SPI_BASE;
> 
> Hmm, these base/irq local variables don't look necessary.
> 
> > +    g_array_append_val(sdev_blob, sdev);
> > +    return 0;
> > +}
> > +
> >  /*
> >   * Input Output Remapping Table (IORT)
> >   * Conforms to "IO Remapping Table System Software on ARM
> Platforms",
> > @@ -275,25 +330,42 @@ static void
> >  build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState
> *vms)
> >  {
> >      int i, nb_nodes, rc_mapping_count;
> > -    size_t node_size, smmu_offset = 0;
> > +    size_t node_size, *smmu_offset = NULL;
> >      AcpiIortIdMapping *idmap;
> > +    int num_smmus = 0;
> >      uint32_t id = 0;
> >      GArray *smmu_idmaps = g_array_new(false, true,
> sizeof(AcpiIortIdMapping));
> >      GArray *its_idmaps = g_array_new(false, true,
> sizeof(AcpiIortIdMapping));
> > +    GArray *smmuv3_devices = g_array_new(false, true,
> sizeof(SMMUv3Device));
> >
> >      AcpiTable table = { .sig = "IORT", .rev = 3, .oem_id = vms->oem_id,
> >                          .oem_table_id = vms->oem_table_id };
> >      /* Table 2 The IORT */
> >      acpi_table_begin(&table, table_data);
> >
> > -    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> > -        AcpiIortIdMapping next_range = {0};
> > -
> > +    nb_nodes = 2; /* RC, ITS */
> > +    if (vms->iommu == VIRT_IOMMU_SMMUV3_DEV) {
> > +        object_child_foreach_recursive(object_get_root(),
> > +                                       get_smmuv3_device, smmuv3_devices);
> > +         /* Sort the smmuv3-devices by smmu idmap input_base */
> > +        g_array_sort(smmuv3_devices, smmuv3_dev_idmap_compare);
> > +        /*  Fill smmu idmap from sorted smmuv3 devices array */
> > +        for (i = 0; i < smmuv3_devices->len; i++) {
> > +            SMMUv3Device *s = &g_array_index(smmuv3_devices,
> SMMUv3Device, i);
> > +            g_array_append_val(smmu_idmaps, s->smmu_idmap);
> > +        }
> > +        num_smmus = smmuv3_devices->len;
> > +    } else if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> >          object_child_foreach_recursive(object_get_root(),
> >                                         iort_host_bridges, smmu_idmaps);
> >
> >          /* Sort the smmu idmap by input_base */
> >          g_array_sort(smmu_idmaps, iort_idmap_compare);
> 
> VIRT_IOMMU_SMMUV3 seems to fit the struct SMMUv3Device very well,
> given that it has base, irq, and smmu_idmaps too?

One difference though is the legacy VIRT_IOMMU_SMMUV3 one is a global
Machine wide one nad can be associated with multiple PCIe root complexes
which will result in smmu_idmaps array. See the iort_host_bridges() fn.

> 
> Maybe we could generalize the struct SMMUv3Device to store both
> cases. Perhaps just SMMUv3AcpiInfo? And then ..

Could do. But then you have to have  a smmu_idmaps array and then check
the length of it to cover legacy SMMUv3 case I guess.

 > > @@ -341,10 +413,20 @@ build_iort(GArray *table_data, BIOSLinker
> *linker, VirtMachineState *vms)
> >      /* GIC ITS Identifier Array */
> >      build_append_int_noprefix(table_data, 0 /* MADT translation_id */,
> 4);
> >
> > -    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> > -        int irq =  vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
> > +    for (i = 0; i < num_smmus; i++) {
> > +        hwaddr base;
> > +        int irq;
> > +
> > +        if (vms->iommu == VIRT_IOMMU_SMMUV3_DEV) {
> > +            SMMUv3Device *s = &g_array_index(smmuv3_devices,
> SMMUv3Device, i);
> > +            base = s->base;
> > +            irq = s->irq;
> > +        } else {
> > +            base = vms->memmap[VIRT_SMMU].base;
> > +            irq = vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
> > +        }
> 
> .. we wouldn't need two paths here.
> 
> > @@ -404,15 +486,26 @@ build_iort(GArray *table_data, BIOSLinker
> *linker, VirtMachineState *vms)
> >      build_append_int_noprefix(table_data, 0, 3); /* Reserved */
> >
> >      /* Output Reference */
> > -    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> > +    if (num_smmus) {
> >          AcpiIortIdMapping *range;
> >
> >          /* translated RIDs connect to SMMUv3 node: RC -> SMMUv3 -> ITS
> */
> >          for (i = 0; i < smmu_idmaps->len; i++) {
> > +            size_t offset;
> > +            if (vms->iommu == VIRT_IOMMU_SMMUV3_DEV) {
> > +                offset = smmu_offset[i];
> > +            } else {
> > +                /*
> > +                 * For legacy VIRT_IOMMU_SMMUV3 case, one machine wide
> > +                 * smmuv3 may have multiple smmu_idmaps.
> > +                 */
> > +                offset = smmu_offset[0];
> > +            }
> 
> And I think we can eliminate this too if we stuff an smmu_offset
> in struct AcpiIortIdMapping ..
> 
> >              range = &g_array_index(smmu_idmaps, AcpiIortIdMapping, i);
> >              /* output IORT node is the smmuv3 node */
> >              build_iort_id_mapping(table_data, range->input_base,
> > -                                  range->id_count, smmu_offset);
> > +                                  range->id_count, offset);
> 
> ... and here it would be range->offset.

I will give it a try and if that simplifies things will include it in next respin.

Thanks,
Shameer

RE: [PATCH 3/5] hw/arm/virt: Factor out common SMMUV3 dt bindings code

[Shameerali Kolothum Thodi via]

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Wednesday, April 16, 2025 5:26 AM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
> mochs@nvidia.com; smostafa@google.com; Linuxarm
> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [PATCH 3/5] hw/arm/virt: Factor out common SMMUV3 dt
> bindings code
> 
> On Tue, Apr 15, 2025 at 09:11:02AM +0100, Shameer Kolothum wrote:
> > No functional changes intended. This will be useful when we add
> > support for user-creatable smmuv3 device.
> >
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  hw/arm/virt.c | 55
> > +++++++++++++++++++++++++++------------------------
> >  1 file changed, 29 insertions(+), 26 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c index
> > a96452f17a..729f192558 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1417,19 +1417,42 @@ static void create_pcie_irq_map(const
> MachineState *ms,
> >                             0x7           /* PCI irq */);
> >  }
> >
> > +static void create_smmuv3_dt_bindings(const VirtMachineState *vms,
> hwaddr base,
> > +                                      int irq) {
> > +    char *node;
> > +    const char compat[] = "arm,smmu-v3";
> > +    const char irq_names[] = "eventq\0priq\0cmdq-sync\0gerror";
> > +    MachineState *ms = MACHINE(vms);
> > +
> > +    node = g_strdup_printf("/smmuv3@%" PRIx64, base);
> > +    qemu_fdt_add_subnode(ms->fdt, node);
> > +    qemu_fdt_setprop(ms->fdt, node, "compatible", compat,
> sizeof(compat));
> > +    qemu_fdt_setprop_sized_cells(ms->fdt, node, "reg", 2, base, 2,
> > + 0x20000);
> 
> Nit: I would pass in the size from its caller, just to ease in the future when
> we need to add an instance supporting a different MMIO range.

Yes, that makes sense. And probably will use #define for the current size.

Thanks,
Shameer

Re: [PATCH 0/5] Add support for user creatable SMMUv3 device

[Donald Dutile]

Shameer,
Hi!

First off, like the partitioning of these pieces.

On 4/15/25 4:10 AM, Shameer Kolothum wrote:
> Hi All,
> 
> This patch series introduces support for a user-creatable SMMUv3 device
> (-device arm-smmuv3-dev) in QEMU.
> 
Can we drop the '-dev', as 'arm-smmu' is sufficient, as is '-device'?

I know, internal to QEMU, there's already an ARM_SMMU, but as suggested later,
if it uses the same struct, the qemu cmdline syntax is a bit less redundant.

> The implementation is based on feedback received from the RFCv2[0]:
> "hw/arm/virt: Add support for user-creatable accelerated SMMUv3"
> 
> Currently, QEMU's SMMUv3 emulation (iommu=smmuv3) is tied to the machine
> and does not support instantiating multiple SMMUv3 devices—each associated
> with a separate PCIe root complex. In contrast, real-world ARM systems
> often include multiple SMMUv3 instances, each bound to a different PCIe
> root complex.
> 
> This also lays the groundwork for supporting accelerated SMMUv3, as
> proposed in the aforementioned RFC. Please note, the accelerated SMMUv3
> support is not part of this series and will be sent out as a separate
> series later on top of this one.
> 
> Summary of changes:
> 
>   -Introduces support for multiple -device arm-smmuv3-dev,bus=pcie.x
>    instances.
> 
>    Example usage:
> 
>    -device arm-smmuv3-dev,bus=pcie.0
>    -device virtio-net-pci,bus=pcie.0
>    -device pxb-pcie,id=pcie.1,bus_nr=2
>    -device arm-smmuv3-dev,bus=pcie.1
>    -device pcie-root-port,id=pcie.port1,bus=pcie.1
>    -device virtio-net-pci,bus=pcie.port1
> 
>    -Supports either the legacy iommu=smmuv3 option or the new
>    "-device arm-smmuv3-dev" model.
> 
nice! :)
Can it support both? i.e., some devices using a system-wide, legacy smmuv3,
and some pcie devices using the -device arm-smmuv3 ?

If not, then add a check to min-warn or max-fail, that config.
I can see old machines being converted/upgraded to new machines,
and they will want to switch from legacy->device smmuv3, and catching
an edit/update to a machine change (or enabling automated conversion) would be helpful.

>    -Adds device tree bindings for the new SMMUv3 device on the arm/virt
>     machine only, and only for the default pcie.0 root complex.
>     (Note: pxb-pcie root complexes are currently not supported with the
>      device tree due to known limitations[1].)
>    -Restricts usage of the new SMMUv3 device to virt machine versions > 9.2.
>     This is to avoid creating new smmuv3 device for old virt machine versions
>     and accidently breaking the migration support.
> 
> Please take a look and let me know your feedback.
> 
> Thanks,
> Shameer
> [0]:https://lore.kernel.org/qemu-devel/20250311141045.66620-1-shameerali.kolothum.thodi@huawei.com/
> [1]:https://lore.kernel.org/qemu-devel/20230421165037.2506-1-Jonathan.Cameron@huawei.com/
> 
> Shameer Kolothum (5):
>    hw/arm/smmuv3: Introduce SMMUv3 device
>    hw/arm/virt-acpi-build: Update IORT for multiple smmuv3 devices
>    hw/arm/virt: Factor out common SMMUV3 dt bindings code
>    hw/arm/virt: Add support for smmuv3 device
>    hw/arm/smmuv3: Enable smmuv3 device creation
> 
>   hw/arm/smmuv3.c          |  55 ++++++++++++++++++
>   hw/arm/virt-acpi-build.c | 119 +++++++++++++++++++++++++++++++++++----
>   hw/arm/virt.c            | 109 ++++++++++++++++++++++++++---------
>   hw/core/sysbus-fdt.c     |   3 +
>   include/hw/arm/smmuv3.h  |  16 ++++++
>   include/hw/arm/virt.h    |   2 +
>   6 files changed, 266 insertions(+), 38 deletions(-)
> 

Re: [PATCH 2/5] hw/arm/virt-acpi-build: Update IORT for multiple smmuv3 devices

[Donald Dutile]

On 4/16/25 1:38 AM, Shameerali Kolothum Thodi wrote:
> 
> 
>> -----Original Message-----
>> From: Nicolin Chen <nicolinc@nvidia.com>
>> Sent: Wednesday, April 16, 2025 5:19 AM
>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
>> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
>> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
>> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
>> Subject: Re: [PATCH 2/5] hw/arm/virt-acpi-build: Update IORT for multiple
>> smmuv3 devices
>>
>> On Tue, Apr 15, 2025 at 09:11:01AM +0100, Shameer Kolothum wrote:
>>> +static int get_smmuv3_device(Object *obj, void *opaque)
>>> +{
>>> +    GArray *sdev_blob = opaque;
>>> +    int min_bus, max_bus;
>>> +    VirtMachineState *vms;
>>> +    PlatformBusDevice *pbus;
>>> +    SysBusDevice *sbdev;
>>> +    SMMUv3Device sdev;
>>> +    hwaddr base;
>>> +    int irq;
>>> +    PCIBus *bus;
>>
>> In a reverse christmas tree order? Or we could at least group
>> those similar types together.
> 
> Yeah. Reverse will look better I guess.
>>
>>> +    vms = VIRT_MACHINE(qdev_get_machine());
>>> +    pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
>>> +    sbdev = SYS_BUS_DEVICE(obj);
>>> +    base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
>>> +    irq = platform_bus_get_irqn(pbus, sbdev, 0);
>>> +
>>> +    base += vms->memmap[VIRT_PLATFORM_BUS].base;
>>> +    irq += vms->irqmap[VIRT_PLATFORM_BUS];
>>> +
>>> +    pci_bus_range(bus, &min_bus, &max_bus);
>>> +    sdev.smmu_idmap.input_base = min_bus << 8;
>>> +    sdev.smmu_idmap.id_count = (max_bus - min_bus + 1) << 8;
>>> +    sdev.base = base;
>>> +    sdev.irq = irq + ARM_SPI_BASE;
>>
>> Hmm, these base/irq local variables don't look necessary.
>>
>>> +    g_array_append_val(sdev_blob, sdev);
>>> +    return 0;
>>> +}
>>> +
>>>   /*
>>>    * Input Output Remapping Table (IORT)
>>>    * Conforms to "IO Remapping Table System Software on ARM
>> Platforms",
>>> @@ -275,25 +330,42 @@ static void
>>>   build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState
>> *vms)
>>>   {
>>>       int i, nb_nodes, rc_mapping_count;
>>> -    size_t node_size, smmu_offset = 0;
>>> +    size_t node_size, *smmu_offset = NULL;
>>>       AcpiIortIdMapping *idmap;
>>> +    int num_smmus = 0;
>>>       uint32_t id = 0;
>>>       GArray *smmu_idmaps = g_array_new(false, true,
>> sizeof(AcpiIortIdMapping));
>>>       GArray *its_idmaps = g_array_new(false, true,
>> sizeof(AcpiIortIdMapping));
>>> +    GArray *smmuv3_devices = g_array_new(false, true,
>> sizeof(SMMUv3Device));
>>>
>>>       AcpiTable table = { .sig = "IORT", .rev = 3, .oem_id = vms->oem_id,
>>>                           .oem_table_id = vms->oem_table_id };
>>>       /* Table 2 The IORT */
>>>       acpi_table_begin(&table, table_data);
>>>
>>> -    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
>>> -        AcpiIortIdMapping next_range = {0};
>>> -
>>> +    nb_nodes = 2; /* RC, ITS */
>>> +    if (vms->iommu == VIRT_IOMMU_SMMUV3_DEV) {
>>> +        object_child_foreach_recursive(object_get_root(),
>>> +                                       get_smmuv3_device, smmuv3_devices);
>>> +         /* Sort the smmuv3-devices by smmu idmap input_base */
>>> +        g_array_sort(smmuv3_devices, smmuv3_dev_idmap_compare);
>>> +        /*  Fill smmu idmap from sorted smmuv3 devices array */
>>> +        for (i = 0; i < smmuv3_devices->len; i++) {
>>> +            SMMUv3Device *s = &g_array_index(smmuv3_devices,
>> SMMUv3Device, i);
>>> +            g_array_append_val(smmu_idmaps, s->smmu_idmap);
>>> +        }
>>> +        num_smmus = smmuv3_devices->len;
>>> +    } else if (vms->iommu == VIRT_IOMMU_SMMUV3) {
>>>           object_child_foreach_recursive(object_get_root(),
>>>                                          iort_host_bridges, smmu_idmaps);
>>>
>>>           /* Sort the smmu idmap by input_base */
>>>           g_array_sort(smmu_idmaps, iort_idmap_compare);
>>
>> VIRT_IOMMU_SMMUV3 seems to fit the struct SMMUv3Device very well,
>> given that it has base, irq, and smmu_idmaps too?
> 
> One difference though is the legacy VIRT_IOMMU_SMMUV3 one is a global
> Machine wide one nad can be associated with multiple PCIe root complexes
> which will result in smmu_idmaps array. See the iort_host_bridges() fn.
> 
Didn't quite follow this logic; shouldn't the iort be built for devices
tagged for a virt-iommu or dependent on a pcie rp (bus num/bus-num range of an RP)?
Thus, it's just an IORT with one or more IORT nodes?
I could see how the current iort_host_bridge() may need a revision to work with -device arm-smmu configs.

Which would bring me back to my earlier question, and it's likely tied to this IORT construction:
  -- can you have both types defined in a machine model?


>>
>> Maybe we could generalize the struct SMMUv3Device to store both
>> cases. Perhaps just SMMUv3AcpiInfo? And then ..
> 
I didn't follow 'SMMUv3AcpiInfo' since I don't see that in current qemu tree;
or is the statement trying to say its a 'mere matter of the proper acpi info generation'?

> Could do. But then you have to have  a smmu_idmaps array and then check
> the length of it to cover legacy SMMUv3 case I guess.
> 
Aren't they going to be different idmaps for different IORT nodes?

>   > > @@ -341,10 +413,20 @@ build_iort(GArray *table_data, BIOSLinker
>> *linker, VirtMachineState *vms)
>>>       /* GIC ITS Identifier Array */
>>>       build_append_int_noprefix(table_data, 0 /* MADT translation_id */,
>> 4);
>>>
>>> -    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
>>> -        int irq =  vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
>>> +    for (i = 0; i < num_smmus; i++) {
>>> +        hwaddr base;
>>> +        int irq;
>>> +
>>> +        if (vms->iommu == VIRT_IOMMU_SMMUV3_DEV) {
>>> +            SMMUv3Device *s = &g_array_index(smmuv3_devices,
>> SMMUv3Device, i);
>>> +            base = s->base;
>>> +            irq = s->irq;
>>> +        } else {
>>> +            base = vms->memmap[VIRT_SMMU].base;
>>> +            irq = vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
>>> +        }
>>
>> .. we wouldn't need two paths here.
>>
>>> @@ -404,15 +486,26 @@ build_iort(GArray *table_data, BIOSLinker
>> *linker, VirtMachineState *vms)
>>>       build_append_int_noprefix(table_data, 0, 3); /* Reserved */
>>>
>>>       /* Output Reference */
>>> -    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
>>> +    if (num_smmus) {
>>>           AcpiIortIdMapping *range;
>>>
>>>           /* translated RIDs connect to SMMUv3 node: RC -> SMMUv3 -> ITS
>> */
>>>           for (i = 0; i < smmu_idmaps->len; i++) {
>>> +            size_t offset;
>>> +            if (vms->iommu == VIRT_IOMMU_SMMUV3_DEV) {
>>> +                offset = smmu_offset[i];
>>> +            } else {
>>> +                /*
>>> +                 * For legacy VIRT_IOMMU_SMMUV3 case, one machine wide
>>> +                 * smmuv3 may have multiple smmu_idmaps.
>>> +                 */
>>> +                offset = smmu_offset[0];
>>> +            }
>>
>> And I think we can eliminate this too if we stuff an smmu_offset
>> in struct AcpiIortIdMapping ..
>>
>>>               range = &g_array_index(smmu_idmaps, AcpiIortIdMapping, i);
>>>               /* output IORT node is the smmuv3 node */
>>>               build_iort_id_mapping(table_data, range->input_base,
>>> -                                  range->id_count, smmu_offset);
>>> +                                  range->id_count, offset);
>>
>> ... and here it would be range->offset.
> 
> I will give it a try and if that simplifies things will include it in next respin.
> 
> Thanks,
> Shameer
> 

RE: [PATCH 2/5] hw/arm/virt-acpi-build: Update IORT for multiple smmuv3 devices

[Shameerali Kolothum Thodi via]

Hi Donald,

> -----Original Message-----
> From: Donald Dutile <ddutile@redhat.com>
> Sent: Friday, April 18, 2025 9:34 PM
> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; Nicolin Chen
> <nicolinc@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> berrange@redhat.com; nathanc@nvidia.com; mochs@nvidia.com;
> smostafa@google.com; Linuxarm <linuxarm@huawei.com>; Wangzhou (B)
> <wangzhou1@hisilicon.com>; jiangkunkun <jiangkunkun@huawei.com>;
> Jonathan Cameron <jonathan.cameron@huawei.com>;
> zhangfei.gao@linaro.org
> Subject: Re: [PATCH 2/5] hw/arm/virt-acpi-build: Update IORT for multiple
> smmuv3 devices
> 
> 
> 
> On 4/16/25 1:38 AM, Shameerali Kolothum Thodi wrote:
> >
> >
> >> -----Original Message-----
> >> From: Nicolin Chen <nicolinc@nvidia.com>
> >> Sent: Wednesday, April 16, 2025 5:19 AM
> >> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>
> >> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> >> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> >> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
> >> mochs@nvidia.com; smostafa@google.com; Linuxarm
> >> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> >> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> >> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> >> Subject: Re: [PATCH 2/5] hw/arm/virt-acpi-build: Update IORT for
> multiple
> >> smmuv3 devices
> >>
> >> On Tue, Apr 15, 2025 at 09:11:01AM +0100, Shameer Kolothum wrote:
> >>> +static int get_smmuv3_device(Object *obj, void *opaque)
> >>> +{
> >>> +    GArray *sdev_blob = opaque;
> >>> +    int min_bus, max_bus;
> >>> +    VirtMachineState *vms;
> >>> +    PlatformBusDevice *pbus;
> >>> +    SysBusDevice *sbdev;
> >>> +    SMMUv3Device sdev;
> >>> +    hwaddr base;
> >>> +    int irq;
> >>> +    PCIBus *bus;
> >>
> >> In a reverse christmas tree order? Or we could at least group
> >> those similar types together.
> >
> > Yeah. Reverse will look better I guess.
> >>
> >>> +    vms = VIRT_MACHINE(qdev_get_machine());
> >>> +    pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
> >>> +    sbdev = SYS_BUS_DEVICE(obj);
> >>> +    base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
> >>> +    irq = platform_bus_get_irqn(pbus, sbdev, 0);
> >>> +
> >>> +    base += vms->memmap[VIRT_PLATFORM_BUS].base;
> >>> +    irq += vms->irqmap[VIRT_PLATFORM_BUS];
> >>> +
> >>> +    pci_bus_range(bus, &min_bus, &max_bus);
> >>> +    sdev.smmu_idmap.input_base = min_bus << 8;
> >>> +    sdev.smmu_idmap.id_count = (max_bus - min_bus + 1) << 8;
> >>> +    sdev.base = base;
> >>> +    sdev.irq = irq + ARM_SPI_BASE;
> >>
> >> Hmm, these base/irq local variables don't look necessary.
> >>
> >>> +    g_array_append_val(sdev_blob, sdev);
> >>> +    return 0;
> >>> +}
> >>> +
> >>>   /*
> >>>    * Input Output Remapping Table (IORT)
> >>>    * Conforms to "IO Remapping Table System Software on ARM
> >> Platforms",
> >>> @@ -275,25 +330,42 @@ static void
> >>>   build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState
> >> *vms)
> >>>   {
> >>>       int i, nb_nodes, rc_mapping_count;
> >>> -    size_t node_size, smmu_offset = 0;
> >>> +    size_t node_size, *smmu_offset = NULL;
> >>>       AcpiIortIdMapping *idmap;
> >>> +    int num_smmus = 0;
> >>>       uint32_t id = 0;
> >>>       GArray *smmu_idmaps = g_array_new(false, true,
> >> sizeof(AcpiIortIdMapping));
> >>>       GArray *its_idmaps = g_array_new(false, true,
> >> sizeof(AcpiIortIdMapping));
> >>> +    GArray *smmuv3_devices = g_array_new(false, true,
> >> sizeof(SMMUv3Device));
> >>>
> >>>       AcpiTable table = { .sig = "IORT", .rev = 3, .oem_id = vms->oem_id,
> >>>                           .oem_table_id = vms->oem_table_id };
> >>>       /* Table 2 The IORT */
> >>>       acpi_table_begin(&table, table_data);
> >>>
> >>> -    if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> >>> -        AcpiIortIdMapping next_range = {0};
> >>> -
> >>> +    nb_nodes = 2; /* RC, ITS */
> >>> +    if (vms->iommu == VIRT_IOMMU_SMMUV3_DEV) {
> >>> +        object_child_foreach_recursive(object_get_root(),
> >>> +                                       get_smmuv3_device, smmuv3_devices);
> >>> +         /* Sort the smmuv3-devices by smmu idmap input_base */
> >>> +        g_array_sort(smmuv3_devices, smmuv3_dev_idmap_compare);
> >>> +        /*  Fill smmu idmap from sorted smmuv3 devices array */
> >>> +        for (i = 0; i < smmuv3_devices->len; i++) {
> >>> +            SMMUv3Device *s = &g_array_index(smmuv3_devices,
> >> SMMUv3Device, i);
> >>> +            g_array_append_val(smmu_idmaps, s->smmu_idmap);
> >>> +        }
> >>> +        num_smmus = smmuv3_devices->len;
> >>> +    } else if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> >>>           object_child_foreach_recursive(object_get_root(),
> >>>                                          iort_host_bridges, smmu_idmaps);
> >>>
> >>>           /* Sort the smmu idmap by input_base */
> >>>           g_array_sort(smmu_idmaps, iort_idmap_compare);
> >>
> >> VIRT_IOMMU_SMMUV3 seems to fit the struct SMMUv3Device very well,
> >> given that it has base, irq, and smmu_idmaps too?
> >
> > One difference though is the legacy VIRT_IOMMU_SMMUV3 one is a
> global
> > Machine wide one nad can be associated with multiple PCIe root
> complexes
> > which will result in smmu_idmaps array. See the iort_host_bridges() fn.
> >
> Didn't quite follow this logic; shouldn't the iort be built for devices
> tagged for a virt-iommu or dependent on a pcie rp (bus num/bus-num
> range of an RP)?
> Thus, it's just an IORT with one or more IORT nodes?
> I could see how the current iort_host_bridge() may need a revision to work
> with -device arm-smmu configs.
> 
> Which would bring me back to my earlier question, and it's likely tied to
> this IORT construction:
>   -- can you have both types defined in a machine model?

Let me try to explain.

Suppose we have a virt machine with two PCIe RCs, the default pcie.0
and  a pxb-pcie RC pcie.1.

For legacy "iommu=smmuv3" device case, we can  only have one SMMUv3
device per machine and all the PCIe RCs in the machine are by default
gets attached to that SMMUv3. There is no provision to associate a specific
RC to the legacy SMMUv3 (of course you can use bypass_iommu option to
bypass the SMMU if you want). 

As per IORT specification each SMMUv3 device must have a corresponding
SMMUv3 node, which contains an array of ID mappings.

From the IO remapping table spec,
https://developer.arm.com/documentation/den0049/latest/

"ID mappings represent the formula by which an ID from a source is
 converted to an ID in a destination. For example, for a root complex
 behind an SMMU, the RID originating from that root complex must
 be converted to a StreamID in the destination SMMU."

So the IORT representation for above case is something like below,

[SMMUv3 Node]
       -->id_maps for pcie.0
       -->id_maps for pcie.1

But for the new arm-smmuv3-dev, we have to associate a specific RC with
each SMMUV3 device,

  -device arm-smmuv3-dev,bus=pcie.0,id=smmuv3.1 \ 
   ..
 -device arm-smmuv3-dev,bus=pcie.1,id=smmuv3.2 \

In this case, the IORT would look like:

[smmuv3.1 Node)]
       -->id_maps for pcie.0

[smmuv3.2 Node)]
       -->id_maps for pcie.1

So basically it is not possible/is difficult(and also doesn't make much sense)
to have both legacy SMMUv3 and new SMMUv3 device co-exist in a system 
as the legacy one covers all the RCs in the machine.
 
> 
> >>
> >> Maybe we could generalize the struct SMMUv3Device to store both
> >> cases. Perhaps just SMMUv3AcpiInfo? And then ..
> >
> I didn't follow 'SMMUv3AcpiInfo' since I don't see that in current qemu tree;
> or is the statement trying to say its a 'mere matter of the proper acpi info
> generation'?

I think Nicolin is suggesting to use a common struct 'SMMUv3AcpiInfo'
for both legacy and new cases.

> 
> > Could do. But then you have to have  a smmu_idmaps array and then
> check
> > the length of it to cover legacy SMMUv3 case I guess.
> >
> Aren't they going to be different idmaps for different IORT nodes?

Please see above explanation.

Thanks,
Shameer
 

RE: [PATCH 0/5] Add support for user creatable SMMUv3 device

[Shameerali Kolothum Thodi via]

> -----Original Message-----
> From: Donald Dutile <ddutile@redhat.com>
> Sent: Friday, April 18, 2025 9:34 PM
> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
> qemu-devel@nongnu.org
> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> nicolinc@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
> mochs@nvidia.com; smostafa@google.com; Linuxarm
> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [PATCH 0/5] Add support for user creatable SMMUv3 device
> 
> Shameer,
> Hi!
> 
> First off, like the partitioning of these pieces.
> 
> On 4/15/25 4:10 AM, Shameer Kolothum wrote:
> > Hi All,
> >
> > This patch series introduces support for a user-creatable SMMUv3 device
> > (-device arm-smmuv3-dev) in QEMU.
> >
> Can we drop the '-dev', as 'arm-smmu' is sufficient, as is '-device'?
> 
> I know, internal to QEMU, there's already an ARM_SMMU, but as suggested
> later,
> if it uses the same struct, the qemu cmdline syntax is a bit less redundant.

Trust me I tried that😊. The problem is that, the legacy one doesn't have a bus
associated with it and the moment we change that and have bus specified for the
 "-device arm-smmuv3, bus=pcie.0" the legacy smmuv3 creation in virt,

create_smmu() --> qdev_new(TYPE_ARM_SMMUV3)

will fails as it expects the bus type to be specified for it. I couldn't find a way
to solve that.

> 
> > The implementation is based on feedback received from the RFCv2[0]:
> > "hw/arm/virt: Add support for user-creatable accelerated SMMUv3"
> >
> > Currently, QEMU's SMMUv3 emulation (iommu=smmuv3) is tied to the
> machine
> > and does not support instantiating multiple SMMUv3 devices—each
> associated
> > with a separate PCIe root complex. In contrast, real-world ARM systems
> > often include multiple SMMUv3 instances, each bound to a different PCIe
> > root complex.
> >
> > This also lays the groundwork for supporting accelerated SMMUv3, as
> > proposed in the aforementioned RFC. Please note, the
> accelerated SMMUv3
> > support is not part of this series and will be sent out as a separate
> > series later on top of this one.
> >
> > Summary of changes:
> >
> >   -Introduces support for multiple -device arm-smmuv3-dev,bus=pcie.x
> >    instances.
> >
> >    Example usage:
> >
> >    -device arm-smmuv3-dev,bus=pcie.0
> >    -device virtio-net-pci,bus=pcie.0
> >    -device pxb-pcie,id=pcie.1,bus_nr=2
> >    -device arm-smmuv3-dev,bus=pcie.1
> >    -device pcie-root-port,id=pcie.port1,bus=pcie.1
> >    -device virtio-net-pci,bus=pcie.port1
> >
> >    -Supports either the legacy iommu=smmuv3 option or the new
> >    "-device arm-smmuv3-dev" model.
> >
> nice! :)
> Can it support both? i.e., some devices using a system-wide, legacy
> smmuv3,
> and some pcie devices using the -device arm-smmuv3 ?

Please see my reply to patch #2. In short No, it doesn't support both.

Also I think we should slowly deprecate the use of legacy SMMUv3 going 
forward unless there is a strong use case/reason to support it.

> If not, then add a check to min-warn or max-fail, that config.
> I can see old machines being converted/upgraded to new machines,
> and they will want to switch from legacy->device smmuv3, and catching
> an edit/update to a machine change (or enabling automated conversion)
> would be helpful.

Please see Patch #4. It already checks and prevents if incompatible SMMUv3
types are specified. Or is the suggestion here is to add something extra?
Please let me know.

Thanks,
Shameer

Re: [PATCH 0/5] Add support for user creatable SMMUv3 device

[Donald Dutile]

On 4/22/25 4:57 AM, Shameerali Kolothum Thodi wrote:
> 
>> -----Original Message-----
>> From: Donald Dutile <ddutile@redhat.com>
>> Sent: Friday, April 18, 2025 9:34 PM
>> To: Shameerali Kolothum Thodi
>> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
>> qemu-devel@nongnu.org
>> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
>> nicolinc@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
>> Subject: Re: [PATCH 0/5] Add support for user creatable SMMUv3 device
>>
>> Shameer,
>> Hi!
>>
>> First off, like the partitioning of these pieces.
>>
>> On 4/15/25 4:10 AM, Shameer Kolothum wrote:
>>> Hi All,
>>>
>>> This patch series introduces support for a user-creatable SMMUv3 device
>>> (-device arm-smmuv3-dev) in QEMU.
>>>
>> Can we drop the '-dev', as 'arm-smmu' is sufficient, as is '-device'?
>>
>> I know, internal to QEMU, there's already an ARM_SMMU, but as suggested
>> later,
>> if it uses the same struct, the qemu cmdline syntax is a bit less redundant.
> 
> Trust me I tried that😊. The problem is that, the legacy one doesn't have a bus
> associated with it and the moment we change that and have bus specified for the
>   "-device arm-smmuv3, bus=pcie.0" the legacy smmuv3 creation in virt,
> 
> create_smmu() --> qdev_new(TYPE_ARM_SMMUV3)
> 
> will fails as it expects the bus type to be specified for it. I couldn't find a way
> to solve that.
> 
I guessed that's why there was new syntax for what is basically an extension of previous syntax.
I'll dig more into it and see if there's a way to handle it.
qemu does handle varying added params (id=blah, no id=blah),
so I'm researching how that's done, to figure out how the legacy, smmu-for-all,
and the smmu-for-specific pcie<RC> can both be supported.
Given your stated trials and tribulations, this should be fun. ;-)
- Don

>>
>>> The implementation is based on feedback received from the RFCv2[0]:
>>> "hw/arm/virt: Add support for user-creatable accelerated SMMUv3"
>>>
>>> Currently, QEMU's SMMUv3 emulation (iommu=smmuv3) is tied to the
>> machine
>>> and does not support instantiating multiple SMMUv3 devices—each
>> associated
>>> with a separate PCIe root complex. In contrast, real-world ARM systems
>>> often include multiple SMMUv3 instances, each bound to a different PCIe
>>> root complex.
>>>
>>> This also lays the groundwork for supporting accelerated SMMUv3, as
>>> proposed in the aforementioned RFC. Please note, the
>> accelerated SMMUv3
>>> support is not part of this series and will be sent out as a separate
>>> series later on top of this one.
>>>
>>> Summary of changes:
>>>
>>>    -Introduces support for multiple -device arm-smmuv3-dev,bus=pcie.x
>>>     instances.
>>>
>>>     Example usage:
>>>
>>>     -device arm-smmuv3-dev,bus=pcie.0
>>>     -device virtio-net-pci,bus=pcie.0
>>>     -device pxb-pcie,id=pcie.1,bus_nr=2
>>>     -device arm-smmuv3-dev,bus=pcie.1
>>>     -device pcie-root-port,id=pcie.port1,bus=pcie.1
>>>     -device virtio-net-pci,bus=pcie.port1
>>>
>>>     -Supports either the legacy iommu=smmuv3 option or the new
>>>     "-device arm-smmuv3-dev" model.
>>>
>> nice! :)
>> Can it support both? i.e., some devices using a system-wide, legacy
>> smmuv3,
>> and some pcie devices using the -device arm-smmuv3 ?
> 
> Please see my reply to patch #2. In short No, it doesn't support both.
> 
> Also I think we should slowly deprecate the use of legacy SMMUv3 going
> forward unless there is a strong use case/reason to support it.
> 
>> If not, then add a check to min-warn or max-fail, that config.
>> I can see old machines being converted/upgraded to new machines,
>> and they will want to switch from legacy->device smmuv3, and catching
>> an edit/update to a machine change (or enabling automated conversion)
>> would be helpful.
> 
> Please see Patch #4. It already checks and prevents if incompatible SMMUv3
> types are specified. Or is the suggestion here is to add something extra?
> Please let me know.
> 
> Thanks,
> Shameer
> 

Re: [PATCH 0/5] Add support for user creatable SMMUv3 device

[Eric Auger]

Hi Shameer,

On 4/28/25 8:41 PM, Donald Dutile wrote:
>
>
> On 4/22/25 4:57 AM, Shameerali Kolothum Thodi wrote:
>>
>>> -----Original Message-----
>>> From: Donald Dutile <ddutile@redhat.com>
>>> Sent: Friday, April 18, 2025 9:34 PM
>>> To: Shameerali Kolothum Thodi
>>> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
>>> qemu-devel@nongnu.org
>>> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
>>> nicolinc@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
>>> mochs@nvidia.com; smostafa@google.com; Linuxarm
>>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
>>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
>>> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
>>> Subject: Re: [PATCH 0/5] Add support for user creatable SMMUv3 device
>>>
>>> Shameer,
>>> Hi!
>>>
>>> First off, like the partitioning of these pieces.
>>>
>>> On 4/15/25 4:10 AM, Shameer Kolothum wrote:
>>>> Hi All,
>>>>
>>>> This patch series introduces support for a user-creatable SMMUv3
>>>> device
>>>> (-device arm-smmuv3-dev) in QEMU.
>>>>
>>> Can we drop the '-dev', as 'arm-smmu' is sufficient, as is '-device'?
>>>
>>> I know, internal to QEMU, there's already an ARM_SMMU, but as suggested
>>> later,
>>> if it uses the same struct, the qemu cmdline syntax is a bit less
>>> redundant.
>>
>> Trust me I tried that😊. The problem is that, the legacy one doesn't
>> have a bus
>> associated with it and the moment we change that and have bus
>> specified for the
>>   "-device arm-smmuv3, bus=pcie.0" the legacy smmuv3 creation in virt,
>>
>> create_smmu() --> qdev_new(TYPE_ARM_SMMUV3)
I have the impression you can achieve your goal by replacing
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
by
qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
See https://github.com/eauger/qemu/tree/arm-smmuv3-dev-rc0

and its top patch.

indeed it would be better if we could reuse the same device.

Eric
>>
>> will fails as it expects the bus type to be specified for it. I
>> couldn't find a way
>> to solve that.
>>
> I guessed that's why there was new syntax for what is basically an
> extension of previous syntax.
> I'll dig more into it and see if there's a way to handle it.
> qemu does handle varying added params (id=blah, no id=blah),
> so I'm researching how that's done, to figure out how the legacy,
> smmu-for-all,
> and the smmu-for-specific pcie<RC> can both be supported.
> Given your stated trials and tribulations, this should be fun. ;-)
> - Don
>
>>>
>>>> The implementation is based on feedback received from the RFCv2[0]:
>>>> "hw/arm/virt: Add support for user-creatable accelerated SMMUv3"
>>>>
>>>> Currently, QEMU's SMMUv3 emulation (iommu=smmuv3) is tied to the
>>> machine
>>>> and does not support instantiating multiple SMMUv3 devices—each
>>> associated
>>>> with a separate PCIe root complex. In contrast, real-world ARM systems
>>>> often include multiple SMMUv3 instances, each bound to a different
>>>> PCIe
>>>> root complex.
>>>>
>>>> This also lays the groundwork for supporting accelerated SMMUv3, as
>>>> proposed in the aforementioned RFC. Please note, the
>>> accelerated SMMUv3
>>>> support is not part of this series and will be sent out as a separate
>>>> series later on top of this one.
>>>>
>>>> Summary of changes:
>>>>
>>>>    -Introduces support for multiple -device arm-smmuv3-dev,bus=pcie.x
>>>>     instances.
>>>>
>>>>     Example usage:
>>>>
>>>>     -device arm-smmuv3-dev,bus=pcie.0
>>>>     -device virtio-net-pci,bus=pcie.0
>>>>     -device pxb-pcie,id=pcie.1,bus_nr=2
>>>>     -device arm-smmuv3-dev,bus=pcie.1
>>>>     -device pcie-root-port,id=pcie.port1,bus=pcie.1
>>>>     -device virtio-net-pci,bus=pcie.port1
>>>>
>>>>     -Supports either the legacy iommu=smmuv3 option or the new
>>>>     "-device arm-smmuv3-dev" model.
>>>>
>>> nice! :)
>>> Can it support both? i.e., some devices using a system-wide, legacy
>>> smmuv3,
>>> and some pcie devices using the -device arm-smmuv3 ?
>>
>> Please see my reply to patch #2. In short No, it doesn't support both.
>>
>> Also I think we should slowly deprecate the use of legacy SMMUv3 going
>> forward unless there is a strong use case/reason to support it.
>>
>>> If not, then add a check to min-warn or max-fail, that config.
>>> I can see old machines being converted/upgraded to new machines,
>>> and they will want to switch from legacy->device smmuv3, and catching
>>> an edit/update to a machine change (or enabling automated conversion)
>>> would be helpful.
>>
>> Please see Patch #4. It already checks and prevents if incompatible
>> SMMUv3
>> types are specified. Or is the suggestion here is to add something
>> extra?
>> Please let me know.
>>
>> Thanks,
>> Shameer
>>
>

RE: [PATCH 0/5] Add support for user creatable SMMUv3 device

[Shameerali Kolothum Thodi via]

Hi Eric,

> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: Wednesday, April 30, 2025 6:48 PM
> To: Donald Dutile <ddutile@redhat.com>; Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
> qemu-devel@nongnu.org
> Cc: peter.maydell@linaro.org; jgg@nvidia.com; nicolinc@nvidia.com;
> berrange@redhat.com; nathanc@nvidia.com; mochs@nvidia.com;
> smostafa@google.com; Linuxarm <linuxarm@huawei.com>; Wangzhou (B)
> <wangzhou1@hisilicon.com>; jiangkunkun <jiangkunkun@huawei.com>;
> Jonathan Cameron <jonathan.cameron@huawei.com>;
> zhangfei.gao@linaro.org
> Subject: Re: [PATCH 0/5] Add support for user creatable SMMUv3 device
> 
> Hi Shameer,
> 
> On 4/28/25 8:41 PM, Donald Dutile wrote:
> >
> >
> > On 4/22/25 4:57 AM, Shameerali Kolothum Thodi wrote:
> >>
> >>> -----Original Message-----
> >>> From: Donald Dutile <ddutile@redhat.com>
> >>> Sent: Friday, April 18, 2025 9:34 PM
> >>> To: Shameerali Kolothum Thodi
> >>> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
> >>> qemu-devel@nongnu.org
> >>> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> >>> nicolinc@nvidia.com; berrange@redhat.com; nathanc@nvidia.com;
> >>> mochs@nvidia.com; smostafa@google.com; Linuxarm
> >>> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> >>> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> >>> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> >>> Subject: Re: [PATCH 0/5] Add support for user creatable SMMUv3
> device
> >>>
> >>> Shameer,
> >>> Hi!
> >>>
> >>> First off, like the partitioning of these pieces.
> >>>
> >>> On 4/15/25 4:10 AM, Shameer Kolothum wrote:
> >>>> Hi All,
> >>>>
> >>>> This patch series introduces support for a user-creatable SMMUv3
> >>>> device
> >>>> (-device arm-smmuv3-dev) in QEMU.
> >>>>
> >>> Can we drop the '-dev', as 'arm-smmu' is sufficient, as is '-device'?
> >>>
> >>> I know, internal to QEMU, there's already an ARM_SMMU, but as
> suggested
> >>> later,
> >>> if it uses the same struct, the qemu cmdline syntax is a bit less
> >>> redundant.
> >>
> >> Trust me I tried that😊. The problem is that, the legacy one doesn't
> >> have a bus
> >> associated with it and the moment we change that and have bus
> >> specified for the
> >>   "-device arm-smmuv3, bus=pcie.0" the legacy smmuv3 creation in virt,
> >>
> >> create_smmu() --> qdev_new(TYPE_ARM_SMMUV3)
> I have the impression you can achieve your goal by replacing
> sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> by
> qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
> See https://github.com/eauger/qemu/tree/arm-smmuv3-dev-rc0

Cool!. That does the trick. Thanks.
 
> and its top patch.
> 
> indeed it would be better if we could reuse the same device.

I will respin soon with the above and addressing other comments received.

Thanks,
Shameer