[Qemu-devel] [PATCH v1][RFC] pci: Set PCI-bus device entry before initializing bus master

[Qemu-devel] [PATCH v1][RFC] pci: Set PCI-bus device entry before initializing bus master

[Pierre Morel]

When initializing the PCI-bus master pci_init_bus_master(PCIDevice *)
calls pci_device_iommu_address_space(PCIDevice *) which in turn calls
iommu_bus->iommu_fn(bus, opaque,devfn)

If the device entry of the PCI-bus is not initialized to point to the
PCIDevice structure, the iommu_fn() function, getting only bus and
devno, is unable to retrieve the PCIDevice.

Usually it is not a problem as the DMA address space is depending of
the BUS but in an architecture like S390x, each PCI Device can have
its own DMA address space.
Being able to setup the DMA address space at the right moment will
greatly simplify the hotplug implementation for these architectures.

This patch proposes to setup the bus device entry before calling
pci_init_bus_master() and reset the entry in case of error.


Pierre Morel (1):
  pci: Set PCI-bus device entry before initializing bus master

 hw/pci/pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.7.4

[Qemu-devel] [PATCH v1] pci: Set PCI-bus device entry before initializing bus master

[Pierre Morel]

When initializing the PCI-bus master pci_init_bus_master(PCIDevice *)
calls pci_device_iommu_address_space(PCIDevice *) which in turn calls
iommu_bus->iommu_fn(bus, opaque,devfn)

If the device entry of the PCI-bus is not initialized to point to the
PCIDevice structure, the iommu_fn() function, getting only bus and
devno, is unable to retrieve the PCIDevice.

Usually it is not a problem as the DMA address space is depending of
the BUS but in an architecture like S390, each PCI Device can have
its own DMA address space.
Being able to setup the DMA address space at the right moment will
greatly simplify the implementation.

This patch proposes to setup the bus device entry before calling
pci_init_bus_master() and reset the entry in case of error.

Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
---
 hw/pci/pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 1e6fb88..3e64492 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1026,6 +1026,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     address_space_init(&pci_dev->bus_master_as,
                        &pci_dev->bus_master_container_region, pci_dev->name);
 
+    bus->devices[devfn] = pci_dev;
     if (qdev_hotplug) {
         pci_init_bus_master(pci_dev);
     }
@@ -1062,6 +1063,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     if (local_err) {
         error_propagate(errp, local_err);
         do_pci_unregister_device(pci_dev);
+        bus->devices[devfn] = NULL;
         return NULL;
     }
 
@@ -1071,7 +1073,6 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
         config_write = pci_default_write_config;
     pci_dev->config_read = config_read;
     pci_dev->config_write = config_write;
-    bus->devices[devfn] = pci_dev;
     pci_dev->version_id = 2; /* Current pci device vmstate version */
     return pci_dev;
 }
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1] pci: Set PCI-bus device entry before initializing bus master

[Yi Min Zhao]

I strongly agree. This will make IOMMU assignment easier.


在 2017/10/5 下午8:36, Pierre Morel 写道:
> When initializing the PCI-bus master pci_init_bus_master(PCIDevice *)
> calls pci_device_iommu_address_space(PCIDevice *) which in turn calls
> iommu_bus->iommu_fn(bus, opaque,devfn)
>
> If the device entry of the PCI-bus is not initialized to point to the
> PCIDevice structure, the iommu_fn() function, getting only bus and
> devno, is unable to retrieve the PCIDevice.
>
> Usually it is not a problem as the DMA address space is depending of
> the BUS but in an architecture like S390, each PCI Device can have
> its own DMA address space.
> Being able to setup the DMA address space at the right moment will
> greatly simplify the implementation.
>
> This patch proposes to setup the bus device entry before calling
> pci_init_bus_master() and reset the entry in case of error.
>
> Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
> ---
>   hw/pci/pci.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 1e6fb88..3e64492 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1026,6 +1026,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>       address_space_init(&pci_dev->bus_master_as,
>                          &pci_dev->bus_master_container_region, pci_dev->name);
>
> +    bus->devices[devfn] = pci_dev;
>       if (qdev_hotplug) {
>           pci_init_bus_master(pci_dev);
>       }
> @@ -1062,6 +1063,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>       if (local_err) {
>           error_propagate(errp, local_err);
>           do_pci_unregister_device(pci_dev);
> +        bus->devices[devfn] = NULL;
>           return NULL;
>       }
>
> @@ -1071,7 +1073,6 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>           config_write = pci_default_write_config;
>       pci_dev->config_read = config_read;
>       pci_dev->config_write = config_write;
> -    bus->devices[devfn] = pci_dev;
>       pci_dev->version_id = 2; /* Current pci device vmstate version */
>       return pci_dev;
>   }