All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Xie, Huawei" <huawei.xie@intel.com>
To: Luke Gorrie <luke@snabb.co>,
	"snabb-devel@googlegroups.com" <snabb-devel@googlegroups.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"virtualization@lists.linux-foundation.org"
	<virtualization@lists.linux-foundation.org>,
	"VirtualOpenSystems Technical Team" <tech@virtualopensystems.com>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [dpdk-dev] Re: memory barriers in virtq.lua?
Date: Wed, 8 Apr 2015 15:15:16 +0000	[thread overview]
Message-ID: <C37D651A908B024F974696C65296B57B0F42E766@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: CAA2XHbcU0tV0NrBXT6oh6LOz7sKm9P8jqD1=T-ZgTahSVM_qwQ@mail.gmail.com

On 4/7/2015 10:23 PM, Luke Gorrie wrote:
> Hi Michael,
>
> I'm writing to follow up the previous discussion about memory barriers in
> virtio-net device implementations, and Cc'ing the DPDK list because I
> believe this is relevant to them too.
>
> First, thanks again for getting in touch and reviewing our code.
>
> I have now found a missed case where we *do* require a hardware memory
> barrier on x86 in our vhost/virtio-net device. That is when checking the
> interrupt suppression flag after updating used->idx. This is needed because
> x86 can reorder the write to used->idx after the read from avail->flags,
> and that causes the guest to see a stale value of used->idx after it
> toggles interrupt suppression.
luke:
1. host read the flag. 2 guest toggles the flag 3.guest checks the used.
4. host update used.
Is this your case?

>
> If I may spell out my mental model, for the sake of being corrected and/or
> as an example of how third party developers are reading and interpreting
> the Virtio-net spec:
>
> Relating this to Virtio 1.0, the most relevant section is 3.2.1 (Supplying
> Buffers to the Device) which calls for two "suitable memory barriers". The
> spec talks about these from the driver perspective, but they are both
> relevant to the device side too.
>
> The first barrier (write to descriptor table before write to used->idx) is
> implicit on x86 because writes by the same core are not reordered. This
> means that no explicit hardware barrier is needed. (A compiler barrier may
> be needed, however.)
>
> The second memory barrier (write to used->idx before reading avail->flags)
> is not implicit on x86 because stores are reordered after loads. So an
> explicit hardware memory barrier is needed.
>
> I hope that is a correct assessment of the situation. (Forgive my
> x86centricity, I am sure that seems very foreign to kernel hackers.)
>
> If this assessment is correct then the DPDK developers might also want to
> review librte_vhost/vhost_rxtx.c and consider adding a hardware memory
> barrier between writing used->idx and reading avail->flags.
>
> Cheers,
> -Luke
>
> P.S. I notice that the Linux virtio-net driver does not seem to tolerate
> spurious interrupts, even though the Virtio 1.0 spec requires this
> ("must"). On 3.13.11-ckt15 I see them trigger an "irq nobody cared" kernel
> log message and then the irq is disabled. If that sounds suspicious I can
> supply more information.
>
>

  parent reply	other threads:[~2015-04-08 15:15 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-27 16:01 memory barriers in virtq.lua? Michael S. Tsirkin
2015-01-28 10:27 ` Nikolay Nikolaev
     [not found] ` <CADDJ2=M6hwFwooXqUjUc9+JxjW1sVYvKhY9dBavrmMUrej6Ysw@mail.gmail.com>
     [not found]   ` <CADDJ2=M6hwFwooXqUjUc9+JxjW1sVYvKhY9dBavrmMUrej6Ysw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-04-07 14:22     ` [snabb-devel] " Luke Gorrie
2015-04-07 15:30       ` Michael S. Tsirkin
     [not found]       ` <CAA2XHbcU0tV0NrBXT6oh6LOz7sKm9P8jqD1=T-ZgTahSVM_qwQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-04-07 15:30         ` Michael S. Tsirkin
     [not found]           ` <20150407172849-mutt-send-email-mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-04-08  3:40             ` Luke Gorrie
2015-04-08  3:40           ` Luke Gorrie
2015-04-08 15:15       ` [dpdk-dev] " Xie, Huawei
2015-04-08 15:15       ` Xie, Huawei [this message]
2015-04-09  3:12         ` Luke Gorrie
2015-04-09  3:12         ` [dpdk-dev] " Luke Gorrie
2015-04-09 15:00           ` [dpdk-dev] [snabb-devel] " Xie, Huawei
     [not found]           ` <CAA2XHbdNAB1ZsBFYHW7W1yhnkzaSixwKk4KvjU8G=7OLECpZhQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-04-09 15:00             ` Xie, Huawei
2015-04-07 14:22   ` Luke Gorrie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=C37D651A908B024F974696C65296B57B0F42E766@SHSMSX101.ccr.corp.intel.com \
    --to=huawei.xie@intel.com \
    --cc=dev@dpdk.org \
    --cc=luke@snabb.co \
    --cc=mst@redhat.com \
    --cc=snabb-devel@googlegroups.com \
    --cc=tech@virtualopensystems.com \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.