Allow cold-plugging of an SMMUv3 device on the virt machine when no
global (legacy) SMMUv3 is present or when a virtio-iommu is specified.
This user-created SMMUv3 device is tied to a specific PCI bus provided
by the user, so ensure the IOMMU ops are configured accordingly.
Due to current limitations in QEMU’s device tree support, specifically
its inability to properly present pxb-pcie based root complexes and
their devices, the device tree support for the new SMMUv3 device is
limited to cases where it is attached to the default pcie.0 root complex.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Nathan Chen <nathanc@nvidia.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
hw/arm/smmu-common.c | 8 +++++-
hw/arm/smmuv3.c | 2 ++
hw/arm/virt.c | 50 ++++++++++++++++++++++++++++++++++++
hw/core/sysbus-fdt.c | 3 +++
include/hw/arm/smmu-common.h | 1 +
5 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index b15e7fd0e4..2ee4691299 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -959,7 +959,12 @@ static void smmu_base_realize(DeviceState *dev, Error **errp)
goto out_err;
}
}
- pci_setup_iommu(pci_bus, &smmu_ops, s);
+
+ if (s->smmu_per_bus) {
+ pci_setup_iommu_per_bus(pci_bus, &smmu_ops, s);
+ } else {
+ pci_setup_iommu(pci_bus, &smmu_ops, s);
+ }
return;
}
out_err:
@@ -984,6 +989,7 @@ static void smmu_base_reset_exit(Object *obj, ResetType type)
static const Property smmu_dev_properties[] = {
DEFINE_PROP_UINT8("bus_num", SMMUState, bus_num, 0),
+ DEFINE_PROP_BOOL("smmu_per_bus", SMMUState, smmu_per_bus, false),
DEFINE_PROP_LINK("primary-bus", SMMUState, primary_bus,
TYPE_PCI_BUS, PCIBus *),
};
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index ab67972353..bcf8af8dc7 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1996,6 +1996,8 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
device_class_set_parent_realize(dc, smmu_realize,
&c->parent_realize);
device_class_set_props(dc, smmuv3_properties);
+ dc->hotpluggable = false;
+ dc->user_creatable = true;
}
static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 05a14881cf..8662173c43 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"
@@ -1440,6 +1441,28 @@ static void create_smmuv3_dt_bindings(const VirtMachineState *vms, hwaddr base,
g_free(node);
}
+static void create_smmuv3_dev_dtb(VirtMachineState *vms,
+ DeviceState *dev, PCIBus *bus)
+{
+ 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);
+
+ 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, SMMU_IO_LEN, 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)
{
@@ -2935,6 +2958,16 @@ 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)) {
+ if (vms->legacy_smmuv3_present || vms->iommu == VIRT_IOMMU_VIRTIO) {
+ error_setg(errp, "virt machine already has %s set. "
+ "Doesn't support incompatible iommus",
+ (vms->legacy_smmuv3_present) ?
+ "iommu=smmuv3" : "virtio-iommu");
+ } else if (vms->iommu == VIRT_IOMMU_NONE) {
+ /* The new SMMUv3 device is specific to the PCI bus */
+ object_property_set_bool(OBJECT(dev), "smmu_per_bus", true, NULL);
+ }
}
}
@@ -2958,6 +2991,22 @@ 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)) {
+ if (!vms->legacy_smmuv3_present && vms->platform_bus_dev) {
+ PCIBus *bus;
+
+ bus = PCI_BUS(object_property_get_link(OBJECT(dev), "primary-bus",
+ &error_abort));
+ if (pci_bus_bypass_iommu(bus)) {
+ error_setg(errp, "Bypass option cannot be set for SMMUv3 "
+ "associated PCIe RC");
+ return;
+ }
+
+ create_smmuv3_dev_dtb(vms, dev, bus);
+ }
+ }
+
if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
PCIDevice *pdev = PCI_DEVICE(dev);
@@ -3160,6 +3209,7 @@ static void virt_machine_class_init(ObjectClass *oc, const 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);
#ifdef CONFIG_TPM
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
#endif
diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
index c339a27875..e80776080b 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"
@@ -518,6 +519,8 @@ static const BindingEntry bindings[] = {
#ifdef CONFIG_TPM
TYPE_BINDING(TYPE_TPM_TIS_SYSBUS, add_tpm_tis_fdt_node),
#endif
+ /* No generic DT support for smmuv3 dev. Support added for arm virt only */
+ TYPE_BINDING(TYPE_ARM_SMMUV3, no_fdt_node),
TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
TYPE_BINDING(TYPE_UEFI_VARS_SYSBUS, add_uefi_vars_node),
TYPE_BINDING("", NULL), /* last element */
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index e5e2d09294..80d0fecfde 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -161,6 +161,7 @@ struct SMMUState {
QLIST_HEAD(, SMMUDevice) devices_with_notifiers;
uint8_t bus_num;
PCIBus *primary_bus;
+ bool smmu_per_bus; /* SMMU is specific to the primary_bus */
};
struct SMMUBaseClass {
--
2.34.1
Hi Shameer,
On 7/3/25 10:46 AM, Shameer Kolothum wrote:
> Allow cold-plugging of an SMMUv3 device on the virt machine when no
> global (legacy) SMMUv3 is present or when a virtio-iommu is specified.
>
> This user-created SMMUv3 device is tied to a specific PCI bus provided
> by the user, so ensure the IOMMU ops are configured accordingly.
>
> Due to current limitations in QEMU’s device tree support, specifically
> its inability to properly present pxb-pcie based root complexes and
> their devices, the device tree support for the new SMMUv3 device is
> limited to cases where it is attached to the default pcie.0 root complex.
>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Nathan Chen <nathanc@nvidia.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
> hw/arm/smmu-common.c | 8 +++++-
> hw/arm/smmuv3.c | 2 ++
> hw/arm/virt.c | 50 ++++++++++++++++++++++++++++++++++++
> hw/core/sysbus-fdt.c | 3 +++
> include/hw/arm/smmu-common.h | 1 +
> 5 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index b15e7fd0e4..2ee4691299 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -959,7 +959,12 @@ static void smmu_base_realize(DeviceState *dev, Error **errp)
> goto out_err;
> }
> }
> - pci_setup_iommu(pci_bus, &smmu_ops, s);
> +
> + if (s->smmu_per_bus) {
> + pci_setup_iommu_per_bus(pci_bus, &smmu_ops, s);
> + } else {
> + pci_setup_iommu(pci_bus, &smmu_ops, s);
> + }
> return;
> }
> out_err:
> @@ -984,6 +989,7 @@ static void smmu_base_reset_exit(Object *obj, ResetType type)
>
> static const Property smmu_dev_properties[] = {
> DEFINE_PROP_UINT8("bus_num", SMMUState, bus_num, 0),
> + DEFINE_PROP_BOOL("smmu_per_bus", SMMUState, smmu_per_bus, false),
> DEFINE_PROP_LINK("primary-bus", SMMUState, primary_bus,
> TYPE_PCI_BUS, PCIBus *),
> };
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index ab67972353..bcf8af8dc7 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -1996,6 +1996,8 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
> device_class_set_parent_realize(dc, smmu_realize,
> &c->parent_realize);
> device_class_set_props(dc, smmuv3_properties);
> + dc->hotpluggable = false;
> + dc->user_creatable = true;
> }
>
> static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 05a14881cf..8662173c43 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"
> @@ -1440,6 +1441,28 @@ static void create_smmuv3_dt_bindings(const VirtMachineState *vms, hwaddr base,
> g_free(node);
> }
>
> +static void create_smmuv3_dev_dtb(VirtMachineState *vms,
> + DeviceState *dev, PCIBus *bus)
> +{
> + 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);
> +
> + if (strcmp("pcie.0", bus->qbus.name)) {
> + warn_report("SMMUv3 device only supported with pcie.0 for DT");
while testing the series I hit the warning with a rhel guest which boots
with ACPI.
I think we shall make the check smarter to avoid that.
maybe also check firmware_loaded and virt_is_acpi_enabled()?
Cheers
Eric
> + 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, SMMU_IO_LEN, 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)
> {
> @@ -2935,6 +2958,16 @@ 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)) {
> + if (vms->legacy_smmuv3_present || vms->iommu == VIRT_IOMMU_VIRTIO) {
> + error_setg(errp, "virt machine already has %s set. "
> + "Doesn't support incompatible iommus",
> + (vms->legacy_smmuv3_present) ?
> + "iommu=smmuv3" : "virtio-iommu");
> + } else if (vms->iommu == VIRT_IOMMU_NONE) {
> + /* The new SMMUv3 device is specific to the PCI bus */
> + object_property_set_bool(OBJECT(dev), "smmu_per_bus", true, NULL);
> + }
> }
> }
>
> @@ -2958,6 +2991,22 @@ 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)) {
> + if (!vms->legacy_smmuv3_present && vms->platform_bus_dev) {
> + PCIBus *bus;
> +
> + bus = PCI_BUS(object_property_get_link(OBJECT(dev), "primary-bus",
> + &error_abort));
> + if (pci_bus_bypass_iommu(bus)) {
> + error_setg(errp, "Bypass option cannot be set for SMMUv3 "
> + "associated PCIe RC");
> + return;
> + }
> +
> + create_smmuv3_dev_dtb(vms, dev, bus);
> + }
> + }
> +
> if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> PCIDevice *pdev = PCI_DEVICE(dev);
>
> @@ -3160,6 +3209,7 @@ static void virt_machine_class_init(ObjectClass *oc, const 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);
> #ifdef CONFIG_TPM
> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> #endif
> diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
> index c339a27875..e80776080b 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"
> @@ -518,6 +519,8 @@ static const BindingEntry bindings[] = {
> #ifdef CONFIG_TPM
> TYPE_BINDING(TYPE_TPM_TIS_SYSBUS, add_tpm_tis_fdt_node),
> #endif
> + /* No generic DT support for smmuv3 dev. Support added for arm virt only */
> + TYPE_BINDING(TYPE_ARM_SMMUV3, no_fdt_node),
> TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
> TYPE_BINDING(TYPE_UEFI_VARS_SYSBUS, add_uefi_vars_node),
> TYPE_BINDING("", NULL), /* last element */
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index e5e2d09294..80d0fecfde 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -161,6 +161,7 @@ struct SMMUState {
> QLIST_HEAD(, SMMUDevice) devices_with_notifiers;
> uint8_t bus_num;
> PCIBus *primary_bus;
> + bool smmu_per_bus; /* SMMU is specific to the primary_bus */
> };
>
> struct SMMUBaseClass {
Hi Eric,
> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: Tuesday, July 8, 2025 8:41 AM
> To: 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;
> ddutile@redhat.com; berrange@redhat.com; imammedo@redhat.com;
> nathanc@nvidia.com; mochs@nvidia.com; smostafa@google.com;
> gustavo.romero@linaro.org; mst@redhat.com;
> marcel.apfelbaum@gmail.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 v6 08/12] hw/arm/virt: Allow user-creatable SMMUv3
> dev instantiation
>
> Hi Shameer,
>
> On 7/3/25 10:46 AM, Shameer Kolothum wrote:
> > Allow cold-plugging of an SMMUv3 device on the virt machine when no
> > global (legacy) SMMUv3 is present or when a virtio-iommu is specified.
> >
> > This user-created SMMUv3 device is tied to a specific PCI bus provided
> > by the user, so ensure the IOMMU ops are configured accordingly.
> >
> > Due to current limitations in QEMU’s device tree support, specifically
> > its inability to properly present pxb-pcie based root complexes and
> > their devices, the device tree support for the new SMMUv3 device is
> > limited to cases where it is attached to the default pcie.0 root complex.
> >
> > Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> > Reviewed-by: Eric Auger <eric.auger@redhat.com>
> > Tested-by: Nathan Chen <nathanc@nvidia.com>
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > ---
> > hw/arm/smmu-common.c | 8 +++++-
> > hw/arm/smmuv3.c | 2 ++
> > hw/arm/virt.c | 50 ++++++++++++++++++++++++++++++++++++
> > hw/core/sysbus-fdt.c | 3 +++
> > include/hw/arm/smmu-common.h | 1 +
> > 5 files changed, 63 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> > index b15e7fd0e4..2ee4691299 100644
> > --- a/hw/arm/smmu-common.c
> > +++ b/hw/arm/smmu-common.c
> > @@ -959,7 +959,12 @@ static void smmu_base_realize(DeviceState *dev,
> Error **errp)
> > goto out_err;
> > }
> > }
> > - pci_setup_iommu(pci_bus, &smmu_ops, s);
> > +
> > + if (s->smmu_per_bus) {
> > + pci_setup_iommu_per_bus(pci_bus, &smmu_ops, s);
> > + } else {
> > + pci_setup_iommu(pci_bus, &smmu_ops, s);
> > + }
> > return;
> > }
> > out_err:
> > @@ -984,6 +989,7 @@ static void smmu_base_reset_exit(Object *obj,
> ResetType type)
> >
> > static const Property smmu_dev_properties[] = {
> > DEFINE_PROP_UINT8("bus_num", SMMUState, bus_num, 0),
> > + DEFINE_PROP_BOOL("smmu_per_bus", SMMUState, smmu_per_bus,
> false),
> > DEFINE_PROP_LINK("primary-bus", SMMUState, primary_bus,
> > TYPE_PCI_BUS, PCIBus *),
> > };
> > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> > index ab67972353..bcf8af8dc7 100644
> > --- a/hw/arm/smmuv3.c
> > +++ b/hw/arm/smmuv3.c
> > @@ -1996,6 +1996,8 @@ static void smmuv3_class_init(ObjectClass
> *klass, const void *data)
> > device_class_set_parent_realize(dc, smmu_realize,
> > &c->parent_realize);
> > device_class_set_props(dc, smmuv3_properties);
> > + dc->hotpluggable = false;
> > + dc->user_creatable = true;
> > }
> >
> > static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 05a14881cf..8662173c43 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"
> > @@ -1440,6 +1441,28 @@ static void create_smmuv3_dt_bindings(const
> VirtMachineState *vms, hwaddr base,
> > g_free(node);
> > }
> >
> > +static void create_smmuv3_dev_dtb(VirtMachineState *vms,
> > + DeviceState *dev, PCIBus *bus)
> > +{
> > + 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);
> > +
> > + if (strcmp("pcie.0", bus->qbus.name)) {
> > + warn_report("SMMUv3 device only supported with pcie.0 for DT");
> while testing the series I hit the warning with a rhel guest which boots
> with ACPI.
> I think we shall make the check smarter to avoid that.
> maybe also check firmware_loaded and virt_is_acpi_enabled()?
Thanks for giving it a spin. Yes, just confirmed that the warning appears.
The above check will work, but can we make use of vms->acpi_dev for
this check instead? It is essentially the same and I think that will work.
if (!vms->acpi_dev && strcmp("pcie.0", bus->qbus.name))
Please let me know.
Thanks,
Shameer
Hi Shameer,
On 7/8/25 10:54 AM, Shameerali Kolothum Thodi wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Sent: Tuesday, July 8, 2025 8:41 AM
>> To: 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;
>> ddutile@redhat.com; berrange@redhat.com; imammedo@redhat.com;
>> nathanc@nvidia.com; mochs@nvidia.com; smostafa@google.com;
>> gustavo.romero@linaro.org; mst@redhat.com;
>> marcel.apfelbaum@gmail.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 v6 08/12] hw/arm/virt: Allow user-creatable SMMUv3
>> dev instantiation
>>
>> Hi Shameer,
>>
>> On 7/3/25 10:46 AM, Shameer Kolothum wrote:
>>> Allow cold-plugging of an SMMUv3 device on the virt machine when no
>>> global (legacy) SMMUv3 is present or when a virtio-iommu is specified.
>>>
>>> This user-created SMMUv3 device is tied to a specific PCI bus provided
>>> by the user, so ensure the IOMMU ops are configured accordingly.
>>>
>>> Due to current limitations in QEMU’s device tree support, specifically
>>> its inability to properly present pxb-pcie based root complexes and
>>> their devices, the device tree support for the new SMMUv3 device is
>>> limited to cases where it is attached to the default pcie.0 root complex.
>>>
>>> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>>> Tested-by: Nathan Chen <nathanc@nvidia.com>
>>> Signed-off-by: Shameer Kolothum
>> <shameerali.kolothum.thodi@huawei.com>
>>> ---
>>> hw/arm/smmu-common.c | 8 +++++-
>>> hw/arm/smmuv3.c | 2 ++
>>> hw/arm/virt.c | 50 ++++++++++++++++++++++++++++++++++++
>>> hw/core/sysbus-fdt.c | 3 +++
>>> include/hw/arm/smmu-common.h | 1 +
>>> 5 files changed, 63 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
>>> index b15e7fd0e4..2ee4691299 100644
>>> --- a/hw/arm/smmu-common.c
>>> +++ b/hw/arm/smmu-common.c
>>> @@ -959,7 +959,12 @@ static void smmu_base_realize(DeviceState *dev,
>> Error **errp)
>>> goto out_err;
>>> }
>>> }
>>> - pci_setup_iommu(pci_bus, &smmu_ops, s);
>>> +
>>> + if (s->smmu_per_bus) {
>>> + pci_setup_iommu_per_bus(pci_bus, &smmu_ops, s);
>>> + } else {
>>> + pci_setup_iommu(pci_bus, &smmu_ops, s);
>>> + }
>>> return;
>>> }
>>> out_err:
>>> @@ -984,6 +989,7 @@ static void smmu_base_reset_exit(Object *obj,
>> ResetType type)
>>> static const Property smmu_dev_properties[] = {
>>> DEFINE_PROP_UINT8("bus_num", SMMUState, bus_num, 0),
>>> + DEFINE_PROP_BOOL("smmu_per_bus", SMMUState, smmu_per_bus,
>> false),
>>> DEFINE_PROP_LINK("primary-bus", SMMUState, primary_bus,
>>> TYPE_PCI_BUS, PCIBus *),
>>> };
>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>> index ab67972353..bcf8af8dc7 100644
>>> --- a/hw/arm/smmuv3.c
>>> +++ b/hw/arm/smmuv3.c
>>> @@ -1996,6 +1996,8 @@ static void smmuv3_class_init(ObjectClass
>> *klass, const void *data)
>>> device_class_set_parent_realize(dc, smmu_realize,
>>> &c->parent_realize);
>>> device_class_set_props(dc, smmuv3_properties);
>>> + dc->hotpluggable = false;
>>> + dc->user_creatable = true;
>>> }
>>>
>>> static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 05a14881cf..8662173c43 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"
>>> @@ -1440,6 +1441,28 @@ static void create_smmuv3_dt_bindings(const
>> VirtMachineState *vms, hwaddr base,
>>> g_free(node);
>>> }
>>>
>>> +static void create_smmuv3_dev_dtb(VirtMachineState *vms,
>>> + DeviceState *dev, PCIBus *bus)
>>> +{
>>> + 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);
>>> +
>>> + if (strcmp("pcie.0", bus->qbus.name)) {
>>> + warn_report("SMMUv3 device only supported with pcie.0 for DT");
>> while testing the series I hit the warning with a rhel guest which boots
>> with ACPI.
>> I think we shall make the check smarter to avoid that.
>> maybe also check firmware_loaded and virt_is_acpi_enabled()?
> Thanks for giving it a spin. Yes, just confirmed that the warning appears.
> The above check will work, but can we make use of vms->acpi_dev for
> this check instead? It is essentially the same and I think that will work.
>
> if (!vms->acpi_dev && strcmp("pcie.0", bus->qbus.name))
>
> Please let me know.
with that fixed, feel free to add my
*Tested-by: Eric Auger <eric.auger@redhat.com> I have tested non
regression on legacy SMMU, SMMU device protecting pcie.0 and pxb
topologies. Looks good to me. Thanks Eric *
>
> Thanks,
> Shameer
Hi Eric,
> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: Tuesday, July 8, 2025 10:56 AM
> To: 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;
> ddutile@redhat.com; berrange@redhat.com; imammedo@redhat.com;
> nathanc@nvidia.com; mochs@nvidia.com; smostafa@google.com;
> gustavo.romero@linaro.org; mst@redhat.com;
> marcel.apfelbaum@gmail.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 v6 08/12] hw/arm/virt: Allow user-creatable SMMUv3
> dev instantiation
>
> Hi Shameer,
>
> On 7/8/25 10:54 AM, Shameerali Kolothum Thodi wrote:
> > Hi Eric,
> >
> >> -----Original Message-----
> >> From: Eric Auger <eric.auger@redhat.com>
> >> Sent: Tuesday, July 8, 2025 8:41 AM
> >> To: 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;
> >> ddutile@redhat.com; berrange@redhat.com; imammedo@redhat.com;
> >> nathanc@nvidia.com; mochs@nvidia.com; smostafa@google.com;
> >> gustavo.romero@linaro.org; mst@redhat.com;
> >> marcel.apfelbaum@gmail.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 v6 08/12] hw/arm/virt: Allow user-creatable
> SMMUv3
> >> dev instantiation
> >>
> >> Hi Shameer,
> >>
> >> On 7/3/25 10:46 AM, Shameer Kolothum wrote:
> >>> Allow cold-plugging of an SMMUv3 device on the virt machine when no
> >>> global (legacy) SMMUv3 is present or when a virtio-iommu is specified.
> >>>
> >>> This user-created SMMUv3 device is tied to a specific PCI bus provided
> >>> by the user, so ensure the IOMMU ops are configured accordingly.
> >>>
> >>> Due to current limitations in QEMU’s device tree support, specifically
> >>> its inability to properly present pxb-pcie based root complexes and
> >>> their devices, the device tree support for the new SMMUv3 device is
> >>> limited to cases where it is attached to the default pcie.0 root complex.
> >>>
> >>> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> >>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> >>> Tested-by: Nathan Chen <nathanc@nvidia.com>
> >>> Signed-off-by: Shameer Kolothum
> >> <shameerali.kolothum.thodi@huawei.com>
> >>> ---
> >>> hw/arm/smmu-common.c | 8 +++++-
> >>> hw/arm/smmuv3.c | 2 ++
> >>> hw/arm/virt.c | 50 ++++++++++++++++++++++++++++++++++++
> >>> hw/core/sysbus-fdt.c | 3 +++
> >>> include/hw/arm/smmu-common.h | 1 +
> >>> 5 files changed, 63 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> >>> index b15e7fd0e4..2ee4691299 100644
> >>> --- a/hw/arm/smmu-common.c
> >>> +++ b/hw/arm/smmu-common.c
> >>> @@ -959,7 +959,12 @@ static void smmu_base_realize(DeviceState
> *dev,
> >> Error **errp)
> >>> goto out_err;
> >>> }
> >>> }
> >>> - pci_setup_iommu(pci_bus, &smmu_ops, s);
> >>> +
> >>> + if (s->smmu_per_bus) {
> >>> + pci_setup_iommu_per_bus(pci_bus, &smmu_ops, s);
> >>> + } else {
> >>> + pci_setup_iommu(pci_bus, &smmu_ops, s);
> >>> + }
> >>> return;
> >>> }
> >>> out_err:
> >>> @@ -984,6 +989,7 @@ static void smmu_base_reset_exit(Object *obj,
> >> ResetType type)
> >>> static const Property smmu_dev_properties[] = {
> >>> DEFINE_PROP_UINT8("bus_num", SMMUState, bus_num, 0),
> >>> + DEFINE_PROP_BOOL("smmu_per_bus", SMMUState, smmu_per_bus,
> >> false),
> >>> DEFINE_PROP_LINK("primary-bus", SMMUState, primary_bus,
> >>> TYPE_PCI_BUS, PCIBus *),
> >>> };
> >>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> >>> index ab67972353..bcf8af8dc7 100644
> >>> --- a/hw/arm/smmuv3.c
> >>> +++ b/hw/arm/smmuv3.c
> >>> @@ -1996,6 +1996,8 @@ static void smmuv3_class_init(ObjectClass
> >> *klass, const void *data)
> >>> device_class_set_parent_realize(dc, smmu_realize,
> >>> &c->parent_realize);
> >>> device_class_set_props(dc, smmuv3_properties);
> >>> + dc->hotpluggable = false;
> >>> + dc->user_creatable = true;
> >>> }
> >>>
> >>> static int smmuv3_notify_flag_changed(IOMMUMemoryRegion
> *iommu,
> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >>> index 05a14881cf..8662173c43 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"
> >>> @@ -1440,6 +1441,28 @@ static void
> create_smmuv3_dt_bindings(const
> >> VirtMachineState *vms, hwaddr base,
> >>> g_free(node);
> >>> }
> >>>
> >>> +static void create_smmuv3_dev_dtb(VirtMachineState *vms,
> >>> + DeviceState *dev, PCIBus *bus)
> >>> +{
> >>> + 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);
> >>> +
> >>> + if (strcmp("pcie.0", bus->qbus.name)) {
> >>> + warn_report("SMMUv3 device only supported with pcie.0 for
> DT");
> >> while testing the series I hit the warning with a rhel guest which boots
> >> with ACPI.
> >> I think we shall make the check smarter to avoid that.
> >> maybe also check firmware_loaded and virt_is_acpi_enabled()?
> > Thanks for giving it a spin. Yes, just confirmed that the warning appears.
> > The above check will work, but can we make use of vms->acpi_dev for
> > this check instead? It is essentially the same and I think that will work.
> >
> > if (!vms->acpi_dev && strcmp("pcie.0", bus->qbus.name))
> >
> > Please let me know.
>
> with that fixed, feel free to add my
>
> *Tested-by: Eric Auger <eric.auger@redhat.com> I have tested non
> regression on legacy SMMU, SMMU device protecting pcie.0 and pxb
> topologies. Looks good to me. Thanks Eric *
Ok. Thanks.
Also, if you can take a look at the last patch and happy with the changes,
can I have a R-by for that, please ?
Shameer
Hi Shameer,
On 7/8/25 10:54 AM, Shameerali Kolothum Thodi wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Sent: Tuesday, July 8, 2025 8:41 AM
>> To: 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;
>> ddutile@redhat.com; berrange@redhat.com; imammedo@redhat.com;
>> nathanc@nvidia.com; mochs@nvidia.com; smostafa@google.com;
>> gustavo.romero@linaro.org; mst@redhat.com;
>> marcel.apfelbaum@gmail.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 v6 08/12] hw/arm/virt: Allow user-creatable SMMUv3
>> dev instantiation
>>
>> Hi Shameer,
>>
>> On 7/3/25 10:46 AM, Shameer Kolothum wrote:
>>> Allow cold-plugging of an SMMUv3 device on the virt machine when no
>>> global (legacy) SMMUv3 is present or when a virtio-iommu is specified.
>>>
>>> This user-created SMMUv3 device is tied to a specific PCI bus provided
>>> by the user, so ensure the IOMMU ops are configured accordingly.
>>>
>>> Due to current limitations in QEMU’s device tree support, specifically
>>> its inability to properly present pxb-pcie based root complexes and
>>> their devices, the device tree support for the new SMMUv3 device is
>>> limited to cases where it is attached to the default pcie.0 root complex.
>>>
>>> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>>> Tested-by: Nathan Chen <nathanc@nvidia.com>
>>> Signed-off-by: Shameer Kolothum
>> <shameerali.kolothum.thodi@huawei.com>
>>> ---
>>> hw/arm/smmu-common.c | 8 +++++-
>>> hw/arm/smmuv3.c | 2 ++
>>> hw/arm/virt.c | 50 ++++++++++++++++++++++++++++++++++++
>>> hw/core/sysbus-fdt.c | 3 +++
>>> include/hw/arm/smmu-common.h | 1 +
>>> 5 files changed, 63 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
>>> index b15e7fd0e4..2ee4691299 100644
>>> --- a/hw/arm/smmu-common.c
>>> +++ b/hw/arm/smmu-common.c
>>> @@ -959,7 +959,12 @@ static void smmu_base_realize(DeviceState *dev,
>> Error **errp)
>>> goto out_err;
>>> }
>>> }
>>> - pci_setup_iommu(pci_bus, &smmu_ops, s);
>>> +
>>> + if (s->smmu_per_bus) {
>>> + pci_setup_iommu_per_bus(pci_bus, &smmu_ops, s);
>>> + } else {
>>> + pci_setup_iommu(pci_bus, &smmu_ops, s);
>>> + }
>>> return;
>>> }
>>> out_err:
>>> @@ -984,6 +989,7 @@ static void smmu_base_reset_exit(Object *obj,
>> ResetType type)
>>> static const Property smmu_dev_properties[] = {
>>> DEFINE_PROP_UINT8("bus_num", SMMUState, bus_num, 0),
>>> + DEFINE_PROP_BOOL("smmu_per_bus", SMMUState, smmu_per_bus,
>> false),
>>> DEFINE_PROP_LINK("primary-bus", SMMUState, primary_bus,
>>> TYPE_PCI_BUS, PCIBus *),
>>> };
>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>> index ab67972353..bcf8af8dc7 100644
>>> --- a/hw/arm/smmuv3.c
>>> +++ b/hw/arm/smmuv3.c
>>> @@ -1996,6 +1996,8 @@ static void smmuv3_class_init(ObjectClass
>> *klass, const void *data)
>>> device_class_set_parent_realize(dc, smmu_realize,
>>> &c->parent_realize);
>>> device_class_set_props(dc, smmuv3_properties);
>>> + dc->hotpluggable = false;
>>> + dc->user_creatable = true;
>>> }
>>>
>>> static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 05a14881cf..8662173c43 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"
>>> @@ -1440,6 +1441,28 @@ static void create_smmuv3_dt_bindings(const
>> VirtMachineState *vms, hwaddr base,
>>> g_free(node);
>>> }
>>>
>>> +static void create_smmuv3_dev_dtb(VirtMachineState *vms,
>>> + DeviceState *dev, PCIBus *bus)
>>> +{
>>> + 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);
>>> +
>>> + if (strcmp("pcie.0", bus->qbus.name)) {
>>> + warn_report("SMMUv3 device only supported with pcie.0 for DT");
>> while testing the series I hit the warning with a rhel guest which boots
>> with ACPI.
>> I think we shall make the check smarter to avoid that.
>> maybe also check firmware_loaded and virt_is_acpi_enabled()?
> Thanks for giving it a spin. Yes, just confirmed that the warning appears.
> The above check will work, but can we make use of vms->acpi_dev for
> this check instead? It is essentially the same and I think that will work.
>
> if (!vms->acpi_dev && strcmp("pcie.0", bus->qbus.name))
the logical dependency on acpi_dev is maybe not that straightforward.
Also it has has_ged and aarch64 check
I thing I would simply check !(firmware_loaded &&
virt_is_acpi_enabled(vms)). about the aarch64 check I am not sure this
is needed.
Eric
>
> Please let me know.
>
> Thanks,
> Shameer
> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: Tuesday, July 8, 2025 10:47 AM
> To: 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;
> ddutile@redhat.com; berrange@redhat.com; imammedo@redhat.com;
> nathanc@nvidia.com; mochs@nvidia.com; smostafa@google.com;
> gustavo.romero@linaro.org; mst@redhat.com;
> marcel.apfelbaum@gmail.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 v6 08/12] hw/arm/virt: Allow user-creatable SMMUv3
> dev instantiation
>
> Hi Shameer,
>
> On 7/8/25 10:54 AM, Shameerali Kolothum Thodi wrote:
> > Hi Eric,
> >
> >> -----Original Message-----
> >> From: Eric Auger <eric.auger@redhat.com>
> >> Sent: Tuesday, July 8, 2025 8:41 AM
> >> To: 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;
> >> ddutile@redhat.com; berrange@redhat.com; imammedo@redhat.com;
> >> nathanc@nvidia.com; mochs@nvidia.com; smostafa@google.com;
> >> gustavo.romero@linaro.org; mst@redhat.com;
> >> marcel.apfelbaum@gmail.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 v6 08/12] hw/arm/virt: Allow user-creatable
> SMMUv3
> >> dev instantiation
> >>
> >> Hi Shameer,
> >>
> >> On 7/3/25 10:46 AM, Shameer Kolothum wrote:
> >>> Allow cold-plugging of an SMMUv3 device on the virt machine when no
> >>> global (legacy) SMMUv3 is present or when a virtio-iommu is specified.
> >>>
> >>> This user-created SMMUv3 device is tied to a specific PCI bus provided
> >>> by the user, so ensure the IOMMU ops are configured accordingly.
> >>>
> >>> Due to current limitations in QEMU’s device tree support, specifically
> >>> its inability to properly present pxb-pcie based root complexes and
> >>> their devices, the device tree support for the new SMMUv3 device is
> >>> limited to cases where it is attached to the default pcie.0 root complex.
> >>>
> >>> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> >>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> >>> Tested-by: Nathan Chen <nathanc@nvidia.com>
> >>> Signed-off-by: Shameer Kolothum
> >> <shameerali.kolothum.thodi@huawei.com>
> >>> ---
> >>> hw/arm/smmu-common.c | 8 +++++-
> >>> hw/arm/smmuv3.c | 2 ++
> >>> hw/arm/virt.c | 50 ++++++++++++++++++++++++++++++++++++
> >>> hw/core/sysbus-fdt.c | 3 +++
> >>> include/hw/arm/smmu-common.h | 1 +
> >>> 5 files changed, 63 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> >>> index b15e7fd0e4..2ee4691299 100644
> >>> --- a/hw/arm/smmu-common.c
> >>> +++ b/hw/arm/smmu-common.c
> >>> @@ -959,7 +959,12 @@ static void smmu_base_realize(DeviceState
> *dev,
> >> Error **errp)
> >>> goto out_err;
> >>> }
> >>> }
> >>> - pci_setup_iommu(pci_bus, &smmu_ops, s);
> >>> +
> >>> + if (s->smmu_per_bus) {
> >>> + pci_setup_iommu_per_bus(pci_bus, &smmu_ops, s);
> >>> + } else {
> >>> + pci_setup_iommu(pci_bus, &smmu_ops, s);
> >>> + }
> >>> return;
> >>> }
> >>> out_err:
> >>> @@ -984,6 +989,7 @@ static void smmu_base_reset_exit(Object *obj,
> >> ResetType type)
> >>> static const Property smmu_dev_properties[] = {
> >>> DEFINE_PROP_UINT8("bus_num", SMMUState, bus_num, 0),
> >>> + DEFINE_PROP_BOOL("smmu_per_bus", SMMUState, smmu_per_bus,
> >> false),
> >>> DEFINE_PROP_LINK("primary-bus", SMMUState, primary_bus,
> >>> TYPE_PCI_BUS, PCIBus *),
> >>> };
> >>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> >>> index ab67972353..bcf8af8dc7 100644
> >>> --- a/hw/arm/smmuv3.c
> >>> +++ b/hw/arm/smmuv3.c
> >>> @@ -1996,6 +1996,8 @@ static void smmuv3_class_init(ObjectClass
> >> *klass, const void *data)
> >>> device_class_set_parent_realize(dc, smmu_realize,
> >>> &c->parent_realize);
> >>> device_class_set_props(dc, smmuv3_properties);
> >>> + dc->hotpluggable = false;
> >>> + dc->user_creatable = true;
> >>> }
> >>>
> >>> static int smmuv3_notify_flag_changed(IOMMUMemoryRegion
> *iommu,
> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >>> index 05a14881cf..8662173c43 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"
> >>> @@ -1440,6 +1441,28 @@ static void
> create_smmuv3_dt_bindings(const
> >> VirtMachineState *vms, hwaddr base,
> >>> g_free(node);
> >>> }
> >>>
> >>> +static void create_smmuv3_dev_dtb(VirtMachineState *vms,
> >>> + DeviceState *dev, PCIBus *bus)
> >>> +{
> >>> + 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);
> >>> +
> >>> + if (strcmp("pcie.0", bus->qbus.name)) {
> >>> + warn_report("SMMUv3 device only supported with pcie.0 for
> DT");
> >> while testing the series I hit the warning with a rhel guest which boots
> >> with ACPI.
> >> I think we shall make the check smarter to avoid that.
> >> maybe also check firmware_loaded and virt_is_acpi_enabled()?
> > Thanks for giving it a spin. Yes, just confirmed that the warning appears.
> > The above check will work, but can we make use of vms->acpi_dev for
> > this check instead? It is essentially the same and I think that will work.
> >
> > if (!vms->acpi_dev && strcmp("pcie.0", bus->qbus.name))
> the logical dependency on acpi_dev is maybe not that straightforward.
> Also it has has_ged and aarch64 check
> I thing I would simply check !(firmware_loaded &&
> virt_is_acpi_enabled(vms)). about the aarch64 check I am not sure this
> is needed.
Right. I think has_ged is enable by default from 4.1 and on aarch64, isn't
SMMUV3 only enabled for ARM64 cases? At least on kernel it is, I think.
Anyway, agree, the firmware_loaded && virt_is_acpi_enabled(vms) is
more readable. I will fix it.
Thanks,
Shameer
© 2016 - 2025 Red Hat, Inc.