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

[Igor Mammedov]

On Thu, 5 Jun 2025 12:36:52 +0000
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:

> > -----Original Message-----
> > From: Igor Mammedov <imammedo@redhat.com>
> > Sent: Thursday, June 5, 2025 1:20 PM
> > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> > Cc: Shameer Kolothum via <qemu-devel@nongnu.org>; qemu-
> > arm@nongnu.org; eric.auger@redhat.com; peter.maydell@linaro.org;
> > jgg@nvidia.com; nicolinc@nvidia.com; ddutile@redhat.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 v3 1/6] hw/arm/smmuv3: Check SMMUv3 has PCIe Root
> > Complex association  
> 
> [..]
> 
> > > > 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.fsf@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'  
> 
> I have tried that earlier and gets this,
> -device arm-smmuv3,bus=pcie.0,id=smmuv3.0: Device 'arm-smmuv3' can't go on 
> PCIE bus
> 
> IIRC, the above mentioned patched dc->bus_type = TYPE_PCIE_BUS was used 
> to avoid that.
> 
> Or am I missing something here?

sigh, but you a right.
qdev_device_add_from_qdict() && co can't handle that without heavy refactoring.

 
> > 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.  
> 
> Ok.
> 
> Thanks,
> Shameer
>