Re: [Xen-devel] [PATCH] xen: arm: bitops take unsigned int (Was: Re: [PATCH] xen/arm64: disable alignment check)

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-05-12 at 13:15 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 05/09/2014 05:19 PM, Ian Campbell wrote:
> > (Just adding the other ARM guys...)
> > 
> > On Fri, 2014-05-09 at 14:24 +0100, Ian Campbell wrote:
> >> On Tue, 2014-04-29 at 09:58 +0100, Ian Campbell wrote:
> >>> On Tue, 2014-04-29 at 08:38 +0100, Vladimir Murzin wrote:
> >>>> On Mon, Apr 28, 2014 at 11:43 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> 
> >>>> wrote:
> >>>>> But I also wanted confirmation that the problematic instruction was
> >>>>> generated by gcc and not by some handcoded asm somewhere which we hadn't
> >>>>> properly fixed.
> >>>
> >>>> I believe it comes form test_bit (xen/include/asm-arm/bitops.h).
> >>>
> >>> Ah, then I think this code needs fixing too. Probably switching to
> >>> unsigned int * throughout would work, what do you think?
> >>
> >> I finally managed to upgrade to a new enough kernel to trigger this.
> >>
> >> This Works For Me(tm), along with the Linux patch "xen/events/fifo:
> >> correctly align bitops" which is queued for 3.15 Linus (but not sent
> >> yet?)
> >>
> >> 8<-------------------
> >>
> >> From aa6afe6520ea22241fb0ce430ef315c49a73867f Mon Sep 17 00:00:00 2001
> >> From: Ian Campbell <ian.campbell@xxxxxxxxxx>
> >> Date: Thu, 8 May 2014 16:13:55 +0100
> >> Subject: [PATCH] xen: arm: bitops take unsigned int
> >>
> >> Xen bitmaps can be 4 rather than 8 byte aligned, so use the appropriate 
> >> type.
> >> Otherwise the compiler can generate unaligned 8 byte accesses and cause 
> >> traps.
> >>
> >> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> >> ---
> >>  xen/include/asm-arm/bitops.h |   37 +++++++++++++++++++------------------
> >>  1 file changed, 19 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/xen/include/asm-arm/bitops.h b/xen/include/asm-arm/bitops.h
> >> index 0a7caee..25f96c8 100644
> >> --- a/xen/include/asm-arm/bitops.h
> >> +++ b/xen/include/asm-arm/bitops.h
> >> @@ -18,13 +18,14 @@
> >>  #define __set_bit(n,p)            set_bit(n,p)
> >>  #define __clear_bit(n,p)          clear_bit(n,p)
> >>  
> >> +#define BITS_PER_WORD           32
> 
> Can you define BITS_PER_WORD in asm-arm/config.h rather than here?

For better or worse BITS_PER_BYTE is already defined in bitops.h and
since I've already run the majority of my pre-push commit checks on a
branch containing this fix (along with some other bits and bobs) I'm not
inclined to restart that process just for this change.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel