RE: [PATCH v2 1/6] hw/arm/smmuv3: Add support to associate a PCIe RC

[Shameerali Kolothum Thodi via <qemu-devel@nongnu.org>]

> -----Original Message-----
> From: Donald Dutile <ddutile@redhat.com>
> Sent: Friday, May 2, 2025 7:17 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 v2 1/6] hw/arm/smmuv3: Add support to associate a
> PCIe RC
> 
> 
> 
> On 5/2/25 6:27 AM, Shameer Kolothum wrote:
> > Although this change does not affect functionality at present, it lays
> > the groundwork for enabling user-created SMMUv3 devices in
> > future patches
> >
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > ---
> >   hw/arm/smmuv3.c | 26 ++++++++++++++++++++++++++
> >   hw/arm/virt.c   |  3 ++-
> >   2 files changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> > index ab67972353..605de9b721 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 "exec/target_page.h"
> >   #include "trace.h"
> > @@ -1874,6 +1875,25 @@ static void smmu_reset_exit(Object *obj,
> ResetType type)
> >       smmuv3_init_regs(s);
> >   }
> >
> > +static int smmuv3_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 non-zero as we got the bus and don't need further
> iteration.*/
> > +        return 1;
> > +    }
> > +    return 0;
> > +}
> > +
> >   static void smmu_realize(DeviceState *d, Error **errp)
> >   {
> >       SMMUState *sys = ARM_SMMU(d);
> > @@ -1882,6 +1902,10 @@ static void smmu_realize(DeviceState *d, Error
> **errp)
> >       SysBusDevice *dev = SYS_BUS_DEVICE(d);
> >       Error *local_err = NULL;
> >
> > +    if (!object_property_get_link(OBJECT(d), "primary-bus", &error_abort))
> {
> > +        object_child_foreach_recursive(object_get_root(),
> smmuv3_pcie_bus, d);
> > +    }
> > +
> >       c->parent_realize(d, &local_err);
> >       if (local_err) {
> >           error_propagate(errp, local_err);
> > @@ -1996,6 +2020,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->bus_type = TYPE_PCIE_BUS;
> Does this force legacy SMMUv3 to be tied to a PCIe bus now?
> if so, will that break some existing legacy smmuv3 configs?, i.e., virtio-scsi
> attached to a legacy smmuv3.

No. It won't break the existing config as Eric has already replied. The only difference is
as Eric rightly pointed out, SMMUv3 is a platform device and we are specifying
the device bus type as PCIE, which is a bit odd. The initial RFC was with a specific
"pci-bus" as command line option , ie,

-device arm-smmuv3, pci-bus=pcie0,...

and then there were feedbacks that it is more intuitive and makes life easy with 
libvirt if we can use the Qemu device "bus" option and hence the change.

Thanks,
Shameer