RE: [PATCH v3 1/6] hw/arm/smmuv3: Check SMMUv3 has PCIe Root Complex association

[Shameerali Kolothum Thodi via]

> -----Original Message-----
> From: Igor Mammedov <imamm...@redhat.com>
> Sent: Thursday, June 5, 2025 11:55 AM
> To: Shameer Kolothum via <qemu-devel@nongnu.org>
> Cc: Shameerali Kolothum Thodi
> <shameerali.kolothum.th...@huawei.com>; qemu-...@nongnu.org;
> eric.au...@redhat.com; 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: [PATCH v3 1/6] hw/arm/smmuv3: Check SMMUv3 has PCIe Root
> Complex association
> 
> On Mon, 2 Jun 2025 16:41:05 +0100
> Shameer Kolothum via <qemu-devel@nongnu.org> wrote:
> 
> > Although this change does not affect functionality at present, it is
> > required when we add support for user-creatable SMMUv3 devices in
> > future patches.
> >
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.th...@huawei.com>
> > ---
> >  hw/arm/smmuv3.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> > index ab67972353..7e934336c2 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"
> > @@ -1881,6 +1882,13 @@ static void smmu_realize(DeviceState *d, Error
> **errp)
> >      SMMUv3Class *c = ARM_SMMUV3_GET_CLASS(s);
> >      SysBusDevice *dev = SYS_BUS_DEVICE(d);
> >      Error *local_err = NULL;
> > +    Object *bus;
> > +
> > +    bus = object_property_get_link(OBJECT(d), "primary-bus",
> &error_abort);
> I'd replace this with direct field access like in smmu_base_realize

Ok.
 
> in QEMU with PCI, usually we specify bus to attach to with 'bus' property,
> wouldn't it better to rename "primary-bus" to 'bus' to be consistent with
> the rest of PCI code (and before "primary-bus" shows up as a CLI option,
> so far (before this series) it looks like it's an internal property)?

That was tried in v2 and since SMMUv3 is not a pci device by itself(it is a 
sysbus device) reusing the default "bus" property to establish an association
with a PCI bus created problems,
https://lore.kernel.org/qemu-devel/877c2ut0zk....@pond.sub.org/

 
> > +    if (!bus || !object_dynamic_cast(bus->parent,
> TYPE_PCI_HOST_BRIDGE)) {
> Also looking at smmu_base_realize, it has NULL pointer check already.
> Which also rises question, shouldn't smmu_base_realize check for
> TYPE_PCI_HOST_BRIDGE as well (aka can smmu be attached to anything
> else but a host bridge)?

Not at the moment in Qemu. Though the SMMUv3 specification allows it to
be associated with non-pci devices as well.

Thanks,
Shameer