Re: [Qemu-devel] Missing barriers in hw/virtio.c

[Paolo Bonzini]

On 08/26/2011 07:40 AM, David Gibson wrote:
> Near the top of hw/virtio.c we have this:
>
> /* 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")
>
>
> However, as far as I can tell when using both kvm and io-thread, the
> assertion that barriers aren't necessary just isn't true.  Although it
> probably works on x86 with its strongly ordered model.
>
> We think we've hit a race due to this with kvm on POWER, described in
> the forwarded message below.  Adding a barrier fixes the problem.
> Below we just stuck in a powerpc specific barrier which was useful as
> proof of concept, but I'm not sure how to go about putting a proper,
> architecture-appropriate barrier into this code.

Yes, I think you are right.  To get a full memory barrier you can add 
__sync_synchronize() and require GCC 4.2 or later (or Red Hat 4.1).  Or 
we can import a library of atomic functions from e.g. liburcu, which 
will provide separate macros for mb()/rmb()/wmb().  Alternatively, we 
can define all of

#define mb()     __sync_synchronize()
#define rmb()    __sync_synchronize()
#define wmb()    __sync_synchronize()

So that we can still differentiate at the source-code level, even though 
all the macros will emit full barriers.

Paolo