From patchwork Tue Sep 20 02:05:21 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Gibson X-Patchwork-Id: 115405 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 54493B70BB for ; Tue, 20 Sep 2011 12:05:47 +1000 (EST) Received: from localhost ([::1]:55145 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R5piR-0000zW-9b for incoming@patchwork.ozlabs.org; Mon, 19 Sep 2011 22:05:43 -0400 Received: from eggs.gnu.org ([140.186.70.92]:33174) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R5piJ-0000z7-Et for qemu-devel@nongnu.org; Mon, 19 Sep 2011 22:05:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R5piG-0008T2-F6 for qemu-devel@nongnu.org; Mon, 19 Sep 2011 22:05:35 -0400 Received: from ozlabs.org ([203.10.76.45]:34250) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R5piG-0008Rn-1A for qemu-devel@nongnu.org; Mon, 19 Sep 2011 22:05:32 -0400 Received: by ozlabs.org (Postfix, from userid 1007) id 07CC1B70BF; Tue, 20 Sep 2011 12:05:29 +1000 (EST) From: David Gibson To: aliguori@us.ibm.com, avi@redhat.com Date: Tue, 20 Sep 2011 12:05:21 +1000 Message-Id: <1316484321-6726-2-git-send-email-david@gibson.dropbear.id.au> X-Mailer: git-send-email 1.7.5.4 In-Reply-To: <1316484321-6726-1-git-send-email-david@gibson.dropbear.id.au> References: <1316484321-6726-1-git-send-email-david@gibson.dropbear.id.au> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 203.10.76.45 Cc: mst@redhat.com, aik@ozlabs.ru, rusty@rustcorp.com.au, qemu-devel@nongnu.org, agraf@suse.de, pbonzini@redhat.com Subject: [Qemu-devel] [PATCH 2/2] Barriers in qemu-barrier.h should not be x86 specific X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 Acked-by: Michael S. Tsirkin --- 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