Hi Eric, > -----Original Message----- > From: Eric Auger <eric.au...@redhat.com> > Sent: Friday, June 27, 2025 12:52 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; imamm...@redhat.com; > nath...@nvidia.com; mo...@nvidia.com; smost...@google.com; > gustavo.rom...@linaro.org; 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 v5 01/11] hw/arm/smmu-common: Check SMMU has > PCIe Root Complex association > > Hi Shameer, > > On 6/23/25 11:42 AM, Shameer Kolothum wrote: > > We only allow default PCIe Root Complex(pcie.0) or pxb-pcie based extra > > root complexes to be associated with SMMU. > > > > 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/smmu-common.c | 29 ++++++++++++++++++++++++++--- > > hw/pci-bridge/pci_expander_bridge.c | 1 - > > include/hw/pci/pci_bridge.h | 1 + > > 3 files changed, 27 insertions(+), 4 deletions(-) > > > > diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c > > index f39b99e526..b15e7fd0e4 100644 > > --- a/hw/arm/smmu-common.c > > +++ b/hw/arm/smmu-common.c > > @@ -20,6 +20,7 @@ > > #include "trace.h" > > #include "exec/target_page.h" > > #include "hw/core/cpu.h" > > +#include "hw/pci/pci_bridge.h" > > #include "hw/qdev-properties.h" > > #include "qapi/error.h" > > #include "qemu/jhash.h" > > @@ -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,11 +939,32 @@ 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), TYPE_PXB_PCIE_BUS)) { > > + goto out_err; > > + } > I still wonder whether the above check was mandated as it works for what > it is meant:
Added that check to make sure we don't support pxb-cxl which is of type PCI_HOST_BRIDGE. Once the cxl support for ARM is up streamed and tested with SMMUv3, we can relax this if required. > Reviewed-by: Eric Auger <eric.au...@redhat.com> Thanks, Shameer