All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support
@ 2017-05-11 12:32 Maxime Coquelin
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 1/6] vhost: propagate errors in vhost_device_iotlb_miss() Maxime Coquelin
                   ` (6 more replies)
  0 siblings, 7 replies; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-11 12:32 UTC (permalink / raw)
  To: mst, peterx, marcandre.lureau, vkaplans, jasowang, wexu,
	yuanhan.liu, qemu-devel, jfreiman
  Cc: Maxime Coquelin

This series aims at specifying ans implementing the protocol update
required to support device IOTLB with user backends.

In this first non-RFC version, IOTLB messages forging and parsing have been
extracted from the backend implementation; so that both user and kernel
backends share as most code as possible, only the transport remains backend
specifics. Also various smaller fixes have been implementated, taking into
account Peter's review comments. I tagged this third version as non-RFC,
since there is now a DPDK vhost-user backend prototype supporting IOMMU[0],
making possible testing of this series.

The slave requests channel part is re-used from Marc-André's series submitted
last year[1], with main changes from original version being request/feature
names renaming and addition of the REPLY_ACK feature support.

Regarding IOTLB protocol, one noticeable change is the IOTLB miss request
reply made optionnal (i.e. only if slave requests it by setting the
VHOST_USER_NEED_REPLY flag in the message header). This change provides
more flexibility in the backend implementation of the feature.

The protocol is very close to kernel backends, except that a new
communication channel is introduced to enable the slave to send
requests to the master.

[0]: https://gitlab.com/mcoquelin/dpdk-next-virtio/commits/vhost_iotlb_proto
[1]: https://lists.gnu.org/archive/html/qemu-devel/2016-04/msg00095.html

Marc-André Lureau (2):
  vhost-user: add vhost_user to hold the chr
  vhost-user: add slave-req-fd support

Maxime Coquelin (4):
  vhost: propagate errors in vhost_device_iotlb_miss()
  vhost: rework IOTLB messaging
  vhost: Update rings information for IOTLB earlier
  spec/vhost-user spec: Add IOMMU support

 docs/specs/vhost-user.txt         | 107 ++++++++++++++++++++++-
 hw/virtio/vhost-backend.c         | 135 ++++++++++++++++-------------
 hw/virtio/vhost-user.c            | 177 +++++++++++++++++++++++++++++++++++++-
 hw/virtio/vhost.c                 |  38 ++++----
 include/hw/virtio/vhost-backend.h |  23 +++--
 include/hw/virtio/vhost.h         |   2 +-
 6 files changed, 393 insertions(+), 89 deletions(-)

-- 
2.9.3

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

* [Qemu-devel] [PATCH 1/6] vhost: propagate errors in vhost_device_iotlb_miss()
  2017-05-11 12:32 [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support Maxime Coquelin
@ 2017-05-11 12:32 ` Maxime Coquelin
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 2/6] vhost: rework IOTLB messaging Maxime Coquelin
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-11 12:32 UTC (permalink / raw)
  To: mst, peterx, marcandre.lureau, vkaplans, jasowang, wexu,
	yuanhan.liu, qemu-devel, jfreiman
  Cc: Maxime Coquelin

Some backends might want to know when things went wrong.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/virtio/vhost.c         | 15 ++++++++++-----
 include/hw/virtio/vhost.h |  2 +-
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 0001e60..369373a 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -971,18 +971,20 @@ static int vhost_memory_region_lookup(struct vhost_dev *hdev,
     return -EFAULT;
 }
 
-void vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write)
+int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write)
 {
     IOMMUTLBEntry iotlb;
     uint64_t uaddr, len;
+    int ret = -EFAULT;
 
     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)) {
+        ret = vhost_memory_region_lookup(dev, iotlb.translated_addr,
+                                         &uaddr, &len);
+        if (ret) {
             error_report("Fail to lookup the translated address "
                          "%"PRIx64, iotlb.translated_addr);
             goto out;
@@ -991,14 +993,17 @@ void vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write)
         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)) {
+        ret = dev->vhost_ops->vhost_update_device_iotlb(dev, iova, uaddr,
+                                                      len, iotlb.perm);
+        if (ret) {
             error_report("Fail to update device iotlb");
             goto out;
         }
     }
 out:
     rcu_read_unlock();
+
+    return ret;
 }
 
 static int vhost_virtqueue_start(struct vhost_dev *dev,
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index a450321..467dc77 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -105,5 +105,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);
+int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write);
 #endif
-- 
2.9.3

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

* [Qemu-devel] [PATCH 2/6] vhost: rework IOTLB messaging
  2017-05-11 12:32 [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support Maxime Coquelin
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 1/6] vhost: propagate errors in vhost_device_iotlb_miss() Maxime Coquelin
@ 2017-05-11 12:32 ` Maxime Coquelin
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier Maxime Coquelin
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-11 12:32 UTC (permalink / raw)
  To: mst, peterx, marcandre.lureau, vkaplans, jasowang, wexu,
	yuanhan.liu, qemu-devel, jfreiman
  Cc: Maxime Coquelin

This patch reworks IOTLB messaging to prepare for vhost-user
device IOTLB support.

IOTLB messages handling is extracted from vhost-kernel backend,
so that only the messages transport remains backend specifics.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/virtio/vhost-backend.c         | 135 +++++++++++++++++++++-----------------
 hw/virtio/vhost.c                 |   8 +--
 include/hw/virtio/vhost-backend.h |  23 ++++---
 3 files changed, 94 insertions(+), 72 deletions(-)

diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index be927b8..d6c38cc 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -192,7 +192,6 @@ static void vhost_kernel_iotlb_read(void *opaque)
     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;
@@ -201,70 +200,21 @@ static void vhost_kernel_iotlb_read(void *opaque)
             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;
+        vhost_backend_handle_iotlb_msg(dev, &msg.iotlb);
     }
-
-    return 0;
 }
 
-static int vhost_kernel_invalidate_device_iotlb(struct vhost_dev *dev,
-                                                uint64_t iova, uint64_t len)
+static int vhost_kernel_send_device_iotlb_msg(struct vhost_dev *dev,
+                                              struct vhost_iotlb_msg *imsg)
 {
-    struct vhost_msg msg;
-
-    msg.type = VHOST_IOTLB_MSG;
-    msg.iotlb.iova = iova;
-    msg.iotlb.size = len;
-    msg.iotlb.type = VHOST_IOTLB_INVALIDATE;
+    struct vhost_msg msg = {
+        .type = VHOST_IOTLB_MSG,
+        .iotlb = *imsg,
+    };
 
     if (write((uintptr_t)dev->opaque, &msg, sizeof msg) != sizeof msg) {
-        error_report("Fail to invalidate device iotlb");
+        error_report("Fail to update device iotlb");
         return -EFAULT;
     }
 
@@ -311,8 +261,7 @@ static const VhostOps kernel_ops = {
         .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,
+        .vhost_send_device_iotlb_msg = vhost_kernel_send_device_iotlb_msg,
 };
 
 int vhost_set_backend_type(struct vhost_dev *dev, VhostBackendType backend_type)
@@ -333,3 +282,69 @@ int vhost_set_backend_type(struct vhost_dev *dev, VhostBackendType backend_type)
 
     return r;
 }
+
+int vhost_backend_update_device_iotlb(struct vhost_dev *dev,
+                                             uint64_t iova, uint64_t uaddr,
+                                             uint64_t len,
+                                             IOMMUAccessFlags perm)
+{
+    struct vhost_iotlb_msg imsg;
+
+    imsg.iova =  iova;
+    imsg.uaddr = uaddr;
+    imsg.size = len;
+    imsg.type = VHOST_IOTLB_UPDATE;
+
+    switch (perm) {
+    case IOMMU_RO:
+        imsg.perm = VHOST_ACCESS_RO;
+        break;
+    case IOMMU_WO:
+        imsg.perm = VHOST_ACCESS_WO;
+        break;
+    case IOMMU_RW:
+        imsg.perm = VHOST_ACCESS_RW;
+        break;
+    default:
+        return -EINVAL;
+    }
+
+    return dev->vhost_ops->vhost_send_device_iotlb_msg(dev, &imsg);
+}
+
+int vhost_backend_invalidate_device_iotlb(struct vhost_dev *dev,
+                                                 uint64_t iova, uint64_t len)
+{
+    struct vhost_iotlb_msg imsg;
+
+    imsg.iova = iova;
+    imsg.size = len;
+    imsg.type = VHOST_IOTLB_INVALIDATE;
+
+    return dev->vhost_ops->vhost_send_device_iotlb_msg(dev, &imsg);
+}
+
+int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev,
+                                          struct vhost_iotlb_msg *imsg)
+{
+    int ret = 0;
+
+    switch (imsg->type) {
+    case VHOST_IOTLB_MISS:
+        ret = vhost_device_iotlb_miss(dev, imsg->iova,
+                                      imsg->perm != VHOST_ACCESS_RO);
+        break;
+    case VHOST_IOTLB_ACCESS_FAIL:
+        /* FIXME: report device iotlb error */
+        ret = -ENOTSUP;
+        break;
+    case VHOST_IOTLB_UPDATE:
+    case VHOST_IOTLB_INVALIDATE:
+    default:
+        error_report("Unexpected IOTLB message type");
+        ret = -EINVAL;
+        break;
+    }
+
+    return ret;
+}
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 369373a..748e331 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -724,8 +724,8 @@ static void vhost_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
     struct vhost_dev *hdev = iommu->hdev;
     hwaddr iova = iotlb->iova + iommu->iommu_offset;
 
-    if (hdev->vhost_ops->vhost_invalidate_device_iotlb(hdev, iova,
-                                                       iotlb->addr_mask + 1)) {
+    if (vhost_backend_invalidate_device_iotlb(hdev, iova,
+                                              iotlb->addr_mask + 1)) {
         error_report("Fail to invalidate device iotlb");
     }
 }
@@ -993,8 +993,8 @@ int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write)
         len = MIN(iotlb.addr_mask + 1, len);
         iova = iova & ~iotlb.addr_mask;
 
-        ret = dev->vhost_ops->vhost_update_device_iotlb(dev, iova, uaddr,
-                                                      len, iotlb.perm);
+        ret = vhost_backend_update_device_iotlb(dev, iova, uaddr,
+                                                len, iotlb.perm);
         if (ret) {
             error_report("Fail to update device iotlb");
             goto out;
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index c3cf4a7..a7a5f22 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -27,6 +27,7 @@ struct vhost_vring_file;
 struct vhost_vring_state;
 struct vhost_vring_addr;
 struct vhost_scsi_target;
+struct vhost_iotlb_msg;
 
 typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque);
 typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev);
@@ -81,12 +82,8 @@ typedef int (*vhost_vsock_set_guest_cid_op)(struct vhost_dev *dev,
 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 int (*vhost_send_device_iotlb_msg_op)(struct vhost_dev *dev,
+                                              struct vhost_iotlb_msg *imsg);
 
 typedef struct VhostOps {
     VhostBackendType backend_type;
@@ -120,8 +117,7 @@ typedef struct VhostOps {
     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;
+    vhost_send_device_iotlb_msg_op vhost_send_device_iotlb_msg;
 } VhostOps;
 
 extern const VhostOps user_ops;
@@ -129,4 +125,15 @@ extern const VhostOps user_ops;
 int vhost_set_backend_type(struct vhost_dev *dev,
                            VhostBackendType backend_type);
 
+int vhost_backend_update_device_iotlb(struct vhost_dev *dev,
+                                             uint64_t iova, uint64_t uaddr,
+                                             uint64_t len,
+                                             IOMMUAccessFlags perm);
+
+int vhost_backend_invalidate_device_iotlb(struct vhost_dev *dev,
+                                                 uint64_t iova, uint64_t len);
+
+int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev,
+                                          struct vhost_iotlb_msg *imsg);
+
 #endif /* VHOST_BACKEND_H */
-- 
2.9.3

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

* [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier
  2017-05-11 12:32 [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support Maxime Coquelin
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 1/6] vhost: propagate errors in vhost_device_iotlb_miss() Maxime Coquelin
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 2/6] vhost: rework IOTLB messaging Maxime Coquelin
@ 2017-05-11 12:32 ` Maxime Coquelin
  2017-05-11 17:33   ` Michael S. Tsirkin
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 4/6] vhost-user: add vhost_user to hold the chr Maxime Coquelin
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-11 12:32 UTC (permalink / raw)
  To: mst, peterx, marcandre.lureau, vkaplans, jasowang, wexu,
	yuanhan.liu, qemu-devel, jfreiman
  Cc: Maxime Coquelin

Vhost-kernel backend need to receive IOTLB entries for rings
information early, but vhost-user need the same information
earlier, before VHOST_USER_SET_VRING_ADDR is sent.

This patch also trigger IOTLB miss for all rings informations
for robustness, even if in practice these adresses are on the
same page.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/virtio/vhost.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 748e331..817f6d0 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -799,7 +799,17 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
         .log_guest_addr = vq->used_phys,
         .flags = enable_log ? (1 << VHOST_VRING_F_LOG) : 0,
     };
-    int r = dev->vhost_ops->vhost_set_vring_addr(dev, &addr);
+    int r;
+
+    /* Update rings information for IOTLB to work correctly,
+     * vhost-kernel & vhost-user backends require for this.*/
+    if (vhost_dev_has_iommu(dev)) {
+        vhost_device_iotlb_miss(dev, addr.desc_user_addr, true);
+        vhost_device_iotlb_miss(dev, addr.used_user_addr, true);
+        vhost_device_iotlb_miss(dev, addr.avail_user_addr, true);
+    }
+
+    r = dev->vhost_ops->vhost_set_vring_addr(dev, &addr);
     if (r < 0) {
         VHOST_OPS_DEBUG("vhost_set_vring_addr failed");
         return -errno;
@@ -1551,13 +1561,6 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
 
     if (vhost_dev_has_iommu(hdev)) {
         hdev->vhost_ops->vhost_set_iotlb_callback(hdev, true);
-
-        /* 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:
-- 
2.9.3

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

* [Qemu-devel] [PATCH 4/6] vhost-user: add vhost_user to hold the chr
  2017-05-11 12:32 [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support Maxime Coquelin
                   ` (2 preceding siblings ...)
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier Maxime Coquelin
@ 2017-05-11 12:32 ` Maxime Coquelin
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 5/6] vhost-user: add slave-req-fd support Maxime Coquelin
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-11 12:32 UTC (permalink / raw)
  To: mst, peterx, marcandre.lureau, vkaplans, jasowang, wexu,
	yuanhan.liu, qemu-devel, jfreiman
  Cc: Marc-André Lureau, Maxime Coquelin

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Next patches will add more fields to the structure

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/virtio/vhost-user.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 9334a8a..f0e10d0 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -111,6 +111,10 @@ static VhostUserMsg m __attribute__ ((unused));
 /* The version of the protocol we support */
 #define VHOST_USER_VERSION    (0x1)
 
+struct vhost_user {
+    CharBackend *chr;
+};
+
 static bool ioeventfd_enabled(void)
 {
     return kvm_enabled() && kvm_eventfds_enabled();
@@ -118,7 +122,8 @@ static bool ioeventfd_enabled(void)
 
 static int vhost_user_read(struct vhost_dev *dev, VhostUserMsg *msg)
 {
-    CharBackend *chr = dev->opaque;
+    struct vhost_user *u = dev->opaque;
+    CharBackend *chr = u->chr;
     uint8_t *p = (uint8_t *) msg;
     int r, size = VHOST_USER_HDR_SIZE;
 
@@ -199,7 +204,8 @@ static bool vhost_user_one_time_request(VhostUserRequest request)
 static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
                             int *fds, int fd_num)
 {
-    CharBackend *chr = dev->opaque;
+    struct vhost_user *u = dev->opaque;
+    CharBackend *chr = u->chr;
     int ret, size = VHOST_USER_HDR_SIZE + msg->size;
 
     /*
@@ -571,11 +577,14 @@ static int vhost_user_reset_device(struct vhost_dev *dev)
 static int vhost_user_init(struct vhost_dev *dev, void *opaque)
 {
     uint64_t features;
+    struct vhost_user *u;
     int err;
 
     assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
-    dev->opaque = opaque;
+    u = g_new0(struct vhost_user, 1);
+    u->chr = opaque;
+    dev->opaque = u;
 
     err = vhost_user_get_features(dev, &features);
     if (err < 0) {
@@ -620,8 +629,12 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque)
 
 static int vhost_user_cleanup(struct vhost_dev *dev)
 {
+    struct vhost_user *u;
+
     assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
+    u = dev->opaque;
+    g_free(u);
     dev->opaque = 0;
 
     return 0;
-- 
2.9.3

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

* [Qemu-devel] [PATCH 5/6] vhost-user: add slave-req-fd support
  2017-05-11 12:32 [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support Maxime Coquelin
                   ` (3 preceding siblings ...)
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 4/6] vhost-user: add vhost_user to hold the chr Maxime Coquelin
@ 2017-05-11 12:32 ` Maxime Coquelin
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support Maxime Coquelin
  2017-05-11 13:25 ` [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support no-reply
  6 siblings, 0 replies; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-11 12:32 UTC (permalink / raw)
  To: mst, peterx, marcandre.lureau, vkaplans, jasowang, wexu,
	yuanhan.liu, qemu-devel, jfreiman
  Cc: Marc-André Lureau, Maxime Coquelin

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Learn to give a socket to the slave to let him make requests to the
master.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 docs/specs/vhost-user.txt |  32 +++++++++++-
 hw/virtio/vhost-user.c    | 127 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 157 insertions(+), 2 deletions(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 036890f..5fa7016 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -139,6 +139,7 @@ in the ancillary data:
  * VHOST_USER_SET_VRING_KICK
  * VHOST_USER_SET_VRING_CALL
  * VHOST_USER_SET_VRING_ERR
+ * VHOST_USER_SET_SLAVE_REQ_FD
 
 If Master is unable to send the full message or receives a wrong reply it will
 close the connection. An optional reconnection mechanism can be implemented.
@@ -252,6 +253,18 @@ Once the source has finished migration, rings will be stopped by
 the source. No further update must be done before rings are
 restarted.
 
+Slave communication
+-------------------
+
+An optional communication channel is provided if the slave declares
+VHOST_USER_PROTOCOL_F_SLAVE_REQ protocol feature, to allow the slave to make
+requests to the master.
+
+The fd is provided via VHOST_USER_SET_SLAVE_REQ_FD ancillary data.
+
+A slave may then send VHOST_USER_SLAVE_* messages to the master
+using this fd communication channel.
+
 Protocol features
 -----------------
 
@@ -260,9 +273,10 @@ Protocol features
 #define VHOST_USER_PROTOCOL_F_RARP           2
 #define VHOST_USER_PROTOCOL_F_REPLY_ACK      3
 #define VHOST_USER_PROTOCOL_F_MTU            4
+#define VHOST_USER_PROTOCOL_F_SLAVE_REQ      5
 
-Message types
--------------
+Master message types
+--------------------
 
  * VHOST_USER_GET_FEATURES
 
@@ -486,6 +500,20 @@ Message types
       If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, slave must respond
       with zero in case the specified MTU is valid, or non-zero otherwise.
 
+ * VHOST_USER_SET_SLAVE_REQ_FD
+
+      Id: 21
+      Equivalent ioctl: N/A
+      Master payload: N/A
+
+      Set the socket file descriptor for slave initiated requests. It is passed
+      in the ancillary data.
+      This request should be sent only when VHOST_USER_F_PROTOCOL_FEATURES
+      has been negotiated, and protocol feature bit VHOST_USER_PROTOCOL_F_SLAVE_REQ
+      bit is present in VHOST_USER_GET_PROTOCOL_FEATURES.
+      If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, slave must respond
+      with zero for success, non-zero otherwise.
+
 VHOST_USER_PROTOCOL_F_REPLY_ACK:
 -------------------------------
 The original vhost-user specification only demands replies for certain
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index f0e10d0..fbc09fa 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -33,6 +33,7 @@ enum VhostUserProtocolFeature {
     VHOST_USER_PROTOCOL_F_RARP = 2,
     VHOST_USER_PROTOCOL_F_REPLY_ACK = 3,
     VHOST_USER_PROTOCOL_F_NET_MTU = 4,
+    VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5,
 
     VHOST_USER_PROTOCOL_F_MAX
 };
@@ -61,9 +62,15 @@ typedef enum VhostUserRequest {
     VHOST_USER_SET_VRING_ENABLE = 18,
     VHOST_USER_SEND_RARP = 19,
     VHOST_USER_NET_SET_MTU = 20,
+    VHOST_USER_SET_SLAVE_REQ_FD = 21,
     VHOST_USER_MAX
 } VhostUserRequest;
 
+typedef enum VhostUserSlaveRequest {
+    VHOST_USER_SLAVE_NONE = 0,
+    VHOST_USER_SLAVE_MAX
+}  VhostUserSlaveRequest;
+
 typedef struct VhostUserMemoryRegion {
     uint64_t guest_phys_addr;
     uint64_t memory_size;
@@ -113,6 +120,7 @@ static VhostUserMsg m __attribute__ ((unused));
 
 struct vhost_user {
     CharBackend *chr;
+    int slave_fd;
 };
 
 static bool ioeventfd_enabled(void)
@@ -574,6 +582,115 @@ static int vhost_user_reset_device(struct vhost_dev *dev)
     return 0;
 }
 
+static void slave_read(void *opaque)
+{
+    struct vhost_dev *dev = opaque;
+    struct vhost_user *u = dev->opaque;
+    VhostUserMsg msg = { 0, };
+    int size, ret = 0;
+
+    /* Read header */
+    size = read(u->slave_fd, &msg, VHOST_USER_HDR_SIZE);
+    if (size != VHOST_USER_HDR_SIZE) {
+        error_report("Failed to read from slave.");
+        goto err;
+    }
+
+    if (msg.size > VHOST_USER_PAYLOAD_SIZE) {
+        error_report("Failed to read msg header."
+                " Size %d exceeds the maximum %zu.", msg.size,
+                VHOST_USER_PAYLOAD_SIZE);
+        goto err;
+    }
+
+    /* Read payload */
+    size = read(u->slave_fd, &msg.payload, msg.size);
+    if (size != msg.size) {
+        error_report("Failed to read payload from slave.");
+        goto err;
+    }
+
+    switch (msg.request) {
+    default:
+        error_report("Received unexpected msg type.");
+        ret = -EINVAL;
+    }
+
+    /*
+     * REPLY_ACK feature handling. Other reply types has to be managed
+     * directly in their request handlers.
+     */
+    if (msg.flags & VHOST_USER_NEED_REPLY_MASK) {
+        msg.flags &= ~VHOST_USER_NEED_REPLY_MASK;
+        msg.flags |= VHOST_USER_REPLY_MASK;
+
+        msg.payload.u64 = !!ret;
+        msg.size = sizeof(msg.payload.u64);
+
+        size = write(u->slave_fd, &msg, VHOST_USER_HDR_SIZE + msg.size);
+        if (size != VHOST_USER_HDR_SIZE + msg.size) {
+            error_report("Failed to send msg reply to slave.");
+            goto err;
+        }
+    }
+
+    return;
+
+err:
+    qemu_set_fd_handler(u->slave_fd, NULL, NULL, NULL);
+    close(u->slave_fd);
+    u->slave_fd = -1;
+    return;
+}
+
+static int vhost_setup_slave_channel(struct vhost_dev *dev)
+{
+    VhostUserMsg msg = {
+        .request = VHOST_USER_SET_SLAVE_REQ_FD,
+        .flags = VHOST_USER_VERSION,
+    };
+    struct vhost_user *u = dev->opaque;
+    int sv[2], ret = 0;
+    bool reply_supported = virtio_has_feature(dev->protocol_features,
+                                              VHOST_USER_PROTOCOL_F_REPLY_ACK);
+
+    if (!virtio_has_feature(dev->protocol_features,
+                            VHOST_USER_PROTOCOL_F_SLAVE_REQ)) {
+        return 0;
+    }
+
+    if (socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
+        error_report("socketpair() failed");
+        return -1;
+    }
+
+    u->slave_fd = sv[0];
+    qemu_set_fd_handler(u->slave_fd, slave_read, NULL, dev);
+
+    if (reply_supported) {
+        msg.flags |= VHOST_USER_NEED_REPLY_MASK;
+    }
+
+    ret = vhost_user_write(dev, &msg, &sv[1], 1);
+    if (ret) {
+        goto out;
+    }
+
+    if (reply_supported) {
+        ret = process_message_reply(dev, msg.request);
+    }
+
+out:
+    close(sv[1]);
+    if (ret) {
+        qemu_set_fd_handler(u->slave_fd, NULL, NULL, NULL);
+        close(u->slave_fd);
+        u->slave_fd = -1;
+    }
+
+    return ret;
+}
+
 static int vhost_user_init(struct vhost_dev *dev, void *opaque)
 {
     uint64_t features;
@@ -584,6 +701,7 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque)
 
     u = g_new0(struct vhost_user, 1);
     u->chr = opaque;
+    u->slave_fd = -1;
     dev->opaque = u;
 
     err = vhost_user_get_features(dev, &features);
@@ -624,6 +742,11 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque)
                    "VHOST_USER_PROTOCOL_F_LOG_SHMFD feature.");
     }
 
+    err = vhost_setup_slave_channel(dev);
+    if (err < 0) {
+        return err;
+    }
+
     return 0;
 }
 
@@ -634,6 +757,10 @@ static int vhost_user_cleanup(struct vhost_dev *dev)
     assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
     u = dev->opaque;
+    if (u->slave_fd >= 0) {
+        close(u->slave_fd);
+        u->slave_fd = -1;
+    }
     g_free(u);
     dev->opaque = 0;
 
-- 
2.9.3

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

* [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-11 12:32 [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support Maxime Coquelin
                   ` (4 preceding siblings ...)
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 5/6] vhost-user: add slave-req-fd support Maxime Coquelin
@ 2017-05-11 12:32 ` Maxime Coquelin
  2017-05-11 18:25   ` Michael S. Tsirkin
  2017-05-17 16:48   ` Michael S. Tsirkin
  2017-05-11 13:25 ` [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support no-reply
  6 siblings, 2 replies; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-11 12:32 UTC (permalink / raw)
  To: mst, peterx, marcandre.lureau, vkaplans, jasowang, wexu,
	yuanhan.liu, qemu-devel, jfreiman
  Cc: Maxime Coquelin

This patch specifies and implements the master/slave communication
to support device IOTLB in slave.

The vhost_iotlb_msg structure introduced for kernel backends is
re-used, making the design close between the two backends.

An exception is the use of the secondary channel to enable the
slave to send IOTLB miss requests to the master.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 docs/specs/vhost-user.txt | 75 +++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
 2 files changed, 106 insertions(+)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 5fa7016..4a1f0c3 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -97,6 +97,23 @@ Depending on the request type, payload can be:
    log offset: offset from start of supplied file descriptor
        where logging starts (i.e. where guest address 0 would be logged)
 
+ * An IOTLB message
+   ---------------------------------------------------------
+   | iova | size | user address | permissions flags | type |
+   ---------------------------------------------------------
+
+   IOVA: a 64-bit guest I/O virtual address
+   Size: a 64-bit size
+   User address: a 64-bit user address
+   Permissions flags: a 8-bit bit field:
+    - Bit 0: Read access
+    - Bit 1: Write access
+   Type: a 8-bit IOTLB message type:
+    - 1: IOTLB miss
+    - 2: IOTLB update
+    - 3: IOTLB invalidate
+    - 4: IOTLB access fail
+
 In QEMU the vhost-user message is implemented with the following struct:
 
 typedef struct VhostUserMsg {
@@ -109,6 +126,7 @@ typedef struct VhostUserMsg {
         struct vhost_vring_addr addr;
         VhostUserMemory memory;
         VhostUserLog log;
+        struct vhost_iotlb_msg iotlb;
     };
 } QEMU_PACKED VhostUserMsg;
 
@@ -253,6 +271,31 @@ Once the source has finished migration, rings will be stopped by
 the source. No further update must be done before rings are
 restarted.
 
+IOMMU support
+-------------
+
+When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated, the master has
+to send IOTLB entries update & invalidation by sending VHOST_USER_IOTLB_MSG
+requests to the slave with a struct vhost_iotlb_msg payload. For update events,
+the iotlb payload has to be filled with the update message type (2), the I/O
+virtual address, the size, the user virtual address, and the permissions
+flags. For invalidation events, the iotlb payload has to be filled with the
+invalidation message type (3), the I/O virtual address and the size. On
+success, the slave is expected to reply with a zero payload, non-zero
+otherwise.
+
+When the VHOST_USER_PROTOCOL_F_SLAVE_REQ is supported by the slave, and the
+master initiated the slave to master communication channel using the
+VHOST_USER_SET_SLAVE_REQ_FD request, the slave can send IOTLB miss and access
+failure events by sending VHOST_USER_SLAVE_IOTLB_MSG requests to the master
+with a struct vhost_iotlb_msg payload. For miss events, the iotlb payload has
+to be filled with the miss message type (1), the I/O virtual address and the
+permissions flags. For access failure event, the iotlb payload has to be
+filled with the access failure message type (4), the I/O virtual address and
+the permissions flags. For synchronization purpose, the slave may rely on the
+reply-ack feature, so the master may send a reply when operation is completed
+if the reply-ack feature is negotiated and slaves requests a reply.
+
 Slave communication
 -------------------
 
@@ -514,6 +557,38 @@ Master message types
       If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, slave must respond
       with zero for success, non-zero otherwise.
 
+ * VHOST_USER_IOTLB_MSG
+
+      Id: 22
+      Equivalent ioctl: N/A (equivalent to VHOST_IOTLB_MSG message type)
+      Master payload: struct vhost_iotlb_msg
+      Slave payload: u64
+
+      Send IOTLB messages with struct vhost_iotlb_msg as payload.
+      Master sends such requests to update and invalidate entries in the device
+      IOTLB. The slave has to acknowledge the request with sending zero as u64
+      payload for success, non-zero otherwise.
+      This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature
+      has been successfully negotiated.
+
+Slave message types
+-------------------
+
+ * VHOST_USER_SLAVE_IOTLB_MSG
+
+      Id: 1
+      Equivalent ioctl: N/A (equivalent to VHOST_IOTLB_MSG message type)
+      Slave payload: struct vhost_iotlb_msg
+      Master payload: N/A
+
+      Send IOTLB messages with struct vhost_iotlb_msg as payload.
+      Slave sends such requests to notify of an IOTLB miss, or an IOTLB
+      access failure. If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated,
+      and slave set the VHOST_USER_NEED_REPLY flag, master must respond with
+      zero when operation is successfully completed, or non-zero otherwise.
+      This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature
+      has been successfully negotiated.
+
 VHOST_USER_PROTOCOL_F_REPLY_ACK:
 -------------------------------
 The original vhost-user specification only demands replies for certain
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index fbc09fa..2c93181 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -63,11 +63,13 @@ typedef enum VhostUserRequest {
     VHOST_USER_SEND_RARP = 19,
     VHOST_USER_NET_SET_MTU = 20,
     VHOST_USER_SET_SLAVE_REQ_FD = 21,
+    VHOST_USER_IOTLB_MSG = 22,
     VHOST_USER_MAX
 } VhostUserRequest;
 
 typedef enum VhostUserSlaveRequest {
     VHOST_USER_SLAVE_NONE = 0,
+    VHOST_USER_SLAVE_IOTLB_MSG = 1,
     VHOST_USER_SLAVE_MAX
 }  VhostUserSlaveRequest;
 
@@ -105,6 +107,7 @@ typedef struct VhostUserMsg {
         struct vhost_vring_addr addr;
         VhostUserMemory memory;
         VhostUserLog log;
+        struct vhost_iotlb_msg iotlb;
     } payload;
 } QEMU_PACKED VhostUserMsg;
 
@@ -611,6 +614,9 @@ static void slave_read(void *opaque)
     }
 
     switch (msg.request) {
+    case VHOST_USER_SLAVE_IOTLB_MSG:
+        ret = vhost_backend_handle_iotlb_msg(dev, &msg.payload.iotlb);
+        break;
     default:
         error_report("Received unexpected msg type.");
         ret = -EINVAL;
@@ -858,6 +864,29 @@ static int vhost_user_net_set_mtu(struct vhost_dev *dev, uint16_t mtu)
     return 0;
 }
 
+static int vhost_user_send_device_iotlb_msg(struct vhost_dev *dev,
+                                            struct vhost_iotlb_msg *imsg)
+{
+    VhostUserMsg msg = {
+        .request = VHOST_USER_IOTLB_MSG,
+        .size = sizeof(msg.payload.iotlb),
+        .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
+        .payload.iotlb = *imsg,
+    };
+
+    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+        return -EFAULT;
+    }
+
+    return process_message_reply(dev, msg.request);
+}
+
+
+static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled)
+{
+    /* No-op as the receive channel is not dedicated to IOTLB messages. */
+}
+
 const VhostOps user_ops = {
         .backend_type = VHOST_BACKEND_TYPE_USER,
         .vhost_backend_init = vhost_user_init,
@@ -882,4 +911,6 @@ const VhostOps user_ops = {
         .vhost_migration_done = vhost_user_migration_done,
         .vhost_backend_can_merge = vhost_user_can_merge,
         .vhost_net_set_mtu = vhost_user_net_set_mtu,
+        .vhost_set_iotlb_callback = vhost_user_set_iotlb_callback,
+        .vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg,
 };
-- 
2.9.3

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

* Re: [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support
  2017-05-11 12:32 [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support Maxime Coquelin
                   ` (5 preceding siblings ...)
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support Maxime Coquelin
@ 2017-05-11 13:25 ` no-reply
  6 siblings, 0 replies; 32+ messages in thread
From: no-reply @ 2017-05-11 13:25 UTC (permalink / raw)
  To: maxime.coquelin
  Cc: famz, mst, peterx, marcandre.lureau, vkaplans, jasowang, wexu,
	yuanhan.liu, qemu-devel, jfreiman

Hi,

This series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Subject: [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support
Message-id: 20170511123246.31308-1-maxime.coquelin@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
time make docker-test-mingw@fedora
time make docker-test-build@min-glib
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
0bb1b03 spec/vhost-user spec: Add IOMMU support
512f30c vhost-user: add slave-req-fd support
6dca4e5 vhost-user: add vhost_user to hold the chr
5091e64 vhost: Update rings information for IOTLB earlier
a1375bb vhost: rework IOTLB messaging
4d21521 vhost: propagate errors in vhost_device_iotlb_miss()

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-d_8o6qsf/src/dtc'...
Submodule path 'dtc': checked out '558cd81bdd432769b59bff01240c44f82cfb1a9d'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-d_8o6qsf/src'
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPY    RUNNER
    RUN test-quick in qemu:centos6 
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
ccache-3.1.6-2.el6.x86_64
epel-release-6-8.noarch
gcc-4.4.7-17.el6.x86_64
git-1.7.1-4.el6_7.1.x86_64
glib2-devel-2.28.8-5.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
make-3.81-23.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
tar-1.23-15.el6_8.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=libfdt-devel ccache     tar git make gcc g++     zlib-devel glib2-devel SDL-devel pixman-devel     epel-release
HOSTNAME=ddbdc122b820
TERM=xterm
MAKEFLAGS= -j8
HISTSIZE=1000
J=8
USER=root
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix    /var/tmp/qemu-build/install
BIOS directory    /var/tmp/qemu-build/install/share/qemu
binary directory  /var/tmp/qemu-build/install/bin
library directory /var/tmp/qemu-build/install/lib
module directory  /var/tmp/qemu-build/install/lib/qemu
libexec directory /var/tmp/qemu-build/install/libexec
include directory /var/tmp/qemu-build/install/include
config directory  /var/tmp/qemu-build/install/etc
local state directory   /var/tmp/qemu-build/install/var
Manual directory  /var/tmp/qemu-build/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path       /tmp/qemu-test/src
C compiler        cc
Host C compiler   cc
C++ compiler      
Objective-C compiler cc
ARFLAGS           rv
CFLAGS            -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS       -I/usr/include/pixman-1   -I$(SRC_PATH)/dtc/libfdt -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-all
LDFLAGS           -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make              make
install           install
python            python -B
smbd              /usr/sbin/smbd
module support    no
host CPU          x86_64
host big endian   no
target list       x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled     no
sparse enabled    no
strip binaries    yes
profiler          no
static build      no
pixman            system
SDL support       yes (1.2.14)
GTK support       no 
GTK GL support    no
VTE support       no 
TLS priority      NORMAL
GNUTLS support    no
GNUTLS rnd        no
libgcrypt         no
libgcrypt kdf     no
nettle            no 
nettle kdf        no
libtasn1          no
curses support    no
virgl support     no
curl support      no
mingw32 support   no
Audio drivers     oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS support    no
VNC support       yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support       no
brlapi support    no
bluez  support    no
Documentation     no
PIE               yes
vde support       no
netmap support    no
Linux AIO support no
ATTR/XATTR support yes
Install blobs     yes
KVM support       yes
HAX support       no
RDMA support      no
TCG interpreter   no
fdt support       yes
preadv support    yes
fdatasync         yes
madvise           yes
posix_madvise     yes
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
vhost-vsock support yes
Trace backends    log
spice support     no 
rbd support       no
xfsctl support    no
smartcard support no
libusb            no
usb net redir     no
OpenGL support    no
OpenGL dmabufs    no
libiscsi support  no
libnfs support    no
build guest agent yes
QGA VSS support   no
QGA w32 disk info no
QGA MSI support   no
seccomp support   no
coroutine backend ucontext
coroutine pool    yes
debug stack usage no
GlusterFS support no
gcov              gcov
gcov enabled      no
TPM support       yes
libssh2 support   no
TPM passthrough   yes
QOM debugging     yes
lzo support       no
snappy support    no
bzip2 support     no
NUMA host support no
tcmalloc support  no
jemalloc support  no
avx2 optimization no
replication support yes
VxHS block device no
mkdir -p dtc/libfdt
  GEN     x86_64-softmmu/config-devices.mak.tmp
mkdir -p dtc/tests
  GEN     aarch64-softmmu/config-devices.mak.tmp
  GEN     config-host.h
  GEN     qemu-options.def
  GEN     qmp-commands.h
  GEN     qapi-types.h
  GEN     qapi-visit.h
  GEN     qapi-event.h
  GEN     x86_64-softmmu/config-devices.mak
  GEN     aarch64-softmmu/config-devices.mak
  GEN     qapi-types.c
  GEN     qmp-marshal.c
  GEN     qapi-visit.c
  GEN     qapi-event.c
  GEN     qmp-introspect.h
  GEN     qmp-introspect.c
  GEN     trace/generated-tcg-tracers.h
  GEN     trace/generated-helpers-wrappers.h
  GEN     trace/generated-helpers.h
  GEN     trace/generated-helpers.c
  GEN     module_block.h
  GEN     tests/test-qapi-types.h
  GEN     tests/test-qapi-visit.h
  GEN     tests/test-qmp-commands.h
  GEN     tests/test-qapi-event.h
  GEN     tests/test-qmp-introspect.h
  GEN     trace-root.h
  GEN     util/trace.h
  GEN     crypto/trace.h
  GEN     io/trace.h
  GEN     migration/trace.h
  GEN     block/trace.h
  GEN     backends/trace.h
  GEN     hw/block/trace.h
  GEN     hw/block/dataplane/trace.h
  GEN     hw/char/trace.h
  GEN     hw/intc/trace.h
  GEN     hw/net/trace.h
  GEN     hw/virtio/trace.h
  GEN     hw/audio/trace.h
  GEN     hw/misc/trace.h
  GEN     hw/usb/trace.h
  GEN     hw/scsi/trace.h
  GEN     hw/nvram/trace.h
  GEN     hw/display/trace.h
  GEN     hw/input/trace.h
  GEN     hw/timer/trace.h
  GEN     hw/dma/trace.h
  GEN     hw/sparc/trace.h
  GEN     hw/sd/trace.h
  GEN     hw/isa/trace.h
  GEN     hw/mem/trace.h
  GEN     hw/i386/trace.h
  GEN     hw/i386/xen/trace.h
  GEN     hw/9pfs/trace.h
  GEN     hw/ppc/trace.h
  GEN     hw/pci/trace.h
  GEN     hw/s390x/trace.h
  GEN     hw/vfio/trace.h
  GEN     hw/acpi/trace.h
  GEN     hw/arm/trace.h
  GEN     hw/alpha/trace.h
  GEN     hw/xen/trace.h
  GEN     ui/trace.h
  GEN     audio/trace.h
  GEN     net/trace.h
  GEN     target/arm/trace.h
  GEN     target/i386/trace.h
  GEN     target/mips/trace.h
  GEN     target/sparc/trace.h
  GEN     target/s390x/trace.h
  GEN     target/ppc/trace.h
  GEN     qom/trace.h
  GEN     linux-user/trace.h
  GEN     qapi/trace.h
  GEN     trace-root.c
  GEN     util/trace.c
  GEN     crypto/trace.c
  GEN     io/trace.c
  GEN     migration/trace.c
  GEN     block/trace.c
  GEN     backends/trace.c
  GEN     hw/block/trace.c
  GEN     hw/block/dataplane/trace.c
  GEN     hw/char/trace.c
  GEN     hw/intc/trace.c
  GEN     hw/net/trace.c
  GEN     hw/virtio/trace.c
  GEN     hw/audio/trace.c
  GEN     hw/misc/trace.c
  GEN     hw/usb/trace.c
  GEN     hw/scsi/trace.c
  GEN     hw/nvram/trace.c
  GEN     hw/display/trace.c
  GEN     hw/input/trace.c
  GEN     hw/timer/trace.c
  GEN     hw/dma/trace.c
  GEN     hw/sparc/trace.c
  GEN     hw/sd/trace.c
  GEN     hw/isa/trace.c
  GEN     hw/mem/trace.c
  GEN     hw/i386/trace.c
  GEN     hw/i386/xen/trace.c
  GEN     hw/9pfs/trace.c
  GEN     hw/ppc/trace.c
  GEN     hw/pci/trace.c
  GEN     hw/s390x/trace.c
  GEN     hw/vfio/trace.c
  GEN     hw/acpi/trace.c
  GEN     hw/arm/trace.c
  GEN     hw/alpha/trace.c
  GEN     hw/xen/trace.c
  GEN     ui/trace.c
  GEN     audio/trace.c
  GEN     net/trace.c
  GEN     target/arm/trace.c
  GEN     target/i386/trace.c
  GEN     target/mips/trace.c
  GEN     target/sparc/trace.c
  GEN     target/s390x/trace.c
  GEN     target/ppc/trace.c
  GEN     qom/trace.c
  GEN     linux-user/trace.c
  GEN     qapi/trace.c
  GEN     config-all-devices.mak
	 DEP /tmp/qemu-test/src/dtc/tests/dumptrees.c
	 DEP /tmp/qemu-test/src/dtc/tests/check_path.c
	 DEP /tmp/qemu-test/src/dtc/tests/value-labels.c
	 DEP /tmp/qemu-test/src/dtc/tests/truncated_property.c
	 DEP /tmp/qemu-test/src/dtc/tests/overlay_bad_fixup.c
	 DEP /tmp/qemu-test/src/dtc/tests/asm_tree_dump.c
	 DEP /tmp/qemu-test/src/dtc/tests/testutils.c
	 DEP /tmp/qemu-test/src/dtc/tests/trees.S
	 DEP /tmp/qemu-test/src/dtc/tests/overlay.c
	 DEP /tmp/qemu-test/src/dtc/tests/subnode_iterate.c
	 DEP /tmp/qemu-test/src/dtc/tests/property_iterate.c
	 DEP /tmp/qemu-test/src/dtc/tests/integer-expressions.c
	 DEP /tmp/qemu-test/src/dtc/tests/utilfdt_test.c
	 DEP /tmp/qemu-test/src/dtc/tests/path_offset_aliases.c
	 DEP /tmp/qemu-test/src/dtc/tests/add_subnode_with_nops.c
	 DEP /tmp/qemu-test/src/dtc/tests/dtbs_equal_unordered.c
	 DEP /tmp/qemu-test/src/dtc/tests/dtb_reverse.c
	 DEP /tmp/qemu-test/src/dtc/tests/dtbs_equal_ordered.c
	 DEP /tmp/qemu-test/src/dtc/tests/extra-terminating-null.c
	 DEP /tmp/qemu-test/src/dtc/tests/incbin.c
	 DEP /tmp/qemu-test/src/dtc/tests/boot-cpuid.c
	 DEP /tmp/qemu-test/src/dtc/tests/phandle_format.c
	 DEP /tmp/qemu-test/src/dtc/tests/path-references.c
	 DEP /tmp/qemu-test/src/dtc/tests/references.c
	 DEP /tmp/qemu-test/src/dtc/tests/string_escapes.c
	 DEP /tmp/qemu-test/src/dtc/tests/appendprop2.c
	 DEP /tmp/qemu-test/src/dtc/tests/propname_escapes.c
	 DEP /tmp/qemu-test/src/dtc/tests/appendprop1.c
	 DEP /tmp/qemu-test/src/dtc/tests/del_node.c
	 DEP /tmp/qemu-test/src/dtc/tests/del_property.c
	 DEP /tmp/qemu-test/src/dtc/tests/set_name.c
	 DEP /tmp/qemu-test/src/dtc/tests/rw_tree1.c
	 DEP /tmp/qemu-test/src/dtc/tests/setprop.c
	 DEP /tmp/qemu-test/src/dtc/tests/open_pack.c
	 DEP /tmp/qemu-test/src/dtc/tests/nopulate.c
	 DEP /tmp/qemu-test/src/dtc/tests/mangle-layout.c
	 DEP /tmp/qemu-test/src/dtc/tests/sw_tree1.c
	 DEP /tmp/qemu-test/src/dtc/tests/move_and_save.c
	 DEP /tmp/qemu-test/src/dtc/tests/nop_node.c
	 DEP /tmp/qemu-test/src/dtc/tests/setprop_inplace.c
	 DEP /tmp/qemu-test/src/dtc/tests/nop_property.c
	 DEP /tmp/qemu-test/src/dtc/tests/stringlist.c
	 DEP /tmp/qemu-test/src/dtc/tests/addr_size_cells.c
	 DEP /tmp/qemu-test/src/dtc/tests/notfound.c
	 DEP /tmp/qemu-test/src/dtc/tests/sized_cells.c
	 DEP /tmp/qemu-test/src/dtc/tests/char_literal.c
	 DEP /tmp/qemu-test/src/dtc/tests/get_alias.c
	 DEP /tmp/qemu-test/src/dtc/tests/node_offset_by_compatible.c
	 DEP /tmp/qemu-test/src/dtc/tests/node_check_compatible.c
	 DEP /tmp/qemu-test/src/dtc/tests/node_offset_by_phandle.c
	 DEP /tmp/qemu-test/src/dtc/tests/node_offset_by_prop_value.c
	 DEP /tmp/qemu-test/src/dtc/tests/parent_offset.c
	 DEP /tmp/qemu-test/src/dtc/tests/supernode_atdepth_offset.c
	 DEP /tmp/qemu-test/src/dtc/tests/get_path.c
	 DEP /tmp/qemu-test/src/dtc/tests/getprop.c
	 DEP /tmp/qemu-test/src/dtc/tests/get_phandle.c
	 DEP /tmp/qemu-test/src/dtc/tests/path_offset.c
	 DEP /tmp/qemu-test/src/dtc/tests/subnode_offset.c
	 DEP /tmp/qemu-test/src/dtc/tests/get_name.c
	 DEP /tmp/qemu-test/src/dtc/tests/find_property.c
	 DEP /tmp/qemu-test/src/dtc/tests/root_node.c
	 DEP /tmp/qemu-test/src/dtc/tests/get_mem_rsv.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_overlay.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_addresses.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_strerror.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_empty_tree.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_rw.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_sw.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_wip.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_ro.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt.c
	 DEP /tmp/qemu-test/src/dtc/util.c
	 DEP /tmp/qemu-test/src/dtc/fdtget.c
	 DEP /tmp/qemu-test/src/dtc/fdtput.c
	 DEP /tmp/qemu-test/src/dtc/fdtdump.c
	 LEX convert-dtsv0-lexer.lex.c
make[1]: flex: Command not found
	 DEP /tmp/qemu-test/src/dtc/srcpos.c
	 BISON dtc-parser.tab.c
make[1]: bison: Command not found
	 LEX dtc-lexer.lex.c
make[1]: flex: Command not found
	 DEP /tmp/qemu-test/src/dtc/treesource.c
	 DEP /tmp/qemu-test/src/dtc/livetree.c
	 DEP /tmp/qemu-test/src/dtc/fstree.c
	 DEP /tmp/qemu-test/src/dtc/flattree.c
	 DEP /tmp/qemu-test/src/dtc/dtc.c
	 DEP /tmp/qemu-test/src/dtc/data.c
	 DEP /tmp/qemu-test/src/dtc/checks.c
	CHK version_gen.h
	 LEX convert-dtsv0-lexer.lex.c
	 LEX dtc-lexer.lex.c
make[1]: flex: Command not found
make[1]: flex: Command not found
	 BISON dtc-parser.tab.c
make[1]: bison: Command not found
	UPD version_gen.h
	 DEP /tmp/qemu-test/src/dtc/util.c
	 LEX convert-dtsv0-lexer.lex.c
	 BISON dtc-parser.tab.c
make[1]: flex: Command not found
	 LEX dtc-lexer.lex.c
make[1]: bison: Command not found
make[1]: flex: Command not found
	 CC libfdt/fdt.o
	 CC libfdt/fdt_wip.o
	 CC libfdt/fdt_rw.o
	 CC libfdt/fdt_sw.o
	 CC libfdt/fdt_ro.o
	 CC libfdt/fdt_addresses.o
	 CC libfdt/fdt_empty_tree.o
	 CC libfdt/fdt_strerror.o
	 CC libfdt/fdt_overlay.o
	 AR libfdt/libfdt.a
ar: creating libfdt/libfdt.a
a - libfdt/fdt.o
a - libfdt/fdt_ro.o
a - libfdt/fdt_wip.o
a - libfdt/fdt_sw.o
a - libfdt/fdt_rw.o
a - libfdt/fdt_strerror.o
a - libfdt/fdt_empty_tree.o
a - libfdt/fdt_addresses.o
a - libfdt/fdt_overlay.o
	 LEX convert-dtsv0-lexer.lex.c
make[1]: flex: Command not found
	 LEX dtc-lexer.lex.c
	 BISON dtc-parser.tab.c
make[1]: flex: Command not found
make[1]: bison: Command not found
  GEN     qga/qapi-generated/qga-qapi-visit.h
  GEN     qga/qapi-generated/qga-qapi-types.h
  GEN     qga/qapi-generated/qga-qapi-visit.c
  CC      tests/qemu-iotests/socket_scm_helper.o
  GEN     qga/qapi-generated/qga-qapi-types.c
  CC      qmp-introspect.o
  GEN     qga/qapi-generated/qga-qmp-commands.h
  GEN     qga/qapi-generated/qga-qmp-marshal.c
  CC      qapi-types.o
  CC      qapi-visit.o
  CC      qapi-event.o
  CC      qapi/qapi-visit-core.o
  CC      qapi/qapi-dealloc-visitor.o
  CC      qapi/qobject-input-visitor.o
  CC      qapi/qmp-registry.o
  CC      qapi/qobject-output-visitor.o
  CC      qapi/qmp-dispatch.o
  CC      qapi/string-input-visitor.o
  CC      qapi/string-output-visitor.o
  CC      qapi/opts-visitor.o
  CC      qapi/qapi-clone-visitor.o
  CC      qapi/qmp-event.o
  CC      qapi/qapi-util.o
  CC      qobject/qnull.o
  CC      qobject/qstring.o
  CC      qobject/qint.o
  CC      qobject/qdict.o
  CC      qobject/qlist.o
  CC      qobject/qfloat.o
  CC      qobject/qbool.o
  CC      qobject/qjson.o
  CC      qobject/qobject.o
  CC      qobject/json-lexer.o
  CC      qobject/json-streamer.o
  CC      qobject/json-parser.o
  CC      trace/qmp.o
  CC      trace/control.o
  CC      util/osdep.o
  CC      util/cutils.o
  CC      util/unicode.o
  CC      util/qemu-timer-common.o
  CC      util/bufferiszero.o
  CC      util/aiocb.o
  CC      util/lockcnt.o
  CC      util/async.o
  CC      util/thread-pool.o
  CC      util/qemu-timer.o
  CC      util/main-loop.o
  CC      util/iohandler.o
  CC      util/aio-posix.o
  CC      util/compatfd.o
  CC      util/event_notifier-posix.o
  CC      util/mmap-alloc.o
  CC      util/oslib-posix.o
  CC      util/qemu-openpty.o
  CC      util/qemu-thread-posix.o
  CC      util/memfd.o
  CC      util/envlist.o
  CC      util/module.o
  CC      util/path.o
  CC      util/host-utils.o
  CC      util/bitmap.o
  CC      util/bitops.o
  CC      util/hbitmap.o
  CC      util/fifo8.o
  CC      util/acl.o
  CC      util/qemu-error.o
  CC      util/error.o
  CC      util/id.o
  CC      util/iov.o
  CC      util/qemu-config.o
  CC      util/qemu-sockets.o
  CC      util/uri.o
  CC      util/notify.o
  CC      util/qemu-option.o
  CC      util/qemu-progress.o
  CC      util/keyval.o
  CC      util/hexdump.o
  CC      util/crc32c.o
  CC      util/uuid.o
  CC      util/throttle.o
  CC      util/getauxval.o
  CC      util/readline.o
  CC      util/rcu.o
  CC      util/qemu-coroutine.o
  CC      util/qemu-coroutine-lock.o
  CC      util/qemu-coroutine-sleep.o
  CC      util/qemu-coroutine-io.o
  CC      util/buffer.o
  CC      util/coroutine-ucontext.o
  CC      util/timed-average.o
  CC      util/base64.o
  CC      util/log.o
  CC      util/qdist.o
  CC      util/qht.o
  CC      util/range.o
  CC      util/systemd.o
  CC      trace-root.o
  CC      crypto/trace.o
  CC      util/trace.o
  CC      io/trace.o
  CC      block/trace.o
  CC      migration/trace.o
  CC      backends/trace.o
  CC      hw/block/trace.o
  CC      hw/block/dataplane/trace.o
  CC      hw/char/trace.o
  CC      hw/intc/trace.o
  CC      hw/net/trace.o
  CC      hw/virtio/trace.o
  CC      hw/audio/trace.o
  CC      hw/misc/trace.o
  CC      hw/usb/trace.o
  CC      hw/scsi/trace.o
  CC      hw/nvram/trace.o
  CC      hw/display/trace.o
  CC      hw/input/trace.o
  CC      hw/timer/trace.o
  CC      hw/dma/trace.o
  CC      hw/sparc/trace.o
  CC      hw/sd/trace.o
  CC      hw/isa/trace.o
  CC      hw/mem/trace.o
  CC      hw/i386/trace.o
  CC      hw/i386/xen/trace.o
  CC      hw/9pfs/trace.o
  CC      hw/ppc/trace.o
  CC      hw/pci/trace.o
  CC      hw/s390x/trace.o
  CC      hw/vfio/trace.o
  CC      hw/acpi/trace.o
  CC      hw/alpha/trace.o
  CC      hw/arm/trace.o
  CC      hw/xen/trace.o
  CC      ui/trace.o
  CC      net/trace.o
  CC      audio/trace.o
  CC      target/arm/trace.o
  CC      target/i386/trace.o
  CC      target/mips/trace.o
  CC      target/sparc/trace.o
  CC      target/s390x/trace.o
  CC      target/ppc/trace.o
  CC      qom/trace.o
  CC      linux-user/trace.o
  CC      qapi/trace.o
  CC      crypto/pbkdf-stub.o
  CC      stubs/arch-query-cpu-def.o
  CC      stubs/arch-query-cpu-model-expansion.o
  CC      stubs/arch-query-cpu-model-comparison.o
  CC      stubs/arch-query-cpu-model-baseline.o
  CC      stubs/bdrv-next-monitor-owned.o
  CC      stubs/blk-commit-all.o
  CC      stubs/blockdev-close-all-bdrv-states.o
  CC      stubs/clock-warp.o
  CC      stubs/cpu-get-clock.o
  CC      stubs/cpu-get-icount.o
  CC      stubs/dump.o
  CC      stubs/error-printf.o
  CC      stubs/gdbstub.o
  CC      stubs/fdset.o
  CC      stubs/get-vm-name.o
  CC      stubs/iothread.o
  CC      stubs/iothread-lock.o
  CC      stubs/is-daemonized.o
  CC      stubs/machine-init-done.o
  CC      stubs/monitor.o
  CC      stubs/migr-blocker.o
  CC      stubs/notify-event.o
  CC      stubs/qtest.o
  CC      stubs/replay.o
  CC      stubs/runstate-check.o
  CC      stubs/set-fd-handler.o
  CC      stubs/slirp.o
  CC      stubs/sysbus.o
  CC      stubs/uuid.o
  CC      stubs/trace-control.o
  CC      stubs/vm-stop.o
  CC      stubs/qmp_pc_dimm_device_list.o
  CC      stubs/vmstate.o
  CC      stubs/target-monitor-defs.o
  CC      stubs/target-get-monitor-def.o
  CC      stubs/pc_madt_cpu_entry.o
  CC      stubs/vmgenid.o
  CC      stubs/xen-common.o
  CC      stubs/xen-hvm.o
  CC      contrib/ivshmem-client/ivshmem-client.o
  CC      contrib/ivshmem-client/main.o
  CC      contrib/ivshmem-server/ivshmem-server.o
  CC      contrib/ivshmem-server/main.o
  CC      qemu-nbd.o
  CC      block.o
  CC      blockjob.o
  CC      qemu-io-cmds.o
  CC      replication.o
  CC      block/raw-format.o
  CC      block/qcow.o
  CC      block/vdi.o
  CC      block/vmdk.o
  CC      block/cloop.o
  CC      block/bochs.o
  CC      block/vpc.o
  CC      block/vvfat.o
  CC      block/dmg.o
  CC      block/qcow2.o
  CC      block/qcow2-refcount.o
  CC      block/qcow2-cluster.o
  CC      block/qcow2-snapshot.o
  CC      block/qcow2-cache.o
  CC      block/qed.o
  CC      block/qed-gencb.o
  CC      block/qed-l2-cache.o
  CC      block/qed-table.o
  CC      block/qed-cluster.o
  CC      block/qed-check.o
  CC      block/vhdx.o
  CC      block/vhdx-endian.o
  CC      block/vhdx-log.o
  CC      block/quorum.o
  CC      block/parallels.o
  CC      block/blkdebug.o
  CC      block/blkverify.o
  CC      block/blkreplay.o
  CC      block/block-backend.o
  CC      block/snapshot.o
  CC      block/qapi.o
  CC      block/file-posix.o
  CC      block/null.o
  CC      block/mirror.o
  CC      block/commit.o
  CC      block/io.o
  CC      block/throttle-groups.o
  CC      block/nbd.o
  CC      block/nbd-client.o
  CC      block/sheepdog.o
  CC      block/accounting.o
  CC      block/dirty-bitmap.o
  CC      block/write-threshold.o
  CC      block/backup.o
  CC      block/crypto.o
  CC      block/replication.o
  CC      nbd/server.o
  CC      nbd/client.o
  CC      nbd/common.o
  CC      crypto/init.o
  CC      crypto/hash.o
  CC      crypto/hmac.o
  CC      crypto/hash-glib.o
  CC      crypto/hmac-glib.o
  CC      crypto/aes.o
  CC      crypto/desrfb.o
  CC      crypto/tlscreds.o
  CC      crypto/cipher.o
  CC      crypto/tlscredsanon.o
  CC      crypto/tlscredsx509.o
  CC      crypto/tlssession.o
  CC      crypto/secret.o
  CC      crypto/random-platform.o
  CC      crypto/pbkdf.o
  CC      crypto/ivgen-essiv.o
  CC      crypto/ivgen.o
  CC      crypto/ivgen-plain.o
  CC      crypto/ivgen-plain64.o
  CC      crypto/afsplit.o
  CC      crypto/xts.o
  CC      crypto/block.o
  CC      crypto/block-qcow.o
  CC      crypto/block-luks.o
  CC      io/channel.o
  CC      io/channel-buffer.o
  CC      io/channel-command.o
  CC      io/channel-file.o
  CC      io/channel-socket.o
  CC      io/channel-tls.o
  CC      io/channel-watch.o
  CC      io/channel-websock.o
  CC      io/channel-util.o
  CC      io/dns-resolver.o
  CC      io/task.o
  CC      qom/object.o
  CC      qom/container.o
  CC      qom/qom-qobject.o
  CC      qom/object_interfaces.o
  GEN     qemu-img-cmds.h
  CC      qemu-io.o
  CC      qemu-bridge-helper.o
  CC      blockdev.o
  CC      blockdev-nbd.o
  CC      iothread.o
  CC      qdev-monitor.o
  CC      device-hotplug.o
  CC      os-posix.o
  CC      page_cache.o
  CC      accel.o
  CC      bt-host.o
  CC      bt-vhci.o
  CC      dma-helpers.o
  CC      vl.o
  CC      tpm.o
  CC      device_tree.o
  CC      qmp-marshal.o
  CC      qmp.o
  CC      hmp.o
  CC      cpus-common.o
  CC      audio/audio.o
  CC      audio/noaudio.o
  CC      audio/wavaudio.o
  CC      audio/mixeng.o
  CC      audio/sdlaudio.o
  CC      audio/ossaudio.o
  CC      audio/wavcapture.o
  CC      backends/rng.o
  CC      backends/rng-egd.o
  CC      backends/rng-random.o
  CC      backends/msmouse.o
  CC      backends/wctablet.o
  CC      backends/testdev.o
  CC      backends/tpm.o
  CC      backends/hostmem.o
  CC      backends/hostmem-ram.o
  CC      backends/hostmem-file.o
  CC      backends/cryptodev.o
  CC      backends/cryptodev-builtin.o
  CC      block/stream.o
  CC      disas/arm.o
  CC      disas/i386.o
  CC      fsdev/qemu-fsdev-dummy.o
  CC      fsdev/qemu-fsdev-throttle.o
  CC      fsdev/qemu-fsdev-opts.o
  CC      hw/acpi/core.o
  CC      hw/acpi/piix4.o
  CC      hw/acpi/pcihp.o
  CC      hw/acpi/ich9.o
  CC      hw/acpi/tco.o
  CC      hw/acpi/cpu_hotplug.o
  CC      hw/acpi/memory_hotplug.o
  CC      hw/acpi/cpu.o
  CC      hw/acpi/nvdimm.o
  CC      hw/acpi/vmgenid.o
  CC      hw/acpi/acpi_interface.o
  CC      hw/acpi/bios-linker-loader.o
  CC      hw/acpi/aml-build.o
  CC      hw/acpi/ipmi.o
  CC      hw/acpi/acpi-stub.o
  CC      hw/acpi/ipmi-stub.o
  CC      hw/audio/sb16.o
  CC      hw/audio/es1370.o
  CC      hw/audio/ac97.o
  CC      hw/audio/fmopl.o
  CC      hw/audio/adlib.o
  CC      hw/audio/gus.o
  CC      hw/audio/gusemu_hal.o
  CC      hw/audio/gusemu_mixer.o
  CC      hw/audio/cs4231a.o
  CC      hw/audio/intel-hda.o
  CC      hw/audio/hda-codec.o
  CC      hw/audio/pcspk.o
  CC      hw/audio/wm8750.o
  CC      hw/audio/pl041.o
  CC      hw/audio/lm4549.o
  CC      hw/audio/marvell_88w8618.o
  CC      hw/block/block.o
  CC      hw/block/cdrom.o
  CC      hw/block/hd-geometry.o
  CC      hw/block/fdc.o
  CC      hw/block/m25p80.o
  CC      hw/block/nand.o
  CC      hw/block/pflash_cfi01.o
  CC      hw/block/pflash_cfi02.o
  CC      hw/block/ecc.o
  CC      hw/block/onenand.o
  CC      hw/block/nvme.o
  CC      hw/bt/core.o
  CC      hw/bt/l2cap.o
  CC      hw/bt/sdp.o
  CC      hw/bt/hci.o
  CC      hw/bt/hid.o
  CC      hw/bt/hci-csr.o
  CC      hw/char/ipoctal232.o
  CC      hw/char/parallel.o
  CC      hw/char/pl011.o
  CC      hw/char/serial.o
  CC      hw/char/serial-isa.o
  CC      hw/char/serial-pci.o
  CC      hw/char/virtio-console.o
  CC      hw/char/cadence_uart.o
  CC      hw/char/debugcon.o
  CC      hw/char/imx_serial.o
  CC      hw/core/qdev.o
  CC      hw/core/qdev-properties.o
  CC      hw/core/bus.o
  CC      hw/core/reset.o
  CC      hw/core/irq.o
  CC      hw/core/fw-path-provider.o
  CC      hw/core/hotplug.o
  CC      hw/core/ptimer.o
  CC      hw/core/sysbus.o
  CC      hw/core/machine.o
  CC      hw/core/loader.o
  CC      hw/core/qdev-properties-system.o
  CC      hw/core/register.o
  CC      hw/core/or-irq.o
  CC      hw/core/platform-bus.o
  CC      hw/display/ads7846.o
  CC      hw/display/cirrus_vga.o
  CC      hw/display/pl110.o
  CC      hw/display/ssd0303.o
  CC      hw/display/ssd0323.o
  CC      hw/display/vga-pci.o
  CC      hw/display/vga-isa.o
  CC      hw/display/vmware_vga.o
  CC      hw/display/blizzard.o
  CC      hw/display/exynos4210_fimd.o
  CC      hw/display/framebuffer.o
  CC      hw/display/tc6393xb.o
  CC      hw/dma/pl080.o
  CC      hw/dma/pl330.o
  CC      hw/dma/i8257.o
  CC      hw/dma/xlnx-zynq-devcfg.o
  CC      hw/gpio/max7310.o
  CC      hw/gpio/pl061.o
  CC      hw/gpio/zaurus.o
  CC      hw/gpio/gpio_key.o
  CC      hw/i2c/core.o
  CC      hw/i2c/smbus.o
  CC      hw/i2c/smbus_eeprom.o
  CC      hw/i2c/i2c-ddc.o
  CC      hw/i2c/versatile_i2c.o
  CC      hw/i2c/smbus_ich9.o
  CC      hw/i2c/pm_smbus.o
  CC      hw/i2c/bitbang_i2c.o
  CC      hw/i2c/exynos4210_i2c.o
  CC      hw/i2c/imx_i2c.o
  CC      hw/ide/core.o
  CC      hw/i2c/aspeed_i2c.o
  CC      hw/ide/atapi.o
  CC      hw/ide/qdev.o
  CC      hw/ide/pci.o
  CC      hw/ide/isa.o
  CC      hw/ide/piix.o
  CC      hw/ide/microdrive.o
  CC      hw/ide/ahci.o
  CC      hw/ide/ich.o
  CC      hw/input/hid.o
  CC      hw/input/lm832x.o
  CC      hw/input/pckbd.o
  CC      hw/input/pl050.o
  CC      hw/input/ps2.o
  CC      hw/input/stellaris_input.o
  CC      hw/input/tsc2005.o
  CC      hw/input/vmmouse.o
  CC      hw/input/virtio-input.o
  CC      hw/input/virtio-input-hid.o
  CC      hw/input/virtio-input-host.o
  CC      hw/intc/i8259_common.o
  CC      hw/intc/i8259.o
  CC      hw/intc/pl190.o
  CC      hw/intc/imx_avic.o
  CC      hw/intc/realview_gic.o
  CC      hw/intc/ioapic_common.o
  CC      hw/intc/arm_gic_common.o
  CC      hw/intc/arm_gic.o
  CC      hw/intc/arm_gicv2m.o
  CC      hw/intc/arm_gicv3_common.o
  CC      hw/intc/arm_gicv3.o
  CC      hw/intc/arm_gicv3_redist.o
  CC      hw/intc/arm_gicv3_dist.o
  CC      hw/intc/arm_gicv3_its_common.o
  CC      hw/intc/intc.o
  CC      hw/ipack/ipack.o
  CC      hw/ipack/tpci200.o
  CC      hw/ipmi/ipmi.o
  CC      hw/ipmi/ipmi_bmc_sim.o
  CC      hw/ipmi/ipmi_bmc_extern.o
  CC      hw/ipmi/isa_ipmi_kcs.o
  CC      hw/ipmi/isa_ipmi_bt.o
  CC      hw/isa/isa-bus.o
  CC      hw/isa/apm.o
  CC      hw/mem/pc-dimm.o
  CC      hw/mem/nvdimm.o
  CC      hw/misc/applesmc.o
  CC      hw/misc/max111x.o
  CC      hw/misc/tmp105.o
  CC      hw/misc/debugexit.o
  CC      hw/misc/sga.o
  CC      hw/misc/pc-testdev.o
  CC      hw/misc/pci-testdev.o
  CC      hw/misc/unimp.o
  CC      hw/misc/arm_l2x0.o
  CC      hw/misc/arm_integrator_debug.o
  CC      hw/misc/a9scu.o
  CC      hw/misc/arm11scu.o
  CC      hw/net/ne2000.o
  CC      hw/net/eepro100.o
  CC      hw/net/pcnet-pci.o
  CC      hw/net/pcnet.o
  CC      hw/net/e1000.o
  CC      hw/net/e1000x_common.o
  CC      hw/net/net_tx_pkt.o
  CC      hw/net/net_rx_pkt.o
  CC      hw/net/e1000e.o
  CC      hw/net/e1000e_core.o
  CC      hw/net/rtl8139.o
  CC      hw/net/vmxnet3.o
  CC      hw/net/smc91c111.o
  CC      hw/net/lan9118.o
  CC      hw/net/ne2000-isa.o
  CC      hw/net/xgmac.o
  CC      hw/net/allwinner_emac.o
  CC      hw/net/cadence_gem.o
  CC      hw/net/imx_fec.o
  CC      hw/net/stellaris_enet.o
  CC      hw/net/ftgmac100.o
  CC      hw/net/rocker/rocker.o
  CC      hw/net/rocker/rocker_fp.o
  CC      hw/net/rocker/rocker_desc.o
  CC      hw/net/rocker/rocker_world.o
  CC      hw/net/rocker/rocker_of_dpa.o
  CC      hw/nvram/eeprom93xx.o
  CC      hw/nvram/fw_cfg.o
  CC      hw/nvram/chrp_nvram.o
  CC      hw/pci-bridge/pci_bridge_dev.o
  CC      hw/pci-bridge/pcie_root_port.o
  CC      hw/pci-bridge/gen_pcie_root_port.o
  CC      hw/pci-bridge/pci_expander_bridge.o
  CC      hw/pci-bridge/xio3130_upstream.o
  CC      hw/pci-bridge/xio3130_downstream.o
  CC      hw/pci-bridge/ioh3420.o
  CC      hw/pci-bridge/i82801b11.o
  CC      hw/pci-host/pam.o
  CC      hw/pci-host/versatile.o
  CC      hw/pci-host/piix.o
  CC      hw/pci-host/q35.o
  CC      hw/pci-host/gpex.o
  CC      hw/pci/pci.o
  CC      hw/pci/pci_bridge.o
  CC      hw/pci/msix.o
  CC      hw/pci/msi.o
  CC      hw/pci/shpc.o
  CC      hw/pci/slotid_cap.o
  CC      hw/pci/pci_host.o
  CC      hw/pci/pcie_host.o
  CC      hw/pci/pcie.o
  CC      hw/pci/pcie_aer.o
  CC      hw/pci/pcie_port.o
  CC      hw/pci/pci-stub.o
  CC      hw/scsi/scsi-disk.o
  CC      hw/pcmcia/pcmcia.o
  CC      hw/scsi/scsi-generic.o
  CC      hw/scsi/scsi-bus.o
  CC      hw/scsi/lsi53c895a.o
  CC      hw/scsi/mptconfig.o
  CC      hw/scsi/mptsas.o
  CC      hw/scsi/mptendian.o
  CC      hw/scsi/megasas.o
  CC      hw/scsi/vmw_pvscsi.o
  CC      hw/scsi/esp.o
  CC      hw/scsi/esp-pci.o
  CC      hw/sd/pl181.o
  CC      hw/sd/ssi-sd.o
  CC      hw/sd/sd.o
  CC      hw/sd/core.o
  CC      hw/sd/sdhci.o
  CC      hw/smbios/smbios.o
  CC      hw/smbios/smbios_type_38.o
  CC      hw/smbios/smbios-stub.o
  CC      hw/smbios/smbios_type_38-stub.o
  CC      hw/ssi/pl022.o
  CC      hw/ssi/ssi.o
  CC      hw/ssi/xilinx_spips.o
  CC      hw/ssi/aspeed_smc.o
  CC      hw/ssi/stm32f2xx_spi.o
  CC      hw/timer/arm_timer.o
  CC      hw/timer/armv7m_systick.o
  CC      hw/timer/arm_mptimer.o
  CC      hw/timer/a9gtimer.o
  CC      hw/timer/cadence_ttc.o
  CC      hw/timer/ds1338.o
  CC      hw/timer/hpet.o
  CC      hw/timer/i8254_common.o
  CC      hw/timer/i8254.o
  CC      hw/timer/pl031.o
  CC      hw/timer/imx_gpt.o
  CC      hw/timer/imx_epit.o
  CC      hw/timer/twl92230.o
  CC      hw/timer/stm32f2xx_timer.o
  CC      hw/timer/aspeed_timer.o
  CC      hw/tpm/tpm_tis.o
  CC      hw/tpm/tpm_passthrough.o
  CC      hw/tpm/tpm_util.o
  CC      hw/usb/core.o
  CC      hw/usb/combined-packet.o
  CC      hw/usb/bus.o
  CC      hw/usb/libhw.o
  CC      hw/usb/desc.o
  CC      hw/usb/desc-msos.o
  CC      hw/usb/hcd-uhci.o
  CC      hw/usb/hcd-ohci.o
  CC      hw/usb/hcd-ehci.o
  CC      hw/usb/hcd-ehci-pci.o
  CC      hw/usb/hcd-ehci-sysbus.o
  CC      hw/usb/hcd-xhci.o
  CC      hw/usb/hcd-musb.o
  CC      hw/usb/dev-hub.o
  CC      hw/usb/dev-hid.o
  CC      hw/usb/dev-wacom.o
  CC      hw/usb/dev-storage.o
  CC      hw/usb/dev-uas.o
  CC      hw/usb/dev-audio.o
  CC      hw/usb/dev-serial.o
  CC      hw/usb/dev-network.o
  CC      hw/usb/dev-bluetooth.o
  CC      hw/usb/dev-smartcard-reader.o
  CC      hw/usb/dev-mtp.o
  CC      hw/usb/host-stub.o
  CC      hw/virtio/virtio-rng.o
  CC      hw/virtio/virtio-pci.o
  CC      hw/virtio/virtio-bus.o
  CC      hw/virtio/virtio-mmio.o
  CC      hw/virtio/vhost-stub.o
  CC      hw/watchdog/watchdog.o
  CC      hw/watchdog/wdt_i6300esb.o
  CC      hw/watchdog/wdt_ib700.o
  CC      hw/watchdog/wdt_aspeed.o
  CC      migration/migration.o
  CC      migration/socket.o
  CC      migration/fd.o
  CC      migration/exec.o
  CC      migration/tls.o
  CC      migration/colo-comm.o
  CC      migration/colo.o
  CC      migration/colo-failover.o
  CC      migration/vmstate.o
  CC      migration/qemu-file-channel.o
  CC      migration/qemu-file.o
  CC      migration/xbzrle.o
  CC      migration/postcopy-ram.o
  CC      migration/qjson.o
  CC      migration/block.o
  CC      net/queue.o
  CC      net/net.o
  CC      net/checksum.o
  CC      net/util.o
  CC      net/hub.o
  CC      net/socket.o
  CC      net/dump.o
  CC      net/eth.o
  CC      net/l2tpv3.o
  CC      net/tap.o
  CC      net/vhost-user.o
  CC      net/tap-linux.o
  CC      net/slirp.o
  CC      net/filter.o
  CC      net/filter-buffer.o
  CC      net/filter-mirror.o
  CC      net/colo-compare.o
  CC      net/colo.o
  CC      net/filter-rewriter.o
  CC      net/filter-replay.o
  CC      qom/cpu.o
  CC      replay/replay.o
  CC      replay/replay-internal.o
/tmp/qemu-test/src/replay/replay-internal.c: In function ‘replay_put_array’:
/tmp/qemu-test/src/replay/replay-internal.c:65: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result
  CC      replay/replay-events.o
  CC      replay/replay-time.o
  CC      replay/replay-input.o
  CC      replay/replay-char.o
  CC      replay/replay-snapshot.o
  CC      replay/replay-net.o
  CC      replay/replay-audio.o
  CC      slirp/cksum.o
  CC      slirp/if.o
  CC      slirp/ip_icmp.o
  CC      slirp/ip6_icmp.o
  CC      slirp/ip6_input.o
  CC      slirp/ip6_output.o
  CC      slirp/ip_input.o
  CC      slirp/ip_output.o
  CC      slirp/dnssearch.o
  CC      slirp/dhcpv6.o
  CC      slirp/slirp.o
  CC      slirp/mbuf.o
  CC      slirp/sbuf.o
  CC      slirp/socket.o
  CC      slirp/misc.o
  CC      slirp/tcp_input.o
  CC      slirp/tcp_output.o
  CC      slirp/tcp_subr.o
  CC      slirp/tcp_timer.o
/tmp/qemu-test/src/slirp/tcp_input.c: In function ‘tcp_input’:
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_p’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_len’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_tos’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_id’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_off’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_ttl’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_sum’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_src.s_addr’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_dst.s_addr’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:220: warning: ‘save_ip6.ip_nh’ may be used uninitialized in this function
  CC      slirp/udp.o
  CC      slirp/udp6.o
  CC      slirp/bootp.o
  CC      slirp/arp_table.o
  CC      slirp/tftp.o
  CC      slirp/ndp_table.o
  CC      slirp/ncsi.o
  CC      ui/keymaps.o
  CC      ui/console.o
  CC      ui/qemu-pixman.o
  CC      ui/cursor.o
  CC      ui/input.o
  CC      ui/input-keymap.o
  CC      ui/input-linux.o
  CC      ui/input-legacy.o
  CC      ui/sdl.o
  CC      ui/sdl_zoom.o
  CC      ui/x_keymap.o
  CC      ui/vnc.o
  CC      ui/vnc-enc-zlib.o
  CC      ui/vnc-enc-hextile.o
  CC      ui/vnc-enc-tight.o
  CC      ui/vnc-palette.o
  CC      ui/vnc-enc-zrle.o
  CC      ui/vnc-auth-vencrypt.o
  CC      ui/vnc-ws.o
  CC      ui/vnc-jobs.o
  CC      chardev/char.o
  CC      chardev/char-fd.o
  CC      chardev/char-file.o
  CC      chardev/char-io.o
  CC      chardev/char-mux.o
  CC      chardev/char-null.o
  CC      chardev/char-parallel.o
  CC      chardev/char-pipe.o
  CC      chardev/char-pty.o
  CC      chardev/char-ringbuf.o
  CC      chardev/char-serial.o
  CC      chardev/char-socket.o
  CC      chardev/char-stdio.o
  CC      chardev/char-udp.o
  LINK    tests/qemu-iotests/socket_scm_helper
  CC      qga/commands.o
  CC      qga/guest-agent-command-state.o
  CC      qga/main.o
  CC      qga/commands-posix.o
  CC      qga/channel-posix.o
  CC      qga/qapi-generated/qga-qapi-types.o
  CC      qga/qapi-generated/qga-qapi-visit.o
  CC      qga/qapi-generated/qga-qmp-marshal.o
  AR      libqemuutil.a
  AR      libqemustub.a
  AS      optionrom/multiboot.o
  AS      optionrom/linuxboot.o
  CC      optionrom/linuxboot_dma.o
cc: unrecognized option '-no-integrated-as'
cc: unrecognized option '-no-integrated-as'
  AS      optionrom/kvmvapic.o
  CC      qemu-img.o
  BUILD   optionrom/multiboot.img
  BUILD   optionrom/linuxboot_dma.img
  BUILD   optionrom/linuxboot_dma.raw
  BUILD   optionrom/linuxboot.img
  BUILD   optionrom/multiboot.raw
  BUILD   optionrom/kvmvapic.img
  BUILD   optionrom/linuxboot.raw
  BUILD   optionrom/kvmvapic.raw
  SIGN    optionrom/multiboot.bin
  SIGN    optionrom/linuxboot_dma.bin
  SIGN    optionrom/linuxboot.bin
  SIGN    optionrom/kvmvapic.bin
  LINK    ivshmem-client
  LINK    ivshmem-server
  LINK    qemu-nbd
  LINK    qemu-img
  LINK    qemu-io
  LINK    qemu-bridge-helper
  LINK    qemu-ga
  GEN     x86_64-softmmu/hmp-commands-info.h
  GEN     x86_64-softmmu/hmp-commands.h
  GEN     x86_64-softmmu/config-target.h
  CC      x86_64-softmmu/cpu-exec.o
  CC      x86_64-softmmu/exec.o
  CC      x86_64-softmmu/tcg/tcg.o
  CC      x86_64-softmmu/translate-all.o
  CC      x86_64-softmmu/tcg/tcg-op.o
  CC      x86_64-softmmu/translate-common.o
  CC      x86_64-softmmu/cpu-exec-common.o
  CC      x86_64-softmmu/tcg/optimize.o
  GEN     aarch64-softmmu/hmp-commands.h
  CC      x86_64-softmmu/tcg/tcg-common.o
  CC      x86_64-softmmu/fpu/softfloat.o
  CC      x86_64-softmmu/disas.o
  CC      x86_64-softmmu/tcg-runtime.o
  GEN     x86_64-softmmu/gdbstub-xml.c
  CC      x86_64-softmmu/hax-stub.o
  CC      x86_64-softmmu/arch_init.o
  CC      x86_64-softmmu/cpus.o
  CC      x86_64-softmmu/monitor.o
  CC      x86_64-softmmu/gdbstub.o
  CC      x86_64-softmmu/balloon.o
  GEN     aarch64-softmmu/hmp-commands-info.h
  GEN     aarch64-softmmu/config-target.h
  CC      aarch64-softmmu/exec.o
  CC      x86_64-softmmu/ioport.o
  CC      x86_64-softmmu/numa.o
  CC      aarch64-softmmu/translate-all.o
  CC      aarch64-softmmu/cpu-exec.o
  CC      x86_64-softmmu/qtest.o
  CC      x86_64-softmmu/bootdevice.o
  CC      aarch64-softmmu/cpu-exec-common.o
  CC      x86_64-softmmu/kvm-all.o
  CC      aarch64-softmmu/translate-common.o
  CC      x86_64-softmmu/memory.o
  CC      aarch64-softmmu/tcg/tcg.o
  CC      x86_64-softmmu/cputlb.o
  CC      x86_64-softmmu/memory_mapping.o
  CC      aarch64-softmmu/tcg/tcg-op.o
  CC      aarch64-softmmu/tcg/optimize.o
  CC      aarch64-softmmu/tcg/tcg-common.o
  CC      aarch64-softmmu/fpu/softfloat.o
  CC      aarch64-softmmu/disas.o
  CC      aarch64-softmmu/tcg-runtime.o
  CC      x86_64-softmmu/dump.o
  CC      x86_64-softmmu/migration/ram.o
  CC      x86_64-softmmu/migration/savevm.o
  CC      x86_64-softmmu/hw/block/virtio-blk.o
  CC      x86_64-softmmu/hw/block/dataplane/virtio-blk.o
  CC      x86_64-softmmu/hw/char/virtio-serial-bus.o
  GEN     aarch64-softmmu/gdbstub-xml.c
  CC      aarch64-softmmu/hax-stub.o
  CC      x86_64-softmmu/hw/core/nmi.o
  CC      x86_64-softmmu/hw/core/generic-loader.o
  CC      x86_64-softmmu/hw/cpu/core.o
  CC      x86_64-softmmu/hw/core/null-machine.o
  CC      x86_64-softmmu/hw/display/vga.o
  CC      x86_64-softmmu/hw/display/virtio-gpu.o
  CC      x86_64-softmmu/hw/display/virtio-gpu-3d.o
  CC      x86_64-softmmu/hw/display/virtio-gpu-pci.o
  CC      aarch64-softmmu/kvm-stub.o
  CC      x86_64-softmmu/hw/display/virtio-vga.o
  CC      x86_64-softmmu/hw/intc/apic.o
  CC      x86_64-softmmu/hw/intc/apic_common.o
  CC      aarch64-softmmu/arch_init.o
  CC      x86_64-softmmu/hw/intc/ioapic.o
  CC      aarch64-softmmu/cpus.o
  CC      x86_64-softmmu/hw/isa/lpc_ich9.o
  CC      x86_64-softmmu/hw/misc/vmport.o
  CC      aarch64-softmmu/monitor.o
  CC      x86_64-softmmu/hw/misc/ivshmem.o
  CC      aarch64-softmmu/gdbstub.o
  CC      x86_64-softmmu/hw/misc/pvpanic.o
  CC      aarch64-softmmu/balloon.o
  CC      aarch64-softmmu/ioport.o
  CC      x86_64-softmmu/hw/misc/edu.o
  CC      aarch64-softmmu/numa.o
  CC      x86_64-softmmu/hw/misc/hyperv_testdev.o
  CC      x86_64-softmmu/hw/net/virtio-net.o
  CC      x86_64-softmmu/hw/net/vhost_net.o
  CC      x86_64-softmmu/hw/scsi/virtio-scsi.o
  CC      aarch64-softmmu/qtest.o
  CC      x86_64-softmmu/hw/scsi/virtio-scsi-dataplane.o
  CC      aarch64-softmmu/bootdevice.o
  CC      x86_64-softmmu/hw/scsi/vhost-scsi-common.o
  CC      aarch64-softmmu/cputlb.o
  CC      aarch64-softmmu/memory_mapping.o
  CC      aarch64-softmmu/memory.o
  CC      x86_64-softmmu/hw/scsi/vhost-scsi.o
  CC      x86_64-softmmu/hw/timer/mc146818rtc.o
  CC      x86_64-softmmu/hw/vfio/common.o
  CC      x86_64-softmmu/hw/vfio/pci.o
  CC      aarch64-softmmu/dump.o
  CC      x86_64-softmmu/hw/vfio/pci-quirks.o
  CC      x86_64-softmmu/hw/vfio/platform.o
  CC      x86_64-softmmu/hw/vfio/spapr.o
  CC      x86_64-softmmu/hw/virtio/virtio.o
  CC      x86_64-softmmu/hw/virtio/virtio-balloon.o
  CC      x86_64-softmmu/hw/virtio/vhost.o
  CC      x86_64-softmmu/hw/virtio/vhost-backend.o
/tmp/qemu-test/src/hw/virtio/vhost-backend.c: In function ‘vhost_kernel_send_device_iotlb_msg’:
/tmp/qemu-test/src/hw/virtio/vhost-backend.c:213: error: unknown field ‘iotlb’ specified in initializer
/tmp/qemu-test/src/hw/virtio/vhost-backend.c:213: warning: missing braces around initializer
/tmp/qemu-test/src/hw/virtio/vhost-backend.c:213: warning: (near initialization for ‘msg.<anonymous>’)
make[1]: *** [hw/virtio/vhost-backend.o] Error 1
make[1]: *** Waiting for unfinished jobs....
  CC      aarch64-softmmu/migration/ram.o
  CC      aarch64-softmmu/migration/savevm.o
  CC      aarch64-softmmu/hw/adc/stm32f2xx_adc.o
  CC      aarch64-softmmu/hw/block/virtio-blk.o
  CC      aarch64-softmmu/hw/block/dataplane/virtio-blk.o
  CC      aarch64-softmmu/hw/char/exynos4210_uart.o
  CC      aarch64-softmmu/hw/char/digic-uart.o
  CC      aarch64-softmmu/hw/char/omap_uart.o
  CC      aarch64-softmmu/hw/char/stm32f2xx_usart.o
  CC      aarch64-softmmu/hw/char/bcm2835_aux.o
  CC      aarch64-softmmu/hw/char/virtio-serial-bus.o
  CC      aarch64-softmmu/hw/core/nmi.o
  CC      aarch64-softmmu/hw/core/null-machine.o
  CC      aarch64-softmmu/hw/core/generic-loader.o
  CC      aarch64-softmmu/hw/cpu/arm11mpcore.o
  CC      aarch64-softmmu/hw/cpu/realview_mpcore.o
  CC      aarch64-softmmu/hw/cpu/a9mpcore.o
  CC      aarch64-softmmu/hw/cpu/a15mpcore.o
  CC      aarch64-softmmu/hw/cpu/core.o
  CC      aarch64-softmmu/hw/display/omap_dss.o
  CC      aarch64-softmmu/hw/display/omap_lcdc.o
  CC      aarch64-softmmu/hw/display/pxa2xx_lcd.o
  CC      aarch64-softmmu/hw/display/bcm2835_fb.o
  CC      aarch64-softmmu/hw/display/vga.o
  CC      aarch64-softmmu/hw/display/virtio-gpu.o
  CC      aarch64-softmmu/hw/display/virtio-gpu-3d.o
  CC      aarch64-softmmu/hw/display/virtio-gpu-pci.o
  CC      aarch64-softmmu/hw/display/dpcd.o
  CC      aarch64-softmmu/hw/display/xlnx_dp.o
  CC      aarch64-softmmu/hw/dma/xlnx_dpdma.o
  CC      aarch64-softmmu/hw/dma/omap_dma.o
  CC      aarch64-softmmu/hw/dma/soc_dma.o
  CC      aarch64-softmmu/hw/dma/pxa2xx_dma.o
  CC      aarch64-softmmu/hw/dma/bcm2835_dma.o
  CC      aarch64-softmmu/hw/gpio/omap_gpio.o
  CC      aarch64-softmmu/hw/gpio/bcm2835_gpio.o
  CC      aarch64-softmmu/hw/gpio/imx_gpio.o
  CC      aarch64-softmmu/hw/i2c/omap_i2c.o
  CC      aarch64-softmmu/hw/input/pxa2xx_keypad.o
  CC      aarch64-softmmu/hw/input/tsc210x.o
  CC      aarch64-softmmu/hw/intc/armv7m_nvic.o
  CC      aarch64-softmmu/hw/intc/exynos4210_gic.o
  CC      aarch64-softmmu/hw/intc/exynos4210_combiner.o
  CC      aarch64-softmmu/hw/intc/omap_intc.o
  CC      aarch64-softmmu/hw/intc/bcm2835_ic.o
  CC      aarch64-softmmu/hw/intc/bcm2836_control.o
  CC      aarch64-softmmu/hw/intc/allwinner-a10-pic.o
  CC      aarch64-softmmu/hw/intc/aspeed_vic.o
  CC      aarch64-softmmu/hw/intc/arm_gicv3_cpuif.o
  CC      aarch64-softmmu/hw/misc/ivshmem.o
  CC      aarch64-softmmu/hw/misc/arm_sysctl.o
  CC      aarch64-softmmu/hw/misc/cbus.o
  CC      aarch64-softmmu/hw/misc/exynos4210_pmu.o
  CC      aarch64-softmmu/hw/misc/exynos4210_clk.o
  CC      aarch64-softmmu/hw/misc/imx_ccm.o
  CC      aarch64-softmmu/hw/misc/imx31_ccm.o
  CC      aarch64-softmmu/hw/misc/imx25_ccm.o
  CC      aarch64-softmmu/hw/misc/imx6_ccm.o
  CC      aarch64-softmmu/hw/misc/imx6_src.o
make: *** [subdir-x86_64-softmmu] Error 2
make: *** Waiting for unfinished jobs....
  CC      aarch64-softmmu/hw/misc/mst_fpga.o
  CC      aarch64-softmmu/hw/misc/omap_clk.o
  CC      aarch64-softmmu/hw/misc/omap_gpmc.o
  CC      aarch64-softmmu/hw/misc/omap_l4.o
  CC      aarch64-softmmu/hw/misc/omap_sdrc.o
  CC      aarch64-softmmu/hw/misc/omap_tap.o
  CC      aarch64-softmmu/hw/misc/bcm2835_mbox.o
  CC      aarch64-softmmu/hw/misc/bcm2835_property.o
  CC      aarch64-softmmu/hw/misc/bcm2835_rng.o
  CC      aarch64-softmmu/hw/misc/stm32f2xx_syscfg.o
  CC      aarch64-softmmu/hw/misc/zynq_slcr.o
  CC      aarch64-softmmu/hw/misc/zynq-xadc.o
  CC      aarch64-softmmu/hw/misc/edu.o
  CC      aarch64-softmmu/hw/misc/auxbus.o
  CC      aarch64-softmmu/hw/misc/aspeed_scu.o
  CC      aarch64-softmmu/hw/misc/aspeed_sdmc.o
  CC      aarch64-softmmu/hw/net/virtio-net.o
  CC      aarch64-softmmu/hw/pcmcia/pxa2xx.o
  CC      aarch64-softmmu/hw/net/vhost_net.o
  CC      aarch64-softmmu/hw/scsi/virtio-scsi.o
  CC      aarch64-softmmu/hw/scsi/virtio-scsi-dataplane.o
  CC      aarch64-softmmu/hw/scsi/vhost-scsi.o
  CC      aarch64-softmmu/hw/scsi/vhost-scsi-common.o
  CC      aarch64-softmmu/hw/sd/omap_mmc.o
  CC      aarch64-softmmu/hw/sd/pxa2xx_mmci.o
  CC      aarch64-softmmu/hw/sd/bcm2835_sdhost.o
  CC      aarch64-softmmu/hw/ssi/omap_spi.o
  CC      aarch64-softmmu/hw/timer/exynos4210_mct.o
  CC      aarch64-softmmu/hw/ssi/imx_spi.o
  CC      aarch64-softmmu/hw/timer/exynos4210_pwm.o
  CC      aarch64-softmmu/hw/timer/exynos4210_rtc.o
  CC      aarch64-softmmu/hw/timer/omap_gptimer.o
  CC      aarch64-softmmu/hw/timer/omap_synctimer.o
  CC      aarch64-softmmu/hw/timer/pxa2xx_timer.o
  CC      aarch64-softmmu/hw/timer/digic-timer.o
  CC      aarch64-softmmu/hw/timer/allwinner-a10-pit.o
  CC      aarch64-softmmu/hw/usb/tusb6010.o
  CC      aarch64-softmmu/hw/vfio/common.o
  CC      aarch64-softmmu/hw/vfio/pci.o
  CC      aarch64-softmmu/hw/vfio/amd-xgbe.o
  CC      aarch64-softmmu/hw/vfio/pci-quirks.o
  CC      aarch64-softmmu/hw/vfio/platform.o
  CC      aarch64-softmmu/hw/vfio/calxeda-xgmac.o
  CC      aarch64-softmmu/hw/vfio/spapr.o
  CC      aarch64-softmmu/hw/virtio/virtio.o
  CC      aarch64-softmmu/hw/virtio/virtio-balloon.o
  CC      aarch64-softmmu/hw/virtio/vhost.o
  CC      aarch64-softmmu/hw/virtio/vhost-backend.o
  CC      aarch64-softmmu/hw/virtio/vhost-user.o
  CC      aarch64-softmmu/hw/virtio/vhost-vsock.o
  CC      aarch64-softmmu/hw/virtio/virtio-crypto.o
  CC      aarch64-softmmu/hw/virtio/virtio-crypto-pci.o
  CC      aarch64-softmmu/hw/arm/boot.o
  CC      aarch64-softmmu/hw/arm/collie.o
  CC      aarch64-softmmu/hw/arm/exynos4_boards.o
/tmp/qemu-test/src/hw/virtio/vhost-backend.c: In function ‘vhost_kernel_send_device_iotlb_msg’:
/tmp/qemu-test/src/hw/virtio/vhost-backend.c:213: error: unknown field ‘iotlb’ specified in initializer
/tmp/qemu-test/src/hw/virtio/vhost-backend.c:213: warning: missing braces around initializer
/tmp/qemu-test/src/hw/virtio/vhost-backend.c:213: warning: (near initialization for ‘msg.<anonymous>’)
make[1]: *** [hw/virtio/vhost-backend.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [subdir-aarch64-softmmu] Error 2
tests/docker/Makefile.include:118: recipe for target 'docker-run' failed
make[1]: *** [docker-run] Error 2
make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-d_8o6qsf/src'
tests/docker/Makefile.include:149: recipe for target 'docker-run-test-quick@centos6' failed
make: *** [docker-run-test-quick@centos6] Error 2
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

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

* Re: [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier Maxime Coquelin
@ 2017-05-11 17:33   ` Michael S. Tsirkin
  2017-05-12 11:21     ` Maxime Coquelin
  0 siblings, 1 reply; 32+ messages in thread
From: Michael S. Tsirkin @ 2017-05-11 17:33 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman

On Thu, May 11, 2017 at 02:32:43PM +0200, Maxime Coquelin wrote:
> Vhost-kernel backend need to receive IOTLB entries for rings
> information early, but vhost-user need the same information
> earlier, before VHOST_USER_SET_VRING_ADDR is sent.

Weird. What does VHOST_USER_SET_VRING_ADDR have to do with it?

According to
	Starting and stopping rings
in vhost user spec, vhost user does not access
anything until ring is started and enabled.


> This patch also trigger IOTLB miss for all rings informations
> for robustness, even if in practice these adresses are on the
> same page.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  hw/virtio/vhost.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 748e331..817f6d0 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -799,7 +799,17 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
>          .log_guest_addr = vq->used_phys,
>          .flags = enable_log ? (1 << VHOST_VRING_F_LOG) : 0,
>      };
> -    int r = dev->vhost_ops->vhost_set_vring_addr(dev, &addr);
> +    int r;
> +
> +    /* Update rings information for IOTLB to work correctly,
> +     * vhost-kernel & vhost-user backends require for this.*/

Any requirements must be in the spec. Pls add them there.
Pls fix comment style as you move code.


> +    if (vhost_dev_has_iommu(dev)) {
> +        vhost_device_iotlb_miss(dev, addr.desc_user_addr, true);
> +        vhost_device_iotlb_miss(dev, addr.used_user_addr, true);
> +        vhost_device_iotlb_miss(dev, addr.avail_user_addr, true);
> +    }
> +
> +    r = dev->vhost_ops->vhost_set_vring_addr(dev, &addr);
>      if (r < 0) {
>          VHOST_OPS_DEBUG("vhost_set_vring_addr failed");
>          return -errno;
> @@ -1551,13 +1561,6 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
>  
>      if (vhost_dev_has_iommu(hdev)) {
>          hdev->vhost_ops->vhost_set_iotlb_callback(hdev, true);
> -
> -        /* 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:
> -- 
> 2.9.3

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support Maxime Coquelin
@ 2017-05-11 18:25   ` Michael S. Tsirkin
  2017-05-12 14:21     ` Maxime Coquelin
  2017-05-17 16:48   ` Michael S. Tsirkin
  1 sibling, 1 reply; 32+ messages in thread
From: Michael S. Tsirkin @ 2017-05-11 18:25 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman

On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote:
> This patch specifies and implements the master/slave communication
> to support device IOTLB in slave.
> 
> The vhost_iotlb_msg structure introduced for kernel backends is
> re-used, making the design close between the two backends.
> 
> An exception is the use of the secondary channel to enable the
> slave to send IOTLB miss requests to the master.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  docs/specs/vhost-user.txt | 75 +++++++++++++++++++++++++++++++++++++++++++++++
>  hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
>  2 files changed, 106 insertions(+)
> 
> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> index 5fa7016..4a1f0c3 100644
> --- a/docs/specs/vhost-user.txt
> +++ b/docs/specs/vhost-user.txt
> @@ -97,6 +97,23 @@ Depending on the request type, payload can be:
>     log offset: offset from start of supplied file descriptor
>         where logging starts (i.e. where guest address 0 would be logged)
>  
> + * An IOTLB message
> +   ---------------------------------------------------------
> +   | iova | size | user address | permissions flags | type |
> +   ---------------------------------------------------------
> +
> +   IOVA: a 64-bit guest I/O virtual address

guest -> VM

> +   Size: a 64-bit size

How do you specify "all memory"? give special meaning to size 0?

> +   User address: a 64-bit user address
> +   Permissions flags: a 8-bit bit field:
> +    - Bit 0: Read access
> +    - Bit 1: Write access

Can both bits be set? Can none?

> +   Type: a 8-bit IOTLB message type:
> +    - 1: IOTLB miss
> +    - 2: IOTLB update
> +    - 3: IOTLB invalidate
> +    - 4: IOTLB access fail
> +
>  In QEMU the vhost-user message is implemented with the following struct:
>  
>  typedef struct VhostUserMsg {
> @@ -109,6 +126,7 @@ typedef struct VhostUserMsg {
>          struct vhost_vring_addr addr;
>          VhostUserMemory memory;
>          VhostUserLog log;
> +        struct vhost_iotlb_msg iotlb;
>      };
>  } QEMU_PACKED VhostUserMsg;
>  
> @@ -253,6 +271,31 @@ Once the source has finished migration, rings will be stopped by
>  the source. No further update must be done before rings are
>  restarted.
>  
> +IOMMU support
> +-------------
> +
> +When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated, the master has
> +to send IOTLB entries update & invalidation by sending VHOST_USER_IOTLB_MSG
> +requests to the slave with a struct vhost_iotlb_msg payload.

Always? This seems a bit strange since iommu can be enabled/disabled
dynamically. Closing channel seems like a wrong thing to do for this.

> For update events,
> +the iotlb payload has to be filled with the update message type (2), the I/O
> +virtual address, the size, the user virtual address, and the permissions
> +flags. For invalidation events, the iotlb payload has to be filled with the
> +invalidation message type (3), the I/O virtual address and the size. On
> +success, the slave is expected to reply with a zero payload, non-zero
> +otherwise.
> +
> +When the VHOST_USER_PROTOCOL_F_SLAVE_REQ is supported by the slave, and the
> +master initiated the slave to master communication channel using the
> +VHOST_USER_SET_SLAVE_REQ_FD request, the slave can send IOTLB miss and access
> +failure events by sending VHOST_USER_SLAVE_IOTLB_MSG requests to the master
> +with a struct vhost_iotlb_msg payload. For miss events, the iotlb payload has
> +to be filled with the miss message type (1), the I/O virtual address and the
> +permissions flags. For access failure event, the iotlb payload has to be
> +filled with the access failure message type (4), the I/O virtual address and
> +the permissions flags. For synchronization purpose, the slave may rely on the
> +reply-ack feature, so the master may send a reply when operation is completed
> +if the reply-ack feature is negotiated and slaves requests a reply.
> +
>  Slave communication
>  -------------------
>  
> @@ -514,6 +557,38 @@ Master message types
>        If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, slave must respond
>        with zero for success, non-zero otherwise.
>  
> + * VHOST_USER_IOTLB_MSG
> +
> +      Id: 22
> +      Equivalent ioctl: N/A (equivalent to VHOST_IOTLB_MSG message type)
> +      Master payload: struct vhost_iotlb_msg
> +      Slave payload: u64
> +
> +      Send IOTLB messages with struct vhost_iotlb_msg as payload.
> +      Master sends such requests to update and invalidate entries in the device
> +      IOTLB. The slave has to acknowledge the request with sending zero as u64
> +      payload for success, non-zero otherwise.
> +      This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature
> +      has been successfully negotiated.
> +
> +Slave message types
> +-------------------
> +
> + * VHOST_USER_SLAVE_IOTLB_MSG
> +
> +      Id: 1
> +      Equivalent ioctl: N/A (equivalent to VHOST_IOTLB_MSG message type)
> +      Slave payload: struct vhost_iotlb_msg
> +      Master payload: N/A
> +
> +      Send IOTLB messages with struct vhost_iotlb_msg as payload.
> +      Slave sends such requests to notify of an IOTLB miss, or an IOTLB
> +      access failure. If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated,
> +      and slave set the VHOST_USER_NEED_REPLY flag, master must respond with
> +      zero when operation is successfully completed, or non-zero otherwise.
> +      This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature
> +      has been successfully negotiated.
> +

Are there limitations on number of messages in flight?


>  VHOST_USER_PROTOCOL_F_REPLY_ACK:
>  -------------------------------
>  The original vhost-user specification only demands replies for certain
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index fbc09fa..2c93181 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -63,11 +63,13 @@ typedef enum VhostUserRequest {
>      VHOST_USER_SEND_RARP = 19,
>      VHOST_USER_NET_SET_MTU = 20,
>      VHOST_USER_SET_SLAVE_REQ_FD = 21,
> +    VHOST_USER_IOTLB_MSG = 22,
>      VHOST_USER_MAX
>  } VhostUserRequest;
>  
>  typedef enum VhostUserSlaveRequest {
>      VHOST_USER_SLAVE_NONE = 0,
> +    VHOST_USER_SLAVE_IOTLB_MSG = 1,
>      VHOST_USER_SLAVE_MAX
>  }  VhostUserSlaveRequest;
>  
> @@ -105,6 +107,7 @@ typedef struct VhostUserMsg {
>          struct vhost_vring_addr addr;
>          VhostUserMemory memory;
>          VhostUserLog log;
> +        struct vhost_iotlb_msg iotlb;
>      } payload;
>  } QEMU_PACKED VhostUserMsg;
>  
> @@ -611,6 +614,9 @@ static void slave_read(void *opaque)
>      }
>  
>      switch (msg.request) {
> +    case VHOST_USER_SLAVE_IOTLB_MSG:
> +        ret = vhost_backend_handle_iotlb_msg(dev, &msg.payload.iotlb);
> +        break;
>      default:
>          error_report("Received unexpected msg type.");
>          ret = -EINVAL;
> @@ -858,6 +864,29 @@ static int vhost_user_net_set_mtu(struct vhost_dev *dev, uint16_t mtu)
>      return 0;
>  }
>  
> +static int vhost_user_send_device_iotlb_msg(struct vhost_dev *dev,
> +                                            struct vhost_iotlb_msg *imsg)
> +{
> +    VhostUserMsg msg = {
> +        .request = VHOST_USER_IOTLB_MSG,
> +        .size = sizeof(msg.payload.iotlb),
> +        .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
> +        .payload.iotlb = *imsg,
> +    };
> +
> +    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> +        return -EFAULT;
> +    }
> +
> +    return process_message_reply(dev, msg.request);
> +}
> +
> +
> +static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled)
> +{
> +    /* No-op as the receive channel is not dedicated to IOTLB messages. */
> +}
> +
>  const VhostOps user_ops = {
>          .backend_type = VHOST_BACKEND_TYPE_USER,
>          .vhost_backend_init = vhost_user_init,
> @@ -882,4 +911,6 @@ const VhostOps user_ops = {
>          .vhost_migration_done = vhost_user_migration_done,
>          .vhost_backend_can_merge = vhost_user_can_merge,
>          .vhost_net_set_mtu = vhost_user_net_set_mtu,
> +        .vhost_set_iotlb_callback = vhost_user_set_iotlb_callback,
> +        .vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg,
>  };
> -- 
> 2.9.3

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

* Re: [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier
  2017-05-11 17:33   ` Michael S. Tsirkin
@ 2017-05-12 11:21     ` Maxime Coquelin
  2017-05-17 16:41       ` Michael S. Tsirkin
  0 siblings, 1 reply; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-12 11:21 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman



On 05/11/2017 07:33 PM, Michael S. Tsirkin wrote:
> On Thu, May 11, 2017 at 02:32:43PM +0200, Maxime Coquelin wrote:
>> Vhost-kernel backend need to receive IOTLB entries for rings
>> information early, but vhost-user need the same information
>> earlier, before VHOST_USER_SET_VRING_ADDR is sent.
> 
> Weird. What does VHOST_USER_SET_VRING_ADDR have to do with it?
> 
> According to
> 	Starting and stopping rings
> in vhost user spec, vhost user does not access
> anything until ring is started and enabled.
> 
> 
>> This patch also trigger IOTLB miss for all rings informations
>> for robustness, even if in practice these adresses are on the
>> same page.

Actually, the DPDK vhost-user backend is compliant with the spec,
but when handling VHOST_USER_SET_VRING_ADDR request, it translates the
guest addresses into backend VAs, and check they are valid. I will make 
the commit message clearer about this in next revision.

The check could be done later, for example when the ring are started,
but it wouldn't change the need to trigger a miss at some point.

Or it could be done in the processing threads, the first time the
addresses are used, but it would add a check for every burst of packets
processed.

>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>   hw/virtio/vhost.c | 19 +++++++++++--------
>>   1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index 748e331..817f6d0 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -799,7 +799,17 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
>>           .log_guest_addr = vq->used_phys,
>>           .flags = enable_log ? (1 << VHOST_VRING_F_LOG) : 0,
>>       };
>> -    int r = dev->vhost_ops->vhost_set_vring_addr(dev, &addr);
>> +    int r;
>> +
>> +    /* Update rings information for IOTLB to work correctly,
>> +     * vhost-kernel & vhost-user backends require for this.*/
> 
> Any requirements must be in the spec. Pls add them there.
> Pls fix comment style as you move code.
Sure.

Thanks,
Maxime

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-11 18:25   ` Michael S. Tsirkin
@ 2017-05-12 14:21     ` Maxime Coquelin
  2017-05-13  0:02       ` Michael S. Tsirkin
  2017-05-16  8:19       ` Maxime Coquelin
  0 siblings, 2 replies; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-12 14:21 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman



On 05/11/2017 08:25 PM, Michael S. Tsirkin wrote:
> On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote:
>> This patch specifies and implements the master/slave communication
>> to support device IOTLB in slave.
>>
>> The vhost_iotlb_msg structure introduced for kernel backends is
>> re-used, making the design close between the two backends.
>>
>> An exception is the use of the secondary channel to enable the
>> slave to send IOTLB miss requests to the master.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>   docs/specs/vhost-user.txt | 75 +++++++++++++++++++++++++++++++++++++++++++++++
>>   hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
>>   2 files changed, 106 insertions(+)
>>
>> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>> index 5fa7016..4a1f0c3 100644
>> --- a/docs/specs/vhost-user.txt
>> +++ b/docs/specs/vhost-user.txt
>> @@ -97,6 +97,23 @@ Depending on the request type, payload can be:
>>      log offset: offset from start of supplied file descriptor
>>          where logging starts (i.e. where guest address 0 would be logged)
>>   
>> + * An IOTLB message
>> +   ---------------------------------------------------------
>> +   | iova | size | user address | permissions flags | type |
>> +   ---------------------------------------------------------
>> +
>> +   IOVA: a 64-bit guest I/O virtual address
> 
> guest -> VM

Ok.

> 
>> +   Size: a 64-bit size
> 
> How do you specify "all memory"? give special meaning to size 0?

Good point, it does not support all memory currently.
It is not vhost-user specific, but general to the vhost implementation.

>> +   User address: a 64-bit user address
>> +   Permissions flags: a 8-bit bit field:
>> +    - Bit 0: Read access
>> +    - Bit 1: Write access
> 
> Can both bits be set? Can none?

Both. I will change it by listing values directly:
  - 0 : No access
  - 1 : Read
  - 2 : Write
  - 3 : Read Write

>> +   Type: a 8-bit IOTLB message type:
>> +    - 1: IOTLB miss
>> +    - 2: IOTLB update
>> +    - 3: IOTLB invalidate
>> +    - 4: IOTLB access fail
>> +
>>   In QEMU the vhost-user message is implemented with the following struct:
>>   
>>   typedef struct VhostUserMsg {
>> @@ -109,6 +126,7 @@ typedef struct VhostUserMsg {
>>           struct vhost_vring_addr addr;
>>           VhostUserMemory memory;
>>           VhostUserLog log;
>> +        struct vhost_iotlb_msg iotlb;
>>       };
>>   } QEMU_PACKED VhostUserMsg;
>>   
>> @@ -253,6 +271,31 @@ Once the source has finished migration, rings will be stopped by
>>   the source. No further update must be done before rings are
>>   restarted.
>>   
>> +IOMMU support
>> +-------------
>> +
>> +When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated, the master has
>> +to send IOTLB entries update & invalidation by sending VHOST_USER_IOTLB_MSG
>> +requests to the slave with a struct vhost_iotlb_msg payload.
> 
> Always? This seems a bit strange since iommu can be enabled/disabled
> dynamically.
Ok, what about:

When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated and iommu
is enbaled, the master sends IOTLB entries update & invalidation via
VHOST_USER_IOTLB_MSG requests to the slave with a struct vhost_iotlb_msg
payload.


> Closing channel seems like a wrong thing to do for this.

Sorry, I'm not sure to get your comment.

>> For update events,
>> +the iotlb payload has to be filled with the update message type (2), the I/O
>> +virtual address, the size, the user virtual address, and the permissions
>> +flags. For invalidation events, the iotlb payload has to be filled with the
>> +invalidation message type (3), the I/O virtual address and the size. On
>> +success, the slave is expected to reply with a zero payload, non-zero
>> +otherwise.
>> +
>> +When the VHOST_USER_PROTOCOL_F_SLAVE_REQ is supported by the slave, and the
>> +master initiated the slave to master communication channel using the
>> +VHOST_USER_SET_SLAVE_REQ_FD request, the slave can send IOTLB miss and access
>> +failure events by sending VHOST_USER_SLAVE_IOTLB_MSG requests to the master
>> +with a struct vhost_iotlb_msg payload. For miss events, the iotlb payload has
>> +to be filled with the miss message type (1), the I/O virtual address and the
>> +permissions flags. For access failure event, the iotlb payload has to be
>> +filled with the access failure message type (4), the I/O virtual address and
>> +the permissions flags. For synchronization purpose, the slave may rely on the
>> +reply-ack feature, so the master may send a reply when operation is completed
>> +if the reply-ack feature is negotiated and slaves requests a reply.
>> +
>>   Slave communication
>>   -------------------
>>   
>> @@ -514,6 +557,38 @@ Master message types
>>         If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, slave must respond
>>         with zero for success, non-zero otherwise.
>>   
>> + * VHOST_USER_IOTLB_MSG
>> +
>> +      Id: 22
>> +      Equivalent ioctl: N/A (equivalent to VHOST_IOTLB_MSG message type)
>> +      Master payload: struct vhost_iotlb_msg
>> +      Slave payload: u64
>> +
>> +      Send IOTLB messages with struct vhost_iotlb_msg as payload.
>> +      Master sends such requests to update and invalidate entries in the device
>> +      IOTLB. The slave has to acknowledge the request with sending zero as u64
>> +      payload for success, non-zero otherwise.
>> +      This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature
>> +      has been successfully negotiated.
>> +
>> +Slave message types
>> +-------------------
>> +
>> + * VHOST_USER_SLAVE_IOTLB_MSG
>> +
>> +      Id: 1
>> +      Equivalent ioctl: N/A (equivalent to VHOST_IOTLB_MSG message type)
>> +      Slave payload: struct vhost_iotlb_msg
>> +      Master payload: N/A
>> +
>> +      Send IOTLB messages with struct vhost_iotlb_msg as payload.
>> +      Slave sends such requests to notify of an IOTLB miss, or an IOTLB
>> +      access failure. If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated,
>> +      and slave set the VHOST_USER_NEED_REPLY flag, master must respond with
>> +      zero when operation is successfully completed, or non-zero otherwise.
>> +      This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature
>> +      has been successfully negotiated.
>> +
> 
> Are there limitations on number of messages in flight?

I didn't think about this, I would say the maximum number of messages in
flight is dependent on the socket buffer size (which is kept to default
in this series).

You question highlights a bug in by DPDK prototype, as the MISS request
can be sent by multiple threads, and I didn't protected this with a lock
to prevent concurrent read on the socket when waiting for the REPLY_ACK.

Thanks,
Maxime

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-12 14:21     ` Maxime Coquelin
@ 2017-05-13  0:02       ` Michael S. Tsirkin
  2017-05-15  5:45         ` Jason Wang
  2017-05-17 15:27         ` Maxime Coquelin
  2017-05-16  8:19       ` Maxime Coquelin
  1 sibling, 2 replies; 32+ messages in thread
From: Michael S. Tsirkin @ 2017-05-13  0:02 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman

On Fri, May 12, 2017 at 04:21:58PM +0200, Maxime Coquelin wrote:
> 
> 
> On 05/11/2017 08:25 PM, Michael S. Tsirkin wrote:
> > On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote:
> > > This patch specifies and implements the master/slave communication
> > > to support device IOTLB in slave.
> > > 
> > > The vhost_iotlb_msg structure introduced for kernel backends is
> > > re-used, making the design close between the two backends.
> > > 
> > > An exception is the use of the secondary channel to enable the
> > > slave to send IOTLB miss requests to the master.
> > > 
> > > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> > > ---
> > >   docs/specs/vhost-user.txt | 75 +++++++++++++++++++++++++++++++++++++++++++++++
> > >   hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
> > >   2 files changed, 106 insertions(+)
> > > 
> > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > > index 5fa7016..4a1f0c3 100644
> > > --- a/docs/specs/vhost-user.txt
> > > +++ b/docs/specs/vhost-user.txt
> > > @@ -97,6 +97,23 @@ Depending on the request type, payload can be:
> > >      log offset: offset from start of supplied file descriptor
> > >          where logging starts (i.e. where guest address 0 would be logged)
> > > + * An IOTLB message
> > > +   ---------------------------------------------------------
> > > +   | iova | size | user address | permissions flags | type |
> > > +   ---------------------------------------------------------
> > > +
> > > +   IOVA: a 64-bit guest I/O virtual address
> > 
> > guest -> VM
> 
> Ok.
> 
> > 
> > > +   Size: a 64-bit size
> > 
> > How do you specify "all memory"? give special meaning to size 0?
> 
> Good point, it does not support all memory currently.
> It is not vhost-user specific, but general to the vhost implementation.

But iommu needs it to support passthrough.

> 
> > > +   User address: a 64-bit user address
> > > +   Permissions flags: a 8-bit bit field:
> > > +    - Bit 0: Read access
> > > +    - Bit 1: Write access
> > 
> > Can both bits be set? Can none?
> 
> Both. I will change it by listing values directly:
>  - 0 : No access
>  - 1 : Read
>  - 2 : Write
>  - 3 : Read Write
> 
> > > +   Type: a 8-bit IOTLB message type:
> > > +    - 1: IOTLB miss
> > > +    - 2: IOTLB update
> > > +    - 3: IOTLB invalidate
> > > +    - 4: IOTLB access fail
> > > +
> > >   In QEMU the vhost-user message is implemented with the following struct:
> > >   typedef struct VhostUserMsg {
> > > @@ -109,6 +126,7 @@ typedef struct VhostUserMsg {
> > >           struct vhost_vring_addr addr;
> > >           VhostUserMemory memory;
> > >           VhostUserLog log;
> > > +        struct vhost_iotlb_msg iotlb;
> > >       };
> > >   } QEMU_PACKED VhostUserMsg;
> > > @@ -253,6 +271,31 @@ Once the source has finished migration, rings will be stopped by
> > >   the source. No further update must be done before rings are
> > >   restarted.
> > > +IOMMU support
> > > +-------------
> > > +
> > > +When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated, the master has
> > > +to send IOTLB entries update & invalidation by sending VHOST_USER_IOTLB_MSG
> > > +requests to the slave with a struct vhost_iotlb_msg payload.
> > 
> > Always? This seems a bit strange since iommu can be enabled/disabled
> > dynamically.
> Ok, what about:
> 
> When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated and iommu
> is enbaled, the master sends IOTLB entries update & invalidation via
> VHOST_USER_IOTLB_MSG requests to the slave with a struct vhost_iotlb_msg
> payload.
> 
> 
> > Closing channel seems like a wrong thing to do for this.
> 
> Sorry, I'm not sure to get your comment.

What happens when guest disables the IOMMU?

> > > For update events,
> > > +the iotlb payload has to be filled with the update message type (2), the I/O
> > > +virtual address, the size, the user virtual address, and the permissions
> > > +flags. For invalidation events, the iotlb payload has to be filled with the
> > > +invalidation message type (3), the I/O virtual address and the size. On
> > > +success, the slave is expected to reply with a zero payload, non-zero
> > > +otherwise.
> > > +
> > > +When the VHOST_USER_PROTOCOL_F_SLAVE_REQ is supported by the slave, and the
> > > +master initiated the slave to master communication channel using the
> > > +VHOST_USER_SET_SLAVE_REQ_FD request, the slave can send IOTLB miss and access
> > > +failure events by sending VHOST_USER_SLAVE_IOTLB_MSG requests to the master
> > > +with a struct vhost_iotlb_msg payload. For miss events, the iotlb payload has
> > > +to be filled with the miss message type (1), the I/O virtual address and the
> > > +permissions flags. For access failure event, the iotlb payload has to be
> > > +filled with the access failure message type (4), the I/O virtual address and
> > > +the permissions flags. For synchronization purpose, the slave may rely on the
> > > +reply-ack feature, so the master may send a reply when operation is completed
> > > +if the reply-ack feature is negotiated and slaves requests a reply.
> > > +
> > >   Slave communication
> > >   -------------------
> > > @@ -514,6 +557,38 @@ Master message types
> > >         If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, slave must respond
> > >         with zero for success, non-zero otherwise.
> > > + * VHOST_USER_IOTLB_MSG
> > > +
> > > +      Id: 22
> > > +      Equivalent ioctl: N/A (equivalent to VHOST_IOTLB_MSG message type)
> > > +      Master payload: struct vhost_iotlb_msg
> > > +      Slave payload: u64
> > > +
> > > +      Send IOTLB messages with struct vhost_iotlb_msg as payload.
> > > +      Master sends such requests to update and invalidate entries in the device
> > > +      IOTLB. The slave has to acknowledge the request with sending zero as u64
> > > +      payload for success, non-zero otherwise.
> > > +      This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature
> > > +      has been successfully negotiated.
> > > +
> > > +Slave message types
> > > +-------------------
> > > +
> > > + * VHOST_USER_SLAVE_IOTLB_MSG
> > > +
> > > +      Id: 1
> > > +      Equivalent ioctl: N/A (equivalent to VHOST_IOTLB_MSG message type)
> > > +      Slave payload: struct vhost_iotlb_msg
> > > +      Master payload: N/A
> > > +
> > > +      Send IOTLB messages with struct vhost_iotlb_msg as payload.
> > > +      Slave sends such requests to notify of an IOTLB miss, or an IOTLB
> > > +      access failure. If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated,
> > > +      and slave set the VHOST_USER_NEED_REPLY flag, master must respond with
> > > +      zero when operation is successfully completed, or non-zero otherwise.
> > > +      This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature
> > > +      has been successfully negotiated.
> > > +
> > 
> > Are there limitations on number of messages in flight?
> 
> I didn't think about this, I would say the maximum number of messages in
> flight is dependent on the socket buffer size (which is kept to default
> in this series).
> 
> You question highlights a bug in by DPDK prototype, as the MISS request
> can be sent by multiple threads, and I didn't protected this with a lock
> to prevent concurrent read on the socket when waiting for the REPLY_ACK.
> 
> Thanks,
> Maxime

A way to make it work without a lock might possibly be valuable.
Can be done on top.

-- 
MST

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-13  0:02       ` Michael S. Tsirkin
@ 2017-05-15  5:45         ` Jason Wang
  2017-05-16 15:16           ` Michael S. Tsirkin
  2017-05-17 15:27         ` Maxime Coquelin
  1 sibling, 1 reply; 32+ messages in thread
From: Jason Wang @ 2017-05-15  5:45 UTC (permalink / raw)
  To: Michael S. Tsirkin, Maxime Coquelin
  Cc: yuanhan.liu, qemu-devel, peterx, marcandre.lureau, wexu,
	vkaplans, jfreiman



On 2017年05月13日 08:02, Michael S. Tsirkin wrote:
> On Fri, May 12, 2017 at 04:21:58PM +0200, Maxime Coquelin wrote:
>>
>> On 05/11/2017 08:25 PM, Michael S. Tsirkin wrote:
>>> On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote:
>>>> This patch specifies and implements the master/slave communication
>>>> to support device IOTLB in slave.
>>>>
>>>> The vhost_iotlb_msg structure introduced for kernel backends is
>>>> re-used, making the design close between the two backends.
>>>>
>>>> An exception is the use of the secondary channel to enable the
>>>> slave to send IOTLB miss requests to the master.
>>>>
>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> ---
>>>>    docs/specs/vhost-user.txt | 75 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>    hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
>>>>    2 files changed, 106 insertions(+)
>>>>
>>>> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>>>> index 5fa7016..4a1f0c3 100644
>>>> --- a/docs/specs/vhost-user.txt
>>>> +++ b/docs/specs/vhost-user.txt
>>>> @@ -97,6 +97,23 @@ Depending on the request type, payload can be:
>>>>       log offset: offset from start of supplied file descriptor
>>>>           where logging starts (i.e. where guest address 0 would be logged)
>>>> + * An IOTLB message
>>>> +   ---------------------------------------------------------
>>>> +   | iova | size | user address | permissions flags | type |
>>>> +   ---------------------------------------------------------
>>>> +
>>>> +   IOVA: a 64-bit guest I/O virtual address
>>> guest -> VM
>> Ok.
>>
>>>> +   Size: a 64-bit size
>>> How do you specify "all memory"? give special meaning to size 0?
>> Good point, it does not support all memory currently.
>> It is not vhost-user specific, but general to the vhost implementation.
> But iommu needs it to support passthrough.

Probably not, we will just pass the mappings in vhost_memory_region to 
vhost. Its memory_size is also a __u64.

Thanks

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-12 14:21     ` Maxime Coquelin
  2017-05-13  0:02       ` Michael S. Tsirkin
@ 2017-05-16  8:19       ` Maxime Coquelin
  2017-05-16 13:23         ` Michael S. Tsirkin
  1 sibling, 1 reply; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-16  8:19 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman



On 05/12/2017 04:21 PM, Maxime Coquelin wrote:
> 
> 
> On 05/11/2017 08:25 PM, Michael S. Tsirkin wrote:
>> On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote:
>>> This patch specifies and implements the master/slave communication
>>> to support device IOTLB in slave.
>>>
>>> The vhost_iotlb_msg structure introduced for kernel backends is
>>> re-used, making the design close between the two backends.
>>>
>>> An exception is the use of the secondary channel to enable the
>>> slave to send IOTLB miss requests to the master.
>>>
>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> ---
>>>   docs/specs/vhost-user.txt | 75 
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>   hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
>>>   2 files changed, 106 insertions(+)
>>>
>>> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>>> index 5fa7016..4a1f0c3 100644
>>> --- a/docs/specs/vhost-user.txt
>>> +++ b/docs/specs/vhost-user.txt
>>> @@ -97,6 +97,23 @@ Depending on the request type, payload can be:
>>>      log offset: offset from start of supplied file descriptor
>>>          where logging starts (i.e. where guest address 0 would be 
>>> logged)
>>> + * An IOTLB message
>>> +   ---------------------------------------------------------
>>> +   | iova | size | user address | permissions flags | type |
>>> +   ---------------------------------------------------------
>>> +
>>> +   IOVA: a 64-bit guest I/O virtual address
>>
>> guest -> VM
> 
> Ok.

It seems that VM is never used in the doc, but guest is:
$ grep -i guest docs/specs/vhost-user.txt | wc -l
13
grep -i vm docs/specs/vhost-user.txt | wc -l
0

I think I should keep "guest" for consistency?

Thanks,
Maxime

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-16  8:19       ` Maxime Coquelin
@ 2017-05-16 13:23         ` Michael S. Tsirkin
  0 siblings, 0 replies; 32+ messages in thread
From: Michael S. Tsirkin @ 2017-05-16 13:23 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman

On Tue, May 16, 2017 at 10:19:24AM +0200, Maxime Coquelin wrote:
> 
> 
> On 05/12/2017 04:21 PM, Maxime Coquelin wrote:
> > 
> > 
> > On 05/11/2017 08:25 PM, Michael S. Tsirkin wrote:
> > > On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote:
> > > > This patch specifies and implements the master/slave communication
> > > > to support device IOTLB in slave.
> > > > 
> > > > The vhost_iotlb_msg structure introduced for kernel backends is
> > > > re-used, making the design close between the two backends.
> > > > 
> > > > An exception is the use of the secondary channel to enable the
> > > > slave to send IOTLB miss requests to the master.
> > > > 
> > > > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> > > > ---
> > > >   docs/specs/vhost-user.txt | 75
> > > > +++++++++++++++++++++++++++++++++++++++++++++++
> > > >   hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
> > > >   2 files changed, 106 insertions(+)
> > > > 
> > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > > > index 5fa7016..4a1f0c3 100644
> > > > --- a/docs/specs/vhost-user.txt
> > > > +++ b/docs/specs/vhost-user.txt
> > > > @@ -97,6 +97,23 @@ Depending on the request type, payload can be:
> > > >      log offset: offset from start of supplied file descriptor
> > > >          where logging starts (i.e. where guest address 0 would
> > > > be logged)
> > > > + * An IOTLB message
> > > > +   ---------------------------------------------------------
> > > > +   | iova | size | user address | permissions flags | type |
> > > > +   ---------------------------------------------------------
> > > > +
> > > > +   IOVA: a 64-bit guest I/O virtual address
> > > 
> > > guest -> VM
> > 
> > Ok.
> 
> It seems that VM is never used in the doc, but guest is:
> $ grep -i guest docs/specs/vhost-user.txt | wc -l
> 13
> grep -i vm docs/specs/vhost-user.txt | wc -l
> 0
> 
> I think I should keep "guest" for consistency?
> 
> Thanks,
> Maxime

"I/O virtual address programmed by the guest" ?

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-15  5:45         ` Jason Wang
@ 2017-05-16 15:16           ` Michael S. Tsirkin
  2017-05-17  2:53             ` Jason Wang
  0 siblings, 1 reply; 32+ messages in thread
From: Michael S. Tsirkin @ 2017-05-16 15:16 UTC (permalink / raw)
  To: Jason Wang
  Cc: Maxime Coquelin, yuanhan.liu, qemu-devel, peterx,
	marcandre.lureau, wexu, vkaplans, jfreiman

On Mon, May 15, 2017 at 01:45:28PM +0800, Jason Wang wrote:
> 
> 
> On 2017年05月13日 08:02, Michael S. Tsirkin wrote:
> > On Fri, May 12, 2017 at 04:21:58PM +0200, Maxime Coquelin wrote:
> > > 
> > > On 05/11/2017 08:25 PM, Michael S. Tsirkin wrote:
> > > > On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote:
> > > > > This patch specifies and implements the master/slave communication
> > > > > to support device IOTLB in slave.
> > > > > 
> > > > > The vhost_iotlb_msg structure introduced for kernel backends is
> > > > > re-used, making the design close between the two backends.
> > > > > 
> > > > > An exception is the use of the secondary channel to enable the
> > > > > slave to send IOTLB miss requests to the master.
> > > > > 
> > > > > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> > > > > ---
> > > > >    docs/specs/vhost-user.txt | 75 +++++++++++++++++++++++++++++++++++++++++++++++
> > > > >    hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
> > > > >    2 files changed, 106 insertions(+)
> > > > > 
> > > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > > > > index 5fa7016..4a1f0c3 100644
> > > > > --- a/docs/specs/vhost-user.txt
> > > > > +++ b/docs/specs/vhost-user.txt
> > > > > @@ -97,6 +97,23 @@ Depending on the request type, payload can be:
> > > > >       log offset: offset from start of supplied file descriptor
> > > > >           where logging starts (i.e. where guest address 0 would be logged)
> > > > > + * An IOTLB message
> > > > > +   ---------------------------------------------------------
> > > > > +   | iova | size | user address | permissions flags | type |
> > > > > +   ---------------------------------------------------------
> > > > > +
> > > > > +   IOVA: a 64-bit guest I/O virtual address
> > > > guest -> VM
> > > Ok.
> > > 
> > > > > +   Size: a 64-bit size
> > > > How do you specify "all memory"? give special meaning to size 0?
> > > Good point, it does not support all memory currently.
> > > It is not vhost-user specific, but general to the vhost implementation.
> > But iommu needs it to support passthrough.
> 
> Probably not, we will just pass the mappings in vhost_memory_region to
> vhost. Its memory_size is also a __u64.
> 
> Thanks

That's different since that's chunks of qemu virtual memory.

IOMMU maps IOVA to GPA.

-- 
MST

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-16 15:16           ` Michael S. Tsirkin
@ 2017-05-17  2:53             ` Jason Wang
  2017-05-17 14:10               ` Maxime Coquelin
  0 siblings, 1 reply; 32+ messages in thread
From: Jason Wang @ 2017-05-17  2:53 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: yuanhan.liu, qemu-devel, peterx, Maxime Coquelin,
	marcandre.lureau, wexu, vkaplans, jfreiman



On 2017年05月16日 23:16, Michael S. Tsirkin wrote:
> On Mon, May 15, 2017 at 01:45:28PM +0800, Jason Wang wrote:
>>
>> On 2017年05月13日 08:02, Michael S. Tsirkin wrote:
>>> On Fri, May 12, 2017 at 04:21:58PM +0200, Maxime Coquelin wrote:
>>>> On 05/11/2017 08:25 PM, Michael S. Tsirkin wrote:
>>>>> On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote:
>>>>>> This patch specifies and implements the master/slave communication
>>>>>> to support device IOTLB in slave.
>>>>>>
>>>>>> The vhost_iotlb_msg structure introduced for kernel backends is
>>>>>> re-used, making the design close between the two backends.
>>>>>>
>>>>>> An exception is the use of the secondary channel to enable the
>>>>>> slave to send IOTLB miss requests to the master.
>>>>>>
>>>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>>>> ---
>>>>>>     docs/specs/vhost-user.txt | 75 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>     hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
>>>>>>     2 files changed, 106 insertions(+)
>>>>>>
>>>>>> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>>>>>> index 5fa7016..4a1f0c3 100644
>>>>>> --- a/docs/specs/vhost-user.txt
>>>>>> +++ b/docs/specs/vhost-user.txt
>>>>>> @@ -97,6 +97,23 @@ Depending on the request type, payload can be:
>>>>>>        log offset: offset from start of supplied file descriptor
>>>>>>            where logging starts (i.e. where guest address 0 would be logged)
>>>>>> + * An IOTLB message
>>>>>> +   ---------------------------------------------------------
>>>>>> +   | iova | size | user address | permissions flags | type |
>>>>>> +   ---------------------------------------------------------
>>>>>> +
>>>>>> +   IOVA: a 64-bit guest I/O virtual address
>>>>> guest -> VM
>>>> Ok.
>>>>
>>>>>> +   Size: a 64-bit size
>>>>> How do you specify "all memory"? give special meaning to size 0?
>>>> Good point, it does not support all memory currently.
>>>> It is not vhost-user specific, but general to the vhost implementation.
>>> But iommu needs it to support passthrough.
>> Probably not, we will just pass the mappings in vhost_memory_region to
>> vhost. Its memory_size is also a __u64.
>>
>> Thanks
> That's different since that's chunks of qemu virtual memory.
>
> IOMMU maps IOVA to GPA.
>

But we're in fact cache IOVA -> HVA mapping in the remote IOTLB. When 
passthrough mode is enabled, IOVA == GPA, so passing mappings in 
vhost_memory_region should be fine.

The only possible "issue" with "all memory" is if you can not use a 
single TLB invalidation to invalidate all caches in remote TLB. But this 
is only theoretical problem since it only happen when we have a 1 byte 
mapping [2^64 - 1, 2^64) cached in remote TLB. Consider:

- E.g intel IOMMU has a range limitation for invalidation (1G currently)
- Looks like all existed IOMMU use page aligned mappings

It was probably not a big issue. And for safety we could use two 
invalidations to make sure all caches were flushed remotely. Or just 
change the protocol from start, size to start, end. Vhost-kernel is 
probably too late for this change, but I'm still not quite sure it is 
worthwhile.

Thanks

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-17  2:53             ` Jason Wang
@ 2017-05-17 14:10               ` Maxime Coquelin
  2017-05-19  6:48                 ` Jason Wang
  0 siblings, 1 reply; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-17 14:10 UTC (permalink / raw)
  To: Jason Wang, Michael S. Tsirkin
  Cc: yuanhan.liu, qemu-devel, peterx, marcandre.lureau, wexu,
	vkaplans, jfreiman



On 05/17/2017 04:53 AM, Jason Wang wrote:
> 
> 
> On 2017年05月16日 23:16, Michael S. Tsirkin wrote:
>> On Mon, May 15, 2017 at 01:45:28PM +0800, Jason Wang wrote:
>>>
>>> On 2017年05月13日 08:02, Michael S. Tsirkin wrote:
>>>> On Fri, May 12, 2017 at 04:21:58PM +0200, Maxime Coquelin wrote:
>>>>> On 05/11/2017 08:25 PM, Michael S. Tsirkin wrote:
>>>>>> On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote:
>>>>>>> This patch specifies and implements the master/slave communication
>>>>>>> to support device IOTLB in slave.
>>>>>>>
>>>>>>> The vhost_iotlb_msg structure introduced for kernel backends is
>>>>>>> re-used, making the design close between the two backends.
>>>>>>>
>>>>>>> An exception is the use of the secondary channel to enable the
>>>>>>> slave to send IOTLB miss requests to the master.
>>>>>>>
>>>>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>>>>> ---
>>>>>>>     docs/specs/vhost-user.txt | 75 
>>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>>     hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
>>>>>>>     2 files changed, 106 insertions(+)
>>>>>>>
>>>>>>> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>>>>>>> index 5fa7016..4a1f0c3 100644
>>>>>>> --- a/docs/specs/vhost-user.txt
>>>>>>> +++ b/docs/specs/vhost-user.txt
>>>>>>> @@ -97,6 +97,23 @@ Depending on the request type, payload can be:
>>>>>>>        log offset: offset from start of supplied file descriptor
>>>>>>>            where logging starts (i.e. where guest address 0 would 
>>>>>>> be logged)
>>>>>>> + * An IOTLB message
>>>>>>> +   ---------------------------------------------------------
>>>>>>> +   | iova | size | user address | permissions flags | type |
>>>>>>> +   ---------------------------------------------------------
>>>>>>> +
>>>>>>> +   IOVA: a 64-bit guest I/O virtual address
>>>>>> guest -> VM
>>>>> Ok.
>>>>>
>>>>>>> +   Size: a 64-bit size
>>>>>> How do you specify "all memory"? give special meaning to size 0?
>>>>> Good point, it does not support all memory currently.
>>>>> It is not vhost-user specific, but general to the vhost 
>>>>> implementation.
>>>> But iommu needs it to support passthrough.
>>> Probably not, we will just pass the mappings in vhost_memory_region to
>>> vhost. Its memory_size is also a __u64.
>>>
>>> Thanks
>> That's different since that's chunks of qemu virtual memory.
>>
>> IOMMU maps IOVA to GPA.
>>
> 
> But we're in fact cache IOVA -> HVA mapping in the remote IOTLB. When 
> passthrough mode is enabled, IOVA == GPA, so passing mappings in 
> vhost_memory_region should be fine.

Not sure this is a good idea, because when configured in passthrough,
QEMU will see the IOMMU as enabled, so the the VIRTIO_F_IOMMU_PLATFORM
feature will be negotiated if both guest and backend support it.
So how the backend will know whether it should directly pick the
translation directly into the vhost_memory_region, or translate it
through the device IOTLB?

Maybe the solution would be for QEMU to wrap "all memory" IOTLB updates
& invalidations to vhost_memory_regions, since the backend won't anyway
be able to perform accesses outside these regions?

> The only possible "issue" with "all memory" is if you can not use a 
> single TLB invalidation to invalidate all caches in remote TLB.

If needed, maybe we could introduce a new VHOST_IOTLB_INVALIDATE message
type?
For older kernel backend that doesn't support it, -EINVAL will be
returned, so QEMU could handle it another way in this case.

> But this 
> is only theoretical problem since it only happen when we have a 1 byte 
> mapping [2^64 - 1, 2^64) cached in remote TLB. Consider:
> 
> - E.g intel IOMMU has a range limitation for invalidation (1G currently)
> - Looks like all existed IOMMU use page aligned mappings
> 
> It was probably not a big issue. And for safety we could use two 
> invalidations to make sure all caches were flushed remotely. Or just 
> change the protocol from start, size to start, end. Vhost-kernel is 
> probably too late for this change, but I'm still not quite sure it is 
> worthwhile.

I'm not for diverging the protocol between kernel & user backends.

Thanks,
Maxime

> Thanks

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-13  0:02       ` Michael S. Tsirkin
  2017-05-15  5:45         ` Jason Wang
@ 2017-05-17 15:27         ` Maxime Coquelin
  1 sibling, 0 replies; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-17 15:27 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman



On 05/13/2017 02:02 AM, Michael S. Tsirkin wrote:
>>>> @@ -253,6 +271,31 @@ Once the source has finished migration, rings will be stopped by
>>>>    the source. No further update must be done before rings are
>>>>    restarted.
>>>> +IOMMU support
>>>> +-------------
>>>> +
>>>> +When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated, the master has
>>>> +to send IOTLB entries update & invalidation by sending VHOST_USER_IOTLB_MSG
>>>> +requests to the slave with a struct vhost_iotlb_msg payload.
>>> Always? This seems a bit strange since iommu can be enabled/disabled
>>> dynamically.
>> Ok, what about:
>>
>> When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated and iommu
>> is enbaled, the master sends IOTLB entries update & invalidation via
>> VHOST_USER_IOTLB_MSG requests to the slave with a struct vhost_iotlb_msg
>> payload.
>>
>>
>>> Closing channel seems like a wrong thing to do for this.
>> Sorry, I'm not sure to get your comment.
> What happens when guest disables the IOMMU?

I think you mean for example when rebooting the guest with IOMMU
disabled. In this case, I think that the user backend should clean its
IOTLB cache on the next SET_FEATURE call, as the kernel backend does.

Note that I was wrong when stating:
"When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated and iommu
is enabled..."

Actually, VIRTIO_F_IOMMU_PLATFORM is negotiated if both supported by the
backend and the guest, and if an iommu device is attached. If the guest
kernel doesn't enable the IOMMU (e.g. intel_iommu=off in kernel
cmdline), the master replies to IOTLB misses with identity IOTLB
entries updates (Passthrough) (it requires recent fixes from Peter to
work[0]).

Maxime

[0]: http://patchwork.ozlabs.org/patch/763379/

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

* Re: [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier
  2017-05-12 11:21     ` Maxime Coquelin
@ 2017-05-17 16:41       ` Michael S. Tsirkin
  2017-05-18  7:35         ` Maxime Coquelin
  0 siblings, 1 reply; 32+ messages in thread
From: Michael S. Tsirkin @ 2017-05-17 16:41 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman

On Fri, May 12, 2017 at 01:21:18PM +0200, Maxime Coquelin wrote:
> 
> 
> On 05/11/2017 07:33 PM, Michael S. Tsirkin wrote:
> > On Thu, May 11, 2017 at 02:32:43PM +0200, Maxime Coquelin wrote:
> > > Vhost-kernel backend need to receive IOTLB entries for rings
> > > information early, but vhost-user need the same information
> > > earlier, before VHOST_USER_SET_VRING_ADDR is sent.
> > 
> > Weird. What does VHOST_USER_SET_VRING_ADDR have to do with it?
> > 
> > According to
> > 	Starting and stopping rings
> > in vhost user spec, vhost user does not access
> > anything until ring is started and enabled.
> > 
> > 
> > > This patch also trigger IOTLB miss for all rings informations
> > > for robustness, even if in practice these adresses are on the
> > > same page.
> 
> Actually, the DPDK vhost-user backend is compliant with the spec,
> but when handling VHOST_USER_SET_VRING_ADDR request, it translates the
> guest addresses into backend VAs, and check they are valid. I will make the
> commit message clearer about this in next revision.
> 
> The check could be done later, for example when the ring are started,
> but it wouldn't change the need to trigger a miss at some point.

I think it should be done later, yes. As long as ring is not
started addresses should not be interpreted.

> Or it could be done in the processing threads, the first time the
> addresses are used, but it would add a check for every burst of packets
> processed.

We don't want that, I agree.

> > > 
> > > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> > > ---
> > >   hw/virtio/vhost.c | 19 +++++++++++--------
> > >   1 file changed, 11 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > > index 748e331..817f6d0 100644
> > > --- a/hw/virtio/vhost.c
> > > +++ b/hw/virtio/vhost.c
> > > @@ -799,7 +799,17 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
> > >           .log_guest_addr = vq->used_phys,
> > >           .flags = enable_log ? (1 << VHOST_VRING_F_LOG) : 0,
> > >       };
> > > -    int r = dev->vhost_ops->vhost_set_vring_addr(dev, &addr);
> > > +    int r;
> > > +
> > > +    /* Update rings information for IOTLB to work correctly,
> > > +     * vhost-kernel & vhost-user backends require for this.*/
> > 
> > Any requirements must be in the spec. Pls add them there.
> > Pls fix comment style as you move code.
> Sure.
> 
> Thanks,
> Maxime

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-11 12:32 ` [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support Maxime Coquelin
  2017-05-11 18:25   ` Michael S. Tsirkin
@ 2017-05-17 16:48   ` Michael S. Tsirkin
  2017-05-18  8:43     ` Maxime Coquelin
  1 sibling, 1 reply; 32+ messages in thread
From: Michael S. Tsirkin @ 2017-05-17 16:48 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman

On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote:
> This patch specifies and implements the master/slave communication
> to support device IOTLB in slave.
> 
> The vhost_iotlb_msg structure introduced for kernel backends is
> re-used, making the design close between the two backends.
> 
> An exception is the use of the secondary channel to enable the
> slave to send IOTLB miss requests to the master.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  docs/specs/vhost-user.txt | 75 +++++++++++++++++++++++++++++++++++++++++++++++
>  hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
>  2 files changed, 106 insertions(+)
> 
> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> index 5fa7016..4a1f0c3 100644
> --- a/docs/specs/vhost-user.txt
> +++ b/docs/specs/vhost-user.txt
> @@ -97,6 +97,23 @@ Depending on the request type, payload can be:
>     log offset: offset from start of supplied file descriptor
>         where logging starts (i.e. where guest address 0 would be logged)
>  
> + * An IOTLB message
> +   ---------------------------------------------------------
> +   | iova | size | user address | permissions flags | type |
> +   ---------------------------------------------------------
> +
> +   IOVA: a 64-bit guest I/O virtual address
> +   Size: a 64-bit size
> +   User address: a 64-bit user address
> +   Permissions flags: a 8-bit bit field:
> +    - Bit 0: Read access
> +    - Bit 1: Write access
> +   Type: a 8-bit IOTLB message type:
> +    - 1: IOTLB miss
> +    - 2: IOTLB update
> +    - 3: IOTLB invalidate
> +    - 4: IOTLB access fail
> +
>  In QEMU the vhost-user message is implemented with the following struct:
>  
>  typedef struct VhostUserMsg {
> @@ -109,6 +126,7 @@ typedef struct VhostUserMsg {
>          struct vhost_vring_addr addr;
>          VhostUserMemory memory;
>          VhostUserLog log;
> +        struct vhost_iotlb_msg iotlb;
>      };
>  } QEMU_PACKED VhostUserMsg;
>  
> @@ -253,6 +271,31 @@ Once the source has finished migration, rings will be stopped by
>  the source. No further update must be done before rings are
>  restarted.
>  
> +IOMMU support
> +-------------
> +
> +When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated, the master has
> +to send IOTLB entries update & invalidation by sending VHOST_USER_IOTLB_MSG
> +requests to the slave with a struct vhost_iotlb_msg payload. For update events,
> +the iotlb payload has to be filled with the update message type (2), the I/O
> +virtual address, the size, the user virtual address, and the permissions
> +flags. For invalidation events, the iotlb payload has to be filled with the
> +invalidation message type (3), the I/O virtual address and the size. On
> +success, the slave is expected to reply with a zero payload, non-zero
> +otherwise.
> +
> +When the VHOST_USER_PROTOCOL_F_SLAVE_REQ is supported by the slave, and the
> +master initiated the slave to master communication channel using the
> +VHOST_USER_SET_SLAVE_REQ_FD request, the slave can send IOTLB miss and access
> +failure events by sending VHOST_USER_SLAVE_IOTLB_MSG requests to the master
> +with a struct vhost_iotlb_msg payload. For miss events, the iotlb payload has
> +to be filled with the miss message type (1), the I/O virtual address and the
> +permissions flags. For access failure event, the iotlb payload has to be
> +filled with the access failure message type (4), the I/O virtual address and
> +the permissions flags.

I don't think slave should cache invalid entries. If it does not,
how can it detect access failure as opposed to a miss?

> For synchronization purpose, the slave may rely on the
> +reply-ack feature, so the master may send a reply when operation is completed

What does completed mean in this context?

> +if the reply-ack feature is negotiated and slaves requests a reply.
> +

This is not very clear to me.

So slave sends an access to master. Master finds a pte that
overlaps. What does it send to guest? Initial PTE?
All PTEs to cover the request? Part of the PTE that overlaps
the request?


>  Slave communication
>  -------------------
>  
> @@ -514,6 +557,38 @@ Master message types
>        If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, slave must respond
>        with zero for success, non-zero otherwise.
>  
> + * VHOST_USER_IOTLB_MSG
> +
> +      Id: 22
> +      Equivalent ioctl: N/A (equivalent to VHOST_IOTLB_MSG message type)
> +      Master payload: struct vhost_iotlb_msg
> +      Slave payload: u64
> +
> +      Send IOTLB messages with struct vhost_iotlb_msg as payload.
> +      Master sends such requests to update and invalidate entries in the device
> +      IOTLB. The slave has to acknowledge the request with sending zero as u64
> +      payload for success, non-zero otherwise.
> +      This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature
> +      has been successfully negotiated.
> +
> +Slave message types
> +-------------------
> +
> + * VHOST_USER_SLAVE_IOTLB_MSG
> +
> +      Id: 1
> +      Equivalent ioctl: N/A (equivalent to VHOST_IOTLB_MSG message type)
> +      Slave payload: struct vhost_iotlb_msg
> +      Master payload: N/A
> +
> +      Send IOTLB messages with struct vhost_iotlb_msg as payload.
> +      Slave sends such requests to notify of an IOTLB miss, or an IOTLB
> +      access failure. If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated,
> +      and slave set the VHOST_USER_NEED_REPLY flag, master must respond with
> +      zero when operation is successfully completed, or non-zero otherwise.
> +      This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature
> +      has been successfully negotiated.
> +
>  VHOST_USER_PROTOCOL_F_REPLY_ACK:
>  -------------------------------
>  The original vhost-user specification only demands replies for certain
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index fbc09fa..2c93181 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -63,11 +63,13 @@ typedef enum VhostUserRequest {
>      VHOST_USER_SEND_RARP = 19,
>      VHOST_USER_NET_SET_MTU = 20,
>      VHOST_USER_SET_SLAVE_REQ_FD = 21,
> +    VHOST_USER_IOTLB_MSG = 22,
>      VHOST_USER_MAX
>  } VhostUserRequest;
>  
>  typedef enum VhostUserSlaveRequest {
>      VHOST_USER_SLAVE_NONE = 0,
> +    VHOST_USER_SLAVE_IOTLB_MSG = 1,
>      VHOST_USER_SLAVE_MAX
>  }  VhostUserSlaveRequest;
>  
> @@ -105,6 +107,7 @@ typedef struct VhostUserMsg {
>          struct vhost_vring_addr addr;
>          VhostUserMemory memory;
>          VhostUserLog log;
> +        struct vhost_iotlb_msg iotlb;
>      } payload;
>  } QEMU_PACKED VhostUserMsg;
>  
> @@ -611,6 +614,9 @@ static void slave_read(void *opaque)
>      }
>  
>      switch (msg.request) {
> +    case VHOST_USER_SLAVE_IOTLB_MSG:
> +        ret = vhost_backend_handle_iotlb_msg(dev, &msg.payload.iotlb);
> +        break;
>      default:
>          error_report("Received unexpected msg type.");
>          ret = -EINVAL;
> @@ -858,6 +864,29 @@ static int vhost_user_net_set_mtu(struct vhost_dev *dev, uint16_t mtu)
>      return 0;
>  }
>  
> +static int vhost_user_send_device_iotlb_msg(struct vhost_dev *dev,
> +                                            struct vhost_iotlb_msg *imsg)
> +{
> +    VhostUserMsg msg = {
> +        .request = VHOST_USER_IOTLB_MSG,
> +        .size = sizeof(msg.payload.iotlb),
> +        .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
> +        .payload.iotlb = *imsg,
> +    };
> +
> +    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> +        return -EFAULT;
> +    }
> +
> +    return process_message_reply(dev, msg.request);
> +}
> +
> +
> +static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled)
> +{
> +    /* No-op as the receive channel is not dedicated to IOTLB messages. */
> +}
> +
>  const VhostOps user_ops = {
>          .backend_type = VHOST_BACKEND_TYPE_USER,
>          .vhost_backend_init = vhost_user_init,
> @@ -882,4 +911,6 @@ const VhostOps user_ops = {
>          .vhost_migration_done = vhost_user_migration_done,
>          .vhost_backend_can_merge = vhost_user_can_merge,
>          .vhost_net_set_mtu = vhost_user_net_set_mtu,
> +        .vhost_set_iotlb_callback = vhost_user_set_iotlb_callback,
> +        .vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg,
>  };
> -- 
> 2.9.3

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

* Re: [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier
  2017-05-17 16:41       ` Michael S. Tsirkin
@ 2017-05-18  7:35         ` Maxime Coquelin
  2017-05-18 14:45           ` Maxime Coquelin
  0 siblings, 1 reply; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-18  7:35 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman



On 05/17/2017 06:41 PM, Michael S. Tsirkin wrote:
> On Fri, May 12, 2017 at 01:21:18PM +0200, Maxime Coquelin wrote:
>>
>> On 05/11/2017 07:33 PM, Michael S. Tsirkin wrote:
>>> On Thu, May 11, 2017 at 02:32:43PM +0200, Maxime Coquelin wrote:
>>>> Vhost-kernel backend need to receive IOTLB entries for rings
>>>> information early, but vhost-user need the same information
>>>> earlier, before VHOST_USER_SET_VRING_ADDR is sent.
>>> Weird. What does VHOST_USER_SET_VRING_ADDR have to do with it?
>>>
>>> According to
>>> 	Starting and stopping rings
>>> in vhost user spec, vhost user does not access
>>> anything until ring is started and enabled.
>>>
>>>
>>>> This patch also trigger IOTLB miss for all rings informations
>>>> for robustness, even if in practice these adresses are on the
>>>> same page.
>> Actually, the DPDK vhost-user backend is compliant with the spec,
>> but when handling VHOST_USER_SET_VRING_ADDR request, it translates the
>> guest addresses into backend VAs, and check they are valid. I will make the
>> commit message clearer about this in next revision.
>>
>> The check could be done later, for example when the ring are started,
>> but it wouldn't change the need to trigger a miss at some point.
> I think it should be done later, yes. As long as ring is not
> started addresses should not be interpreted.
> 

Ok, then I'll move these addresses translations in the
VHOST_USER_SET_VRING_KICK handler.

Thanks,
Maxime

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-17 16:48   ` Michael S. Tsirkin
@ 2017-05-18  8:43     ` Maxime Coquelin
  2017-05-19  7:46       ` Jason Wang
  0 siblings, 1 reply; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-18  8:43 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman



On 05/17/2017 06:48 PM, Michael S. Tsirkin wrote:
> On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote:
>> This patch specifies and implements the master/slave communication
>> to support device IOTLB in slave.
>>
>> The vhost_iotlb_msg structure introduced for kernel backends is
>> re-used, making the design close between the two backends.
>>
>> An exception is the use of the secondary channel to enable the
>> slave to send IOTLB miss requests to the master.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>   docs/specs/vhost-user.txt | 75 +++++++++++++++++++++++++++++++++++++++++++++++
>>   hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
>>   2 files changed, 106 insertions(+)
>>
>> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>> index 5fa7016..4a1f0c3 100644
>> --- a/docs/specs/vhost-user.txt
>> +++ b/docs/specs/vhost-user.txt
>> @@ -97,6 +97,23 @@ Depending on the request type, payload can be:
>>      log offset: offset from start of supplied file descriptor
>>          where logging starts (i.e. where guest address 0 would be logged)
>>   
>> + * An IOTLB message
>> +   ---------------------------------------------------------
>> +   | iova | size | user address | permissions flags | type |
>> +   ---------------------------------------------------------
>> +
>> +   IOVA: a 64-bit guest I/O virtual address
>> +   Size: a 64-bit size
>> +   User address: a 64-bit user address
>> +   Permissions flags: a 8-bit bit field:
>> +    - Bit 0: Read access
>> +    - Bit 1: Write access
>> +   Type: a 8-bit IOTLB message type:
>> +    - 1: IOTLB miss
>> +    - 2: IOTLB update
>> +    - 3: IOTLB invalidate
>> +    - 4: IOTLB access fail
>> +
>>   In QEMU the vhost-user message is implemented with the following struct:
>>   
>>   typedef struct VhostUserMsg {
>> @@ -109,6 +126,7 @@ typedef struct VhostUserMsg {
>>           struct vhost_vring_addr addr;
>>           VhostUserMemory memory;
>>           VhostUserLog log;
>> +        struct vhost_iotlb_msg iotlb;
>>       };
>>   } QEMU_PACKED VhostUserMsg;
>>   
>> @@ -253,6 +271,31 @@ Once the source has finished migration, rings will be stopped by
>>   the source. No further update must be done before rings are
>>   restarted.
>>   
>> +IOMMU support
>> +-------------
>> +
>> +When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated, the master has
>> +to send IOTLB entries update & invalidation by sending VHOST_USER_IOTLB_MSG
>> +requests to the slave with a struct vhost_iotlb_msg payload. For update events,
>> +the iotlb payload has to be filled with the update message type (2), the I/O
>> +virtual address, the size, the user virtual address, and the permissions
>> +flags. For invalidation events, the iotlb payload has to be filled with the
>> +invalidation message type (3), the I/O virtual address and the size. On
>> +success, the slave is expected to reply with a zero payload, non-zero
>> +otherwise.
>> +
>> +When the VHOST_USER_PROTOCOL_F_SLAVE_REQ is supported by the slave, and the
>> +master initiated the slave to master communication channel using the
>> +VHOST_USER_SET_SLAVE_REQ_FD request, the slave can send IOTLB miss and access
>> +failure events by sending VHOST_USER_SLAVE_IOTLB_MSG requests to the master
>> +with a struct vhost_iotlb_msg payload. For miss events, the iotlb payload has
>> +to be filled with the miss message type (1), the I/O virtual address and the
>> +permissions flags. For access failure event, the iotlb payload has to be
>> +filled with the access failure message type (4), the I/O virtual address and
>> +the permissions flags.
> 
> I don't think slave should cache invalid entries. If it does not,
> how can it detect access failure as opposed to a miss?

Of course, invalid cache entries should not be cached.
The VHOST_IOTLB_ACCESS_FAIL has been specified for the Kernel backend,
even if the latter does not implement it yet.

In the case of user backend, I think it could be used without caching
entries.

After the backend sends a miss requests, the master will send an update
requests.

If for some reasons, the update entry is invalid (e.g. is outside the
memory ranges shared by set_mem_table), it won't be inserted into the
cache. So, when looking again at the cache, the backend will still face
a miss, and so can notify the master of the failing access.

That said, I specified this message type to mimic the kernel backend, I
don't what the master could do of such information.

> 
>> For synchronization purpose, the slave may rely on the
>> +reply-ack feature, so the master may send a reply when operation is completed
> 
> What does completed mean in this context?

Completed means either the master has sent the IOTLB entry update to the
slave through the master->slave channel, or the IOVA is invalid (so
nothing sent).

> 
>> +if the reply-ack feature is negotiated and slaves requests a reply.
>> +
> 
> This is not very clear to me.
> So slave sends an access to master. Master finds a pte that
> overlaps. What does it send to guest? Initial PTE?
> All PTEs to cover the request? Part of the PTE that overlaps
> the request?

Actually, the slave only sends the IOVA and access permissions, not the
size. So the master will send the PTE overlapping this IOVA if
permissions match.
When received the slave will again lookup into its cache, and may send
another request with the next first missing IOVA if needed.

Maxime

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

* Re: [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier
  2017-05-18  7:35         ` Maxime Coquelin
@ 2017-05-18 14:45           ` Maxime Coquelin
  2017-05-18 15:24             ` Michael S. Tsirkin
  0 siblings, 1 reply; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-18 14:45 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman

Hi Michael,

On 05/18/2017 09:35 AM, Maxime Coquelin wrote:
> 
> 
> On 05/17/2017 06:41 PM, Michael S. Tsirkin wrote:
>> On Fri, May 12, 2017 at 01:21:18PM +0200, Maxime Coquelin wrote:
>>>
>>> On 05/11/2017 07:33 PM, Michael S. Tsirkin wrote:
>>>> On Thu, May 11, 2017 at 02:32:43PM +0200, Maxime Coquelin wrote:
>>>>> Vhost-kernel backend need to receive IOTLB entries for rings
>>>>> information early, but vhost-user need the same information
>>>>> earlier, before VHOST_USER_SET_VRING_ADDR is sent.
>>>> Weird. What does VHOST_USER_SET_VRING_ADDR have to do with it?
>>>>
>>>> According to
>>>>     Starting and stopping rings
>>>> in vhost user spec, vhost user does not access
>>>> anything until ring is started and enabled.
>>>>
>>>>
>>>>> This patch also trigger IOTLB miss for all rings informations
>>>>> for robustness, even if in practice these adresses are on the
>>>>> same page.
>>> Actually, the DPDK vhost-user backend is compliant with the spec,
>>> but when handling VHOST_USER_SET_VRING_ADDR request, it translates the
>>> guest addresses into backend VAs, and check they are valid. I will 
>>> make the
>>> commit message clearer about this in next revision.
>>>
>>> The check could be done later, for example when the ring are started,
>>> but it wouldn't change the need to trigger a miss at some point.
>> I think it should be done later, yes. As long as ring is not
>> started addresses should not be interpreted.
>>
> 
> Ok, then I'll move these addresses translations in the
> VHOST_USER_SET_VRING_KICK handler.
s/VHOST_USER_SET_VRING_KICK/VHOST_USER_SET_VRING_ENABLE/

I just looked at implementing this change, but I'm not convinced this is
the right thing to do.

On backend side, it means saving temporarily the vhost_vring_addr struct
into the vq struct, and moving all what is done currently in
SET_VRING_ADDR handler to SET_VRING_ENABLE one.

My understanding of the "Starting and stopping rings" chapter of the
spec is that the ring must not be processed as long as not started and
enabled, not that the addresses passed should not be checked/translated
as it is done today both in DPDK and libvhost-user.

If the addresses are invalid, isn't it better to know as soon as
possible?

Cheers,
Maxime

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

* Re: [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier
  2017-05-18 14:45           ` Maxime Coquelin
@ 2017-05-18 15:24             ` Michael S. Tsirkin
  2017-05-19  9:48               ` Maxime Coquelin
  0 siblings, 1 reply; 32+ messages in thread
From: Michael S. Tsirkin @ 2017-05-18 15:24 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman

On Thu, May 18, 2017 at 04:45:23PM +0200, Maxime Coquelin wrote:
> Hi Michael,
> 
> On 05/18/2017 09:35 AM, Maxime Coquelin wrote:
> > 
> > 
> > On 05/17/2017 06:41 PM, Michael S. Tsirkin wrote:
> > > On Fri, May 12, 2017 at 01:21:18PM +0200, Maxime Coquelin wrote:
> > > > 
> > > > On 05/11/2017 07:33 PM, Michael S. Tsirkin wrote:
> > > > > On Thu, May 11, 2017 at 02:32:43PM +0200, Maxime Coquelin wrote:
> > > > > > Vhost-kernel backend need to receive IOTLB entries for rings
> > > > > > information early, but vhost-user need the same information
> > > > > > earlier, before VHOST_USER_SET_VRING_ADDR is sent.
> > > > > Weird. What does VHOST_USER_SET_VRING_ADDR have to do with it?
> > > > > 
> > > > > According to
> > > > >     Starting and stopping rings
> > > > > in vhost user spec, vhost user does not access
> > > > > anything until ring is started and enabled.
> > > > > 
> > > > > 
> > > > > > This patch also trigger IOTLB miss for all rings informations
> > > > > > for robustness, even if in practice these adresses are on the
> > > > > > same page.
> > > > Actually, the DPDK vhost-user backend is compliant with the spec,
> > > > but when handling VHOST_USER_SET_VRING_ADDR request, it translates the
> > > > guest addresses into backend VAs, and check they are valid. I
> > > > will make the
> > > > commit message clearer about this in next revision.
> > > > 
> > > > The check could be done later, for example when the ring are started,
> > > > but it wouldn't change the need to trigger a miss at some point.
> > > I think it should be done later, yes. As long as ring is not
> > > started addresses should not be interpreted.
> > > 
> > 
> > Ok, then I'll move these addresses translations in the
> > VHOST_USER_SET_VRING_KICK handler.
> s/VHOST_USER_SET_VRING_KICK/VHOST_USER_SET_VRING_ENABLE/

Note that when protocol features are off ring is started in
enabled state, but iommu requires protocol features.

> I just looked at implementing this change, but I'm not convinced this is
> the right thing to do.
> 
> On backend side, it means saving temporarily the vhost_vring_addr struct
> into the vq struct, and moving all what is done currently in
> SET_VRING_ADDR handler to SET_VRING_ENABLE one.

Yes, and this is consistent with what the kernel does.

> My understanding of the "Starting and stopping rings" chapter of the
> spec is that the ring must not be processed as long as not started and
> enabled, not that the addresses passed should not be checked/translated
> as it is done today both in DPDK and libvhost-user.
> 
> If the addresses are invalid, isn't it better to know as soon as
> possible?
> 
> Cheers,
> Maxime

There could be valid reasons to set an invalid address temporarily.
For example to make sure connection is reset.



-- 
MST

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-17 14:10               ` Maxime Coquelin
@ 2017-05-19  6:48                 ` Jason Wang
  2017-05-19  8:35                   ` Maxime Coquelin
  0 siblings, 1 reply; 32+ messages in thread
From: Jason Wang @ 2017-05-19  6:48 UTC (permalink / raw)
  To: Maxime Coquelin, Michael S. Tsirkin
  Cc: yuanhan.liu, qemu-devel, peterx, marcandre.lureau, wexu,
	vkaplans, jfreiman



On 2017年05月17日 22:10, Maxime Coquelin wrote:
>
>
> On 05/17/2017 04:53 AM, Jason Wang wrote:
>>
>>
>> On 2017年05月16日 23:16, Michael S. Tsirkin wrote:
>>> On Mon, May 15, 2017 at 01:45:28PM +0800, Jason Wang wrote:
>>>>
>>>> On 2017年05月13日 08:02, Michael S. Tsirkin wrote:
>>>>> On Fri, May 12, 2017 at 04:21:58PM +0200, Maxime Coquelin wrote:
>>>>>> On 05/11/2017 08:25 PM, Michael S. Tsirkin wrote:
>>>>>>> On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote:
>>>>>>>> This patch specifies and implements the master/slave communication
>>>>>>>> to support device IOTLB in slave.
>>>>>>>>
>>>>>>>> The vhost_iotlb_msg structure introduced for kernel backends is
>>>>>>>> re-used, making the design close between the two backends.
>>>>>>>>
>>>>>>>> An exception is the use of the secondary channel to enable the
>>>>>>>> slave to send IOTLB miss requests to the master.
>>>>>>>>
>>>>>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>>>>>> ---
>>>>>>>>     docs/specs/vhost-user.txt | 75 
>>>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>     hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
>>>>>>>>     2 files changed, 106 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>>>>>>>> index 5fa7016..4a1f0c3 100644
>>>>>>>> --- a/docs/specs/vhost-user.txt
>>>>>>>> +++ b/docs/specs/vhost-user.txt
>>>>>>>> @@ -97,6 +97,23 @@ Depending on the request type, payload can be:
>>>>>>>>        log offset: offset from start of supplied file descriptor
>>>>>>>>            where logging starts (i.e. where guest address 0 
>>>>>>>> would be logged)
>>>>>>>> + * An IOTLB message
>>>>>>>> + ---------------------------------------------------------
>>>>>>>> +   | iova | size | user address | permissions flags | type |
>>>>>>>> + ---------------------------------------------------------
>>>>>>>> +
>>>>>>>> +   IOVA: a 64-bit guest I/O virtual address
>>>>>>> guest -> VM
>>>>>> Ok.
>>>>>>
>>>>>>>> +   Size: a 64-bit size
>>>>>>> How do you specify "all memory"? give special meaning to size 0?
>>>>>> Good point, it does not support all memory currently.
>>>>>> It is not vhost-user specific, but general to the vhost 
>>>>>> implementation.
>>>>> But iommu needs it to support passthrough.
>>>> Probably not, we will just pass the mappings in vhost_memory_region to
>>>> vhost. Its memory_size is also a __u64.
>>>>
>>>> Thanks
>>> That's different since that's chunks of qemu virtual memory.
>>>
>>> IOMMU maps IOVA to GPA.
>>>
>>
>> But we're in fact cache IOVA -> HVA mapping in the remote IOTLB. When 
>> passthrough mode is enabled, IOVA == GPA, so passing mappings in 
>> vhost_memory_region should be fine.
>
> Not sure this is a good idea, because when configured in passthrough,
> QEMU will see the IOMMU as enabled, so the the VIRTIO_F_IOMMU_PLATFORM
> feature will be negotiated if both guest and backend support it.
> So how the backend will know whether it should directly pick the
> translation directly into the vhost_memory_region, or translate it
> through the device IOTLB?

This no need for backend to know about this, since IOVA equals to GPA, 
vhost_memory_region stores IOVA -> HVA mapping. If we pass them, device 
IOTLB should work as usual?

>
> Maybe the solution would be for QEMU to wrap "all memory" IOTLB updates
> & invalidations to vhost_memory_regions, since the backend won't anyway
> be able to perform accesses outside these regions?

This is just what I mean, you can refer Peter's series.

>
>> The only possible "issue" with "all memory" is if you can not use a 
>> single TLB invalidation to invalidate all caches in remote TLB.
>
> If needed, maybe we could introduce a new VHOST_IOTLB_INVALIDATE message
> type?
> For older kernel backend that doesn't support it, -EINVAL will be
> returned, so QEMU could handle it another way in this case.

We could, but not sure it was really needed.

>
>> But this is only theoretical problem since it only happen when we 
>> have a 1 byte mapping [2^64 - 1, 2^64) cached in remote TLB. Consider:
>>
>> - E.g intel IOMMU has a range limitation for invalidation (1G currently)
>> - Looks like all existed IOMMU use page aligned mappings
>>
>> It was probably not a big issue. And for safety we could use two 
>> invalidations to make sure all caches were flushed remotely. Or just 
>> change the protocol from start, size to start, end. Vhost-kernel is 
>> probably too late for this change, but I'm still not quite sure it is 
>> worthwhile.
>
> I'm not for diverging the protocol between kernel & user backends.
>
> Thanks,
> Maxime

Yes, agree.

Thanks

>
>> Thanks

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-18  8:43     ` Maxime Coquelin
@ 2017-05-19  7:46       ` Jason Wang
  2017-05-19 16:42         ` Michael S. Tsirkin
  0 siblings, 1 reply; 32+ messages in thread
From: Jason Wang @ 2017-05-19  7:46 UTC (permalink / raw)
  To: Maxime Coquelin, Michael S. Tsirkin
  Cc: yuanhan.liu, qemu-devel, peterx, marcandre.lureau, wexu,
	vkaplans, jfreiman



On 2017年05月18日 16:43, Maxime Coquelin wrote:
>>>
>>> +When the VHOST_USER_PROTOCOL_F_SLAVE_REQ is supported by the slave, 
>>> and the
>>> +master initiated the slave to master communication channel using the
>>> +VHOST_USER_SET_SLAVE_REQ_FD request, the slave can send IOTLB miss 
>>> and access
>>> +failure events by sending VHOST_USER_SLAVE_IOTLB_MSG requests to 
>>> the master
>>> +with a struct vhost_iotlb_msg payload. For miss events, the iotlb 
>>> payload has
>>> +to be filled with the miss message type (1), the I/O virtual 
>>> address and the
>>> +permissions flags. For access failure event, the iotlb payload has 
>>> to be
>>> +filled with the access failure message type (4), the I/O virtual 
>>> address and
>>> +the permissions flags.
>>
>> I don't think slave should cache invalid entries. If it does not,
>> how can it detect access failure as opposed to a miss?
>
> Of course, invalid cache entries should not be cached.
> The VHOST_IOTLB_ACCESS_FAIL has been specified for the Kernel backend,
> even if the latter does not implement it yet. 

Yes, I leave this for future use e.g reporting copy_to_user() failure to 
userspace.

Thanks

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-19  6:48                 ` Jason Wang
@ 2017-05-19  8:35                   ` Maxime Coquelin
  0 siblings, 0 replies; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-19  8:35 UTC (permalink / raw)
  To: Jason Wang, Michael S. Tsirkin
  Cc: yuanhan.liu, qemu-devel, peterx, marcandre.lureau, wexu,
	vkaplans, jfreiman



On 05/19/2017 08:48 AM, Jason Wang wrote:
> 
> 
> On 2017年05月17日 22:10, Maxime Coquelin wrote:
>>
>>
>> On 05/17/2017 04:53 AM, Jason Wang wrote:
>>>
>>>
>>> On 2017年05月16日 23:16, Michael S. Tsirkin wrote:
>>>> On Mon, May 15, 2017 at 01:45:28PM +0800, Jason Wang wrote:
>>>>>
>>>>> On 2017年05月13日 08:02, Michael S. Tsirkin wrote:
>>>>>> On Fri, May 12, 2017 at 04:21:58PM +0200, Maxime Coquelin wrote:
>>>>>>> On 05/11/2017 08:25 PM, Michael S. Tsirkin wrote:
>>>>>>>> On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote:
>>>>>>>>> This patch specifies and implements the master/slave communication
>>>>>>>>> to support device IOTLB in slave.
>>>>>>>>>
>>>>>>>>> The vhost_iotlb_msg structure introduced for kernel backends is
>>>>>>>>> re-used, making the design close between the two backends.
>>>>>>>>>
>>>>>>>>> An exception is the use of the secondary channel to enable the
>>>>>>>>> slave to send IOTLB miss requests to the master.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>>>>>>> ---
>>>>>>>>>     docs/specs/vhost-user.txt | 75 
>>>>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>>     hw/virtio/vhost-user.c    | 31 ++++++++++++++++++++
>>>>>>>>>     2 files changed, 106 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>>>>>>>>> index 5fa7016..4a1f0c3 100644
>>>>>>>>> --- a/docs/specs/vhost-user.txt
>>>>>>>>> +++ b/docs/specs/vhost-user.txt
>>>>>>>>> @@ -97,6 +97,23 @@ Depending on the request type, payload can be:
>>>>>>>>>        log offset: offset from start of supplied file descriptor
>>>>>>>>>            where logging starts (i.e. where guest address 0 
>>>>>>>>> would be logged)
>>>>>>>>> + * An IOTLB message
>>>>>>>>> + ---------------------------------------------------------
>>>>>>>>> +   | iova | size | user address | permissions flags | type |
>>>>>>>>> + ---------------------------------------------------------
>>>>>>>>> +
>>>>>>>>> +   IOVA: a 64-bit guest I/O virtual address
>>>>>>>> guest -> VM
>>>>>>> Ok.
>>>>>>>
>>>>>>>>> +   Size: a 64-bit size
>>>>>>>> How do you specify "all memory"? give special meaning to size 0?
>>>>>>> Good point, it does not support all memory currently.
>>>>>>> It is not vhost-user specific, but general to the vhost 
>>>>>>> implementation.
>>>>>> But iommu needs it to support passthrough.
>>>>> Probably not, we will just pass the mappings in vhost_memory_region to
>>>>> vhost. Its memory_size is also a __u64.
>>>>>
>>>>> Thanks
>>>> That's different since that's chunks of qemu virtual memory.
>>>>
>>>> IOMMU maps IOVA to GPA.
>>>>
>>>
>>> But we're in fact cache IOVA -> HVA mapping in the remote IOTLB. When 
>>> passthrough mode is enabled, IOVA == GPA, so passing mappings in 
>>> vhost_memory_region should be fine.
>>
>> Not sure this is a good idea, because when configured in passthrough,
>> QEMU will see the IOMMU as enabled, so the the VIRTIO_F_IOMMU_PLATFORM
>> feature will be negotiated if both guest and backend support it.
>> So how the backend will know whether it should directly pick the
>> translation directly into the vhost_memory_region, or translate it
>> through the device IOTLB?
> 
> This no need for backend to know about this, since IOVA equals to GPA, 
> vhost_memory_region stores IOVA -> HVA mapping. If we pass them, device 
> IOTLB should work as usual?

Ok, I think there were a misunderstanding. I understood you said there
were no need to use the device IOTLB in this case.

> 
>>
>> Maybe the solution would be for QEMU to wrap "all memory" IOTLB updates
>> & invalidations to vhost_memory_regions, since the backend won't anyway
>> be able to perform accesses outside these regions?
> 
> This is just what I mean, you can refer Peter's series. >
>>
>>> The only possible "issue" with "all memory" is if you can not use a 
>>> single TLB invalidation to invalidate all caches in remote TLB.
>>
>> If needed, maybe we could introduce a new VHOST_IOTLB_INVALIDATE message
>> type?
>> For older kernel backend that doesn't support it, -EINVAL will be
>> returned, so QEMU could handle it another way in this case.
> 
> We could, but not sure it was really needed.

I meant VHOST_IOTLB_INVALIDATE_ALL, and yes, I'm not sure this is
needed. But this is an option we have it turns out to be at some point.

Thanks,
Maxime

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

* Re: [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier
  2017-05-18 15:24             ` Michael S. Tsirkin
@ 2017-05-19  9:48               ` Maxime Coquelin
  2017-05-19 20:37                 ` Michael S. Tsirkin
  0 siblings, 1 reply; 32+ messages in thread
From: Maxime Coquelin @ 2017-05-19  9:48 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman



On 05/18/2017 05:24 PM, Michael S. Tsirkin wrote:
> On Thu, May 18, 2017 at 04:45:23PM +0200, Maxime Coquelin wrote:
>> Hi Michael,
>>
>> On 05/18/2017 09:35 AM, Maxime Coquelin wrote:
>>>
>>>
>>> On 05/17/2017 06:41 PM, Michael S. Tsirkin wrote:
>>>> On Fri, May 12, 2017 at 01:21:18PM +0200, Maxime Coquelin wrote:
>>>>>
>>>>> On 05/11/2017 07:33 PM, Michael S. Tsirkin wrote:
>>>>>> On Thu, May 11, 2017 at 02:32:43PM +0200, Maxime Coquelin wrote:
>>>>>>> Vhost-kernel backend need to receive IOTLB entries for rings
>>>>>>> information early, but vhost-user need the same information
>>>>>>> earlier, before VHOST_USER_SET_VRING_ADDR is sent.
>>>>>> Weird. What does VHOST_USER_SET_VRING_ADDR have to do with it?
>>>>>>
>>>>>> According to
>>>>>>      Starting and stopping rings
>>>>>> in vhost user spec, vhost user does not access
>>>>>> anything until ring is started and enabled.
>>>>>>
>>>>>>
>>>>>>> This patch also trigger IOTLB miss for all rings informations
>>>>>>> for robustness, even if in practice these adresses are on the
>>>>>>> same page.
>>>>> Actually, the DPDK vhost-user backend is compliant with the spec,
>>>>> but when handling VHOST_USER_SET_VRING_ADDR request, it translates the
>>>>> guest addresses into backend VAs, and check they are valid. I
>>>>> will make the
>>>>> commit message clearer about this in next revision.
>>>>>
>>>>> The check could be done later, for example when the ring are started,
>>>>> but it wouldn't change the need to trigger a miss at some point.
>>>> I think it should be done later, yes. As long as ring is not
>>>> started addresses should not be interpreted.
>>>>
>>>
>>> Ok, then I'll move these addresses translations in the
>>> VHOST_USER_SET_VRING_KICK handler.
>> s/VHOST_USER_SET_VRING_KICK/VHOST_USER_SET_VRING_ENABLE/
> 
> Note that when protocol features are off ring is started in
> enabled state, but iommu requires protocol features.

OK, I will take care of this.

Note that currently in DPDK, the ring is created in enabled state,
so it is enabled as soon as started even with protocol features.
I have done the patch to fix this, will be posted with the patch that
do the ring addresses translations only when starting/enabling the ring.

Also, note that disabling VHOST_USER_F_PROTOCOL_FEATURES with latest
DPDK and QEMU seems broken. I'll add this to my todo list to understand
where the problem is, but this is lower priority.

>> I just looked at implementing this change, but I'm not convinced this is
>> the right thing to do.
>>
>> On backend side, it means saving temporarily the vhost_vring_addr struct
>> into the vq struct, and moving all what is done currently in
>> SET_VRING_ADDR handler to SET_VRING_ENABLE one.
> 
> Yes, and this is consistent with what the kernel does.
> 
>> My understanding of the "Starting and stopping rings" chapter of the
>> spec is that the ring must not be processed as long as not started and
>> enabled, not that the addresses passed should not be checked/translated
>> as it is done today both in DPDK and libvhost-user.
>>
>> If the addresses are invalid, isn't it better to know as soon as
>> possible?
>>
>> Cheers,
>> Maxime
> 
> There could be valid reasons to set an invalid address temporarily.
> For example to make sure connection is reset.

Ok.

Thanks,
Maxime

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

* Re: [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support
  2017-05-19  7:46       ` Jason Wang
@ 2017-05-19 16:42         ` Michael S. Tsirkin
  0 siblings, 0 replies; 32+ messages in thread
From: Michael S. Tsirkin @ 2017-05-19 16:42 UTC (permalink / raw)
  To: Jason Wang
  Cc: Maxime Coquelin, yuanhan.liu, qemu-devel, peterx,
	marcandre.lureau, wexu, vkaplans, jfreiman

On Fri, May 19, 2017 at 03:46:36PM +0800, Jason Wang wrote:
> 
> 
> On 2017年05月18日 16:43, Maxime Coquelin wrote:
> > > > 
> > > > +When the VHOST_USER_PROTOCOL_F_SLAVE_REQ is supported by the
> > > > slave, and the
> > > > +master initiated the slave to master communication channel using the
> > > > +VHOST_USER_SET_SLAVE_REQ_FD request, the slave can send IOTLB
> > > > miss and access
> > > > +failure events by sending VHOST_USER_SLAVE_IOTLB_MSG requests
> > > > to the master
> > > > +with a struct vhost_iotlb_msg payload. For miss events, the
> > > > iotlb payload has
> > > > +to be filled with the miss message type (1), the I/O virtual
> > > > address and the
> > > > +permissions flags. For access failure event, the iotlb payload
> > > > has to be
> > > > +filled with the access failure message type (4), the I/O
> > > > virtual address and
> > > > +the permissions flags.
> > > 
> > > I don't think slave should cache invalid entries. If it does not,
> > > how can it detect access failure as opposed to a miss?
> > 
> > Of course, invalid cache entries should not be cached.
> > The VHOST_IOTLB_ACCESS_FAIL has been specified for the Kernel backend,
> > even if the latter does not implement it yet.
> 
> Yes, I leave this for future use e.g reporting copy_to_user() failure to
> userspace.
> 
> Thanks

Interesting. And it's not handled now.
So let's add a text "reserved for reporting internal access
errors in the future. Should not be used for now.".

-- 
MST

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

* Re: [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier
  2017-05-19  9:48               ` Maxime Coquelin
@ 2017-05-19 20:37                 ` Michael S. Tsirkin
  0 siblings, 0 replies; 32+ messages in thread
From: Michael S. Tsirkin @ 2017-05-19 20:37 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: peterx, marcandre.lureau, vkaplans, jasowang, wexu, yuanhan.liu,
	qemu-devel, jfreiman

On Fri, May 19, 2017 at 11:48:52AM +0200, Maxime Coquelin wrote:
> 
> 
> On 05/18/2017 05:24 PM, Michael S. Tsirkin wrote:
> > On Thu, May 18, 2017 at 04:45:23PM +0200, Maxime Coquelin wrote:
> > > Hi Michael,
> > > 
> > > On 05/18/2017 09:35 AM, Maxime Coquelin wrote:
> > > > 
> > > > 
> > > > On 05/17/2017 06:41 PM, Michael S. Tsirkin wrote:
> > > > > On Fri, May 12, 2017 at 01:21:18PM +0200, Maxime Coquelin wrote:
> > > > > > 
> > > > > > On 05/11/2017 07:33 PM, Michael S. Tsirkin wrote:
> > > > > > > On Thu, May 11, 2017 at 02:32:43PM +0200, Maxime Coquelin wrote:
> > > > > > > > Vhost-kernel backend need to receive IOTLB entries for rings
> > > > > > > > information early, but vhost-user need the same information
> > > > > > > > earlier, before VHOST_USER_SET_VRING_ADDR is sent.
> > > > > > > Weird. What does VHOST_USER_SET_VRING_ADDR have to do with it?
> > > > > > > 
> > > > > > > According to
> > > > > > >      Starting and stopping rings
> > > > > > > in vhost user spec, vhost user does not access
> > > > > > > anything until ring is started and enabled.
> > > > > > > 
> > > > > > > 
> > > > > > > > This patch also trigger IOTLB miss for all rings informations
> > > > > > > > for robustness, even if in practice these adresses are on the
> > > > > > > > same page.
> > > > > > Actually, the DPDK vhost-user backend is compliant with the spec,
> > > > > > but when handling VHOST_USER_SET_VRING_ADDR request, it translates the
> > > > > > guest addresses into backend VAs, and check they are valid. I
> > > > > > will make the
> > > > > > commit message clearer about this in next revision.
> > > > > > 
> > > > > > The check could be done later, for example when the ring are started,
> > > > > > but it wouldn't change the need to trigger a miss at some point.
> > > > > I think it should be done later, yes. As long as ring is not
> > > > > started addresses should not be interpreted.
> > > > > 
> > > > 
> > > > Ok, then I'll move these addresses translations in the
> > > > VHOST_USER_SET_VRING_KICK handler.
> > > s/VHOST_USER_SET_VRING_KICK/VHOST_USER_SET_VRING_ENABLE/
> > 
> > Note that when protocol features are off ring is started in
> > enabled state, but iommu requires protocol features.
> 
> OK, I will take care of this.
> 
> Note that currently in DPDK, the ring is created in enabled state,
> so it is enabled as soon as started even with protocol features.
> I have done the patch to fix this, will be posted with the patch that
> do the ring addresses translations only when starting/enabling the ring.
> 
> Also, note that disabling VHOST_USER_F_PROTOCOL_FEATURES with latest
> DPDK and QEMU seems broken. I'll add this to my todo list to understand
> where the problem is, but this is lower priority.

And hopefully add a unit test without this so we don't break
it in the future.

> > > I just looked at implementing this change, but I'm not convinced this is
> > > the right thing to do.
> > > 
> > > On backend side, it means saving temporarily the vhost_vring_addr struct
> > > into the vq struct, and moving all what is done currently in
> > > SET_VRING_ADDR handler to SET_VRING_ENABLE one.
> > 
> > Yes, and this is consistent with what the kernel does.
> > 
> > > My understanding of the "Starting and stopping rings" chapter of the
> > > spec is that the ring must not be processed as long as not started and
> > > enabled, not that the addresses passed should not be checked/translated
> > > as it is done today both in DPDK and libvhost-user.
> > > 
> > > If the addresses are invalid, isn't it better to know as soon as
> > > possible?
> > > 
> > > Cheers,
> > > Maxime
> > 
> > There could be valid reasons to set an invalid address temporarily.
> > For example to make sure connection is reset.
> 
> Ok.
> 
> Thanks,
> Maxime

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

end of thread, other threads:[~2017-05-19 20:37 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-11 12:32 [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support Maxime Coquelin
2017-05-11 12:32 ` [Qemu-devel] [PATCH 1/6] vhost: propagate errors in vhost_device_iotlb_miss() Maxime Coquelin
2017-05-11 12:32 ` [Qemu-devel] [PATCH 2/6] vhost: rework IOTLB messaging Maxime Coquelin
2017-05-11 12:32 ` [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier Maxime Coquelin
2017-05-11 17:33   ` Michael S. Tsirkin
2017-05-12 11:21     ` Maxime Coquelin
2017-05-17 16:41       ` Michael S. Tsirkin
2017-05-18  7:35         ` Maxime Coquelin
2017-05-18 14:45           ` Maxime Coquelin
2017-05-18 15:24             ` Michael S. Tsirkin
2017-05-19  9:48               ` Maxime Coquelin
2017-05-19 20:37                 ` Michael S. Tsirkin
2017-05-11 12:32 ` [Qemu-devel] [PATCH 4/6] vhost-user: add vhost_user to hold the chr Maxime Coquelin
2017-05-11 12:32 ` [Qemu-devel] [PATCH 5/6] vhost-user: add slave-req-fd support Maxime Coquelin
2017-05-11 12:32 ` [Qemu-devel] [PATCH 6/6] spec/vhost-user spec: Add IOMMU support Maxime Coquelin
2017-05-11 18:25   ` Michael S. Tsirkin
2017-05-12 14:21     ` Maxime Coquelin
2017-05-13  0:02       ` Michael S. Tsirkin
2017-05-15  5:45         ` Jason Wang
2017-05-16 15:16           ` Michael S. Tsirkin
2017-05-17  2:53             ` Jason Wang
2017-05-17 14:10               ` Maxime Coquelin
2017-05-19  6:48                 ` Jason Wang
2017-05-19  8:35                   ` Maxime Coquelin
2017-05-17 15:27         ` Maxime Coquelin
2017-05-16  8:19       ` Maxime Coquelin
2017-05-16 13:23         ` Michael S. Tsirkin
2017-05-17 16:48   ` Michael S. Tsirkin
2017-05-18  8:43     ` Maxime Coquelin
2017-05-19  7:46       ` Jason Wang
2017-05-19 16:42         ` Michael S. Tsirkin
2017-05-11 13:25 ` [Qemu-devel] [PATCH 0/6] vhost-user: Specify and implement device IOTLB support no-reply

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.