From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:50921) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R7AqU-00009f-Lv for qemu-devel@nongnu.org; Fri, 23 Sep 2011 14:51:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R7AqR-0000V4-MV for qemu-devel@nongnu.org; Fri, 23 Sep 2011 14:51:34 -0400 Received: from e3.ny.us.ibm.com ([32.97.182.143]:46547) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R7AqR-0000V0-Gq for qemu-devel@nongnu.org; Fri, 23 Sep 2011 14:51:31 -0400 Received: from /spool/local by us.ibm.com with XMail ESMTP for from ; Fri, 23 Sep 2011 14:51:30 -0400 Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by d01relay05.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p8NIpRkb224398 for ; Fri, 23 Sep 2011 14:51:27 -0400 Received: from d01av03.pok.ibm.com (loopback [127.0.0.1]) by d01av03.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p8NIpPbx023101 for ; Fri, 23 Sep 2011 15:51:27 -0300 Message-ID: <4E7CD52B.5020102@us.ibm.com> Date: Fri, 23 Sep 2011 13:51:23 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1316484321-6726-1-git-send-email-david@gibson.dropbear.id.au> In-Reply-To: <1316484321-6726-1-git-send-email-david@gibson.dropbear.id.au> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: mst@redhat.com, aik@ozlabs.ru, rusty@rustcorp.com.au, agraf@suse.de, qemu-devel@nongnu.org, avi@redhat.com, pbonzini@redhat.com On 09/19/2011 09:05 PM, 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 Applied all. Thanks. Regards, Anthony Liguori > --- > 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);