From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:38009)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <prvs=903cf46b3=david.vrabel@citrix.com>)
	id 1apxsu-0006RJ-Oy
	for qemu-devel@nongnu.org; Tue, 12 Apr 2016 08:57:37 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <prvs=903cf46b3=david.vrabel@citrix.com>)
	id 1apxsr-0006pe-F7
	for qemu-devel@nongnu.org; Tue, 12 Apr 2016 08:57:36 -0400
Received: from smtp.citrix.com ([66.165.176.89]:51253)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <prvs=903cf46b3=david.vrabel@citrix.com>)
	id 1apxsr-0006pV-9O
	for qemu-devel@nongnu.org; Tue, 12 Apr 2016 08:57:33 -0400
References: <1460457796-1779-1-git-send-email-wei.liu2@citrix.com>
	<1460457796-1779-3-git-send-email-wei.liu2@citrix.com>
From: David Vrabel <david.vrabel@citrix.com>
Message-ID: <570CF0B1.9070507@citrix.com>
Date: Tue, 12 Apr 2016 13:57:21 +0100
MIME-Version: 1.0
In-Reply-To: <1460457796-1779-3-git-send-email-wei.liu2@citrix.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v2 2/3] xenfb: move xen_rmb to
 the correct location
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Wei Liu <wei.liu2@citrix.com>, qemu-devel@nongnu.org
Cc: Anthony Perard <anthony.perard@citrix.com>, Xen-devel <xen-devel@lists.xenproject.org>, Stefano Stabellini <sstabellini@kernel.org>

On 12/04/16 11:43, Wei Liu wrote:
> It should be placed before first time producer and consumer are used.

This change isn't necessary and is confusing as this is not what this
barrier is for.

The barrier needs to be between the load of prod and the load of the
ring contents (there's even a comment that says this).  This pairs with
the corresponding write barrier between the store of the ring contents
and the store of prod (in the other end).

David

> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> 
> Backport candidate to our own tree.
> ---
>  hw/display/xenfb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 9866dfd..7f4fad7 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -775,10 +775,10 @@ static void xenfb_handle_events(struct XenFB *xenfb)
>  
>      prod = page->out_prod;
>      out_cons = page->out_cons;
> +    xen_rmb();
>      if (prod - out_cons > XENFB_OUT_RING_LEN) {
>          return;
>      }
> -    xen_rmb();		/* ensure we see ring contents up to prod */
>      for (cons = out_cons; cons != prod; cons++) {
>  	union xenfb_out_event *event = &XENFB_OUT_RING_REF(page, cons);
>          uint8_t type = event->type;
>