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(a)redhat.com>
Cc: Gleb Natapov <gleb(a)redhat.com>
---
This depends on the NO_NOTIFY patch I sent earlier.
Note: compile-tested only
Gleb, could you try it out pls?
src/virtio-ring.c | 13 ++++++-------
src/virtio-ring.h | 5 +++--
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/virtio-ring.c b/src/virtio-ring.c
index 493d8b5..97a3ffe 100644
--- 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 3fb86fe..8b546f4 100644
--- 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) */
--
1.7.1.12.g42b7f