Re: [PATCH v4 02/15] hw/pci: Refactor pci_device_iommu_address_space()
On 06/10/2023 09:52, Eric Auger wrote: > Hi Joao, > > On 6/22/23 23:48, Joao Martins wrote: >> From: Yi Liu >> >> 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 >> [joao: Commit message, and better splitting] >> Signed-off-by: Joao Martins > Reviewed-by: Eric Auger Thank you
Re: [PATCH v4 02/15] hw/pci: Refactor pci_device_iommu_address_space()
Hi Joao,
On 6/22/23 23:48, Joao Martins wrote:
> From: Yi Liu
>
> 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
> [joao: Commit message, and better splitting]
> Signed-off-by: Joao Martins
> ---
> Splitted from v1:
> https://lore.kernel.org/all/20210302203827.437645-6-yi.l@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, );
> if (!pci_bus_bypass_iommu(bus) && iommu_bus) {
> if (iommu_bus->iommu_fn) {
> return iommu_bus->iommu_fn(bus, iommu_bus->iommu_opaque, devfn);
Re: [PATCH v4 02/15] hw/pci: Refactor pci_device_iommu_address_space()
Hi Joao,
On 6/22/23 23:48, Joao Martins wrote:
> From: Yi Liu
>
> 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
> [joao: Commit message, and better splitting]
> Signed-off-by: Joao Martins
Reviewed-by: Eric Auger
Eric
> ---
> Splitted from v1:
> https://lore.kernel.org/all/20210302203827.437645-6-yi.l@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, );
> if (!pci_bus_bypass_iommu(bus) && iommu_bus) {
> if (iommu_bus->iommu_fn) {
> return iommu_bus->iommu_fn(bus, iommu_bus->iommu_opaque, devfn);
Re: [PATCH v4 02/15] hw/pci: Refactor pci_device_iommu_address_space()
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 >>> >>> 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.
Re: [PATCH v4 02/15] hw/pci: Refactor pci_device_iommu_address_space()
On 02/10/2023 16:22, Cédric Le Goater wrote:
> On 6/22/23 23:48, Joao Martins wrote:
>> From: Yi Liu
>>
>> 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
>> [joao: Commit message, and better splitting]
>> Signed-off-by: Joao Martins
>> ---
>> Splitted from v1:
>> https://lore.kernel.org/all/20210302203827.437645-6-yi.l@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, );
>> if (!pci_bus_bypass_iommu(bus) && iommu_bus) {
>> if (iommu_bus->iommu_fn) {
>> return iommu_bus->iommu_fn(bus, iommu_bus->iommu_opaque, devfn);
>
Re: [PATCH v4 02/15] hw/pci: Refactor pci_device_iommu_address_space()
On 6/22/23 23:48, Joao Martins wrote:
From: Yi Liu
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
[joao: Commit message, and better splitting]
Signed-off-by: Joao Martins
---
Splitted from v1:
https://lore.kernel.org/all/20210302203827.437645-6-yi.l@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, );
if (!pci_bus_bypass_iommu(bus) && iommu_bus) {
if (iommu_bus->iommu_fn) {
return iommu_bus->iommu_fn(bus, iommu_bus->iommu_opaque, devfn);
