[Qemu-devel] [PATCH 2/2] Barriers in qemu-barrier.h should not be x86 specific

[David Gibson]

qemu-barrier.h contains a few macros implementing memory barrier
primitives used in several places throughout qemu.  However, apart
from the compiler-only barrier, the defined wmb() is correct only for
x86, or platforms which are similarly strongly ordered.

This patch addresses the FIXME about this by making the wmb() macro
arch dependent.  On x86, it remains a compiler barrier only, but with
a comment explaining in more detail the conditions under which this is
correct.  On weakly-ordered powerpc, an "eieio" instruction is used,
again with explanation of the conditions under which it is sufficient.

On other platforms, we use the __sync_synchronize() primitive,
available in sufficiently recent gcc (4.2 and after?).  This should
implement a full barrier which will be sufficient on all platforms,
although it may be overkill in some cases.  Other platforms can add
optimized versions in future if it's worth it for them.

Without proper memory barriers, it is easy to reproduce ordering
problems with virtio on powerpc; specifically, the QEMU puts new
element into the "used" ring and then updates the ring free-running
counter.  Without a barrier between these under the right
circumstances, the guest linux driver can receive an interrupt, read
the counter change but find the ring element to be handled still has
an old value, leading to an "id %u is not a head!\n" error message.
Similar problems are likely to be possible with kvm on other weakly
ordered platforms.

Signed-off-by: Alexey Kardashevskiy <address@hidden>
Signed-off-by: David Gibson <address@hidden>
---
 qemu-barrier.h |   34 +++++++++++++++++++++++++++++++---
 1 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/qemu-barrier.h b/qemu-barrier.h
index b77fce2..735eea6 100644
--- a/qemu-barrier.h
+++ b/qemu-barrier.h
@@ -1,10 +1,38 @@
 #ifndef __QEMU_BARRIER_H
 #define __QEMU_BARRIER_H 1
 
-/* FIXME: arch dependant, x86 version */
-#define smp_wmb()   asm volatile("" ::: "memory")
-
 /* Compiler barrier */
 #define barrier()   asm volatile("" ::: "memory")
 
+#if defined(__i386__) || defined(__x86_64__)
+
+/*
+ * 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()
+
+#elif defined(__powerpc__)
+
+/*
+ * We use an eieio() for a 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")
+
+#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.
+ */
+#define smp_wmb()   __sync_synchronize()
+
+#endif
+
 #endif
-- 
1.7.5.4