From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <Ian.Campbell@citrix.com>
Subject: Re: [PATCH] xen/arm: Correctly handle non-page aligned
 pointer in raw_copy_*
Date: Tue, 18 Feb 2014 15:25:12 +0000
Message-ID: <1392737112.11080.102.camel@kazak.uk.xensource.com>
References: <1392397809-13255-1-git-send-email-julien.grall@linaro.org>
	<1392735576.11080.87.camel@kazak.uk.xensource.com>
	<1392735659.11080.88.camel@kazak.uk.xensource.com>
	<53037788.4010702@linaro.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta3.messagelabs.com ([195.245.230.39])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <Ian.Campbell@citrix.com>) id 1WFmXt-0002Uw-RZ
	for xen-devel@lists.xenproject.org; Tue, 18 Feb 2014 15:25:18 +0000
In-Reply-To: <53037788.4010702@linaro.org>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Julien Grall <julien.grall@linaro.org>
Cc: xen-devel@lists.xenproject.org, tim@xen.org, george.dunlap@citrix.com, stefano.stabellini@citrix.com
List-Id: xen-devel@lists.xenproject.org

On Tue, 2014-02-18 at 15:08 +0000, Julien Grall wrote:
> On 02/18/2014 03:00 PM, Ian Campbell wrote:
> > On Tue, 2014-02-18 at 14:59 +0000, Ian Campbell wrote:
> > 
> >>> As the offset is only computed one time per function,
> >>
> >> We set offset = 0 at the end of the first iteration.
> > 
> > Ah, we do in raw_copy_to_guest_helper and raw_clear_guest but not
> > raw_copy_from_guest -- which I think is the actual bug here.
> 
> I didn't notice the offset = 0 at then end of raw_copy_to_guest.
> 
> I can send a patch to only set offset to 0 in raw_copy_from_guest. But I
> think it's less clear than this patch. What do you think?

I think the approach currently used by the (non-buggy) functions is
better -- it makes it obvious that after the first iteration things
*have* to now be aligned.

I also wouldn't be surprised if the compiler had trouble proving this
and so ended up needlessly recalculating offset instead of optimising it
out.

If you find the code unclear please feel free to add comments etc.

Ian.