From e22517086bbdf8d09de2b9ba8b3dfa4c42ec0f6c Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 20 Sep 2011 12:05:21 +1000 Subject: [PATCH] Barriers in qemu-barrier.h should not be x86 specific 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 Signed-off-by: David Gibson Signed-off-by: Anthony Liguori --- qemu-barrier.h | 34 +++++++++++++++++++++++++++++++--- 1 file 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 -- 2.7.4