Hi Eric,
> -----Original Message-----
> From: Eric Auger <eric.au...@redhat.com>
> Sent: Wednesday, March 12, 2025 4:08 PM
> 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; nath...@nvidia.com;
> mo...@nvidia.com; smost...@google.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: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
> pcie bus
>
> Hi Shameer,
>
>
> On 3/11/25 3:10 PM, Shameer Kolothum wrote:
> > User must associate a pxb-pcie root bus to smmuv3-accel
> > and that is set as the primary-bus for the smmu dev.
> why do we require a pxb-pcie root bus? why can't pci.0 root bus be used
> for simpler use cases (ie. I just want to passthough a NIC in
> accelerated mode). Or may pci.0 is also called a pax-pcie root bus?
The idea was since pcie.0 is the default RC with virt, leave that to cases where
we want to attach any emulated devices and use pxb-pcie based RCs for vfio-pci.
>
> Besides, why do we put the constraint to plug on a root bus. I know that
> at this point we always plug to pci.0 but with the new -device option it
> would be possible to plug it anywhere in the pcie hierarchy. At SOC
> level can't an SMMU be plugged anywhere protecting just a few RIDs?
In my understanding normally(or atleast in the most common cases) it is
attached
to root complexes. Also IORT mappings are at the root complex level, right?
To
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.th...@huawei.com>
> > ---
> > hw/arm/smmuv3-accel.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> > index c327661636..1471b65374 100644
> > --- a/hw/arm/smmuv3-accel.c
> > +++ b/hw/arm/smmuv3-accel.c
> > @@ -9,6 +9,21 @@
> > #include "qemu/osdep.h"
> >
> > #include "hw/arm/smmuv3-accel.h"
> > +#include "hw/pci/pci_bridge.h"
> > +
> > +static int smmuv3_accel_pxb_pcie_bus(Object *obj, void *opaque)
> > +{
> > + DeviceState *d = opaque;
> > +
> > + if (object_dynamic_cast(obj, "pxb-pcie-bus")) {
> > + PCIBus *bus = PCI_HOST_BRIDGE(obj->parent)->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);
> if you want to stop the recursive search I think you need to return
> something != 0 here.
Ok.
> I don't really understand why we don't simply set the primary-bus to
> <bus> where -device arm-smmuv3-accel, bus=<bus>? or maybe enforce that
> this bus is an actual root bus if we really need that?
The primary-bus here is actually the property of the parent SMMU device.
This one now has,
-device arm-smmuv3-accel, bus format.
> > + }
> > + }
> > + return 0;
> > +}
> >
> > static void smmu_accel_realize(DeviceState *d, Error **errp)
> > {
> > @@ -17,6 +32,9 @@ static void smmu_accel_realize(DeviceState *d, Error
> **errp)
> > SysBusDevice *dev = SYS_BUS_DEVICE(d);
> > Error *local_err = NULL;
> >
> > + object_child_foreach_recursive(object_get_root(),
> > + smmuv3_accel_pxb_pcie_bus, d);
> > +
> > object_property_set_bool(OBJECT(dev), "accel", true, &error_abort);
> > c->parent_realize(d, &local_err);
> > if (local_err) {
> > @@ -33,6 +51,7 @@ static void smmuv3_accel_class_init(ObjectClass
> *klass, void *data)
> > device_class_set_parent_realize(dc, smmu_accel_realize,
> > &c->parent_realize);
> > dc->hotpluggable = false;
> > + dc->bus_type = TYPE_PCIE_BUS;
> shouldn't it below to 3/20? It is not really related to primary_bus
> setting?
This is to set the bus property of this smmuv3-accel device. As mentioned
above primary-bus is the property of parent TYPE_ARM_SMMU device.
Thanks,
Shameer
