[Qemu-devel] [PULL 02/17] Revert "virtio: turn vq->notification into a nested counter"

[Michael S. Tsirkin]

From: Stefan Hajnoczi <address@hidden>

This reverts commit aff8fd18f1786fc5af259a9bc0077727222f51ca.

Both virtio-net and virtio-crypto do not balance
virtio_queue_set_notification() enable and disable calls.  This makes
the notifications_disabled counter unreliable and Doug Goldstein
reported the following assertion failure:

  #3  0x00007ffff44d1c62 in __GI___assert_fail (
      address@hidden "vq->notification_disabled > 0",
      address@hidden "/home/doug/work/qemu/hw/virtio/virtio.c",
      address@hidden,
      address@hidden <__PRETTY_FUNCTION__.43707>
      "virtio_queue_set_notification") at assert.c:101
  #4  0x00005555557f25d6 in virtio_queue_set_notification (vq=0x55555666aa90,
      address@hidden) at /home/doug/work/qemu/hw/virtio/virtio.c:215
  #5  0x00005555557dc311 in virtio_net_has_buffers (q=<optimized out>,
      q=<optimized out>, bufsize=102)
      at /home/doug/work/qemu/hw/net/virtio-net.c:1008
  #6  virtio_net_receive (nc=<optimized out>, buf=0x555557386b88 "", size=102)
      at /home/doug/work/qemu/hw/net/virtio-net.c:1148
  #7  0x00005555559cad33 in nc_sendv_compat (flags=<optimized out>, iovcnt=1,
      iov=0x7fffead746d0, nc=0x55555788b340) at net/net.c:705
  #8  qemu_deliver_packet_iov (sender=<optimized out>, flags=<optimized out>,
      iov=0x7fffead746d0, iovcnt=1, opaque=0x55555788b340) at net/net.c:732
  #9  0x00005555559cd929 in qemu_net_queue_deliver (size=<optimized out>,
      data=<optimized out>, flags=<optimized out>, sender=<optimized out>,
      queue=0x55555788b550) at net/queue.c:164
  #10 qemu_net_queue_flush (queue=0x55555788b550) at net/queue.c:261

This patch is safe to revert since it's just an optimization for
virtqueue polling.  The next patch will improve the situation again
without resorting to nesting.

Reported-by: Doug Goldstein <address@hidden>
Signed-off-by: Stefan Hajnoczi <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Michael S. Tsirkin <address@hidden>
Tested-by: Richard Henderson <address@hidden>
Tested-by: Laszlo Ersek <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Michael S. Tsirkin <address@hidden>
---
 hw/virtio/virtio.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index aa4f38f..f04ab7a 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -88,8 +88,8 @@ struct VirtQueue
     /* Last used index value we have signalled on */
     bool signalled_used_valid;
 
-    /* Nested host->guest notification disabled counter */
-    unsigned int notification_disabled;
+    /* Notification enabled? */
+    bool notification;
 
     uint16_t queue_index;
 
@@ -202,7 +202,7 @@ static inline void vring_used_flags_unset_bit(VirtQueue 
*vq, int mask)
 static inline void vring_set_avail_event(VirtQueue *vq, uint16_t val)
 {
     hwaddr pa;
-    if (vq->notification_disabled) {
+    if (!vq->notification) {
         return;
     }
     pa = vq->vring.used + offsetof(VRingUsed, ring[vq->vring.num]);
@@ -211,13 +211,7 @@ static inline void vring_set_avail_event(VirtQueue *vq, 
uint16_t val)
 
 void virtio_queue_set_notification(VirtQueue *vq, int enable)
 {
-    if (enable) {
-        assert(vq->notification_disabled > 0);
-        vq->notification_disabled--;
-    } else {
-        vq->notification_disabled++;
-    }
-
+    vq->notification = enable;
     if (virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
         vring_set_avail_event(vq, vring_avail_idx(vq));
     } else if (enable) {
@@ -1020,7 +1014,7 @@ void virtio_reset(void *opaque)
         virtio_queue_set_vector(vdev, i, VIRTIO_NO_VECTOR);
         vdev->vq[i].signalled_used = 0;
         vdev->vq[i].signalled_used_valid = false;
-        vdev->vq[i].notification_disabled = 0;
+        vdev->vq[i].notification = true;
         vdev->vq[i].vring.num = vdev->vq[i].vring.num_default;
         vdev->vq[i].inuse = 0;
     }
@@ -1831,7 +1825,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int 
version_id)
         vdev->vq[i].vring.desc = qemu_get_be64(f);
         qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
         vdev->vq[i].signalled_used_valid = false;
-        vdev->vq[i].notification_disabled = 0;
+        vdev->vq[i].notification = true;
 
         if (vdev->vq[i].vring.desc) {
             /* XXX virtio-1 devices */
-- 
MST