RE: [PATCH v7 02/12] hw/arm/smmu-common: Check SMMU has PCIe Root Complex association

Wed, 09 Jul 2025 01:10:17 -0700 


> -----Original Message-----
> From: Nicolin Chen <[email protected]>
> Sent: Tuesday, July 8, 2025 9:57 PM
> To: Shameerali Kolothum Thodi <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Linuxarm <[email protected]>;
> Wangzhou (B) <[email protected]>; jiangkunkun
> <[email protected]>; Jonathan Cameron
> <[email protected]>; [email protected]
> Subject: Re: [PATCH v7 02/12] hw/arm/smmu-common: Check SMMU has
> PCIe Root Complex association
> 
> On Tue, Jul 08, 2025 at 04:40:45PM +0100, Shameer Kolothum wrote:
> > @@ -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);
> 
> Although this is not introduced by this patch, is there a
> g_hash_table_remove() somewhere in the code?

g_hash_table_remove()  is to remove a key/value pair, isn't it? Or you meant
a corresponding free in case of failure here? It's a realize() fn and errp is 
set
if something goes wrong and QEMU will exit. Not sure we need an explicit
free here.
 
> > +    /*
> > +     * 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)) {
> 
> The pci_bus_is_express(pci_bus) at the top is equivalent to:
>       object_dynamic_cast(OBJECT(pci_bus), TYPE_PCIE_BUS)
> Then here it is doing:
>       object_dynamic_cast(OBJECT(pci_bus), TYPE_PXB_PCIE_BUS)

Yes.

> So, this checks the same pci_bus but expects two different types?

In QEMU,  we can have three types of PCIe root complexes to be specified for
virt machine. 

1. default pcie.0 (TYPE_GPEX_HOST --> TYPE_PCIE_HOST_BRIDGE --> 
TYPE_PCI_HOST_BRIDGE)
2. pxb-pcie (TYPE_PXB_HOST  -->TYPE_PCI_HOST_BRIDGE)
2. pxb-cxl (TYPE_PXB_CXL_HOST  --> TYPE_PCI_HOST_BRIDGE)

The above first check is to see whether the bus is  PCIE && root bus && parent 
of type TYPE_PCI_HOST_BRIDGE. This will identify all the above three cases.

Both pxb-pcie and pxb-cxl are special extra root complexes based on PCI
expansion bridges and has a parent_dev set(both has pcie.0 has parent bus).

Hence we check to see parent_dev is set and make sure it is indeed 
TYPE_PXB_PCIE_BUS to avoid attaching to pxb-cxl. 

As mentioned in the commit log above, cxl support for virt is currently in
progress and once it has verified the functionality with SMMUv3
we can relax that check.

> I don't see the code check "PCIe Root Complex" explicitly, which
> should be TYPE_GPEX_HOST?

Hope it is clear now.

Thanks,
Shameer
























					Reply via email to