[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V3] virtio: do not require IOMMU to be created i

From: Marcel Apfelbaum
Subject: Re: [Qemu-devel] [PATCH V3] virtio: do not require IOMMU to be created in advance
Date: Wed, 8 Mar 2017 10:24:40 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1

On 03/08/2017 04:43 AM, Peter Xu wrote:
On Tue, Mar 07, 2017 at 02:47:30PM +0200, Marcel Apfelbaum wrote:
On 03/07/2017 11:09 AM, Jason Wang wrote:
After commit 96a8821d2141 ("virtio: unbreak virtio-pci with IOMMU
after caching ring translations"), IOMMU was required to be created in
advance. This is because we can only get the correct dma_as after pci
IOMMU (e.g intel_iommu) was initialized. This is suboptimal and
inconvenient for user. This patch releases this by:

- introduce a bus_master_ready method for PCIDeviceClass and trigger
 this during pci_init_bus_master
- implement virtio-pci method and 1) reset the dma_as 2) re-register
 the memory listener to the new dma_as

Hi Jason,

Cc: Paolo Bonzini <address@hidden>
Signed-off-by: Jason Wang <address@hidden>
Changes from V2:
- delay pci_init_bus_master() after pci device is realized to make
 bus_master_ready a more generic method
hw/pci/pci.c               | 11 ++++++++---
hw/virtio/virtio-pci.c     |  9 +++++++++
hw/virtio/virtio.c         | 19 +++++++++++++++++++
include/hw/pci/pci.h       |  1 +
include/hw/virtio/virtio.h |  1 +
5 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 273f1e4..22e6ad9 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -83,6 +83,7 @@ static const VMStateDescription vmstate_pcibus = {
static void pci_init_bus_master(PCIDevice *pci_dev)
    AddressSpace *dma_as = pci_device_iommu_address_space(pci_dev);
+    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);

                             OBJECT(pci_dev), "bus master",
@@ -90,6 +91,9 @@ static void pci_init_bus_master(PCIDevice *pci_dev)
    memory_region_set_enabled(&pci_dev->bus_master_enable_region, false);
                       &pci_dev->bus_master_enable_region, pci_dev->name);
+    if (pc->bus_master_ready) {
+        pc->bus_master_ready(pci_dev);
+    }

static void pcibus_machine_done(Notifier *notifier, void *data)
@@ -995,9 +999,6 @@ static PCIDevice *do_pci_register_device(PCIDevice 
*pci_dev, PCIBus *bus,
    pci_dev->devfn = devfn;
    pci_dev->requester_id_cache = pci_req_id_cache_get(pci_dev);

-    if (qdev_hotplug) {
-        pci_init_bus_master(pci_dev);
-    }
    pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
    pci_dev->irq_state = 0;
@@ -1995,6 +1996,10 @@ static void pci_qdev_realize(DeviceState *qdev, Error 

+    if (qdev_hotplug) {
+        pci_init_bus_master(pci_dev);
+    }

How does it help if we move qdev_hotplug check outside "do_pci_register_device"?
I suppose you want the code to run after the "realize" function?
If so, what happens if a "realize" function of another device needs the 
(at hotplug time)?

My unmature understanding is that, we can just call
pci_device_iommu_address_space() if device realization really needs
the address space, rather than using bus_master_as.

Yes, each device that support IOMMU needs to call it instead of bus_master_as.
As if it really needs this information at realize time is debatable.
The vfio-pci is a special case and Alex Williamson explain why it is needed
at the "realize" time.

An example is vfio-pci device. It is using
pci_device_iommu_address_space() in vfio_realize(). Though I _guess_
there may be other reasons behind, e.g., VFIOAddressSpace should be
1:1 mapped to bus master address space, so we may want to make sure
this address space will be the same when different devices are using
the same address space (while bus_master_as is one per device, even if
two devices have the same backend address space, there will be two
distinct bus_master_as).


IIUC, bus_master_as is only used to emulate the master bit in PCI
command register. In that case, that should only be there for the
guest operations, while imho device realization is "emulation code",
which can directly use pci_device_iommu_address_space(). Actually, if
it plays with bus_master_as even if it can, I guess it'll just fail
since that region has not yet been enabled.

Please kindly correct me if I made a mistake...

You are right the device does not need the bus master address space before the
VM is up, this is we the init process of this region is delayed until machine 

My question was why do you need to move pci_init_bus_master after device 
at hotplug time and how would influence the other devices.



-- peterx

reply via email to

[Prev in Thread] Current Thread [Next in Thread]