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

[Julien Grall <julien.grall@linaro.org>]

On 05/12/2014 01:18 PM, Ian Campbell wrote:
> 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@citrix.com> 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@citrix.com>
>>>> 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@citrix.com>
>>>> ---
>>>>  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.

No problem. I guess you plan to backport this patch for Xen 4.4?

Regards,

-- 
Julien Grall