Re: [Qemu-devel] [PATCH] virtio: add missing mb() on notification

[Stefan Hajnoczi]

On Sun, Apr 22, 2012 at 3:54 PM, Michael S. Tsirkin <address@hidden> wrote:
> During normal operation, virtio host first writes a used index
> and then checks whether it should interrupt the guest
> by reading guest avail flag/used event index values.
> Guest does the reverse: writes the index/flag,
> then checks the used ring.
>
> The ordering is important: if host avail flag read bypasses the used
> index write, we could in effect get this timing:
>
> host avail flag/used event index read
>                guest enable interrupts: this performs
>                         avail flag/used event index write
>                guest check used ring: ring is empty
> host used index write
>
> This timing results in a lost interrupt: guest will never be
> notified about the used ring update.
>
> This has actually been observed in the field, when using qemu-kvm such
> that the guest vcpu and qemu io run on different host cpus,
> but only seems to trigger on some specific hardware,
> and only with userspace virtio: vhost has the necessary smp_mb() in
> place to prevent the reordering, so the same workload stalls forever
> waiting for an interrupt with vhost=off but works fine with vhost=on.
>
> Insert an smp_mb() barrier operation in userspace virtio to
> ensure the correct ordering.
> Applying this patch fixed the race condition we have observed.
> Tested on x86_64. I checked the code generated by the new macro
> for i386 and ppc but didn't run virtio.
>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
>  hw/virtio.c    |    2 ++
>  qemu-barrier.h |   23 ++++++++++++++++++++---
>  2 files changed, 22 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi <address@hidden>