virtio: clean up memory barrier usage

cleanup memory barrier usage bringing it
in sync with what linux guest does.
The rules are simple:
- read barrier after index read
- write barrier before index write
Also, call macros smp_rmb/smp_wmb to stress
we are not syncing with a real io device here.

While I don't think compiler is crazy/powerful
enough to reorder these, anyway, the bogus
barriers we currently have in code will confuse
anyone who tries to copy/reuse it.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Cc: Gleb Natapov <gleb@redhat.com>

diff --git a/src/virtio-ring.c b/src/virtio-ring.c
index 493d8b5e983ae648b98ffad311b5472ddc43bf4c..97a3ffee519bef72d7238deecae32e6474602058 100644 (file)
--- a/src/virtio-ring.c
+++ b/src/virtio-ring.c
@@ -38,8 +38,10 @@
 int vring_more_used(struct vring_virtqueue *vq)
 {
     struct vring_used *used = GET_FLATPTR(vq->vring.used);
-    wmb();
-    return GET_FLATPTR(vq->last_used_idx) != GET_FLATPTR(used->idx);
+    int more = GET_FLATPTR(vq->last_used_idx) != GET_FLATPTR(used->idx);
+    /* Make sure ring reads are done after idx read above. */
+    smp_rmb();
+    return more;
 }
 
 /*
@@ -63,7 +65,6 @@ void vring_detach(struct vring_virtqueue *vq, unsigned int head)
     /* link it with free list and point to it */
 
     SET_FLATPTR(desc[i].next, GET_FLATPTR(vq->free_head));
-    wmb();
     SET_FLATPTR(vq->free_head, head);
 }
 
@@ -85,7 +86,6 @@ int vring_get_buf(struct vring_virtqueue *vq, unsigned int *len)
 //    BUG_ON(!vring_more_used(vq));
 
     elem = &used->ring[GET_FLATPTR(vq->last_used_idx) % GET_FLATPTR(vr->num)];
-    wmb();
     id = GET_FLATPTR(elem->id);
     if (len != NULL)
         *len = GET_FLATPTR(elem->len);
@@ -136,7 +136,6 @@ void vring_add_buf(struct vring_virtqueue *vq,
 
     av = (GET_FLATPTR(avail->idx) + num_added) % GET_FLATPTR(vr->num);
     SET_FLATPTR(avail->ring[av], head);
-    wmb();
 }
 
 void vring_kick(unsigned int ioaddr, struct vring_virtqueue *vq, int num_added)
@@ -144,9 +143,9 @@ void vring_kick(unsigned int ioaddr, struct vring_virtqueue *vq, int num_added)
     struct vring *vr = &vq->vring;
     struct vring_avail *avail = GET_FLATPTR(vr->avail);
 
-    wmb();
+    /* Make sure idx update is done after ring write. */
+    smp_wmb();
     SET_FLATPTR(avail->idx, GET_FLATPTR(avail->idx) + num_added);
 
-    mb();
     vp_notify(ioaddr, GET_FLATPTR(vq->queue_index));
 }

diff --git a/src/virtio-ring.h b/src/virtio-ring.h
index 014defc34917c3b3888e38e22a05d872343f8d04..b7a7aafb238df38d5fb19ac6e92f74deb4e9a8de 100644 (file)
--- a/src/virtio-ring.h
+++ b/src/virtio-ring.h
@@ -9,8 +9,9 @@
 
 #define virt_to_phys(v) (unsigned long)(v)
 #define phys_to_virt(p) (void*)(p)
-#define wmb() barrier()
-#define mb() barrier()
+/* Compiler barrier is enough as an x86 CPU does not reorder reads or writes */
+#define smp_rmb() barrier()
+#define smp_wmb() barrier()
 
 /* Status byte for guest to report progress, and synchronize features. */
 /* We have seen device and processed generic fields (VIRTIO_CONFIG_F_VIRTIO) */