virtio: add missing mb() on notification

During normal operation, virtio first writes a used index
and then checks whether it should interrupt the guest
by reading guest avail index/flag values.

Guest does the reverse: writes the index/flag,
then checks the used ring.

The ordering is important: if host avail flag read bypasses the used
index write, we could in effect get this timing:

host avail flag read
		guest enable interrupts: avail flag write
		guest check used ring: ring is empty
host used index write

which results in a lost interrupt: guest will never be notified
about the used ring update.

This actually can happen when using kvm with an io thread,
such that the guest vcpu and qemu run on different host cpus,
and this has actually been observed in the field
(but only seems to trigger on very specific processor types)
with userspace virtio: vhost has the necessary smp_mb()
in place to prevent the regordering, so the same workload stalls
forever waiting for an interrupt with vhost=off but works
fine with vhost=on.

Insert an smp_mb barrier operation in userspace virtio to
ensure the correct ordering.
Applying this patch fixed the race condition we have observed.
Tested on x86_64. I checked the code generated by the new macro
for i386 and ppc but didn't run virtio.

Note: mb could in theory be implemented by __sync_synchronize, but this
would make us hit old GCC bugs. Besides old GCC
not implementing __sync_synchronize at all, there were bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36793
in this functionality as recently as in 4.3.

As we need asm for rmb,wmb anyway, it's just as well to
use it for mb.

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

1 files changed, 2 insertions, 0 deletions

diff --git a/hw/virtio.c b/hw/virtio.c
index 314abf8a18..bb994c9b51 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -700,6 +700,8 @@ static bool vring_notify(VirtIODevice *vdev, VirtQueue *vq)
 {
     uint16_t old, new;
     bool v;
+    /* We need to expose used array entries before checking used event. */
+    smp_mb();
     /* Always notify when queue is empty (when feature acknowledge) */
     if (((vdev->guest_features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY)) &&
          !vq->inuse && vring_avail_idx(vq) == vq->last_avail_idx)) {