RE: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has PCIe Root Complex association

[Shameerali Kolothum Thodi via]

> -----Original Message-----
> From: Jonathan Cameron <jonathan.came...@huawei.com>
> Sent: Tuesday, June 17, 2025 5:53 PM
> To: Eric Auger <eric.au...@redhat.com>
> Cc: Shameerali Kolothum Thodi
> <shameerali.kolothum.th...@huawei.com>; Linuxarm
> <linux...@huawei.com>; qemu-...@nongnu.org; qemu-
> de...@nongnu.org; peter.mayd...@linaro.org; j...@nvidia.com;
> nicol...@nvidia.com; ddut...@redhat.com; berra...@redhat.com;
> imamm...@redhat.com; nath...@nvidia.com; mo...@nvidia.com;
> smost...@google.com; Wangzhou (B) <wangzh...@hisilicon.com>;
> jiangkunkun <jiangkun...@huawei.com>; zhangfei....@linaro.org
> Subject: Re: [PATCH v4 1/7] hw/arm/smmu-common: Check SMMU has PCIe
> Root Complex association
> 
> On Tue, 17 Jun 2025 09:49:54 +0200
> Eric Auger <eric.au...@redhat.com> wrote:
> 
> > On 6/16/25 12:20 PM, Jonathan Cameron wrote:
> > > On Fri, 13 Jun 2025 15:44:43 +0100
> > > Shameer Kolothum <shameerali.kolothum.th...@huawei.com> wrote:
> > >
> > >> Although this change does not affect functionality at present, it is
> > > Patch title says PCIe.  This check is vs PCI host bridge.
> > >
> > > No idea which one you wanted, but if it is PCIe needs to be
> > > TYPC_PCIE_HOST_BRIDGE from pcie_host.h not the pci_host.h one
> > > I think.
> > I think we need TYPE_PCI_HOST_BRIDGE as we want to check against pxb
> >
> > pci-bridge/pci_expander_bridge.c:    .parent        =
> TYPE_PCI_HOST_BRIDGE,
> 
> Hmm. That's awkward and I'd forgotten that wrinkle.
> Need a stronger test but which one?  The PXB root bus has a parent of
> TYPE_PCIE_BUS.  Maybe we can check that?

Ok. How about we do something like below?


@@ -925,6 +926,7 @@ static void smmu_base_realize(DeviceState *dev,
Error **errp)
 {
     SMMUState *s = ARM_SMMU(dev);
     SMMUBaseClass *sbc = ARM_SMMU_GET_CLASS(dev);
+    PCIBus *pci_bus = s->primary_bus;
     Error *local_err = NULL;

     sbc->parent_realize(dev, &local_err);
@@ -937,10 +939,31 @@ static void smmu_base_realize(DeviceState *dev,
Error **errp)
                                      g_free, g_free);
     s->smmu_pcibus_by_busptr = g_hash_table_new(NULL, NULL);

-    if (s->primary_bus) {
-        pci_setup_iommu(s->primary_bus, &smmu_ops, s);
-    } else {
+    if (!pci_bus) {
         error_setg(errp, "SMMU is not attached to any PCI bus!");
+        return;
+    }
+
+    /*
+     * We only allow default PCIe Root Complex(pcie.0) or pxb-pcie based extra
+     * root complexes to be associated with SMMU.
+     */
+    if (pci_bus_is_express(pci_bus) && pci_bus_is_root(pci_bus) &&
+        object_dynamic_cast(OBJECT(pci_bus)->parent, TYPE_PCI_HOST_BRIDGE)) {
+        /*
+         * For pxb-pcie, parent_dev will be set. Make sure it is
+         * pxb-pcie indeed.
+         */
+        if (pci_bus->parent_dev) {
+            if (!object_dynamic_cast(OBJECT(pci_bus), "pxb-pcie-bus")) {
+                error_setg(errp, "SMMU is not attached to pxb-pcie bus!");
+                return;
+            }
+        }
+        pci_setup_iommu(pci_bus, &smmu_ops, s);
+    } else {
+       error_setg(errp, "SMMU should be attached to a default PCIe
root complex"
+                  "(pcie.0) or a pxb-pcie based root complex");
     }
 }

Please let me know if this is good enough or not.

Thanks,
Shameer