Hi Eric,
> -----Original Message-----
> From: Eric Auger <eric.au...@redhat.com>
> Sent: Tuesday, July 8, 2025 8:41 AM
> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.th...@huawei.com>; qemu-...@nongnu.org;
> qemu-devel@nongnu.org
> Cc: peter.mayd...@linaro.org; j...@nvidia.com; nicol...@nvidia.com;
> ddut...@redhat.com; berra...@redhat.com; imamm...@redhat.com;
> nath...@nvidia.com; mo...@nvidia.com; smost...@google.com;
> gustavo.rom...@linaro.org; m...@redhat.com;
> marcel.apfelb...@gmail.com; Linuxarm <linux...@huawei.com>;
> Wangzhou (B) <wangzh...@hisilicon.com>; jiangkunkun
> <jiangkun...@huawei.com>; Jonathan Cameron
> <jonathan.came...@huawei.com>; zhangfei....@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.came...@huawei.com>
> > Reviewed-by: Eric Auger <eric.au...@redhat.com>
> > Tested-by: Nathan Chen <nath...@nvidia.com>
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.th...@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
