From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:33139) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RAIto-0004Tj-A5 for qemu-devel@nongnu.org; Sun, 02 Oct 2011 06:04:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RAItl-0008Lf-N5 for qemu-devel@nongnu.org; Sun, 02 Oct 2011 06:03:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18738) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RAItl-0008L4-Du for qemu-devel@nongnu.org; Sun, 02 Oct 2011 06:03:53 -0400 Date: Sun, 2 Oct 2011 12:04:54 +0200 From: "Michael S. Tsirkin" Message-ID: <20111002100453.GA30747@redhat.com> References: <1316484321-6726-1-git-send-email-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1316484321-6726-1-git-send-email-david@gibson.dropbear.id.au> Subject: Re: [Qemu-devel] [PATCH 1/2] virtio: Use global memory barrier macros List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: aliguori@us.ibm.com, aik@ozlabs.ru, rusty@rustcorp.com.au, agraf@suse.de, qemu-devel@nongnu.org, avi@redhat.com, pbonzini@redhat.com On Tue, Sep 20, 2011 at 12:05:20PM +1000, David Gibson wrote: > The virtio code uses wmb() macros in several places, as required by the > SMP-aware virtio protocol. However the wmb() macro is locally defined > to be a compiler barrier only. This is probably sufficient on x86 > due to its strong storage ordering model, but it certainly isn't on other > platforms, such as ppc. > > In any case, qemu already has some globally defined memory barrier macros > in qemu-barrier.h. This patch, therefore converts virtio.c to use those > barrier macros. The macros in qemu-barrier.h are also wrong (or at least, > safe for x86 only) but this way at least there's only one place to fix > them. > > Signed-off-by: Alexey Kardashevskiy > Signed-off-by: David Gibson Acked-by: Michael S. Tsirkin > --- > hw/virtio.c | 14 +++----------- > 1 files changed, 3 insertions(+), 11 deletions(-) > > diff --git a/hw/virtio.c b/hw/virtio.c > index 946d911..9663294 100644 > --- a/hw/virtio.c > +++ b/hw/virtio.c > @@ -16,20 +16,12 @@ > #include "trace.h" > #include "qemu-error.h" > #include "virtio.h" > +#include "qemu-barrier.h" > > /* The alignment to use between consumer and producer parts of vring. > * x86 pagesize again. */ > #define VIRTIO_PCI_VRING_ALIGN 4096 > > -/* QEMU doesn't strictly need write barriers since everything runs in > - * lock-step. We'll leave the calls to wmb() in though to make it obvious for > - * KVM or if kqemu gets SMP support. > - * In any case, we must prevent the compiler from reordering the code. > - * TODO: we likely need some rmb()/mb() as well. > - */ > - > -#define wmb() __asm__ __volatile__("": : :"memory") > - > typedef struct VRingDesc > { > uint64_t addr; > @@ -264,7 +256,7 @@ void virtqueue_flush(VirtQueue *vq, unsigned int count) > { > uint16_t old, new; > /* Make sure buffer is written before we update index. */ > - wmb(); > + smp_wmb(); > trace_virtqueue_flush(vq, count); > old = vring_used_idx(vq); > new = old + count; > @@ -324,7 +316,7 @@ static unsigned virtqueue_next_desc(target_phys_addr_t desc_pa, > /* Check they're not leading us off end of descriptors. */ > next = vring_desc_next(desc_pa, i); > /* Make sure compiler knows to grab that: we don't want it changing! */ > - wmb(); > + smp_wmb(); > > if (next >= max) { > error_report("Desc next is %u", next); > -- > 1.7.5.4