[Qemu-devel] [PATCH 0/6] virtio: use ioeventfd in TCG and qtest mode

[Qemu-devel] [PATCH 0/6] virtio: use ioeventfd in TCG and qtest mode

[Stefan Hajnoczi]

This patch series fixes qemu-iotests 068.  Since commit
ea4f3cebc4e0224605ab9dd9724aa4e7768fe372 ("qemu-iotests: 068: test iothread
mode") the test case has attempted to use dataplane without -M accel=kvm.
Although QEMU is capable of running TCG or qtest with emulated ioeventfd/irqfd
we haven't enabled it yet.

Unfortunately the virtio test cases fail when ioeventfd is enabled in qtest
mode.  This is because they make assumptions about virtqueue ISR signalling.
They assume that a request is completed when ISR becomes 1.  However, the ISR
can be set to 1 even though no new request has completed since commit
83d768b5640946b7da55ce8335509df297e2c7cd "virtio: set ISR on dataplane
notifications".

This issue is solved by introducing a proper qvirtqueue_get_buf() API (similar
to the Linux guest drivers) instead of making assumptions about the ISR.  Most
of the patches update the test cases to use the new API.

Stefan Hajnoczi (6):
  libqos: fix typo in virtio.h QVirtQueue->used comment
  libqos: add virtio used ring support
  tests: fix virtio-scsi-test ISR dependence
  tests: fix virtio-blk-test ISR dependence
  tests: fix virtio-net-test ISR dependence
  virtio-pci: use ioeventfd even when KVM is disabled

 tests/libqos/virtio.h    |  8 ++++++-
 hw/virtio/virtio-pci.c   |  2 +-
 tests/libqos/virtio.c    | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
 tests/virtio-blk-test.c  | 27 ++++++++++++++--------
 tests/virtio-net-test.c  |  6 ++---
 tests/virtio-scsi-test.c |  2 +-
 6 files changed, 89 insertions(+), 16 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH 1/6] libqos: fix typo in virtio.h QVirtQueue->used comment

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqos/virtio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 3397a08..829de5e 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -26,7 +26,7 @@ typedef struct QVirtioDevice {
 typedef struct QVirtQueue {
     uint64_t desc; /* This points to an array of struct vring_desc */
     uint64_t avail; /* This points to a struct vring_avail */
-    uint64_t used; /* This points to a struct vring_desc */
+    uint64_t used; /* This points to a struct vring_used */
     uint16_t index;
     uint32_t size;
     uint32_t free_head;
-- 
2.9.4

Re: [Qemu-devel] [PATCH 1/6] libqos: fix typo in virtio.h QVirtQueue->used comment

[Fam Zheng]

On Wed, 06/28 19:47, Stefan Hajnoczi wrote:
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/libqos/virtio.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
> index 3397a08..829de5e 100644
> --- a/tests/libqos/virtio.h
> +++ b/tests/libqos/virtio.h
> @@ -26,7 +26,7 @@ typedef struct QVirtioDevice {
>  typedef struct QVirtQueue {
>      uint64_t desc; /* This points to an array of struct vring_desc */
>      uint64_t avail; /* This points to a struct vring_avail */
> -    uint64_t used; /* This points to a struct vring_desc */
> +    uint64_t used; /* This points to a struct vring_used */
>      uint16_t index;
>      uint32_t size;
>      uint32_t free_head;
> -- 
> 2.9.4
> 
> 

Reviewed-by: Fam Zheng <famz@redhat.com>

[Qemu-devel] [PATCH 2/6] libqos: add virtio used ring support

[Stefan Hajnoczi]

Existing tests do not touch the virtqueue used ring.  Instead they poll
the virtqueue ISR register and peek into their request's device-specific
status field.

It turns out that the virtqueue ISR register can be set to 1 more than
once for a single notification (see commit
83d768b5640946b7da55ce8335509df297e2c7cd "virtio: set ISR on dataplane
notifications").  This causes problems for tests that assume a 1:1
correspondence between the ISR being 1 and request completion.

Peeking at device-specific status fields is also problematic if the
device has no field that can be abused for EINPROGRESS polling
semantics.  This is the case if all the field's values may be set by the
device; there's no magic constant left for polling.

It's time to process the used ring for completed requests, just like a
real virtio guest driver.  This patch adds the necessary APIs.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqos/virtio.h |  6 ++++++
 tests/libqos/virtio.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 829de5e..8fbcd18 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -32,6 +32,7 @@ typedef struct QVirtQueue {
     uint32_t free_head;
     uint32_t num_free;
     uint32_t align;
+    uint16_t last_used_idx;
     bool indirect;
     bool event;
 } QVirtQueue;
@@ -120,6 +121,10 @@ uint8_t qvirtio_wait_status_byte_no_isr(QVirtioDevice *d,
                                         QVirtQueue *vq,
                                         uint64_t addr,
                                         gint64 timeout_us);
+void qvirtio_wait_used_elem(QVirtioDevice *d,
+                            QVirtQueue *vq,
+                            uint32_t desc_idx,
+                            gint64 timeout_us);
 void qvirtio_wait_config_isr(QVirtioDevice *d, gint64 timeout_us);
 QVirtQueue *qvirtqueue_setup(QVirtioDevice *d,
                              QGuestAllocator *alloc, uint16_t index);
@@ -135,6 +140,7 @@ uint32_t qvirtqueue_add(QVirtQueue *vq, uint64_t data, uint32_t len, bool write,
                                                                     bool next);
 uint32_t qvirtqueue_add_indirect(QVirtQueue *vq, QVRingIndirectDesc *indirect);
 void qvirtqueue_kick(QVirtioDevice *d, QVirtQueue *vq, uint32_t free_head);
+bool qvirtqueue_get_buf(QVirtQueue *vq, uint32_t *desc_idx);
 
 void qvirtqueue_set_used_event(QVirtQueue *vq, uint16_t idx);
 #endif
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index ec30cb9..9880a69 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -116,6 +116,35 @@ uint8_t qvirtio_wait_status_byte_no_isr(QVirtioDevice *d,
     return val;
 }
 
+/*
+ * qvirtio_wait_used_elem:
+ * @desc_idx: The next expected vq->desc[] index in the used ring
+ * @timeout_us: How many microseconds to wait before failing
+ *
+ * This function waits for the next completed request on the used ring.
+ */
+void qvirtio_wait_used_elem(QVirtioDevice *d,
+                            QVirtQueue *vq,
+                            uint32_t desc_idx,
+                            gint64 timeout_us)
+{
+    gint64 start_time = g_get_monotonic_time();
+
+    for (;;) {
+        uint32_t got_desc_idx;
+
+        clock_step(100);
+
+        if (d->bus->get_queue_isr_status(d, vq) &&
+            qvirtqueue_get_buf(vq, &got_desc_idx)) {
+            g_assert_cmpint(got_desc_idx, ==, desc_idx);
+            return;
+        }
+
+        g_assert(g_get_monotonic_time() - start_time <= timeout_us);
+    }
+}
+
 void qvirtio_wait_config_isr(QVirtioDevice *d, gint64 timeout_us)
 {
     gint64 start_time = g_get_monotonic_time();
@@ -272,6 +301,37 @@ void qvirtqueue_kick(QVirtioDevice *d, QVirtQueue *vq, uint32_t free_head)
     }
 }
 
+/*
+ * qvirtqueue_get_buf:
+ * @desc_idx: A pointer that is filled with the vq->desc[] index, may be NULL
+ *
+ * This function gets the next used element if there is one ready.
+ *
+ * Returns: true if an element was ready, false otherwise
+ */
+bool qvirtqueue_get_buf(QVirtQueue *vq, uint32_t *desc_idx)
+{
+    uint16_t idx;
+
+    idx = readw(vq->used + offsetof(struct vring_used, idx));
+    if (idx == vq->last_used_idx) {
+        return false;
+    }
+
+    if (desc_idx) {
+        uint64_t elem_addr;
+
+        elem_addr = vq->used +
+                    offsetof(struct vring_used, ring) +
+                    (vq->last_used_idx % vq->size) *
+                    sizeof(struct vring_used_elem);
+        *desc_idx = readl(elem_addr + offsetof(struct vring_used_elem, id));
+    }
+
+    vq->last_used_idx++;
+    return true;
+}
+
 void qvirtqueue_set_used_event(QVirtQueue *vq, uint16_t idx)
 {
     g_assert(vq->event);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 2/6] libqos: add virtio used ring support

[Fam Zheng]

On Wed, 06/28 19:47, Stefan Hajnoczi wrote:
> Existing tests do not touch the virtqueue used ring.  Instead they poll
> the virtqueue ISR register and peek into their request's device-specific
> status field.
> 
> It turns out that the virtqueue ISR register can be set to 1 more than
> once for a single notification (see commit
> 83d768b5640946b7da55ce8335509df297e2c7cd "virtio: set ISR on dataplane
> notifications").  This causes problems for tests that assume a 1:1
> correspondence between the ISR being 1 and request completion.
> 
> Peeking at device-specific status fields is also problematic if the
> device has no field that can be abused for EINPROGRESS polling
> semantics.  This is the case if all the field's values may be set by the
> device; there's no magic constant left for polling.
> 
> It's time to process the used ring for completed requests, just like a
> real virtio guest driver.  This patch adds the necessary APIs.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Fam Zheng <famz@redhat.com>

[Qemu-devel] [PATCH 3/6] tests: fix virtio-scsi-test ISR dependence

[Stefan Hajnoczi]

Use the new used ring APIs instead of assuming ISR being set means the
request has completed.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/virtio-scsi-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index eff71df..87a3b6e 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -121,7 +121,7 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, const uint8_t *cdb,
     }
 
     qvirtqueue_kick(vs->dev, vq, free_head);
-    qvirtio_wait_queue_isr(vs->dev, vq, QVIRTIO_SCSI_TIMEOUT_US);
+    qvirtio_wait_used_elem(vs->dev, vq, free_head, QVIRTIO_SCSI_TIMEOUT_US);
 
     response = readb(resp_addr +
                      offsetof(struct virtio_scsi_cmd_resp, response));
-- 
2.9.4

Re: [Qemu-devel] [PATCH 3/6] tests: fix virtio-scsi-test ISR dependence

[Fam Zheng]

On Wed, 06/28 19:47, Stefan Hajnoczi wrote:
> Use the new used ring APIs instead of assuming ISR being set means the
> request has completed.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Fam Zheng <famz@redhat.com>

[Qemu-devel] [PATCH 4/6] tests: fix virtio-blk-test ISR dependence

[Stefan Hajnoczi]

Use the new used ring APIs instead of assuming ISR being set means the
request has completed.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/virtio-blk-test.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index fd2078c..0576cb1 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -196,7 +196,7 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
 
     qvirtqueue_kick(dev, vq, free_head);
 
-    qvirtio_wait_queue_isr(dev, vq, QVIRTIO_BLK_TIMEOUT_US);
+    qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_BLK_TIMEOUT_US);
     status = readb(req_addr + 528);
     g_assert_cmpint(status, ==, 0);
 
@@ -218,7 +218,7 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
 
     qvirtqueue_kick(dev, vq, free_head);
 
-    qvirtio_wait_queue_isr(dev, vq, QVIRTIO_BLK_TIMEOUT_US);
+    qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_BLK_TIMEOUT_US);
     status = readb(req_addr + 528);
     g_assert_cmpint(status, ==, 0);
 
@@ -246,7 +246,7 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
         qvirtqueue_add(vq, req_addr + 528, 1, true, false);
         qvirtqueue_kick(dev, vq, free_head);
 
-        qvirtio_wait_queue_isr(dev, vq, QVIRTIO_BLK_TIMEOUT_US);
+        qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_BLK_TIMEOUT_US);
         status = readb(req_addr + 528);
         g_assert_cmpint(status, ==, 0);
 
@@ -267,7 +267,7 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
 
         qvirtqueue_kick(dev, vq, free_head);
 
-        qvirtio_wait_queue_isr(dev, vq, QVIRTIO_BLK_TIMEOUT_US);
+        qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_BLK_TIMEOUT_US);
         status = readb(req_addr + 528);
         g_assert_cmpint(status, ==, 0);
 
@@ -348,7 +348,7 @@ static void pci_indirect(void)
     free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
     qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
 
-    qvirtio_wait_queue_isr(&dev->vdev, &vqpci->vq,
+    qvirtio_wait_used_elem(&dev->vdev, &vqpci->vq, free_head,
                            QVIRTIO_BLK_TIMEOUT_US);
     status = readb(req_addr + 528);
     g_assert_cmpint(status, ==, 0);
@@ -373,7 +373,7 @@ static void pci_indirect(void)
     free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
     qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
 
-    qvirtio_wait_queue_isr(&dev->vdev, &vqpci->vq,
+    qvirtio_wait_used_elem(&dev->vdev, &vqpci->vq, free_head,
                            QVIRTIO_BLK_TIMEOUT_US);
     status = readb(req_addr + 528);
     g_assert_cmpint(status, ==, 0);
@@ -484,7 +484,7 @@ static void pci_msix(void)
     qvirtqueue_add(&vqpci->vq, req_addr + 528, 1, true, false);
     qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
 
-    qvirtio_wait_queue_isr(&dev->vdev, &vqpci->vq,
+    qvirtio_wait_used_elem(&dev->vdev, &vqpci->vq, free_head,
                            QVIRTIO_BLK_TIMEOUT_US);
 
     status = readb(req_addr + 528);
@@ -509,7 +509,7 @@ static void pci_msix(void)
     qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
 
 
-    qvirtio_wait_queue_isr(&dev->vdev, &vqpci->vq,
+    qvirtio_wait_used_elem(&dev->vdev, &vqpci->vq, free_head,
                            QVIRTIO_BLK_TIMEOUT_US);
 
     status = readb(req_addr + 528);
@@ -540,6 +540,8 @@ static void pci_idx(void)
     uint64_t capacity;
     uint32_t features;
     uint32_t free_head;
+    uint32_t write_head;
+    uint32_t desc_idx;
     uint8_t status;
     char *data;
 
@@ -581,7 +583,8 @@ static void pci_idx(void)
     qvirtqueue_add(&vqpci->vq, req_addr + 528, 1, true, false);
     qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
 
-    qvirtio_wait_queue_isr(&dev->vdev, &vqpci->vq, QVIRTIO_BLK_TIMEOUT_US);
+    qvirtio_wait_used_elem(&dev->vdev, &vqpci->vq, free_head,
+                           QVIRTIO_BLK_TIMEOUT_US);
 
     /* Write request */
     req.type = VIRTIO_BLK_T_OUT;
@@ -600,6 +603,7 @@ static void pci_idx(void)
     qvirtqueue_add(&vqpci->vq, req_addr + 16, 512, false, true);
     qvirtqueue_add(&vqpci->vq, req_addr + 528, 1, true, false);
     qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
+    write_head = free_head;
 
     /* No notification expected */
     status = qvirtio_wait_status_byte_no_isr(&dev->vdev,
@@ -625,8 +629,11 @@ static void pci_idx(void)
 
     qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
 
-    qvirtio_wait_queue_isr(&dev->vdev, &vqpci->vq,
+    /* We get just one notification for both requests */
+    qvirtio_wait_used_elem(&dev->vdev, &vqpci->vq, write_head,
                            QVIRTIO_BLK_TIMEOUT_US);
+    g_assert(qvirtqueue_get_buf(&vqpci->vq, &desc_idx));
+    g_assert_cmpint(desc_idx, ==, free_head);
 
     status = readb(req_addr + 528);
     g_assert_cmpint(status, ==, 0);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 4/6] tests: fix virtio-blk-test ISR dependence

[Fam Zheng]

On Wed, 06/28 19:47, Stefan Hajnoczi wrote:
> Use the new used ring APIs instead of assuming ISR being set means the
> request has completed.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Fam Zheng <famz@redhat.com>

[Qemu-devel] [PATCH 5/6] tests: fix virtio-net-test ISR dependence

[Stefan Hajnoczi]

Use the new used ring APIs instead of assuming ISR being set means the
request has completed.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/virtio-net-test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index 8f94360..635b942 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -108,7 +108,7 @@ static void rx_test(QVirtioDevice *dev,
     ret = iov_send(socket, iov, 2, 0, sizeof(len) + sizeof(test));
     g_assert_cmpint(ret, ==, sizeof(test) + sizeof(len));
 
-    qvirtio_wait_queue_isr(dev, vq, QVIRTIO_NET_TIMEOUT_US);
+    qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_NET_TIMEOUT_US);
     memread(req_addr + VNET_HDR_SIZE, buffer, sizeof(test));
     g_assert_cmpstr(buffer, ==, "TEST");
 
@@ -131,7 +131,7 @@ static void tx_test(QVirtioDevice *dev,
     free_head = qvirtqueue_add(vq, req_addr, 64, false, false);
     qvirtqueue_kick(dev, vq, free_head);
 
-    qvirtio_wait_queue_isr(dev, vq, QVIRTIO_NET_TIMEOUT_US);
+    qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_NET_TIMEOUT_US);
     guest_free(alloc, req_addr);
 
     ret = qemu_recv(socket, &len, sizeof(len), 0);
@@ -182,7 +182,7 @@ static void rx_stop_cont_test(QVirtioDevice *dev,
     rsp = qmp("{ 'execute' : 'cont'}");
     QDECREF(rsp);
 
-    qvirtio_wait_queue_isr(dev, vq, QVIRTIO_NET_TIMEOUT_US);
+    qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_NET_TIMEOUT_US);
     memread(req_addr + VNET_HDR_SIZE, buffer, sizeof(test));
     g_assert_cmpstr(buffer, ==, "TEST");
 
-- 
2.9.4

Re: [Qemu-devel] [PATCH 5/6] tests: fix virtio-net-test ISR dependence

[Fam Zheng]

On Wed, 06/28 19:47, Stefan Hajnoczi wrote:
> Use the new used ring APIs instead of assuming ISR being set means the
> request has completed.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Fam Zheng <famz@redhat.com>

[Qemu-devel] [PATCH 6/6] virtio-pci: use ioeventfd even when KVM is disabled

[Stefan Hajnoczi]

Old kvm.ko versions only supported a tiny number of ioeventfds so
virtio-pci avoids ioeventfds when kvm_has_many_ioeventfds() returns 0.

Do not check kvm_has_many_ioeventfds() when KVM is disabled since it
always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.

This patch makes -device virtio-blk-pci,iothread=iothread0 work even
when KVM is disabled.

I have tested that virtio-blk-pci works under TCG both with and without
iothread.

This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.

Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-id: 20170615163813.7255-2-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/virtio/virtio-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 20d6a08..301920e 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1740,7 +1740,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
     bool pcie_port = pci_bus_is_express(pci_dev->bus) &&
                      !pci_bus_is_root(pci_dev->bus);
 
-    if (!kvm_has_many_ioeventfds()) {
+    if (kvm_enabled() && !kvm_has_many_ioeventfds()) {
         proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
     }
 
-- 
2.9.4

Re: [Qemu-devel] [PATCH 6/6] virtio-pci: use ioeventfd even when KVM is disabled

[Fam Zheng]

On Wed, 06/28 19:47, Stefan Hajnoczi wrote:
> Old kvm.ko versions only supported a tiny number of ioeventfds so
> virtio-pci avoids ioeventfds when kvm_has_many_ioeventfds() returns 0.
> 
> Do not check kvm_has_many_ioeventfds() when KVM is disabled since it
> always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> qtest or TCG mode.
> 
> This patch makes -device virtio-blk-pci,iothread=iothread0 work even
> when KVM is disabled.
> 
> I have tested that virtio-blk-pci works under TCG both with and without
> iothread.
> 
> This patch fixes qemu-iotests 068, which was accidentally merged early
> despite the dependency on ioeventfd.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Message-id: 20170615163813.7255-2-stefanha@redhat.com
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH 0/6] virtio: use ioeventfd in TCG and qtest mode

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1636 bytes --]

On 06/28/2017 01:47 PM, Stefan Hajnoczi wrote:
> This patch series fixes qemu-iotests 068.  Since commit
> ea4f3cebc4e0224605ab9dd9724aa4e7768fe372 ("qemu-iotests: 068: test iothread
> mode") the test case has attempted to use dataplane without -M accel=kvm.
> Although QEMU is capable of running TCG or qtest with emulated ioeventfd/irqfd
> we haven't enabled it yet.
> 
> Unfortunately the virtio test cases fail when ioeventfd is enabled in qtest
> mode.  This is because they make assumptions about virtqueue ISR signalling.
> They assume that a request is completed when ISR becomes 1.  However, the ISR
> can be set to 1 even though no new request has completed since commit
> 83d768b5640946b7da55ce8335509df297e2c7cd "virtio: set ISR on dataplane
> notifications".
> 
> This issue is solved by introducing a proper qvirtqueue_get_buf() API (similar
> to the Linux guest drivers) instead of making assumptions about the ISR.  Most
> of the patches update the test cases to use the new API.
> 
> Stefan Hajnoczi (6):
>   libqos: fix typo in virtio.h QVirtQueue->used comment
>   libqos: add virtio used ring support
>   tests: fix virtio-scsi-test ISR dependence
>   tests: fix virtio-blk-test ISR dependence
>   tests: fix virtio-net-test ISR dependence
>   virtio-pci: use ioeventfd even when KVM is disabled

I'm less familiar with the code in question, so I'll let others review,
but it did fix the failure of 068 for me.

Tested-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 0/6] virtio: use ioeventfd in TCG and qtest mode

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 1724 bytes --]

Am 28.06.2017 um 21:38 hat Eric Blake geschrieben:
> On 06/28/2017 01:47 PM, Stefan Hajnoczi wrote:
> > This patch series fixes qemu-iotests 068.  Since commit
> > ea4f3cebc4e0224605ab9dd9724aa4e7768fe372 ("qemu-iotests: 068: test iothread
> > mode") the test case has attempted to use dataplane without -M accel=kvm.
> > Although QEMU is capable of running TCG or qtest with emulated ioeventfd/irqfd
> > we haven't enabled it yet.
> > 
> > Unfortunately the virtio test cases fail when ioeventfd is enabled in qtest
> > mode.  This is because they make assumptions about virtqueue ISR signalling.
> > They assume that a request is completed when ISR becomes 1.  However, the ISR
> > can be set to 1 even though no new request has completed since commit
> > 83d768b5640946b7da55ce8335509df297e2c7cd "virtio: set ISR on dataplane
> > notifications".
> > 
> > This issue is solved by introducing a proper qvirtqueue_get_buf() API (similar
> > to the Linux guest drivers) instead of making assumptions about the ISR.  Most
> > of the patches update the test cases to use the new API.
> > 
> > Stefan Hajnoczi (6):
> >   libqos: fix typo in virtio.h QVirtQueue->used comment
> >   libqos: add virtio used ring support
> >   tests: fix virtio-scsi-test ISR dependence
> >   tests: fix virtio-blk-test ISR dependence
> >   tests: fix virtio-net-test ISR dependence
> >   virtio-pci: use ioeventfd even when KVM is disabled
> 
> I'm less familiar with the code in question, so I'll let others review,
> but it did fix the failure of 068 for me.
> 
> Tested-by: Eric Blake <eblake@redhat.com>

Also passes the virtio-scsi qtest under valgrind for me now.

Tested-by: Kevin Wolf <kwolf@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH 0/6] virtio: use ioeventfd in TCG and qtest mode

[Michael S. Tsirkin]

On Wed, Jun 28, 2017 at 07:47:18PM +0100, Stefan Hajnoczi wrote:
> This patch series fixes qemu-iotests 068.  Since commit
> ea4f3cebc4e0224605ab9dd9724aa4e7768fe372 ("qemu-iotests: 068: test iothread
> mode") the test case has attempted to use dataplane without -M accel=kvm.
> Although QEMU is capable of running TCG or qtest with emulated ioeventfd/irqfd
> we haven't enabled it yet.
> 
> Unfortunately the virtio test cases fail when ioeventfd is enabled in qtest
> mode.  This is because they make assumptions about virtqueue ISR signalling.
> They assume that a request is completed when ISR becomes 1.  However, the ISR
> can be set to 1 even though no new request has completed since commit
> 83d768b5640946b7da55ce8335509df297e2c7cd "virtio: set ISR on dataplane
> notifications".
> 
> This issue is solved by introducing a proper qvirtqueue_get_buf() API (similar
> to the Linux guest drivers) instead of making assumptions about the ISR.  Most
> of the patches update the test cases to use the new API.
> 
> Stefan Hajnoczi (6):
>   libqos: fix typo in virtio.h QVirtQueue->used comment
>   libqos: add virtio used ring support
>   tests: fix virtio-scsi-test ISR dependence
>   tests: fix virtio-blk-test ISR dependence
>   tests: fix virtio-net-test ISR dependence
>   virtio-pci: use ioeventfd even when KVM is disabled


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Feel free to merge.

>  tests/libqos/virtio.h    |  8 ++++++-
>  hw/virtio/virtio-pci.c   |  2 +-
>  tests/libqos/virtio.c    | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/virtio-blk-test.c  | 27 ++++++++++++++--------
>  tests/virtio-net-test.c  |  6 ++---
>  tests/virtio-scsi-test.c |  2 +-
>  6 files changed, 89 insertions(+), 16 deletions(-)
> 
> -- 
> 2.9.4

Re: [Qemu-devel] [PATCH 0/6] virtio: use ioeventfd in TCG and qtest mode

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

On Wed, Jun 28, 2017 at 07:47:18PM +0100, Stefan Hajnoczi wrote:
> This patch series fixes qemu-iotests 068.  Since commit
> ea4f3cebc4e0224605ab9dd9724aa4e7768fe372 ("qemu-iotests: 068: test iothread
> mode") the test case has attempted to use dataplane without -M accel=kvm.
> Although QEMU is capable of running TCG or qtest with emulated ioeventfd/irqfd
> we haven't enabled it yet.
> 
> Unfortunately the virtio test cases fail when ioeventfd is enabled in qtest
> mode.  This is because they make assumptions about virtqueue ISR signalling.
> They assume that a request is completed when ISR becomes 1.  However, the ISR
> can be set to 1 even though no new request has completed since commit
> 83d768b5640946b7da55ce8335509df297e2c7cd "virtio: set ISR on dataplane
> notifications".
> 
> This issue is solved by introducing a proper qvirtqueue_get_buf() API (similar
> to the Linux guest drivers) instead of making assumptions about the ISR.  Most
> of the patches update the test cases to use the new API.
> 
> Stefan Hajnoczi (6):
>   libqos: fix typo in virtio.h QVirtQueue->used comment
>   libqos: add virtio used ring support
>   tests: fix virtio-scsi-test ISR dependence
>   tests: fix virtio-blk-test ISR dependence
>   tests: fix virtio-net-test ISR dependence
>   virtio-pci: use ioeventfd even when KVM is disabled
> 
>  tests/libqos/virtio.h    |  8 ++++++-
>  hw/virtio/virtio-pci.c   |  2 +-
>  tests/libqos/virtio.c    | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/virtio-blk-test.c  | 27 ++++++++++++++--------
>  tests/virtio-net-test.c  |  6 ++---
>  tests/virtio-scsi-test.c |  2 +-
>  6 files changed, 89 insertions(+), 16 deletions(-)
> 
> -- 
> 2.9.4
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]