a281ebc11a
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>
56 lines
1.5 KiB
C
56 lines
1.5 KiB
C
#ifndef __QEMU_BARRIER_H
|
|
#define __QEMU_BARRIER_H 1
|
|
|
|
/* Compiler barrier */
|
|
#define barrier() asm volatile("" ::: "memory")
|
|
|
|
#if defined(__i386__)
|
|
|
|
/*
|
|
* Because of the strongly ordered x86 storage model, wmb() is a nop
|
|
* on x86(well, a compiler barrier only). Well, at least as long as
|
|
* qemu doesn't do accesses to write-combining memory or non-temporal
|
|
* load/stores from C code.
|
|
*/
|
|
#define smp_wmb() barrier()
|
|
/*
|
|
* We use GCC builtin if it's available, as that can use
|
|
* mfence on 32 bit as well, e.g. if built with -march=pentium-m.
|
|
* However, on i386, there seem to be known bugs as recently as 4.3.
|
|
* */
|
|
#if defined(__GNUC__) && __GNUC__ >= 4 && __GNUC_MINOR__ >= 4
|
|
#define smp_mb() __sync_synchronize()
|
|
#else
|
|
#define smp_mb() asm volatile("lock; addl $0,0(%%esp) " ::: "memory")
|
|
#endif
|
|
|
|
#elif defined(__x86_64__)
|
|
|
|
#define smp_wmb() barrier()
|
|
#define smp_mb() asm volatile("mfence" ::: "memory")
|
|
|
|
#elif defined(_ARCH_PPC)
|
|
|
|
/*
|
|
* We use an eieio() for wmb() on powerpc. This assumes we don't
|
|
* need to order cacheable and non-cacheable stores with respect to
|
|
* each other
|
|
*/
|
|
#define smp_wmb() asm volatile("eieio" ::: "memory")
|
|
#define smp_mb() asm volatile("sync" ::: "memory")
|
|
|
|
#else
|
|
|
|
/*
|
|
* For (host) platforms we don't have explicit barrier definitions
|
|
* for, we use the gcc __sync_synchronize() primitive to generate a
|
|
* full barrier. This should be safe on all platforms, though it may
|
|
* be overkill for wmb().
|
|
*/
|
|
#define smp_wmb() __sync_synchronize()
|
|
#define smp_mb() __sync_synchronize()
|
|
|
|
#endif
|
|
|
|
#endif
|