Re: [PATCH v7 07/12] hw/pci: Introduce pci_setup_iommu_per_bus() for per-bus IOMMU ops retrieval

[Nicolin Chen <nicolinc@nvidia.com>]

On Wed, Jul 09, 2025 at 08:20:35AM +0000, Shameerali Kolothum Thodi wrote:
> > On Tue, Jul 08, 2025 at 04:40:50PM +0100, Shameer Kolothum wrote:
> > > @@ -2909,6 +2909,19 @@ static void
> > pci_device_get_iommu_bus_devfn(PCIDevice *dev,
> > >              }
> > >          }
> > >
> > > +        /*
> > > +         * When multiple PCI Express Root Buses are defined using pxb-
> > pcie,
> > > +         * the IOMMU configuration may be specific to each root bus.
> > However,
> > > +         * pxb-pcie acts as a special root complex whose parent is
> > effectively
> > > +         * the default root complex(pcie.0). Ensure that we retrieve the
> > > +         * correct IOMMU ops(if any) in such cases.
> > > +         */
> > > +        if (pci_bus_is_express(iommu_bus) &&
> > pci_bus_is_root(iommu_bus)) {
> > > +            if (!iommu_bus->iommu_per_bus && parent_bus-
> > >iommu_per_bus) {
> > > +                break;
> > 
> > Mind elaborating why the current bus must unset iommu_per_bus while
> > its parent sets iommu_per_bus?
> > 
> > My understanding is that for a pxb-pcie we should set iommu_per_bus
> > but for its parent (the default root complex) we should unset its
> > iommu_per_bus?
> 
> Well, for new arm-smmuv3 dev you need an associated pcie root
> complex. Either the default pcie.0 or a pxb-pcie one. And as I
> mentioned in my reply to the other thread(patch #2) and commit log here,
> the pxb-pcie is special extra root complex in Qemu which has pcie.0 has
> parent bus.
> 
> The above pci_device_get_iommu_bus_devfn() at present, iterate over the
> parent_dev if it is set and returns the parent_bus IOMMU ops even if the
> associated pxb-pcie bus doesn't have any IOMMU. This creates problem
> for a case that is described here in the cover letter here,
> https://lore.kernel.org/qemu-devel/20250708154055.101012-1-shameerali.kolothum.thodi@huawei.com/
> 
> (Please see "Major changes from v4:" section)
> 
> To address that issue, this patch introduces an new helper function to specify that
> the IOMMU ops are specific to the associated root complex(iommu_per_bus) and
> use that to return the correct IOMMU ops.
> 
> Hope with that context it is clear now.

Hmm, I was not questioning the context, I get what the patch is
supposed to do.

I was asking the logic that is unclear to me why it breaks when:
    !pxb-pcie->iommu_per_bus && pcie.0->iommu_per_bus

Or in which case pcie.0 would be set to iommu_per_bus=true?

Thanks
Nicolin