All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V3 00/10] vhost device IOTLB support
@ 2016-11-07  7:09 Jason Wang
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 01/10] virtio: convert to use DMA api Jason Wang
                   ` (9 more replies)
  0 siblings, 10 replies; 23+ messages in thread
From: Jason Wang @ 2016-11-07  7:09 UTC (permalink / raw)
  To: qemu-devel, mst
  Cc: pbonzini, peterx, cornelia.huck, wexu, vkaplans, Jason Wang

Hi all:

As the userspace vitio driver became popular, more and more request
were received for secure DMA environemt (DMAR). So this series tries
to make DMAR works for virtio/vhost. The idea is let virtio/vhost
co-work with userspace iommu implememtation. This is done through:

- for virtio, when platform supports IOMMU (VIRTIO_F_IOMMU_PLATFORM),
  virtio will not assume address_space_memory, instead a transport
  specific method were introduced for querying the dma address space
  and dma helpers were used in device emulation codes.
- for vhost, implement a device IOTLB by using device IOTLB API
  supported by recent kernel. With this API, vhost kernel can query
  IOTLB entry for a specified iova from qemu, qemu can invalidate an
  arbitrary range of iova in vhost kernel.

The device IOTLB API is totaly architecture independent, an example
implementation was done with intel iommu by:

- implement basic ATS (Address Translation Service) for virtio-pci,
  this will make device IOTLB visible for iommu driver in guest.
- implement device IOTLB descriptor processing in intel iommu (enabled
  through device-iotlb=on), and trigger the device IOTLB invalidation
  in vhost through iommu notifier.

It could be easily ported to other IOMMU or architecture even if it
doesn't support device IOTLB. (e.g just invalidate the vhost IOTLB
during IOMMU IOTLB invalidation). But this will be slow since several
devics were contending userspace IOTLB entries.

AMD IOMMU suppot for device IOTLB is ready, but it depends on other
fixes to work correctly.

Test was done by:

- intel_iommu=on/strict in guest.
- vfio l2fwd in guest.

This main use case is the programs that use fixed mapping in guest
(e.g dpdk). If 1G hugepage were used in guest, thanks to the SLLPS
support, we can get 100% TLB hit rate for l2fwd in guest.

For the normal kernel driver which uses lots of dynamic mapping and
unmapping, we may see performance penalty, this could be optimized in
the future.

TODO:
- more platforms and IOMMU support (AMD IOMMU support is ongoing)
- performance optimizations (e.g merging adjacent mappings)
- non ATS support (userspace IOTLB snooping) ?

Changes from V2:
- rebase to HEAD
- avoid querying dma_as each time by using vdev->dma_as directly
- fix and improve address_space_get_iotlb_entry()
- drop patch 1 (which has been posted as an independent fix)
- fix ECAP when device-iotlb=off
- fix centos6 build

Changes from V1:
- rebase to HEAD
- avoid calling transport specific dma as fetching method each time by
  caching it in vdev
- convert to use new IOMMU notifier API
- silent checkpatch warnings and fix 32bit build
- use "device-iotlb" instead of "device_iotlb"
- rename virtio_memory_map() to vhost_memory_map() and move it to vhost.c
- use memory_region_is_iommu() instead of inventing new one

Changes from RFC:
- rebase to HEAD
- switch to use new vhost device IOTLB API
- use the new feature bit VIRITO_F_IOMMU_PLATFORM
- finalize basic ATS implementation
- add ATSR for Root port ATS transaction
- fix the iommu notifier handling during unregistering
- use snprintf() in patch 3
- correc the loop in address_space_get_iotlb_entry()
- small tweak on the address calculation during device iotlb
  descriptor processing.

Jason Wang (10):
  virtio: convert to use DMA api
  intel_iommu: name vtd address space with devfn
  intel_iommu: allocate new key when creating new address space
  exec: introduce address_space_get_iotlb_entry()
  intel_iommu: support device iotlb descriptor
  virtio-pci: address space translation service (ATS) support
  acpi: add ATSR for q35
  memory: handle alias for iommu notifier
  memory: handle alias in memory_region_is_iommu()
  vhost_net: device IOTLB support

 exec.c                                    |  33 ++++++
 hw/block/virtio-blk.c                     |   2 +-
 hw/char/virtio-serial-bus.c               |   3 +-
 hw/i386/acpi-build.c                      |   9 ++
 hw/i386/intel_iommu.c                     |  90 +++++++++++++--
 hw/i386/intel_iommu_internal.h            |  13 ++-
 hw/i386/x86-iommu.c                       |  17 +++
 hw/pci/pcie.c                             |  16 +++
 hw/scsi/virtio-scsi.c                     |   4 +-
 hw/virtio/vhost-backend.c                 |  99 ++++++++++++++++
 hw/virtio/vhost.c                         | 180 ++++++++++++++++++++++++++----
 hw/virtio/virtio-bus.c                    |   8 ++
 hw/virtio/virtio-pci.c                    |  21 ++++
 hw/virtio/virtio-pci.h                    |   4 +
 hw/virtio/virtio.c                        |  57 ++++++----
 include/exec/memory.h                     |   9 ++
 include/hw/acpi/acpi-defs.h               |  12 ++
 include/hw/i386/x86-iommu.h               |   1 +
 include/hw/pci/pcie.h                     |   4 +
 include/hw/virtio/vhost-backend.h         |  13 +++
 include/hw/virtio/vhost.h                 |   4 +
 include/hw/virtio/virtio-access.h         |  31 +++--
 include/hw/virtio/virtio-bus.h            |   1 +
 include/hw/virtio/virtio.h                |   9 +-
 include/standard-headers/linux/pci_regs.h |   1 +
 memory.c                                  |   9 ++
 net/tap.c                                 |   1 +
 27 files changed, 582 insertions(+), 69 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH V3 01/10] virtio: convert to use DMA api
  2016-11-07  7:09 [Qemu-devel] [PATCH V3 00/10] vhost device IOTLB support Jason Wang
@ 2016-11-07  7:09 ` Jason Wang
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 02/10] intel_iommu: name vtd address space with devfn Jason Wang
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Jason Wang @ 2016-11-07  7:09 UTC (permalink / raw)
  To: qemu-devel, mst
  Cc: pbonzini, peterx, cornelia.huck, wexu, vkaplans, Jason Wang,
	Stefan Hajnoczi, Kevin Wolf, Amit Shah, qemu-block

Currently, all virtio devices bypass IOMMU completely. This is because
address_space_memory is assumed and used during DMA emulation. This
patch converts the virtio core API to use DMA API. This idea is

- introducing a new transport specific helper to query the dma address
  space. (only pci version is implemented).
- query and use this address space during virtio device guest memory
  accessing when iommu platform (VIRTIO_F_IOMMU_PLATFORM) was enabled
  for this device.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Amit Shah <amit.shah@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/block/virtio-blk.c             |  2 +-
 hw/char/virtio-serial-bus.c       |  3 ++-
 hw/scsi/virtio-scsi.c             |  4 ++-
 hw/virtio/virtio-bus.c            |  8 ++++++
 hw/virtio/virtio-pci.c            | 14 ++++++++++
 hw/virtio/virtio.c                | 57 +++++++++++++++++++++++++--------------
 include/hw/virtio/virtio-access.h | 31 ++++++++++++++-------
 include/hw/virtio/virtio-bus.h    |  1 +
 include/hw/virtio/virtio.h        |  9 ++++---
 9 files changed, 93 insertions(+), 36 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 37fe72b..6a2dbaf 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -860,7 +860,7 @@ static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
             }
         }
 
-        req = qemu_get_virtqueue_element(f, sizeof(VirtIOBlockReq));
+        req = qemu_get_virtqueue_element(vdev, f, sizeof(VirtIOBlockReq));
         virtio_blk_init_request(s, virtio_get_queue(vdev, vq_idx), req);
         req->next = s->rq;
         s->rq = req;
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 7975c2c..d544cd9 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -732,6 +732,7 @@ static void virtio_serial_post_load_timer_cb(void *opaque)
 static int fetch_active_ports_list(QEMUFile *f,
                                    VirtIOSerial *s, uint32_t nr_active_ports)
 {
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
     uint32_t i;
 
     s->post_load = g_malloc0(sizeof(*s->post_load));
@@ -765,7 +766,7 @@ static int fetch_active_ports_list(QEMUFile *f,
             qemu_get_be64s(f, &port->iov_offset);
 
             port->elem =
-                qemu_get_virtqueue_element(f, sizeof(VirtQueueElement));
+                qemu_get_virtqueue_element(vdev, f, sizeof(VirtQueueElement));
 
             /*
              *  Port was throttled on source machine.  Let's
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 4762f05..1519cee 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -198,12 +198,14 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
     SCSIBus *bus = sreq->bus;
     VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
     VirtIOSCSIReq *req;
     uint32_t n;
 
     qemu_get_be32s(f, &n);
     assert(n < vs->conf.num_queues);
-    req = qemu_get_virtqueue_element(f, sizeof(VirtIOSCSIReq) + vs->cdb_size);
+    req = qemu_get_virtqueue_element(vdev, f,
+                                     sizeof(VirtIOSCSIReq) + vs->cdb_size);
     virtio_scsi_init_req(s, vs->cmd_vqs[n], req);
 
     if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 11f65bd..8597762 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -28,6 +28,7 @@
 #include "hw/qdev.h"
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio.h"
+#include "exec/address-spaces.h"
 
 /* #define DEBUG_VIRTIO_BUS */
 
@@ -61,6 +62,13 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp)
     if (klass->device_plugged != NULL) {
         klass->device_plugged(qbus->parent, errp);
     }
+
+    if (klass->get_dma_as != NULL &&
+        virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) {
+        vdev->dma_as = klass->get_dma_as(qbus->parent);
+    } else {
+        vdev->dma_as = &address_space_memory;
+    }
 }
 
 /* Reset the virtio_bus */
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 06831de..6ceb43e 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1168,6 +1168,14 @@ static int virtio_pci_query_nvectors(DeviceState *d)
     return proxy->nvectors;
 }
 
+static AddressSpace *virtio_pci_get_dma_as(DeviceState *d)
+{
+    VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
+    PCIDevice *dev = &proxy->pci_dev;
+
+    return pci_get_address_space(dev);
+}
+
 static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
                                    struct virtio_pci_cap *cap)
 {
@@ -1624,6 +1632,11 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp)
     }
 
     if (legacy) {
+        if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) {
+            error_setg(errp, "VIRTIO_F_IOMMU_PLATFORM was supported by"
+                       "neither legacy nor transitional device.");
+            return ;
+        }
         /* legacy and transitional */
         pci_set_word(config + PCI_SUBSYSTEM_VENDOR_ID,
                      pci_get_word(config + PCI_VENDOR_ID));
@@ -2544,6 +2557,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
     k->ioeventfd_disabled = virtio_pci_ioeventfd_disabled;
     k->ioeventfd_set_disabled = virtio_pci_ioeventfd_set_disabled;
     k->ioeventfd_assign = virtio_pci_ioeventfd_assign;
+    k->get_dma_as = virtio_pci_get_dma_as;
 }
 
 static const TypeInfo virtio_pci_bus_info = {
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index d48d1a9..943db77 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -23,6 +23,7 @@
 #include "hw/virtio/virtio-bus.h"
 #include "migration/migration.h"
 #include "hw/virtio/virtio-access.h"
+#include "sysemu/dma.h"
 
 /*
  * The alignment to use between consumer and producer parts of vring.
@@ -122,7 +123,7 @@ void virtio_queue_update_rings(VirtIODevice *vdev, int n)
 static void vring_desc_read(VirtIODevice *vdev, VRingDesc *desc,
                             hwaddr desc_pa, int i)
 {
-    address_space_read(&address_space_memory, desc_pa + i * sizeof(VRingDesc),
+    address_space_read(vdev->dma_as, desc_pa + i * sizeof(VRingDesc),
                        MEMTXATTRS_UNSPECIFIED, (void *)desc, sizeof(VRingDesc));
     virtio_tswap64s(vdev, &desc->addr);
     virtio_tswap32s(vdev, &desc->len);
@@ -164,7 +165,7 @@ static inline void vring_used_write(VirtQueue *vq, VRingUsedElem *uelem,
     virtio_tswap32s(vq->vdev, &uelem->id);
     virtio_tswap32s(vq->vdev, &uelem->len);
     pa = vq->vring.used + offsetof(VRingUsed, ring[i]);
-    address_space_write(&address_space_memory, pa, MEMTXATTRS_UNSPECIFIED,
+    address_space_write(vq->vdev->dma_as, pa, MEMTXATTRS_UNSPECIFIED,
                        (void *)uelem, sizeof(VRingUsedElem));
 }
 
@@ -244,6 +245,7 @@ int virtio_queue_empty(VirtQueue *vq)
 static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
                                unsigned int len)
 {
+    AddressSpace *dma_as = vq->vdev->dma_as;
     unsigned int offset;
     int i;
 
@@ -251,17 +253,18 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
     for (i = 0; i < elem->in_num; i++) {
         size_t size = MIN(len - offset, elem->in_sg[i].iov_len);
 
-        cpu_physical_memory_unmap(elem->in_sg[i].iov_base,
-                                  elem->in_sg[i].iov_len,
-                                  1, size);
+        dma_memory_unmap(dma_as, elem->in_sg[i].iov_base,
+                         elem->in_sg[i].iov_len,
+                         DMA_DIRECTION_FROM_DEVICE, size);
 
         offset += size;
     }
 
     for (i = 0; i < elem->out_num; i++)
-        cpu_physical_memory_unmap(elem->out_sg[i].iov_base,
-                                  elem->out_sg[i].iov_len,
-                                  0, elem->out_sg[i].iov_len);
+        dma_memory_unmap(dma_as, elem->out_sg[i].iov_base,
+                         elem->out_sg[i].iov_len,
+                         DMA_DIRECTION_TO_DEVICE,
+                         elem->out_sg[i].iov_len);
 }
 
 /* virtqueue_detach_element:
@@ -555,7 +558,10 @@ static bool virtqueue_map_desc(VirtIODevice *vdev, unsigned int *p_num_sg,
             goto out;
         }
 
-        iov[num_sg].iov_base = cpu_physical_memory_map(pa, &len, is_write);
+        iov[num_sg].iov_base = dma_memory_map(vdev->dma_as, pa, &len,
+                                              is_write ?
+                                              DMA_DIRECTION_FROM_DEVICE :
+                                              DMA_DIRECTION_TO_DEVICE);
         if (!iov[num_sg].iov_base) {
             virtio_error(vdev, "virtio: bogus descriptor or out of resources");
             goto out;
@@ -592,9 +598,9 @@ static void virtqueue_undo_map_desc(unsigned int out_num, unsigned int in_num,
     }
 }
 
-static void virtqueue_map_iovec(struct iovec *sg, hwaddr *addr,
-                                unsigned int *num_sg, unsigned int max_size,
-                                int is_write)
+static void virtqueue_map_iovec(VirtIODevice *vdev, struct iovec *sg,
+                                hwaddr *addr, unsigned int *num_sg,
+                                unsigned int max_size, int is_write)
 {
     unsigned int i;
     hwaddr len;
@@ -613,7 +619,10 @@ static void virtqueue_map_iovec(struct iovec *sg, hwaddr *addr,
 
     for (i = 0; i < *num_sg; i++) {
         len = sg[i].iov_len;
-        sg[i].iov_base = cpu_physical_memory_map(addr[i], &len, is_write);
+        sg[i].iov_base = dma_memory_map(vdev->dma_as,
+                                        addr[i], &len, is_write ?
+                                        DMA_DIRECTION_FROM_DEVICE :
+                                        DMA_DIRECTION_TO_DEVICE);
         if (!sg[i].iov_base) {
             error_report("virtio: error trying to map MMIO memory");
             exit(1);
@@ -625,12 +634,15 @@ static void virtqueue_map_iovec(struct iovec *sg, hwaddr *addr,
     }
 }
 
-void virtqueue_map(VirtQueueElement *elem)
+void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem)
 {
-    virtqueue_map_iovec(elem->in_sg, elem->in_addr, &elem->in_num,
-                        VIRTQUEUE_MAX_SIZE, 1);
-    virtqueue_map_iovec(elem->out_sg, elem->out_addr, &elem->out_num,
-                        VIRTQUEUE_MAX_SIZE, 0);
+    virtqueue_map_iovec(vdev, elem->in_sg, elem->in_addr, &elem->in_num,
+                        MIN(ARRAY_SIZE(elem->in_sg), ARRAY_SIZE(elem->in_addr)),
+                        1);
+    virtqueue_map_iovec(vdev, elem->out_sg, elem->out_addr, &elem->out_num,
+                        MIN(ARRAY_SIZE(elem->out_sg),
+                        ARRAY_SIZE(elem->out_addr)),
+                        0);
 }
 
 void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_num)
@@ -783,7 +795,7 @@ typedef struct VirtQueueElementOld {
     struct iovec out_sg[VIRTQUEUE_MAX_SIZE];
 } VirtQueueElementOld;
 
-void *qemu_get_virtqueue_element(QEMUFile *f, size_t sz)
+void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz)
 {
     VirtQueueElement *elem;
     VirtQueueElementOld data;
@@ -814,7 +826,7 @@ void *qemu_get_virtqueue_element(QEMUFile *f, size_t sz)
         elem->out_sg[i].iov_len = data.out_sg[i].iov_len;
     }
 
-    virtqueue_map(elem);
+    virtqueue_map(vdev, elem);
     return elem;
 }
 
@@ -873,6 +885,11 @@ static int virtio_validate_features(VirtIODevice *vdev)
 {
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
 
+    if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM) &&
+        !virtio_vdev_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) {
+        return -EFAULT;
+    }
+
     if (k->validate_features) {
         return k->validate_features(vdev);
     } else {
diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index 440b455..91ae14d 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -17,6 +17,7 @@
 #define QEMU_VIRTIO_ACCESS_H
 
 #include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-bus.h"
 #include "exec/address-spaces.h"
 
 #if defined(TARGET_PPC64) || defined(TARGET_ARM)
@@ -40,45 +41,55 @@ static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
 
 static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, hwaddr pa)
 {
+    AddressSpace *dma_as = vdev->dma_as;
+
     if (virtio_access_is_big_endian(vdev)) {
-        return lduw_be_phys(&address_space_memory, pa);
+        return lduw_be_phys(dma_as, pa);
     }
-    return lduw_le_phys(&address_space_memory, pa);
+    return lduw_le_phys(dma_as, pa);
 }
 
 static inline uint32_t virtio_ldl_phys(VirtIODevice *vdev, hwaddr pa)
 {
+    AddressSpace *dma_as = vdev->dma_as;
+
     if (virtio_access_is_big_endian(vdev)) {
-        return ldl_be_phys(&address_space_memory, pa);
+        return ldl_be_phys(dma_as, pa);
     }
-    return ldl_le_phys(&address_space_memory, pa);
+    return ldl_le_phys(dma_as, pa);
 }
 
 static inline uint64_t virtio_ldq_phys(VirtIODevice *vdev, hwaddr pa)
 {
+    AddressSpace *dma_as = vdev->dma_as;
+
     if (virtio_access_is_big_endian(vdev)) {
-        return ldq_be_phys(&address_space_memory, pa);
+        return ldq_be_phys(dma_as, pa);
     }
-    return ldq_le_phys(&address_space_memory, pa);
+    return ldq_le_phys(dma_as, pa);
 }
 
 static inline void virtio_stw_phys(VirtIODevice *vdev, hwaddr pa,
                                    uint16_t value)
 {
+    AddressSpace *dma_as = vdev->dma_as;
+
     if (virtio_access_is_big_endian(vdev)) {
-        stw_be_phys(&address_space_memory, pa, value);
+        stw_be_phys(dma_as, pa, value);
     } else {
-        stw_le_phys(&address_space_memory, pa, value);
+        stw_le_phys(dma_as, pa, value);
     }
 }
 
 static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa,
                                    uint32_t value)
 {
+    AddressSpace *dma_as = vdev->dma_as;
+
     if (virtio_access_is_big_endian(vdev)) {
-        stl_be_phys(&address_space_memory, pa, value);
+        stl_be_phys(dma_as, pa, value);
     } else {
-        stl_le_phys(&address_space_memory, pa, value);
+        stl_le_phys(dma_as, pa, value);
     }
 }
 
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 2e4b67e..9d5aa79 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -98,6 +98,7 @@ typedef struct VirtioBusClass {
      * Note that changing this will break migration for this transport.
      */
     bool has_variable_vring_alignment;
+    AddressSpace *(*get_dma_as)(DeviceState *d);
 } VirtioBusClass;
 
 struct VirtioBusState {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b913aac..1245831 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -92,6 +92,7 @@ struct VirtIODevice
     char *bus_name;
     uint8_t device_endian;
     bool use_guest_notifier_mask;
+    AddressSpace *dma_as;
     QLIST_HEAD(, VirtQueue) *vector_queues;
 };
 
@@ -163,9 +164,9 @@ bool virtqueue_rewind(VirtQueue *vq, unsigned int num);
 void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
                     unsigned int len, unsigned int idx);
 
-void virtqueue_map(VirtQueueElement *elem);
+void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem);
 void *virtqueue_pop(VirtQueue *vq, size_t sz);
-void *qemu_get_virtqueue_element(QEMUFile *f, size_t sz);
+void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz);
 void qemu_put_virtqueue_element(QEMUFile *f, VirtQueueElement *elem);
 int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes,
                           unsigned int out_bytes);
@@ -247,7 +248,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
     DEFINE_PROP_BIT64("notify_on_empty", _state, _field,  \
                       VIRTIO_F_NOTIFY_ON_EMPTY, true), \
     DEFINE_PROP_BIT64("any_layout", _state, _field, \
-                      VIRTIO_F_ANY_LAYOUT, true)
+                      VIRTIO_F_ANY_LAYOUT, true), \
+    DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
+                      VIRTIO_F_IOMMU_PLATFORM, false)
 
 hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH V3 02/10] intel_iommu: name vtd address space with devfn
  2016-11-07  7:09 [Qemu-devel] [PATCH V3 00/10] vhost device IOTLB support Jason Wang
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 01/10] virtio: convert to use DMA api Jason Wang
@ 2016-11-07  7:09 ` Jason Wang
  2016-12-16  2:53   ` Peter Xu
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 03/10] intel_iommu: allocate new key when creating new address space Jason Wang
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Jason Wang @ 2016-11-07  7:09 UTC (permalink / raw)
  To: qemu-devel, mst
  Cc: pbonzini, peterx, cornelia.huck, wexu, vkaplans, Jason Wang,
	Richard Henderson, Eduardo Habkost

To avoid duplicated name and ease debugging.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/i386/intel_iommu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 1655a65..5272c30 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2323,6 +2323,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
     uintptr_t key = (uintptr_t)bus;
     VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key);
     VTDAddressSpace *vtd_dev_as;
+    char name[128];
 
     if (!vtd_bus) {
         /* No corresponding free() */
@@ -2336,6 +2337,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
     vtd_dev_as = vtd_bus->dev_as[devfn];
 
     if (!vtd_dev_as) {
+        snprintf(name, sizeof(name), "intel_iommu_devfn_%d", devfn);
         vtd_bus->dev_as[devfn] = vtd_dev_as = g_malloc0(sizeof(VTDAddressSpace));
 
         vtd_dev_as->bus = bus;
@@ -2350,7 +2352,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
         memory_region_add_subregion(&vtd_dev_as->iommu, VTD_INTERRUPT_ADDR_FIRST,
                                     &vtd_dev_as->iommu_ir);
         address_space_init(&vtd_dev_as->as,
-                           &vtd_dev_as->iommu, "intel_iommu");
+                           &vtd_dev_as->iommu, name);
     }
     return vtd_dev_as;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH V3 03/10] intel_iommu: allocate new key when creating new address space
  2016-11-07  7:09 [Qemu-devel] [PATCH V3 00/10] vhost device IOTLB support Jason Wang
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 01/10] virtio: convert to use DMA api Jason Wang
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 02/10] intel_iommu: name vtd address space with devfn Jason Wang
@ 2016-11-07  7:09 ` Jason Wang
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 04/10] exec: introduce address_space_get_iotlb_entry() Jason Wang
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Jason Wang @ 2016-11-07  7:09 UTC (permalink / raw)
  To: qemu-devel, mst
  Cc: pbonzini, peterx, cornelia.huck, wexu, vkaplans, Jason Wang,
	Richard Henderson, Eduardo Habkost

We use the pointer to stack for key for new address space, this will break hash
table searching, fixing by g_malloc() a new key instead.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/i386/intel_iommu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 5272c30..a2678bc 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2326,12 +2326,13 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
     char name[128];
 
     if (!vtd_bus) {
+        uintptr_t *new_key = g_malloc(sizeof(*new_key));
+        *new_key = (uintptr_t)bus;
         /* No corresponding free() */
         vtd_bus = g_malloc0(sizeof(VTDBus) + sizeof(VTDAddressSpace *) * \
                             X86_IOMMU_PCI_DEVFN_MAX);
         vtd_bus->bus = bus;
-        key = (uintptr_t)bus;
-        g_hash_table_insert(s->vtd_as_by_busptr, &key, vtd_bus);
+        g_hash_table_insert(s->vtd_as_by_busptr, new_key, vtd_bus);
     }
 
     vtd_dev_as = vtd_bus->dev_as[devfn];
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH V3 04/10] exec: introduce address_space_get_iotlb_entry()
  2016-11-07  7:09 [Qemu-devel] [PATCH V3 00/10] vhost device IOTLB support Jason Wang
                   ` (2 preceding siblings ...)
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 03/10] intel_iommu: allocate new key when creating new address space Jason Wang
@ 2016-11-07  7:09 ` Jason Wang
  2016-11-07 17:07   ` Paolo Bonzini
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 05/10] intel_iommu: support device iotlb descriptor Jason Wang
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Jason Wang @ 2016-11-07  7:09 UTC (permalink / raw)
  To: qemu-devel, mst
  Cc: pbonzini, peterx, cornelia.huck, wexu, vkaplans, Jason Wang,
	Peter Crosthwaite, Richard Henderson

This patch introduces a helper to query the iotlb entry for a
possible iova. This will be used by later device IOTLB API to enable
the capability for a dataplane (e.g vhost) to query the IOTLB.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 exec.c                | 33 +++++++++++++++++++++++++++++++++
 include/exec/memory.h |  6 ++++++
 2 files changed, 39 insertions(+)

diff --git a/exec.c b/exec.c
index b1094c0..f108cf6 100644
--- a/exec.c
+++ b/exec.c
@@ -449,6 +449,39 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
 }
 
 /* Called from RCU critical section */
+IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
+                                            bool is_write)
+{
+    IOMMUTLBEntry iotlb = {0};
+    MemoryRegionSection *section;
+    MemoryRegion *mr;
+
+    for (;;) {
+        AddressSpaceDispatch *d = atomic_rcu_read(&as->dispatch);
+        section = address_space_lookup_region(d, addr, false);
+        addr = addr - section->offset_within_address_space
+               + section->offset_within_region;
+        mr = section->mr;
+
+        if (!mr->iommu_ops) {
+            break;
+        }
+
+        iotlb = mr->iommu_ops->translate(mr, addr, is_write);
+        if (!(iotlb.perm & (1 << is_write))) {
+            iotlb.target_as = NULL;
+            break;
+        }
+
+        addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
+                | (addr & iotlb.addr_mask));
+        as = iotlb.target_as;
+    }
+
+    return iotlb;
+}
+
+/* Called from RCU critical section */
 MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
                                       hwaddr *xlat, hwaddr *plen,
                                       bool is_write)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 9728a2f..e605de3 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1404,6 +1404,12 @@ void address_space_stq_le(AddressSpace *as, hwaddr addr, uint64_t val,
 void address_space_stq_be(AddressSpace *as, hwaddr addr, uint64_t val,
                             MemTxAttrs attrs, MemTxResult *result);
 
+/* address_space_get_iotlb_entry: translate an address into an IOTLB
+ * entry. Should be called from an RCU critical section.
+ */
+IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
+                                            bool is_write);
+
 /* address_space_translate: translate an address range into an address space
  * into a MemoryRegion and an address range into that section.  Should be
  * called from an RCU critical section, to avoid that the last reference
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH V3 05/10] intel_iommu: support device iotlb descriptor
  2016-11-07  7:09 [Qemu-devel] [PATCH V3 00/10] vhost device IOTLB support Jason Wang
                   ` (3 preceding siblings ...)
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 04/10] exec: introduce address_space_get_iotlb_entry() Jason Wang
@ 2016-11-07  7:09 ` Jason Wang
  2016-11-07 23:35   ` Peter Xu
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 06/10] virtio-pci: address space translation service (ATS) support Jason Wang
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Jason Wang @ 2016-11-07  7:09 UTC (permalink / raw)
  To: qemu-devel, mst
  Cc: pbonzini, peterx, cornelia.huck, wexu, vkaplans, Jason Wang,
	Richard Henderson, Eduardo Habkost

This patch enables device IOTLB support for intel iommu. The major
work is to implement QI device IOTLB descriptor processing and notify
the device through iommu notifier.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/i386/intel_iommu.c          | 81 +++++++++++++++++++++++++++++++++++++++---
 hw/i386/intel_iommu_internal.h | 13 +++++--
 hw/i386/x86-iommu.c            | 17 +++++++++
 include/hw/i386/x86-iommu.h    |  1 +
 4 files changed, 105 insertions(+), 7 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index a2678bc..c1da0a6 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -738,11 +738,18 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num,
                     "context-entry hi 0x%"PRIx64 " lo 0x%"PRIx64,
                     ce->hi, ce->lo);
         return -VTD_FR_CONTEXT_ENTRY_INV;
-    } else if (ce->lo & VTD_CONTEXT_ENTRY_TT) {
-        VTD_DPRINTF(GENERAL, "error: unsupported Translation Type in "
-                    "context-entry hi 0x%"PRIx64 " lo 0x%"PRIx64,
-                    ce->hi, ce->lo);
-        return -VTD_FR_CONTEXT_ENTRY_INV;
+    } else {
+        switch (ce->lo & VTD_CONTEXT_ENTRY_TT) {
+        case VTD_CONTEXT_TT_MULTI_LEVEL:
+            /* fall through */
+        case VTD_CONTEXT_TT_DEV_IOTLB:
+            break;
+        default:
+            VTD_DPRINTF(GENERAL, "error: unsupported Translation Type in "
+                        "context-entry hi 0x%"PRIx64 " lo 0x%"PRIx64,
+                        ce->hi, ce->lo);
+            return -VTD_FR_CONTEXT_ENTRY_INV;
+        }
     }
     return 0;
 }
@@ -1437,7 +1444,59 @@ static bool vtd_process_inv_iec_desc(IntelIOMMUState *s,
     vtd_iec_notify_all(s, !inv_desc->iec.granularity,
                        inv_desc->iec.index,
                        inv_desc->iec.index_mask);
+    return true;
+}
+
+static bool vtd_process_device_iotlb_desc(IntelIOMMUState *s,
+                                          VTDInvDesc *inv_desc)
+{
+    VTDAddressSpace *vtd_dev_as;
+    IOMMUTLBEntry entry;
+    struct VTDBus *vtd_bus;
+    hwaddr addr;
+    uint64_t sz;
+    uint16_t sid;
+    uint8_t devfn;
+    bool size;
+    uint8_t bus_num;
+
+    addr = VTD_INV_DESC_DEVICE_IOTLB_ADDR(inv_desc->hi);
+    sid = VTD_INV_DESC_DEVICE_IOTLB_SID(inv_desc->lo);
+    devfn = sid & 0xff;
+    bus_num = sid >> 8;
+    size = VTD_INV_DESC_DEVICE_IOTLB_SIZE(inv_desc->hi);
+
+    if ((inv_desc->lo & VTD_INV_DESC_DEVICE_IOTLB_RSVD_LO) ||
+        (inv_desc->hi & VTD_INV_DESC_DEVICE_IOTLB_RSVD_HI)) {
+        VTD_DPRINTF(GENERAL, "error: non-zero reserved field in Device "
+                    "IOTLB Invalidate Descriptor hi 0x%"PRIx64 " lo 0x%"PRIx64,
+                    inv_desc->hi, inv_desc->lo);
+        return false;
+    }
+
+    vtd_bus = vtd_find_as_from_bus_num(s, bus_num);
+    if (!vtd_bus) {
+        goto done;
+    }
+
+    vtd_dev_as = vtd_bus->dev_as[devfn];
+    if (!vtd_dev_as) {
+        goto done;
+    }
+
+    if (size) {
+        sz = 1 << (ctz64(~(addr | (VTD_PAGE_MASK_4K - 1))) + 1);
+        addr &= ~(sz - 1);
+    } else {
+        sz = VTD_PAGE_SIZE;
+    }
 
+    entry.target_as = &vtd_dev_as->as;
+    entry.addr_mask = sz - 1;
+    entry.iova = addr;
+    memory_region_notify_iommu(entry.target_as->root, entry);
+
+done:
     return true;
 }
 
@@ -1489,6 +1548,14 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
         }
         break;
 
+    case VTD_INV_DESC_DEVICE:
+        VTD_DPRINTF(INV, "Device IOTLB Invalidation Descriptor hi 0x%"PRIx64
+                    " lo 0x%"PRIx64, inv_desc.hi, inv_desc.lo);
+        if (!vtd_process_device_iotlb_desc(s, &inv_desc)) {
+            return false;
+        }
+        break;
+
     default:
         VTD_DPRINTF(GENERAL, "error: unkonw Invalidation Descriptor type "
                     "hi 0x%"PRIx64 " lo 0x%"PRIx64 " type %"PRIu8,
@@ -2394,6 +2461,10 @@ static void vtd_init(IntelIOMMUState *s)
         assert(s->intr_eim != ON_OFF_AUTO_AUTO);
     }
 
+    if (x86_iommu->dt_supported) {
+        s->ecap |= VTD_ECAP_DT;
+    }
+
     vtd_reset_context_cache(s);
     vtd_reset_iotlb(s);
 
diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 0829a50..b195d8a 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -183,6 +183,7 @@
 /* (offset >> 4) << 8 */
 #define VTD_ECAP_IRO                (DMAR_IOTLB_REG_OFFSET << 4)
 #define VTD_ECAP_QI                 (1ULL << 1)
+#define VTD_ECAP_DT                 (1ULL << 2)
 /* Interrupt Remapping support */
 #define VTD_ECAP_IR                 (1ULL << 3)
 #define VTD_ECAP_EIM                (1ULL << 4)
@@ -326,6 +327,7 @@ typedef union VTDInvDesc VTDInvDesc;
 #define VTD_INV_DESC_TYPE               0xf
 #define VTD_INV_DESC_CC                 0x1 /* Context-cache Invalidate Desc */
 #define VTD_INV_DESC_IOTLB              0x2
+#define VTD_INV_DESC_DEVICE             0x3
 #define VTD_INV_DESC_IEC                0x4 /* Interrupt Entry Cache
                                                Invalidate Descriptor */
 #define VTD_INV_DESC_WAIT               0x5 /* Invalidation Wait Descriptor */
@@ -361,6 +363,13 @@ typedef union VTDInvDesc VTDInvDesc;
 #define VTD_INV_DESC_IOTLB_RSVD_LO      0xffffffff0000ff00ULL
 #define VTD_INV_DESC_IOTLB_RSVD_HI      0xf80ULL
 
+/* Mask for Device IOTLB Invalidate Descriptor */
+#define VTD_INV_DESC_DEVICE_IOTLB_ADDR(val) ((val) & 0xfffffffffffff000ULL)
+#define VTD_INV_DESC_DEVICE_IOTLB_SIZE(val) ((val) & 0x1)
+#define VTD_INV_DESC_DEVICE_IOTLB_SID(val) (((val) >> 32) & 0xFFFFULL)
+#define VTD_INV_DESC_DEVICE_IOTLB_RSVD_HI 0xffeULL
+#define VTD_INV_DESC_DEVICE_IOTLB_RSVD_LO 0xffff0000ffe0fff8
+
 /* Information about page-selective IOTLB invalidate */
 struct VTDIOTLBPageInvInfo {
     uint16_t domain_id;
@@ -399,8 +408,8 @@ typedef struct VTDRootEntry VTDRootEntry;
 #define VTD_CONTEXT_ENTRY_FPD       (1ULL << 1) /* Fault Processing Disable */
 #define VTD_CONTEXT_ENTRY_TT        (3ULL << 2) /* Translation Type */
 #define VTD_CONTEXT_TT_MULTI_LEVEL  0
-#define VTD_CONTEXT_TT_DEV_IOTLB    1
-#define VTD_CONTEXT_TT_PASS_THROUGH 2
+#define VTD_CONTEXT_TT_DEV_IOTLB    (1ULL << 2)
+#define VTD_CONTEXT_TT_PASS_THROUGH (2ULL << 2)
 /* Second Level Page Translation Pointer*/
 #define VTD_CONTEXT_ENTRY_SLPTPTR   (~0xfffULL)
 #define VTD_CONTEXT_ENTRY_RSVD_LO   (0xff0ULL | ~VTD_HAW_MASK)
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 2278af7..23dcd3f 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -106,6 +106,18 @@ static void x86_iommu_intremap_prop_set(Object *o, bool value, Error **errp)
     s->intr_supported = value;
 }
 
+static bool x86_iommu_device_iotlb_prop_get(Object *o, Error **errp)
+{
+    X86IOMMUState *s = X86_IOMMU_DEVICE(o);
+    return s->dt_supported;
+}
+
+static void x86_iommu_device_iotlb_prop_set(Object *o, bool value, Error **errp)
+{
+    X86IOMMUState *s = X86_IOMMU_DEVICE(o);
+    s->dt_supported = value;
+}
+
 static void x86_iommu_instance_init(Object *o)
 {
     X86IOMMUState *s = X86_IOMMU_DEVICE(o);
@@ -114,6 +126,11 @@ static void x86_iommu_instance_init(Object *o)
     s->intr_supported = false;
     object_property_add_bool(o, "intremap", x86_iommu_intremap_prop_get,
                              x86_iommu_intremap_prop_set, NULL);
+    s->dt_supported = false;
+    object_property_add_bool(o, "device-iotlb",
+                             x86_iommu_device_iotlb_prop_get,
+                             x86_iommu_device_iotlb_prop_set,
+                             NULL);
 }
 
 static const TypeInfo x86_iommu_info = {
diff --git a/include/hw/i386/x86-iommu.h b/include/hw/i386/x86-iommu.h
index 0c89d98..361c07c 100644
--- a/include/hw/i386/x86-iommu.h
+++ b/include/hw/i386/x86-iommu.h
@@ -73,6 +73,7 @@ typedef struct IEC_Notifier IEC_Notifier;
 struct X86IOMMUState {
     SysBusDevice busdev;
     bool intr_supported;        /* Whether vIOMMU supports IR */
+    bool dt_supported;          /* Whether vIOMMU supports DT */
     IommuType type;             /* IOMMU type - AMD/Intel     */
     QLIST_HEAD(, IEC_Notifier) iec_notifiers; /* IEC notify list */
 };
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH V3 06/10] virtio-pci: address space translation service (ATS) support
  2016-11-07  7:09 [Qemu-devel] [PATCH V3 00/10] vhost device IOTLB support Jason Wang
                   ` (4 preceding siblings ...)
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 05/10] intel_iommu: support device iotlb descriptor Jason Wang
@ 2016-11-07  7:09 ` Jason Wang
  2016-11-08 12:25   ` Greg Kurz
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 07/10] acpi: add ATSR for q35 Jason Wang
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Jason Wang @ 2016-11-07  7:09 UTC (permalink / raw)
  To: qemu-devel, mst
  Cc: pbonzini, peterx, cornelia.huck, wexu, vkaplans, Jason Wang

This patches enable the Address Translation Service support for virtio
pci devices. This is needed for a guest visible Device IOTLB
implementation and will be required by vhost device IOTLB API
implementation for intel IOMMU.

Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/pci/pcie.c                             | 16 ++++++++++++++++
 hw/virtio/virtio-pci.c                    |  7 +++++++
 hw/virtio/virtio-pci.h                    |  4 ++++
 include/hw/pci/pcie.h                     |  4 ++++
 include/standard-headers/linux/pci_regs.h |  1 +
 5 files changed, 32 insertions(+)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 99cfb45..02195d9 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -717,3 +717,19 @@ void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num)
                         PCI_EXT_CAP_DSN_SIZEOF);
     pci_set_quad(dev->config + offset + pci_dsn_cap, ser_num);
 }
+
+void pcie_ats_init(PCIDevice *dev, uint16_t offset)
+{
+    pcie_add_capability(dev, PCI_EXT_CAP_ID_ATS, 0x1,
+                        offset, PCI_EXT_CAP_ATS_SIZEOF);
+
+    dev->exp.ats_cap = offset;
+
+    /* Invalidate Queue Depth 0, Page Aligned Request 0 */
+    pci_set_word(dev->config + offset + PCI_ATS_CAP, 0);
+    /* STU 0, Disabled by default */
+    pci_set_word(dev->config + offset + PCI_ATS_CTRL, 0);
+
+    pci_set_word(dev->wmask + dev->exp.ats_cap + PCI_ATS_CTRL, 0x800f);
+}
+
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 6ceb43e..e357bdf 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1838,6 +1838,11 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
          * PCI Power Management Interface Specification.
          */
         pci_set_word(pci_dev->config + pos + PCI_PM_PMC, 0x3);
+
+        if (proxy->flags & VIRTIO_PCI_FLAG_ATS) {
+            pcie_ats_init(pci_dev, 256);
+        }
+
     } else {
         /*
          * make future invocations of pci_is_express() return false
@@ -1889,6 +1894,8 @@ static Property virtio_pci_properties[] = {
                     VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, false),
     DEFINE_PROP_BIT("page-per-vq", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT, false),
+    DEFINE_PROP_BIT("ats", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_ATS_BIT, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index b4edea6..057d49d 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -69,6 +69,7 @@ enum {
     VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT,
     VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT,
     VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT,
+    VIRTIO_PCI_FLAG_ATS_BIT,
 };
 
 /* Need to activate work-arounds for buggy guests at vmstate load. */
@@ -93,6 +94,9 @@ enum {
 #define VIRTIO_PCI_FLAG_PAGE_PER_VQ \
     (1 << VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT)
 
+/* address space translation service */
+#define VIRTIO_PCI_FLAG_ATS (1 << VIRTIO_PCI_FLAG_ATS_BIT)
+
 typedef struct {
     MSIMessage msg;
     int virq;
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index 056d25e..b08451d 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -74,6 +74,9 @@ struct PCIExpressDevice {
     /* AER */
     uint16_t aer_cap;
     PCIEAERLog aer_log;
+
+    /* Offset of ATS capability in config space */
+    uint16_t ats_cap;
 };
 
 #define COMPAT_PROP_PCP "power_controller_present"
@@ -120,6 +123,7 @@ void pcie_add_capability(PCIDevice *dev,
 
 void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
 void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);
+void pcie_ats_init(PCIDevice *dev, uint16_t offset);
 
 extern const VMStateDescription vmstate_pcie_device;
 
diff --git a/include/standard-headers/linux/pci_regs.h b/include/standard-headers/linux/pci_regs.h
index 4040951..ac426a0 100644
--- a/include/standard-headers/linux/pci_regs.h
+++ b/include/standard-headers/linux/pci_regs.h
@@ -674,6 +674,7 @@
 #define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_DPC
 
 #define PCI_EXT_CAP_DSN_SIZEOF	12
+#define PCI_EXT_CAP_ATS_SIZEOF	8
 #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
 
 /* Advanced Error Reporting */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH V3 07/10] acpi: add ATSR for q35
  2016-11-07  7:09 [Qemu-devel] [PATCH V3 00/10] vhost device IOTLB support Jason Wang
                   ` (5 preceding siblings ...)
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 06/10] virtio-pci: address space translation service (ATS) support Jason Wang
@ 2016-11-07  7:09 ` Jason Wang
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 08/10] memory: handle alias for iommu notifier Jason Wang
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Jason Wang @ 2016-11-07  7:09 UTC (permalink / raw)
  To: qemu-devel, mst
  Cc: pbonzini, peterx, cornelia.huck, wexu, vkaplans, Jason Wang

This patch provides ATSR which was a requirement for software that
wants to enable ATS on endpoint devices behind a Root Port. This is
done simply by setting ALL_PORTS which indicates all PCI-Express Root
Ports support ATS transactions.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/i386/acpi-build.c        |  9 +++++++++
 include/hw/acpi/acpi-defs.h | 12 ++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5cd1da9..2d10b0f 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2568,6 +2568,7 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
 
     AcpiTableDmar *dmar;
     AcpiDmarHardwareUnit *drhd;
+    AcpiDmarRootPortATS *atsr;
     uint8_t dmar_flags = 0;
     X86IOMMUState *iommu = x86_iommu_get_default();
     AcpiDmarDeviceScope *scope = NULL;
@@ -2600,6 +2601,14 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
     scope->bus = Q35_PSEUDO_BUS_PLATFORM;
     scope->path[0] = cpu_to_le16(Q35_PSEUDO_DEVFN_IOAPIC);
 
+    if (iommu->dt_supported) {
+        atsr = acpi_data_push(table_data, sizeof(*atsr));
+        atsr->type = cpu_to_le16(ACPI_DMAR_TYPE_ATSR);
+        atsr->length = cpu_to_le16(sizeof(*atsr));
+        atsr->flags = ACPI_DMAR_ATSR_ALL_PORTS;
+        atsr->pci_segment = cpu_to_le16(0);
+    }
+
     build_header(linker, table_data, (void *)(table_data->data + dmar_start),
                  "DMAR", table_data->len - dmar_start, 1, NULL, NULL);
 }
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index d1d1d61..420ea36 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -635,8 +635,20 @@ struct AcpiDmarHardwareUnit {
 } QEMU_PACKED;
 typedef struct AcpiDmarHardwareUnit AcpiDmarHardwareUnit;
 
+/* Type 2: Root Port ATS Capability Reporting Structure */
+struct AcpiDmarRootPortATS {
+    uint16_t type;
+    uint16_t length;
+    uint8_t flags;
+    uint8_t reserved;
+    uint16_t pci_segment;
+    AcpiDmarDeviceScope scope[0];
+} QEMU_PACKED;
+typedef struct AcpiDmarRootPortATS AcpiDmarRootPortATS;
+
 /* Masks for Flags field above */
 #define ACPI_DMAR_INCLUDE_PCI_ALL   1
+#define ACPI_DMAR_ATSR_ALL_PORTS    1
 
 /*
  * Input Output Remapping Table (IORT)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH V3 08/10] memory: handle alias for iommu notifier
  2016-11-07  7:09 [Qemu-devel] [PATCH V3 00/10] vhost device IOTLB support Jason Wang
                   ` (6 preceding siblings ...)
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 07/10] acpi: add ATSR for q35 Jason Wang
@ 2016-11-07  7:09 ` Jason Wang
  2016-11-07 23:41   ` Peter Xu
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 09/10] memory: handle alias in memory_region_is_iommu() Jason Wang
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 10/10] vhost_net: device IOTLB support Jason Wang
  9 siblings, 1 reply; 23+ messages in thread
From: Jason Wang @ 2016-11-07  7:09 UTC (permalink / raw)
  To: qemu-devel, mst
  Cc: pbonzini, peterx, cornelia.huck, wexu, vkaplans, Jason Wang

Cc: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 memory.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/memory.c b/memory.c
index 33110e9..2bfc37f 100644
--- a/memory.c
+++ b/memory.c
@@ -1603,6 +1603,11 @@ static void memory_region_update_iommu_notify_flags(MemoryRegion *mr)
 void memory_region_register_iommu_notifier(MemoryRegion *mr,
                                            IOMMUNotifier *n)
 {
+    if (mr->alias) {
+        memory_region_register_iommu_notifier(mr->alias, n);
+        return;
+    }
+
     /* We need to register for at least one bitfield */
     assert(n->notifier_flags != IOMMU_NOTIFIER_NONE);
     QLIST_INSERT_HEAD(&mr->iommu_notify, n, node);
@@ -1643,6 +1648,10 @@ void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n,
 void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
                                              IOMMUNotifier *n)
 {
+    if (mr->alias) {
+        memory_region_unregister_iommu_notifier(mr->alias, n);
+        return;
+    }
     QLIST_REMOVE(n, node);
     memory_region_update_iommu_notify_flags(mr);
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH V3 09/10] memory: handle alias in memory_region_is_iommu()
  2016-11-07  7:09 [Qemu-devel] [PATCH V3 00/10] vhost device IOTLB support Jason Wang
                   ` (7 preceding siblings ...)
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 08/10] memory: handle alias for iommu notifier Jason Wang
@ 2016-11-07  7:09 ` Jason Wang
  2016-11-07 23:42   ` Peter Xu
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 10/10] vhost_net: device IOTLB support Jason Wang
  9 siblings, 1 reply; 23+ messages in thread
From: Jason Wang @ 2016-11-07  7:09 UTC (permalink / raw)
  To: qemu-devel, mst
  Cc: pbonzini, peterx, cornelia.huck, wexu, vkaplans, Jason Wang

Cc: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 include/exec/memory.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index e605de3..ab37499 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -628,6 +628,9 @@ static inline bool memory_region_is_romd(MemoryRegion *mr)
  */
 static inline bool memory_region_is_iommu(MemoryRegion *mr)
 {
+    if (mr->alias) {
+        return memory_region_is_iommu(mr->alias);
+    }
     return mr->iommu_ops;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH V3 10/10] vhost_net: device IOTLB support
  2016-11-07  7:09 [Qemu-devel] [PATCH V3 00/10] vhost device IOTLB support Jason Wang
                   ` (8 preceding siblings ...)
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 09/10] memory: handle alias in memory_region_is_iommu() Jason Wang
@ 2016-11-07  7:09 ` Jason Wang
  9 siblings, 0 replies; 23+ messages in thread
From: Jason Wang @ 2016-11-07  7:09 UTC (permalink / raw)
  To: qemu-devel, mst
  Cc: pbonzini, peterx, cornelia.huck, wexu, vkaplans, Jason Wang

This patches implements Device IOTLB support for vhost kernel. This is
done through:

1) switch to use dma helpers when map/unmap vrings from vhost codes
2) introduce a set of VhostOps to:
   - setting up device IOTLB request callback
   - processing device IOTLB request
   - processing device IOTLB invalidation
2) kernel support for Device IOTLB API:

- allow vhost-net to query the IOMMU IOTLB entry through eventfd
- enable the ability for qemu to update a specified mapping of vhost
- through ioctl.
- enable the ability to invalidate a specified range of iova for the
  device IOTLB of vhost through ioctl. In x86/intel_iommu case this is
  triggered through iommu memory region notifier from device IOTLB
  invalidation descriptor processing routine.

With all the above, kernel vhost_net can co-operate with userspace
IOMMU. For vhost-user, the support could be easily done on top by
implementing the VhostOps.

Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/vhost-backend.c         |  99 +++++++++++++++++++++
 hw/virtio/vhost.c                 | 180 +++++++++++++++++++++++++++++++++-----
 include/hw/virtio/vhost-backend.h |  13 +++
 include/hw/virtio/vhost.h         |   4 +
 net/tap.c                         |   1 +
 5 files changed, 274 insertions(+), 23 deletions(-)

diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 272a5ec..be927b8 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -185,6 +185,102 @@ static int vhost_kernel_vsock_set_running(struct vhost_dev *dev, int start)
 }
 #endif /* CONFIG_VHOST_VSOCK */
 
+static void vhost_kernel_iotlb_read(void *opaque)
+{
+    struct vhost_dev *dev = opaque;
+    struct vhost_msg msg;
+    ssize_t len;
+
+    while ((len = read((uintptr_t)dev->opaque, &msg, sizeof msg)) > 0) {
+        struct vhost_iotlb_msg *imsg = &msg.iotlb;
+        if (len < sizeof msg) {
+            error_report("Wrong vhost message len: %d", (int)len);
+            break;
+        }
+        if (msg.type != VHOST_IOTLB_MSG) {
+            error_report("Unknown vhost iotlb message type");
+            break;
+        }
+        switch (imsg->type) {
+        case VHOST_IOTLB_MISS:
+            vhost_device_iotlb_miss(dev, imsg->iova,
+                                    imsg->perm != VHOST_ACCESS_RO);
+            break;
+        case VHOST_IOTLB_UPDATE:
+        case VHOST_IOTLB_INVALIDATE:
+            error_report("Unexpected IOTLB message type");
+            break;
+        case VHOST_IOTLB_ACCESS_FAIL:
+            /* FIXME: report device iotlb error */
+            break;
+        default:
+            break;
+        }
+    }
+}
+
+static int vhost_kernel_update_device_iotlb(struct vhost_dev *dev,
+                                            uint64_t iova, uint64_t uaddr,
+                                            uint64_t len,
+                                            IOMMUAccessFlags perm)
+{
+    struct vhost_msg msg;
+    msg.type = VHOST_IOTLB_MSG;
+    msg.iotlb.iova =  iova;
+    msg.iotlb.uaddr = uaddr;
+    msg.iotlb.size = len;
+    msg.iotlb.type = VHOST_IOTLB_UPDATE;
+
+    switch (perm) {
+    case IOMMU_RO:
+        msg.iotlb.perm = VHOST_ACCESS_RO;
+        break;
+    case IOMMU_WO:
+        msg.iotlb.perm = VHOST_ACCESS_WO;
+        break;
+    case IOMMU_RW:
+        msg.iotlb.perm = VHOST_ACCESS_RW;
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    if (write((uintptr_t)dev->opaque, &msg, sizeof msg) != sizeof msg) {
+        error_report("Fail to update device iotlb");
+        return -EFAULT;
+    }
+
+    return 0;
+}
+
+static int vhost_kernel_invalidate_device_iotlb(struct vhost_dev *dev,
+                                                uint64_t iova, uint64_t len)
+{
+    struct vhost_msg msg;
+
+    msg.type = VHOST_IOTLB_MSG;
+    msg.iotlb.iova = iova;
+    msg.iotlb.size = len;
+    msg.iotlb.type = VHOST_IOTLB_INVALIDATE;
+
+    if (write((uintptr_t)dev->opaque, &msg, sizeof msg) != sizeof msg) {
+        error_report("Fail to invalidate device iotlb");
+        return -EFAULT;
+    }
+
+    return 0;
+}
+
+static void vhost_kernel_set_iotlb_callback(struct vhost_dev *dev,
+                                           int enabled)
+{
+    if (enabled)
+        qemu_set_fd_handler((uintptr_t)dev->opaque,
+                            vhost_kernel_iotlb_read, NULL, dev);
+    else
+        qemu_set_fd_handler((uintptr_t)dev->opaque, NULL, NULL, NULL);
+}
+
 static const VhostOps kernel_ops = {
         .backend_type = VHOST_BACKEND_TYPE_KERNEL,
         .vhost_backend_init = vhost_kernel_init,
@@ -214,6 +310,9 @@ static const VhostOps kernel_ops = {
         .vhost_vsock_set_guest_cid = vhost_kernel_vsock_set_guest_cid,
         .vhost_vsock_set_running = vhost_kernel_vsock_set_running,
 #endif /* CONFIG_VHOST_VSOCK */
+        .vhost_set_iotlb_callback = vhost_kernel_set_iotlb_callback,
+        .vhost_update_device_iotlb = vhost_kernel_update_device_iotlb,
+        .vhost_invalidate_device_iotlb = vhost_kernel_invalidate_device_iotlb,
 };
 
 int vhost_set_backend_type(struct vhost_dev *dev, VhostBackendType backend_type)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index bd051ab..6e186f7 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -26,6 +26,7 @@
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-access.h"
 #include "migration/migration.h"
+#include "sysemu/dma.h"
 
 /* enabled until disconnected backend stabilizes */
 #define _VHOST_DEBUG 1
@@ -421,6 +422,33 @@ static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
     dev->log_size = size;
 }
 
+static void *vhost_memory_map(VirtIODevice *vdev, hwaddr addr,
+                              hwaddr *plen, int is_write)
+{
+    AddressSpace *dma_as = vdev->dma_as;
+
+    if (!memory_region_is_iommu(dma_as->root)) {
+        return dma_memory_map(dma_as, addr, plen, is_write ?
+                              DMA_DIRECTION_FROM_DEVICE :
+                              DMA_DIRECTION_TO_DEVICE);
+    } else {
+        return (void *)(uintptr_t)addr;
+    }
+}
+
+static void vhost_memory_unmap(VirtIODevice *vdev, void *buffer,
+                               hwaddr len, int is_write,
+                               hwaddr access_len)
+{
+    AddressSpace *dma_as = vdev->dma_as;
+
+    if (!memory_region_is_iommu(dma_as->root)) {
+        dma_memory_unmap(dma_as, buffer, len, is_write ?
+                         DMA_DIRECTION_FROM_DEVICE : DMA_DIRECTION_TO_DEVICE,
+                         access_len);
+    }
+}
+
 static int vhost_verify_ring_mappings(struct vhost_dev *dev,
                                       uint64_t start_addr,
                                       uint64_t size)
@@ -437,7 +465,7 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
             continue;
         }
         l = vq->ring_size;
-        p = cpu_physical_memory_map(vq->ring_phys, &l, 1);
+        p = vhost_memory_map(dev->vdev, vq->ring_phys, &l, 1);
         if (!p || l != vq->ring_size) {
             error_report("Unable to map ring buffer for ring %d", i);
             r = -ENOMEM;
@@ -446,7 +474,7 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
             error_report("Ring buffer relocated for ring %d", i);
             r = -EBUSY;
         }
-        cpu_physical_memory_unmap(p, l, 0, 0);
+        vhost_memory_unmap(dev->vdev, p, l, 0, 0);
     }
     return r;
 }
@@ -674,13 +702,27 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
     return 0;
 }
 
-static int vhost_dev_set_features(struct vhost_dev *dev, bool enable_log)
+static int vhost_dev_has_iommu(struct vhost_dev *dev)
+{
+    VirtIODevice *vdev = dev->vdev;
+    AddressSpace *dma_as = vdev->dma_as;
+
+    return memory_region_is_iommu(dma_as->root) &&
+           virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
+}
+
+static int vhost_dev_set_features(struct vhost_dev *dev,
+                                  bool enable_log)
 {
     uint64_t features = dev->acked_features;
+    bool has_iommu = vhost_dev_has_iommu(dev);
     int r;
     if (enable_log) {
         features |= 0x1ULL << VHOST_F_LOG_ALL;
     }
+    if (has_iommu) {
+        features |= 0x1ULL << VIRTIO_F_IOMMU_PLATFORM;
+    }
     r = dev->vhost_ops->vhost_set_features(dev, features);
     if (r < 0) {
         VHOST_OPS_DEBUG("vhost_set_features failed");
@@ -817,6 +859,56 @@ static int vhost_virtqueue_set_vring_endian_legacy(struct vhost_dev *dev,
     return -errno;
 }
 
+static int vhost_memory_region_lookup(struct vhost_dev *hdev,
+                                      uint64_t gpa, uint64_t *uaddr,
+                                      uint64_t *len)
+{
+    int i;
+
+    for (i = 0; i < hdev->mem->nregions; i++) {
+        struct vhost_memory_region *reg = hdev->mem->regions + i;
+
+        if (gpa >= reg->guest_phys_addr &&
+            reg->guest_phys_addr + reg->memory_size > gpa) {
+            *uaddr = reg->userspace_addr + gpa - reg->guest_phys_addr;
+            *len = reg->guest_phys_addr + reg->memory_size - gpa;
+            return 0;
+        }
+    }
+
+    return -EFAULT;
+}
+
+void vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write)
+{
+    IOMMUTLBEntry iotlb;
+    uint64_t uaddr, len;
+
+    rcu_read_lock();
+
+    iotlb = address_space_get_iotlb_entry(dev->vdev->dma_as,
+                                          iova, write);
+    if (iotlb.target_as != NULL) {
+        if (vhost_memory_region_lookup(dev, iotlb.translated_addr,
+                                       &uaddr, &len)) {
+            error_report("Fail to lookup the translated address "
+                         "%"PRIx64, iotlb.translated_addr);
+            goto out;
+        }
+
+        len = MIN(iotlb.addr_mask + 1, len);
+        iova = iova & ~iotlb.addr_mask;
+
+        if (dev->vhost_ops->vhost_update_device_iotlb(dev, iova, uaddr,
+                                                      len, iotlb.perm)) {
+            error_report("Fail to update device iotlb");
+            goto out;
+        }
+    }
+out:
+    rcu_read_unlock();
+}
+
 static int vhost_virtqueue_start(struct vhost_dev *dev,
                                 struct VirtIODevice *vdev,
                                 struct vhost_virtqueue *vq,
@@ -862,21 +954,21 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
 
     s = l = virtio_queue_get_desc_size(vdev, idx);
     a = virtio_queue_get_desc_addr(vdev, idx);
-    vq->desc = cpu_physical_memory_map(a, &l, 0);
+    vq->desc = vhost_memory_map(vdev, a, &l, 0);
     if (!vq->desc || l != s) {
         r = -ENOMEM;
         goto fail_alloc_desc;
     }
     s = l = virtio_queue_get_avail_size(vdev, idx);
     a = virtio_queue_get_avail_addr(vdev, idx);
-    vq->avail = cpu_physical_memory_map(a, &l, 0);
+    vq->avail = vhost_memory_map(vdev, a, &l, 0);
     if (!vq->avail || l != s) {
         r = -ENOMEM;
         goto fail_alloc_avail;
     }
     vq->used_size = s = l = virtio_queue_get_used_size(vdev, idx);
     vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
-    vq->used = cpu_physical_memory_map(a, &l, 1);
+    vq->used = vhost_memory_map(vdev, a, &l, 1);
     if (!vq->used || l != s) {
         r = -ENOMEM;
         goto fail_alloc_used;
@@ -884,7 +976,7 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
 
     vq->ring_size = s = l = virtio_queue_get_ring_size(vdev, idx);
     vq->ring_phys = a = virtio_queue_get_ring_addr(vdev, idx);
-    vq->ring = cpu_physical_memory_map(a, &l, 1);
+    vq->ring = vhost_memory_map(vdev, a, &l, 1);
     if (!vq->ring || l != s) {
         r = -ENOMEM;
         goto fail_alloc_ring;
@@ -930,17 +1022,17 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
 fail_vector:
 fail_kick:
 fail_alloc:
-    cpu_physical_memory_unmap(vq->ring, virtio_queue_get_ring_size(vdev, idx),
-                              0, 0);
+    vhost_memory_unmap(vdev, vq->ring, virtio_queue_get_ring_size(vdev, idx),
+                       0, 0);
 fail_alloc_ring:
-    cpu_physical_memory_unmap(vq->used, virtio_queue_get_used_size(vdev, idx),
-                              0, 0);
+    vhost_memory_unmap(vdev, vq->used, virtio_queue_get_used_size(vdev, idx),
+                       0, 0);
 fail_alloc_used:
-    cpu_physical_memory_unmap(vq->avail, virtio_queue_get_avail_size(vdev, idx),
-                              0, 0);
+    vhost_memory_unmap(vdev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
+                       0, 0);
 fail_alloc_avail:
-    cpu_physical_memory_unmap(vq->desc, virtio_queue_get_desc_size(vdev, idx),
-                              0, 0);
+    vhost_memory_unmap(vdev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
+                       0, 0);
 fail_alloc_desc:
     return r;
 }
@@ -973,14 +1065,14 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
                                                 vhost_vq_index);
     }
 
-    cpu_physical_memory_unmap(vq->ring, virtio_queue_get_ring_size(vdev, idx),
-                              0, virtio_queue_get_ring_size(vdev, idx));
-    cpu_physical_memory_unmap(vq->used, virtio_queue_get_used_size(vdev, idx),
-                              1, virtio_queue_get_used_size(vdev, idx));
-    cpu_physical_memory_unmap(vq->avail, virtio_queue_get_avail_size(vdev, idx),
-                              0, virtio_queue_get_avail_size(vdev, idx));
-    cpu_physical_memory_unmap(vq->desc, virtio_queue_get_desc_size(vdev, idx),
-                              0, virtio_queue_get_desc_size(vdev, idx));
+    vhost_memory_unmap(vdev, vq->ring, virtio_queue_get_ring_size(vdev, idx),
+                       0, virtio_queue_get_ring_size(vdev, idx));
+    vhost_memory_unmap(vdev, vq->used, virtio_queue_get_used_size(vdev, idx),
+                       1, virtio_queue_get_used_size(vdev, idx));
+    vhost_memory_unmap(vdev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
+                       0, virtio_queue_get_avail_size(vdev, idx));
+    vhost_memory_unmap(vdev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
+                       0, virtio_queue_get_desc_size(vdev, idx));
 }
 
 static void vhost_eventfd_add(MemoryListener *listener,
@@ -1037,6 +1129,9 @@ static int vhost_virtqueue_init(struct vhost_dev *dev,
         r = -errno;
         goto fail_call;
     }
+
+    vq->dev = dev;
+
     return 0;
 fail_call:
     event_notifier_cleanup(&vq->masked_notifier);
@@ -1048,12 +1143,24 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
     event_notifier_cleanup(&vq->masked_notifier);
 }
 
+static void vhost_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
+{
+    struct vhost_dev *hdev = container_of(n, struct vhost_dev, n);
+
+    if (hdev->vhost_ops->vhost_invalidate_device_iotlb(hdev,
+                                                       iotlb->iova,
+                                                       iotlb->addr_mask + 1)) {
+        error_report("Fail to invalidate device iotlb");
+    }
+}
+
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
                    VhostBackendType backend_type, uint32_t busyloop_timeout)
 {
     uint64_t features;
     int i, r, n_initialized_vqs = 0;
 
+    hdev->vdev = NULL;
     hdev->migration_blocker = NULL;
 
     r = vhost_set_backend_type(hdev, backend_type);
@@ -1118,6 +1225,9 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
         .priority = 10
     };
 
+    hdev->n.notify = vhost_iommu_unmap_notify;
+    hdev->n.notifier_flags = IOMMU_NOTIFIER_UNMAP;
+
     if (hdev->migration_blocker == NULL) {
         if (!(hdev->features & (0x1ULL << VHOST_F_LOG_ALL))) {
             error_setg(&hdev->migration_blocker,
@@ -1310,11 +1420,18 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
     assert(hdev->vhost_ops);
 
     hdev->started = true;
+    hdev->vdev = vdev;
 
     r = vhost_dev_set_features(hdev, hdev->log_enabled);
     if (r < 0) {
         goto fail_features;
     }
+
+    if (vhost_dev_has_iommu(hdev)) {
+        memory_region_register_iommu_notifier(vdev->dma_as->root,
+                                              &hdev->n);
+    }
+
     r = hdev->vhost_ops->vhost_set_mem_table(hdev, hdev->mem);
     if (r < 0) {
         VHOST_OPS_DEBUG("vhost_set_mem_table failed");
@@ -1348,6 +1465,16 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
         }
     }
 
+    hdev->vhost_ops->vhost_set_iotlb_callback(hdev, true);
+
+    if (vhost_dev_has_iommu(hdev)) {
+        /* Update used ring information for IOTLB to work correctly,
+         * vhost-kernel code requires for this.*/
+        for (i = 0; i < hdev->nvqs; ++i) {
+            struct vhost_virtqueue *vq = hdev->vqs + i;
+            vhost_device_iotlb_miss(hdev, vq->used_phys, true);
+        }
+    }
     return 0;
 fail_log:
     vhost_log_put(hdev, false);
@@ -1359,6 +1486,7 @@ fail_vq:
                              hdev->vq_index + i);
     }
     i = hdev->nvqs;
+
 fail_mem:
 fail_features:
 
@@ -1373,6 +1501,7 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
 
     /* should only be called after backend is connected */
     assert(hdev->vhost_ops);
+    hdev->vhost_ops->vhost_set_iotlb_callback(hdev, false);
 
     for (i = 0; i < hdev->nvqs; ++i) {
         vhost_virtqueue_stop(hdev,
@@ -1381,8 +1510,13 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
                              hdev->vq_index + i);
     }
 
+    if (vhost_dev_has_iommu(hdev)) {
+        memory_region_unregister_iommu_notifier(vdev->dma_as->root,
+                                                &hdev->n);
+    }
     vhost_log_put(hdev, true);
     hdev->started = false;
+    hdev->vdev = NULL;
 }
 
 int vhost_net_set_backend(struct vhost_dev *hdev,
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index 6e90703..236eb85 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -11,6 +11,8 @@
 #ifndef VHOST_BACKEND_H
 #define VHOST_BACKEND_H
 
+#include "exec/memory.h"
+
 typedef enum VhostBackendType {
     VHOST_BACKEND_TYPE_NONE = 0,
     VHOST_BACKEND_TYPE_KERNEL = 1,
@@ -76,6 +78,14 @@ typedef bool (*vhost_backend_can_merge_op)(struct vhost_dev *dev,
 typedef int (*vhost_vsock_set_guest_cid_op)(struct vhost_dev *dev,
                                             uint64_t guest_cid);
 typedef int (*vhost_vsock_set_running_op)(struct vhost_dev *dev, int start);
+typedef void (*vhost_set_iotlb_callback_op)(struct vhost_dev *dev,
+                                           int enabled);
+typedef int (*vhost_update_device_iotlb_op)(struct vhost_dev *dev,
+                                            uint64_t iova, uint64_t uaddr,
+                                            uint64_t len,
+                                            IOMMUAccessFlags perm);
+typedef int (*vhost_invalidate_device_iotlb_op)(struct vhost_dev *dev,
+                                                uint64_t iova, uint64_t len);
 
 typedef struct VhostOps {
     VhostBackendType backend_type;
@@ -107,6 +117,9 @@ typedef struct VhostOps {
     vhost_backend_can_merge_op vhost_backend_can_merge;
     vhost_vsock_set_guest_cid_op vhost_vsock_set_guest_cid;
     vhost_vsock_set_running_op vhost_vsock_set_running;
+    vhost_set_iotlb_callback_op vhost_set_iotlb_callback;
+    vhost_update_device_iotlb_op vhost_update_device_iotlb;
+    vhost_invalidate_device_iotlb_op vhost_invalidate_device_iotlb;
 } VhostOps;
 
 extern const VhostOps user_ops;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index e433089..4e52a13 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -20,6 +20,7 @@ struct vhost_virtqueue {
     unsigned long long ring_phys;
     unsigned ring_size;
     EventNotifier masked_notifier;
+    struct vhost_dev *dev;
 };
 
 typedef unsigned long vhost_log_chunk_t;
@@ -37,6 +38,7 @@ struct vhost_log {
 
 struct vhost_memory;
 struct vhost_dev {
+    VirtIODevice *vdev;
     MemoryListener memory_listener;
     struct vhost_memory *mem;
     int n_mem_sections;
@@ -61,6 +63,7 @@ struct vhost_dev {
     void *opaque;
     struct vhost_log *log;
     QLIST_ENTRY(vhost_dev) entry;
+    IOMMUNotifier n;
 };
 
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
@@ -90,4 +93,5 @@ bool vhost_has_free_slot(void);
 int vhost_net_set_backend(struct vhost_dev *hdev,
                           struct vhost_vring_file *file);
 
+void vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write);
 #endif
diff --git a/net/tap.c b/net/tap.c
index b6896a7..86071b2 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -696,6 +696,7 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
                                  "tap: open vhost char device failed");
                 return;
             }
+            fcntl(vhostfd, F_SETFL, O_NONBLOCK);
         }
         options.opaque = (void *)(uintptr_t)vhostfd;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH V3 04/10] exec: introduce address_space_get_iotlb_entry()
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 04/10] exec: introduce address_space_get_iotlb_entry() Jason Wang
@ 2016-11-07 17:07   ` Paolo Bonzini
  0 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2016-11-07 17:07 UTC (permalink / raw)
  To: Jason Wang, qemu-devel, mst
  Cc: peterx, cornelia.huck, wexu, vkaplans, Peter Crosthwaite,
	Richard Henderson



On 07/11/2016 08:09, Jason Wang wrote:
> This patch introduces a helper to query the iotlb entry for a
> possible iova. This will be used by later device IOTLB API to enable
> the capability for a dataplane (e.g vhost) to query the IOTLB.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  exec.c                | 33 +++++++++++++++++++++++++++++++++
>  include/exec/memory.h |  6 ++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/exec.c b/exec.c
> index b1094c0..f108cf6 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -449,6 +449,39 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
>  }
>  
>  /* Called from RCU critical section */
> +IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
> +                                            bool is_write)
> +{
> +    IOMMUTLBEntry iotlb = {0};
> +    MemoryRegionSection *section;
> +    MemoryRegion *mr;
> +
> +    for (;;) {
> +        AddressSpaceDispatch *d = atomic_rcu_read(&as->dispatch);
> +        section = address_space_lookup_region(d, addr, false);
> +        addr = addr - section->offset_within_address_space
> +               + section->offset_within_region;
> +        mr = section->mr;
> +
> +        if (!mr->iommu_ops) {
> +            break;
> +        }
> +
> +        iotlb = mr->iommu_ops->translate(mr, addr, is_write);
> +        if (!(iotlb.perm & (1 << is_write))) {
> +            iotlb.target_as = NULL;
> +            break;
> +        }
> +
> +        addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
> +                | (addr & iotlb.addr_mask));
> +        as = iotlb.target_as;
> +    }
> +
> +    return iotlb;
> +}
> +
> +/* Called from RCU critical section */
>  MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
>                                        hwaddr *xlat, hwaddr *plen,
>                                        bool is_write)
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 9728a2f..e605de3 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -1404,6 +1404,12 @@ void address_space_stq_le(AddressSpace *as, hwaddr addr, uint64_t val,
>  void address_space_stq_be(AddressSpace *as, hwaddr addr, uint64_t val,
>                              MemTxAttrs attrs, MemTxResult *result);
>  
> +/* address_space_get_iotlb_entry: translate an address into an IOTLB
> + * entry. Should be called from an RCU critical section.
> + */
> +IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
> +                                            bool is_write);
> +
>  /* address_space_translate: translate an address range into an address space
>   * into a MemoryRegion and an address range into that section.  Should be
>   * called from an RCU critical section, to avoid that the last reference
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH V3 05/10] intel_iommu: support device iotlb descriptor
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 05/10] intel_iommu: support device iotlb descriptor Jason Wang
@ 2016-11-07 23:35   ` Peter Xu
  2016-11-08  6:54     ` Jason Wang
  0 siblings, 1 reply; 23+ messages in thread
From: Peter Xu @ 2016-11-07 23:35 UTC (permalink / raw)
  To: Jason Wang
  Cc: qemu-devel, mst, pbonzini, cornelia.huck, wexu, vkaplans,
	Richard Henderson, Eduardo Habkost

On Mon, Nov 07, 2016 at 03:09:50PM +0800, Jason Wang wrote:

[...]

> +static bool vtd_process_device_iotlb_desc(IntelIOMMUState *s,
> +                                          VTDInvDesc *inv_desc)
> +{
> +    VTDAddressSpace *vtd_dev_as;
> +    IOMMUTLBEntry entry;

Since "entry" is allocated on the stack...

[...]

> +    entry.target_as = &vtd_dev_as->as;
> +    entry.addr_mask = sz - 1;
> +    entry.iova = addr;
> +    memory_region_notify_iommu(entry.target_as->root, entry);

... here we need to assign entry.perm explicitly to IOMMU_NONE, right?

Also I think it'll be nice that we set all the fields even not used,
to avoid rubbish from the stack passed down to notifier handlers.

[...]

> +static bool x86_iommu_device_iotlb_prop_get(Object *o, Error **errp)
> +{
> +    X86IOMMUState *s = X86_IOMMU_DEVICE(o);
> +    return s->dt_supported;
> +}
> +
> +static void x86_iommu_device_iotlb_prop_set(Object *o, bool value, Error **errp)
> +{
> +    X86IOMMUState *s = X86_IOMMU_DEVICE(o);
> +    s->dt_supported = value;
> +}
> +
>  static void x86_iommu_instance_init(Object *o)
>  {
>      X86IOMMUState *s = X86_IOMMU_DEVICE(o);
> @@ -114,6 +126,11 @@ static void x86_iommu_instance_init(Object *o)
>      s->intr_supported = false;
>      object_property_add_bool(o, "intremap", x86_iommu_intremap_prop_get,
>                               x86_iommu_intremap_prop_set, NULL);
> +    s->dt_supported = false;
> +    object_property_add_bool(o, "device-iotlb",
> +                             x86_iommu_device_iotlb_prop_get,
> +                             x86_iommu_device_iotlb_prop_set,
> +                             NULL);

Again, a nit-pick here is to use Property for "device-iotlb":

    static Property vtd_properties[] = {
        DEFINE_PROP_UINT32("device-iotlb", X86IOMMUState, dt_supported, false),
        DEFINE_PROP_END_OF_LIST(),
    };

However not worth a repost.

Thanks,

-- peterx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH V3 08/10] memory: handle alias for iommu notifier
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 08/10] memory: handle alias for iommu notifier Jason Wang
@ 2016-11-07 23:41   ` Peter Xu
  0 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2016-11-07 23:41 UTC (permalink / raw)
  To: Jason Wang; +Cc: qemu-devel, mst, pbonzini, cornelia.huck, wexu, vkaplans

On Mon, Nov 07, 2016 at 03:09:53PM +0800, Jason Wang wrote:
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH V3 09/10] memory: handle alias in memory_region_is_iommu()
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 09/10] memory: handle alias in memory_region_is_iommu() Jason Wang
@ 2016-11-07 23:42   ` Peter Xu
  0 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2016-11-07 23:42 UTC (permalink / raw)
  To: Jason Wang; +Cc: qemu-devel, mst, pbonzini, cornelia.huck, wexu, vkaplans

On Mon, Nov 07, 2016 at 03:09:54PM +0800, Jason Wang wrote:
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH V3 05/10] intel_iommu: support device iotlb descriptor
  2016-11-07 23:35   ` Peter Xu
@ 2016-11-08  6:54     ` Jason Wang
  2016-11-08 20:31       ` Michael S. Tsirkin
  0 siblings, 1 reply; 23+ messages in thread
From: Jason Wang @ 2016-11-08  6:54 UTC (permalink / raw)
  To: Peter Xu
  Cc: Eduardo Habkost, mst, qemu-devel, vkaplans, wexu, cornelia.huck,
	pbonzini, Richard Henderson



On 2016年11月08日 07:35, Peter Xu wrote:
> On Mon, Nov 07, 2016 at 03:09:50PM +0800, Jason Wang wrote:
>
> [...]
>
>> +static bool vtd_process_device_iotlb_desc(IntelIOMMUState *s,
>> +                                          VTDInvDesc *inv_desc)
>> +{
>> +    VTDAddressSpace *vtd_dev_as;
>> +    IOMMUTLBEntry entry;
> Since "entry" is allocated on the stack...
>
> [...]
>
>> +    entry.target_as = &vtd_dev_as->as;
>> +    entry.addr_mask = sz - 1;
>> +    entry.iova = addr;
>> +    memory_region_notify_iommu(entry.target_as->root, entry);
> ... here we need to assign entry.perm explicitly to IOMMU_NONE, right?
>
> Also I think it'll be nice that we set all the fields even not used,
> to avoid rubbish from the stack passed down to notifier handlers.
>
> [...]

This is better, if no other comments on the series I will post a patch 
on top to fix this.

>
>> +static bool x86_iommu_device_iotlb_prop_get(Object *o, Error **errp)
>> +{
>> +    X86IOMMUState *s = X86_IOMMU_DEVICE(o);
>> +    return s->dt_supported;
>> +}
>> +
>> +static void x86_iommu_device_iotlb_prop_set(Object *o, bool value, Error **errp)
>> +{
>> +    X86IOMMUState *s = X86_IOMMU_DEVICE(o);
>> +    s->dt_supported = value;
>> +}
>> +
>>   static void x86_iommu_instance_init(Object *o)
>>   {
>>       X86IOMMUState *s = X86_IOMMU_DEVICE(o);
>> @@ -114,6 +126,11 @@ static void x86_iommu_instance_init(Object *o)
>>       s->intr_supported = false;
>>       object_property_add_bool(o, "intremap", x86_iommu_intremap_prop_get,
>>                                x86_iommu_intremap_prop_set, NULL);
>> +    s->dt_supported = false;
>> +    object_property_add_bool(o, "device-iotlb",
>> +                             x86_iommu_device_iotlb_prop_get,
>> +                             x86_iommu_device_iotlb_prop_set,
>> +                             NULL);
> Again, a nit-pick here is to use Property for "device-iotlb":
>
>      static Property vtd_properties[] = {
>          DEFINE_PROP_UINT32("device-iotlb", X86IOMMUState, dt_supported, false),
>          DEFINE_PROP_END_OF_LIST(),
>      };
>
> However not worth a repost.
>
> Thanks,
>
> -- peterx
>

We may want to share this with AMD IOMMU. (Looking at AMD IOMMU codes, 
its device-iotlb support is buggy).

Thanks

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH V3 06/10] virtio-pci: address space translation service (ATS) support
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 06/10] virtio-pci: address space translation service (ATS) support Jason Wang
@ 2016-11-08 12:25   ` Greg Kurz
  0 siblings, 0 replies; 23+ messages in thread
From: Greg Kurz @ 2016-11-08 12:25 UTC (permalink / raw)
  To: Jason Wang
  Cc: qemu-devel, mst, peterx, vkaplans, wexu, cornelia.huck, pbonzini

On Mon,  7 Nov 2016 15:09:51 +0800
Jason Wang <jasowang@redhat.com> wrote:

> This patches enable the Address Translation Service support for virtio
> pci devices. This is needed for a guest visible Device IOTLB
> implementation and will be required by vhost device IOTLB API
> implementation for intel IOMMU.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  hw/pci/pcie.c                             | 16 ++++++++++++++++
>  hw/virtio/virtio-pci.c                    |  7 +++++++
>  hw/virtio/virtio-pci.h                    |  4 ++++
>  include/hw/pci/pcie.h                     |  4 ++++
>  include/standard-headers/linux/pci_regs.h |  1 +
>  5 files changed, 32 insertions(+)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 99cfb45..02195d9 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -717,3 +717,19 @@ void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num)
>                          PCI_EXT_CAP_DSN_SIZEOF);
>      pci_set_quad(dev->config + offset + pci_dsn_cap, ser_num);
>  }
> +
> +void pcie_ats_init(PCIDevice *dev, uint16_t offset)
> +{
> +    pcie_add_capability(dev, PCI_EXT_CAP_ID_ATS, 0x1,
> +                        offset, PCI_EXT_CAP_ATS_SIZEOF);
> +
> +    dev->exp.ats_cap = offset;
> +
> +    /* Invalidate Queue Depth 0, Page Aligned Request 0 */
> +    pci_set_word(dev->config + offset + PCI_ATS_CAP, 0);
> +    /* STU 0, Disabled by default */
> +    pci_set_word(dev->config + offset + PCI_ATS_CTRL, 0);
> +
> +    pci_set_word(dev->wmask + dev->exp.ats_cap + PCI_ATS_CTRL, 0x800f);
> +}
> +

The extra blank line makes git am unhappy:

Applying: virtio-pci: address space translation service (ATS) support
/home/greg/Work/qemu/qemu-master/.git/rebase-apply/patch:32: new blank line at EOF.

> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 6ceb43e..e357bdf 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1838,6 +1838,11 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
>           * PCI Power Management Interface Specification.
>           */
>          pci_set_word(pci_dev->config + pos + PCI_PM_PMC, 0x3);
> +
> +        if (proxy->flags & VIRTIO_PCI_FLAG_ATS) {
> +            pcie_ats_init(pci_dev, 256);
> +        }
> +
>      } else {
>          /*
>           * make future invocations of pci_is_express() return false
> @@ -1889,6 +1894,8 @@ static Property virtio_pci_properties[] = {
>                      VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, false),
>      DEFINE_PROP_BIT("page-per-vq", VirtIOPCIProxy, flags,
>                      VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT, false),
> +    DEFINE_PROP_BIT("ats", VirtIOPCIProxy, flags,
> +                    VIRTIO_PCI_FLAG_ATS_BIT, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> index b4edea6..057d49d 100644
> --- a/hw/virtio/virtio-pci.h
> +++ b/hw/virtio/virtio-pci.h
> @@ -69,6 +69,7 @@ enum {
>      VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT,
>      VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT,
>      VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT,
> +    VIRTIO_PCI_FLAG_ATS_BIT,
>  };
>  
>  /* Need to activate work-arounds for buggy guests at vmstate load. */
> @@ -93,6 +94,9 @@ enum {
>  #define VIRTIO_PCI_FLAG_PAGE_PER_VQ \
>      (1 << VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT)
>  
> +/* address space translation service */
> +#define VIRTIO_PCI_FLAG_ATS (1 << VIRTIO_PCI_FLAG_ATS_BIT)
> +
>  typedef struct {
>      MSIMessage msg;
>      int virq;
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index 056d25e..b08451d 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -74,6 +74,9 @@ struct PCIExpressDevice {
>      /* AER */
>      uint16_t aer_cap;
>      PCIEAERLog aer_log;
> +
> +    /* Offset of ATS capability in config space */
> +    uint16_t ats_cap;
>  };
>  
>  #define COMPAT_PROP_PCP "power_controller_present"
> @@ -120,6 +123,7 @@ void pcie_add_capability(PCIDevice *dev,
>  
>  void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
>  void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);
> +void pcie_ats_init(PCIDevice *dev, uint16_t offset);
>  
>  extern const VMStateDescription vmstate_pcie_device;
>  
> diff --git a/include/standard-headers/linux/pci_regs.h b/include/standard-headers/linux/pci_regs.h
> index 4040951..ac426a0 100644
> --- a/include/standard-headers/linux/pci_regs.h
> +++ b/include/standard-headers/linux/pci_regs.h
> @@ -674,6 +674,7 @@
>  #define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_DPC
>  
>  #define PCI_EXT_CAP_DSN_SIZEOF	12
> +#define PCI_EXT_CAP_ATS_SIZEOF	8
>  #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
>  
>  /* Advanced Error Reporting */

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH V3 05/10] intel_iommu: support device iotlb descriptor
  2016-11-08  6:54     ` Jason Wang
@ 2016-11-08 20:31       ` Michael S. Tsirkin
  0 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2016-11-08 20:31 UTC (permalink / raw)
  To: Jason Wang
  Cc: Peter Xu, Eduardo Habkost, qemu-devel, vkaplans, wexu,
	cornelia.huck, pbonzini, Richard Henderson

On Tue, Nov 08, 2016 at 02:54:19PM +0800, Jason Wang wrote:
> 
> 
> On 2016年11月08日 07:35, Peter Xu wrote:
> > On Mon, Nov 07, 2016 at 03:09:50PM +0800, Jason Wang wrote:
> > 
> > [...]
> > 
> > > +static bool vtd_process_device_iotlb_desc(IntelIOMMUState *s,
> > > +                                          VTDInvDesc *inv_desc)
> > > +{
> > > +    VTDAddressSpace *vtd_dev_as;
> > > +    IOMMUTLBEntry entry;
> > Since "entry" is allocated on the stack...
> > 
> > [...]
> > 
> > > +    entry.target_as = &vtd_dev_as->as;
> > > +    entry.addr_mask = sz - 1;
> > > +    entry.iova = addr;
> > > +    memory_region_notify_iommu(entry.target_as->root, entry);
> > ... here we need to assign entry.perm explicitly to IOMMU_NONE, right?
> > 
> > Also I think it'll be nice that we set all the fields even not used,
> > to avoid rubbish from the stack passed down to notifier handlers.
> > 
> > [...]
> 
> This is better, if no other comments on the series I will post a patch on
> top to fix this.

If you do, pls remember to use the fixup! prefix.

> > 
> > > +static bool x86_iommu_device_iotlb_prop_get(Object *o, Error **errp)
> > > +{
> > > +    X86IOMMUState *s = X86_IOMMU_DEVICE(o);
> > > +    return s->dt_supported;
> > > +}
> > > +
> > > +static void x86_iommu_device_iotlb_prop_set(Object *o, bool value, Error **errp)
> > > +{
> > > +    X86IOMMUState *s = X86_IOMMU_DEVICE(o);
> > > +    s->dt_supported = value;
> > > +}
> > > +
> > >   static void x86_iommu_instance_init(Object *o)
> > >   {
> > >       X86IOMMUState *s = X86_IOMMU_DEVICE(o);
> > > @@ -114,6 +126,11 @@ static void x86_iommu_instance_init(Object *o)
> > >       s->intr_supported = false;
> > >       object_property_add_bool(o, "intremap", x86_iommu_intremap_prop_get,
> > >                                x86_iommu_intremap_prop_set, NULL);
> > > +    s->dt_supported = false;
> > > +    object_property_add_bool(o, "device-iotlb",
> > > +                             x86_iommu_device_iotlb_prop_get,
> > > +                             x86_iommu_device_iotlb_prop_set,
> > > +                             NULL);
> > Again, a nit-pick here is to use Property for "device-iotlb":
> > 
> >      static Property vtd_properties[] = {
> >          DEFINE_PROP_UINT32("device-iotlb", X86IOMMUState, dt_supported, false),
> >          DEFINE_PROP_END_OF_LIST(),
> >      };
> > 
> > However not worth a repost.
> > 
> > Thanks,
> > 
> > -- peterx
> > 
> 
> We may want to share this with AMD IOMMU. (Looking at AMD IOMMU codes, its
> device-iotlb support is buggy).
> 
> Thanks

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH V3 02/10] intel_iommu: name vtd address space with devfn
  2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 02/10] intel_iommu: name vtd address space with devfn Jason Wang
@ 2016-12-16  2:53   ` Peter Xu
  2016-12-16  3:53     ` Jason Wang
  0 siblings, 1 reply; 23+ messages in thread
From: Peter Xu @ 2016-12-16  2:53 UTC (permalink / raw)
  To: Jason Wang
  Cc: qemu-devel, mst, pbonzini, cornelia.huck, wexu, vkaplans,
	Richard Henderson, Eduardo Habkost

On Mon, Nov 07, 2016 at 03:09:47PM +0800, Jason Wang wrote:
> To avoid duplicated name and ease debugging.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Acked-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  hw/i386/intel_iommu.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 1655a65..5272c30 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2323,6 +2323,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
>      uintptr_t key = (uintptr_t)bus;
>      VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key);
>      VTDAddressSpace *vtd_dev_as;
> +    char name[128];
>  
>      if (!vtd_bus) {
>          /* No corresponding free() */
> @@ -2336,6 +2337,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
>      vtd_dev_as = vtd_bus->dev_as[devfn];
>  
>      if (!vtd_dev_as) {
> +        snprintf(name, sizeof(name), "intel_iommu_devfn_%d", devfn);

Better with bus number as well? ;)

Like: "%02x:%02x.%x" for (bus, dev, fn).

Thanks,

-- peterx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH V3 02/10] intel_iommu: name vtd address space with devfn
  2016-12-16  2:53   ` Peter Xu
@ 2016-12-16  3:53     ` Jason Wang
  2016-12-30  8:19       ` Jason Wang
  0 siblings, 1 reply; 23+ messages in thread
From: Jason Wang @ 2016-12-16  3:53 UTC (permalink / raw)
  To: Peter Xu
  Cc: qemu-devel, mst, pbonzini, cornelia.huck, wexu, vkaplans,
	Richard Henderson, Eduardo Habkost



On 2016年12月16日 10:53, Peter Xu wrote:
> On Mon, Nov 07, 2016 at 03:09:47PM +0800, Jason Wang wrote:
>> To avoid duplicated name and ease debugging.
>>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Richard Henderson <rth@twiddle.net>
>> Cc: Eduardo Habkost <ehabkost@redhat.com>
>> Acked-by: Peter Xu <peterx@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>   hw/i386/intel_iommu.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 1655a65..5272c30 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -2323,6 +2323,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
>>       uintptr_t key = (uintptr_t)bus;
>>       VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key);
>>       VTDAddressSpace *vtd_dev_as;
>> +    char name[128];
>>   
>>       if (!vtd_bus) {
>>           /* No corresponding free() */
>> @@ -2336,6 +2337,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
>>       vtd_dev_as = vtd_bus->dev_as[devfn];
>>   
>>       if (!vtd_dev_as) {
>> +        snprintf(name, sizeof(name), "intel_iommu_devfn_%d", devfn);
> Better with bus number as well? ;)
>
> Like: "%02x:%02x.%x" for (bus, dev, fn).
>
> Thanks,
>
> -- peterx

Yes, will do it in next version.

Thanks

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH V3 02/10] intel_iommu: name vtd address space with devfn
  2016-12-16  3:53     ` Jason Wang
@ 2016-12-30  8:19       ` Jason Wang
  2016-12-30  9:22         ` Peter Xu
  0 siblings, 1 reply; 23+ messages in thread
From: Jason Wang @ 2016-12-30  8:19 UTC (permalink / raw)
  To: Peter Xu
  Cc: Eduardo Habkost, mst, qemu-devel, vkaplans, wexu, cornelia.huck,
	pbonzini, Richard Henderson



On 2016年12月16日 11:53, Jason Wang wrote:
>
>
> On 2016年12月16日 10:53, Peter Xu wrote:
>> On Mon, Nov 07, 2016 at 03:09:47PM +0800, Jason Wang wrote:
>>> To avoid duplicated name and ease debugging.
>>>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Richard Henderson <rth@twiddle.net>
>>> Cc: Eduardo Habkost <ehabkost@redhat.com>
>>> Acked-by: Peter Xu <peterx@redhat.com>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>> ---
>>>   hw/i386/intel_iommu.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>> index 1655a65..5272c30 100644
>>> --- a/hw/i386/intel_iommu.c
>>> +++ b/hw/i386/intel_iommu.c
>>> @@ -2323,6 +2323,7 @@ VTDAddressSpace 
>>> *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
>>>       uintptr_t key = (uintptr_t)bus;
>>>       VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key);
>>>       VTDAddressSpace *vtd_dev_as;
>>> +    char name[128];
>>>         if (!vtd_bus) {
>>>           /* No corresponding free() */
>>> @@ -2336,6 +2337,7 @@ VTDAddressSpace 
>>> *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
>>>       vtd_dev_as = vtd_bus->dev_as[devfn];
>>>         if (!vtd_dev_as) {
>>> +        snprintf(name, sizeof(name), "intel_iommu_devfn_%d", devfn);
>> Better with bus number as well? ;)
>>
>> Like: "%02x:%02x.%x" for (bus, dev, fn).
>>
>> Thanks,
>>
>> -- peterx
>
> Yes, will do it in next version.
>
> Thanks
>

Actually, I choose not to bother since bus and fn number needs more 
codes to get.

Thanks

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH V3 02/10] intel_iommu: name vtd address space with devfn
  2016-12-30  8:19       ` Jason Wang
@ 2016-12-30  9:22         ` Peter Xu
  2016-12-30 10:06           ` Jason Wang
  0 siblings, 1 reply; 23+ messages in thread
From: Peter Xu @ 2016-12-30  9:22 UTC (permalink / raw)
  To: Jason Wang
  Cc: Eduardo Habkost, mst, qemu-devel, vkaplans, wexu, cornelia.huck,
	pbonzini, Richard Henderson

On Fri, Dec 30, 2016 at 04:19:31PM +0800, Jason Wang wrote:
> 
> 
> On 2016年12月16日 11:53, Jason Wang wrote:
> >
> >
> >On 2016年12月16日 10:53, Peter Xu wrote:
> >>On Mon, Nov 07, 2016 at 03:09:47PM +0800, Jason Wang wrote:
> >>>To avoid duplicated name and ease debugging.
> >>>
> >>>Cc: Michael S. Tsirkin <mst@redhat.com>
> >>>Cc: Paolo Bonzini <pbonzini@redhat.com>
> >>>Cc: Richard Henderson <rth@twiddle.net>
> >>>Cc: Eduardo Habkost <ehabkost@redhat.com>
> >>>Acked-by: Peter Xu <peterx@redhat.com>
> >>>Signed-off-by: Jason Wang <jasowang@redhat.com>
> >>>---
> >>>  hw/i386/intel_iommu.c | 4 +++-
> >>>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>
> >>>diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> >>>index 1655a65..5272c30 100644
> >>>--- a/hw/i386/intel_iommu.c
> >>>+++ b/hw/i386/intel_iommu.c
> >>>@@ -2323,6 +2323,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState
> >>>*s, PCIBus *bus, int devfn)
> >>>      uintptr_t key = (uintptr_t)bus;
> >>>      VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key);
> >>>      VTDAddressSpace *vtd_dev_as;
> >>>+    char name[128];
> >>>        if (!vtd_bus) {
> >>>          /* No corresponding free() */
> >>>@@ -2336,6 +2337,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState
> >>>*s, PCIBus *bus, int devfn)
> >>>      vtd_dev_as = vtd_bus->dev_as[devfn];
> >>>        if (!vtd_dev_as) {
> >>>+        snprintf(name, sizeof(name), "intel_iommu_devfn_%d", devfn);
> >>Better with bus number as well? ;)
> >>
> >>Like: "%02x:%02x.%x" for (bus, dev, fn).
> >>
> >>Thanks,
> >>
> >>-- peterx
> >
> >Yes, will do it in next version.
> >
> >Thanks
> >
> 
> Actually, I choose not to bother since bus and fn number needs more codes to
> get.

Should this work?

    snprintf(name, sizeof(name), "intel_iommu_%02x:%02x.%x",
             pci_bus_num(bus), VTD_PCI_SLOT(devfn),
             VTD_PCI_FUNC(devfn));

Anyway, I am okay as well to keep it as it is. :)

-- peterx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH V3 02/10] intel_iommu: name vtd address space with devfn
  2016-12-30  9:22         ` Peter Xu
@ 2016-12-30 10:06           ` Jason Wang
  0 siblings, 0 replies; 23+ messages in thread
From: Jason Wang @ 2016-12-30 10:06 UTC (permalink / raw)
  To: Peter Xu
  Cc: Eduardo Habkost, mst, qemu-devel, cornelia.huck, wexu, vkaplans,
	pbonzini, Richard Henderson



On 2016年12月30日 17:22, Peter Xu wrote:
> On Fri, Dec 30, 2016 at 04:19:31PM +0800, Jason Wang wrote:
>>
>> On 2016年12月16日 11:53, Jason Wang wrote:
>>>
>>> On 2016年12月16日 10:53, Peter Xu wrote:
>>>> On Mon, Nov 07, 2016 at 03:09:47PM +0800, Jason Wang wrote:
>>>>> To avoid duplicated name and ease debugging.
>>>>>
>>>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>>>> Cc: Richard Henderson <rth@twiddle.net>
>>>>> Cc: Eduardo Habkost <ehabkost@redhat.com>
>>>>> Acked-by: Peter Xu <peterx@redhat.com>
>>>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>>>> ---
>>>>>   hw/i386/intel_iommu.c | 4 +++-
>>>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>>>> index 1655a65..5272c30 100644
>>>>> --- a/hw/i386/intel_iommu.c
>>>>> +++ b/hw/i386/intel_iommu.c
>>>>> @@ -2323,6 +2323,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState
>>>>> *s, PCIBus *bus, int devfn)
>>>>>       uintptr_t key = (uintptr_t)bus;
>>>>>       VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key);
>>>>>       VTDAddressSpace *vtd_dev_as;
>>>>> +    char name[128];
>>>>>         if (!vtd_bus) {
>>>>>           /* No corresponding free() */
>>>>> @@ -2336,6 +2337,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState
>>>>> *s, PCIBus *bus, int devfn)
>>>>>       vtd_dev_as = vtd_bus->dev_as[devfn];
>>>>>         if (!vtd_dev_as) {
>>>>> +        snprintf(name, sizeof(name), "intel_iommu_devfn_%d", devfn);
>>>> Better with bus number as well? ;)
>>>>
>>>> Like: "%02x:%02x.%x" for (bus, dev, fn).
>>>>
>>>> Thanks,
>>>>
>>>> -- peterx
>>> Yes, will do it in next version.
>>>
>>> Thanks
>>>
>> Actually, I choose not to bother since bus and fn number needs more codes to
>> get.
> Should this work?
>
>      snprintf(name, sizeof(name), "intel_iommu_%02x:%02x.%x",
>               pci_bus_num(bus), VTD_PCI_SLOT(devfn),
>               VTD_PCI_FUNC(devfn));
>
> Anyway, I am okay as well to keep it as it is. :)
>
> -- peterx

This may work but I tend to keep it as is and make this on top :)

Thanks

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2016-12-30 10:06 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-07  7:09 [Qemu-devel] [PATCH V3 00/10] vhost device IOTLB support Jason Wang
2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 01/10] virtio: convert to use DMA api Jason Wang
2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 02/10] intel_iommu: name vtd address space with devfn Jason Wang
2016-12-16  2:53   ` Peter Xu
2016-12-16  3:53     ` Jason Wang
2016-12-30  8:19       ` Jason Wang
2016-12-30  9:22         ` Peter Xu
2016-12-30 10:06           ` Jason Wang
2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 03/10] intel_iommu: allocate new key when creating new address space Jason Wang
2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 04/10] exec: introduce address_space_get_iotlb_entry() Jason Wang
2016-11-07 17:07   ` Paolo Bonzini
2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 05/10] intel_iommu: support device iotlb descriptor Jason Wang
2016-11-07 23:35   ` Peter Xu
2016-11-08  6:54     ` Jason Wang
2016-11-08 20:31       ` Michael S. Tsirkin
2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 06/10] virtio-pci: address space translation service (ATS) support Jason Wang
2016-11-08 12:25   ` Greg Kurz
2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 07/10] acpi: add ATSR for q35 Jason Wang
2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 08/10] memory: handle alias for iommu notifier Jason Wang
2016-11-07 23:41   ` Peter Xu
2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 09/10] memory: handle alias in memory_region_is_iommu() Jason Wang
2016-11-07 23:42   ` Peter Xu
2016-11-07  7:09 ` [Qemu-devel] [PATCH V3 10/10] vhost_net: device IOTLB support Jason Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.