| Message ID | 20230622214845.3980-3-joao.m.martins@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | vfio: VFIO migration support with vIOMMU | expand |
On 6/22/23 23:48, Joao Martins wrote: > From: Yi Liu <yi.l.liu@intel.com> > > Refactor pci_device_iommu_address_space() and move the > code that fetches the device bus and iommu bus into its > own private helper pci_device_get_iommu_bus_devfn(). > > This is in preparation to introduce pci_device_iommu_get_attr() > which will need to use it too. Where is this routine used ? Thanks, C. > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > [joao: Commit message, and better splitting] > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > --- > Splitted from v1: > https://lore.kernel.org/all/20210302203827.437645-6-yi.l.liu@intel.com/ > --- > hw/pci/pci.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 4e32c09e81d6..90ae92a43d85 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -2632,8 +2632,8 @@ static void pci_device_class_base_init(ObjectClass *klass, void *data) > assert(conventional || pcie || cxl); > } > } > - > -AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > +static void pci_device_get_iommu_bus_devfn(PCIDevice *dev, PCIBus **pdevbus, > + PCIBus **pbus, uint8_t *pdevfn) > { > PCIBus *bus = pci_get_bus(dev); > PCIBus *iommu_bus = bus; > @@ -2686,6 +2686,18 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > > iommu_bus = parent_bus; > } > + > + *pdevbus = bus; > + *pbus = iommu_bus; > + *pdevfn = devfn; > +} > + > +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > +{ > + PCIBus *bus, *iommu_bus; > + uint8_t devfn; > + > + pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn); > if (!pci_bus_bypass_iommu(bus) && iommu_bus) { > if (iommu_bus->iommu_fn) { > return iommu_bus->iommu_fn(bus, iommu_bus->iommu_opaque, devfn);
On 02/10/2023 16:22, Cédric Le Goater wrote: > On 6/22/23 23:48, Joao Martins wrote: >> From: Yi Liu <yi.l.liu@intel.com> >> >> Refactor pci_device_iommu_address_space() and move the >> code that fetches the device bus and iommu bus into its >> own private helper pci_device_get_iommu_bus_devfn(). >> >> This is in preparation to introduce pci_device_iommu_get_attr() >> which will need to use it too. > > Where is this routine used ? > Patch 7 to understand if a configured vIOMMU supports DMA translation, regardless of whether the guest is doing. > Thanks, > > C. > > >> >> Signed-off-by: Yi Liu <yi.l.liu@intel.com> >> [joao: Commit message, and better splitting] >> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >> --- >> Splitted from v1: >> https://lore.kernel.org/all/20210302203827.437645-6-yi.l.liu@intel.com/ >> --- >> hw/pci/pci.c | 16 ++++++++++++++-- >> 1 file changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index 4e32c09e81d6..90ae92a43d85 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -2632,8 +2632,8 @@ static void pci_device_class_base_init(ObjectClass >> *klass, void *data) >> assert(conventional || pcie || cxl); >> } >> } >> - >> -AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) >> +static void pci_device_get_iommu_bus_devfn(PCIDevice *dev, PCIBus **pdevbus, >> + PCIBus **pbus, uint8_t *pdevfn) >> { >> PCIBus *bus = pci_get_bus(dev); >> PCIBus *iommu_bus = bus; >> @@ -2686,6 +2686,18 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice >> *dev) >> iommu_bus = parent_bus; >> } >> + >> + *pdevbus = bus; >> + *pbus = iommu_bus; >> + *pdevfn = devfn; >> +} >> + >> +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) >> +{ >> + PCIBus *bus, *iommu_bus; >> + uint8_t devfn; >> + >> + pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn); >> if (!pci_bus_bypass_iommu(bus) && iommu_bus) { >> if (iommu_bus->iommu_fn) { >> return iommu_bus->iommu_fn(bus, iommu_bus->iommu_opaque, devfn); >
On 06/10/2023 09:39, Joao Martins wrote: > > > On 02/10/2023 16:22, Cédric Le Goater wrote: >> On 6/22/23 23:48, Joao Martins wrote: >>> From: Yi Liu <yi.l.liu@intel.com> >>> >>> Refactor pci_device_iommu_address_space() and move the >>> code that fetches the device bus and iommu bus into its >>> own private helper pci_device_get_iommu_bus_devfn(). >>> >>> This is in preparation to introduce pci_device_iommu_get_attr() >>> which will need to use it too. >> >> Where is this routine used ? >> > > Patch 7 to understand if a configured vIOMMU supports DMA translation, > regardless of whether the guest is doing. > And then patch 12 to see the max IOVA boundaries of the vIOMMU possible address space.
Hi Joao, On 6/22/23 23:48, Joao Martins wrote: > From: Yi Liu <yi.l.liu@intel.com> > > Refactor pci_device_iommu_address_space() and move the > code that fetches the device bus and iommu bus into its > own private helper pci_device_get_iommu_bus_devfn(). > > This is in preparation to introduce pci_device_iommu_get_attr() > which will need to use it too. you may add: no functional change intended. > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > [joao: Commit message, and better splitting] > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > --- > Splitted from v1: > https://lore.kernel.org/all/20210302203827.437645-6-yi.l.liu@intel.com/ > --- > hw/pci/pci.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 4e32c09e81d6..90ae92a43d85 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -2632,8 +2632,8 @@ static void pci_device_class_base_init(ObjectClass *klass, void *data) > assert(conventional || pcie || cxl); > } > } > - > -AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > +static void pci_device_get_iommu_bus_devfn(PCIDevice *dev, PCIBus **pdevbus, > + PCIBus **pbus, uint8_t *pdevfn) > { > PCIBus *bus = pci_get_bus(dev); > PCIBus *iommu_bus = bus; > @@ -2686,6 +2686,18 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > > iommu_bus = parent_bus; > } > + > + *pdevbus = bus; > + *pbus = iommu_bus; > + *pdevfn = devfn; > +} > + > +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > +{ > + PCIBus *bus, *iommu_bus; > + uint8_t devfn; > + > + pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn); > if (!pci_bus_bypass_iommu(bus) && iommu_bus) { > if (iommu_bus->iommu_fn) { > return iommu_bus->iommu_fn(bus, iommu_bus->iommu_opaque, devfn);
Hi Joao, On 6/22/23 23:48, Joao Martins wrote: > From: Yi Liu <yi.l.liu@intel.com> > > Refactor pci_device_iommu_address_space() and move the > code that fetches the device bus and iommu bus into its > own private helper pci_device_get_iommu_bus_devfn(). > > This is in preparation to introduce pci_device_iommu_get_attr() > which will need to use it too. > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > [joao: Commit message, and better splitting] > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > --- > Splitted from v1: > https://lore.kernel.org/all/20210302203827.437645-6-yi.l.liu@intel.com/ > --- > hw/pci/pci.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 4e32c09e81d6..90ae92a43d85 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -2632,8 +2632,8 @@ static void pci_device_class_base_init(ObjectClass *klass, void *data) > assert(conventional || pcie || cxl); > } > } > - > -AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > +static void pci_device_get_iommu_bus_devfn(PCIDevice *dev, PCIBus **pdevbus, > + PCIBus **pbus, uint8_t *pdevfn) actually I would rename pbus arg to match what it is, ie the iommu_bus also not sure the 'p' prefix is needed By the way can the iommu_bus be null? I see it initialized to pci_get_bus(dev); What does characterize an iommu bus? It must have either iommu_fn or iommu_ops set, right? Eric > { > PCIBus *bus = pci_get_bus(dev); > PCIBus *iommu_bus = bus; > @@ -2686,6 +2686,18 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > > iommu_bus = parent_bus; > } > + > + *pdevbus = bus; > + *pbus = iommu_bus; > + *pdevfn = devfn; > +} > + > +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > +{ > + PCIBus *bus, *iommu_bus; > + uint8_t devfn; > + > + pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn); > if (!pci_bus_bypass_iommu(bus) && iommu_bus) { > if (iommu_bus->iommu_fn) { > return iommu_bus->iommu_fn(bus, iommu_bus->iommu_opaque, devfn);
On 06/10/2023 09:52, Eric Auger wrote: > Hi Joao, > > On 6/22/23 23:48, Joao Martins wrote: >> From: Yi Liu <yi.l.liu@intel.com> >> >> Refactor pci_device_iommu_address_space() and move the >> code that fetches the device bus and iommu bus into its >> own private helper pci_device_get_iommu_bus_devfn(). >> >> This is in preparation to introduce pci_device_iommu_get_attr() >> which will need to use it too. > > you may add: > no functional change intended. OK >> >> Signed-off-by: Yi Liu <yi.l.liu@intel.com> >> [joao: Commit message, and better splitting] >> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > Reviewed-by: Eric Auger <eric.auger@redhat.com> Thank you
diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 4e32c09e81d6..90ae92a43d85 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -2632,8 +2632,8 @@ static void pci_device_class_base_init(ObjectClass *klass, void *data) assert(conventional || pcie || cxl); } } - -AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) +static void pci_device_get_iommu_bus_devfn(PCIDevice *dev, PCIBus **pdevbus, + PCIBus **pbus, uint8_t *pdevfn) { PCIBus *bus = pci_get_bus(dev); PCIBus *iommu_bus = bus; @@ -2686,6 +2686,18 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) iommu_bus = parent_bus; } + + *pdevbus = bus; + *pbus = iommu_bus; + *pdevfn = devfn; +} + +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) +{ + PCIBus *bus, *iommu_bus; + uint8_t devfn; + + pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn); if (!pci_bus_bypass_iommu(bus) && iommu_bus) { if (iommu_bus->iommu_fn) { return iommu_bus->iommu_fn(bus, iommu_bus->iommu_opaque, devfn);