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 <a...@ozlabs.ru>
> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
Acked-by: Michael S. Tsirkin <m...@redhat.com>
> ---
> 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
