On Thu, 5 Jun 2025 11:29:59 +0000 Shameerali Kolothum Thodi <shameerali.kolothum.th...@huawei.com> wrote:
> > -----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/ that was an approach was trying to workaround by patching dc->bus_type, which is obviously wrong. I'm not talking about changing device type or something similar, but about renaming 'primary-bus' property name to 'bus' so it would be consistent interface wise with PCI or other QEMU devices that are attached to a bus. > > > + 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. then perhaps move, the check to smmu_base_realize() for now? if smmu + non-pci ever materialize, it can be refactored at that time. > Thanks, > Shameer >
