On 02/10/2023 16:12, Cédric Le Goater wrote: > Hello Joao, > > On 6/22/23 23:48, Joao Martins wrote: >> From: Yi Liu <yi.l....@intel.com> >> >> Add a pci_setup_iommu_ops() that uses a newly added structure >> (PCIIOMMUOps) instead of using PCIIOMMUFunc. The old pci_setup_iommu() >> that uses PCIIOMMUFunc is still kept for other IOMMUs to get an >> an address space for a PCI device in vendor specific way. >> >> In preparation to expand to supplying vIOMMU attributes, add a >> alternate helper pci_setup_iommu_ops() to setup the PCI device IOMMU. >> For now the PCIIOMMUOps just defines the address_space, but it will >> be extended to have another callback. >> >> Signed-off-by: Yi Liu <yi.l....@intel.com> >> [joao: Massage commit message and subject, and make it a complementary >> rather than changing every single consumer of pci_setup_iommu()] >> Signed-off-by: Joao Martins <joao.m.mart...@oracle.com> >> --- >> v1: https://lore.kernel.org/all/20210302203827.437645-5-yi.l....@intel.com/ >> --- >> include/hw/pci/pci.h | 7 +++++++ >> include/hw/pci/pci_bus.h | 1 + >> hw/pci/pci.c | 26 +++++++++++++++++++++++--- >> 3 files changed, 31 insertions(+), 3 deletions(-) >> >> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >> index e6d0574a2999..f59aef5a329a 100644 >> --- a/include/hw/pci/pci.h >> +++ b/include/hw/pci/pci.h >> @@ -368,6 +368,13 @@ typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, >> int); >> AddressSpace *pci_device_iommu_address_space(PCIDevice *dev); >> void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque); >> +typedef struct PCIIOMMUOps PCIIOMMUOps; >> +struct PCIIOMMUOps { >> + AddressSpace * (*get_address_space)(PCIBus *bus, >> + void *opaque, int32_t devfn); >> +}; >> +void pci_setup_iommu_ops(PCIBus *bus, const PCIIOMMUOps *iommu_ops, void >> *opaque); >> + > > I think you should first convert all PHBs to PCIIOMMUOps to avoid all the > tests as below and adapt pci_setup_iommu_ops() with the new parameter. >
OK, that's Yi's original patch: https://lore.kernel.org/all/20210302203827.437645-5-yi.l....@intel.com/ I went with this one is that 1) it might take eons to get every single IOMMU maintainer ack; and 2) it would allow each IOMMU to move at its own speed specially as I can't test most of the other ones. essentially iterative, rather than invasive change? Does that make sense? Joao