[PATCH] xen: fix alignment for bitops

[PATCH] xen: fix alignment for bitops

[Vladimir Murzin]

Bitops operations like set/clear/change mandate world aligned pointer, mainly
because architectures specific implementation.

Looks that DEFINE_PER_CPU does required alignment for cpu_control_block;
however, local copy used for bitops might not be world aligned.

For arm64 it ends up with unaligned access trap:

Unhandled fault: alignment fault (0x96000021) at 0xffffffc01cf07d64
Internal error: : 96000021 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 563 Comm: udevd Not tainted 3.14.0+ #1
task: ffffffc01de95c40 ti: ffffffc01cf04000 task.ti: ffffffc01cf04000
PC is at clear_bit+0x14/0x30
LR is at evtchn_fifo_handle_events+0x168/0x184
pc : [<ffffffc00027b6c4>] lr : [<ffffffc0002b17d8>] pstate: 600001c5
sp : ffffffc01cf07d00
x29: ffffffc01cf07d00 x28: ffffffc01dce5000
x27: 0000000000000008 x26: 0000000000000002
x25: 00000000dffe0000 x24: 0000000000000000
x23: 0000000000000007 x22: ffffffc000660000
x21: ffffffc00060d900 x20: ffffffc01efdd900
x19: ffffffc01dc26000 x18: 0000007ff1d5a570
x17: 0000007faf848824 x16: 000000000044bf10
x15: 0000007faf8ed598 x14: ffffffffffffffff
x13: 0000000000000028 x12: 0101010101010101
x11: 7f7f7f7f7f7f7f7f x10: 622e607360632e75
x9 : 7f7f7f7f7f7f7f7f x8 : 000000001e9d0000
x7 : ffffffc01d800120 x6 : 00000000a0000000
x5 : 00000000a0000000 x4 : 0000000000000000
x3 : 0000000000000080 x2 : 0000000000000001
x1 : ffffffc01cf07d64 x0 : 0000000000000000

Process udevd (pid: 563, stack limit = 0xffffffc01cf04058)
Stack: (0xffffffc01cf07d00 to 0xffffffc01cf08000)
7d00: 1cf07d80 ffffffc0 002aeb7c ffffffc0 0060d6ec ffffffc0 1efe1c90 ffffffc0
7d20: 0056e418 ffffffc0 0066c000 ffffffc0 00000000 00000000 005659a0 ffffffc0
7d40: 00565998 ffffffc0 004053b0 00000000 00423bb0 00000000 00423000 00000000
7d60: a0000000 00000080 002aeb1c ffffffc0 00000000 00000000 002aeb4c ffffffc0
7d80: 1cf07dd0 ffffffc0 002aec2c ffffffc0 1dc09f00 ffffffc0 00630360 ffffffc0
7da0: 1dc23400 ffffffc0 00567c08 ffffffc0 0000001f 00000000 1efdc400 ffffffc0
7dc0: 000001ed 00000000 004053b0 00000000 1cf07de0 ffffffc0 00092eec ffffffc0
7de0: 1cf07df0 ffffffc0 000d79bc ffffffc0 1cf07e30 ffffffc0 000d3cf0 ffffffc0
7e00: 0000001f 00000000 0060d000 ffffffc0 00565000 ffffffc0 00565000 ffffffc0
7e20: 0000001f 00000000 00000000 00000000 1cf07e50 ffffffc0 000848bc ffffffc0
7e40: 0060d5a0 ffffffc0 000848a4 ffffffc0 1cf07ea0 ffffffc0 0008128c ffffffc0
7e60: 00667000 ffffffc0 0000400c ffffff80 1cf07ed0 ffffffc0 00004010 ffffff80
7e80: 80000000 00000000 2dca8010 00000000 1cf07ed0 ffffffc0 00000012 00000000
7ea0: f1d5b710 0000007f 000841bc ffffffc0 2dca8170 00000000 f1d5b870 0000007f
7ec0: ffffffff ffffffff af848838 0000007f 00000000 00000000 f1d5b7a0 0000007f
7ee0: f1d5b720 0000007f 00000000 00000000 61642f76 632f6174 2e353a34 00706d74
7f00: f1d5b7ae 0000007f 00000000 00000000 0000004f 00000000 7f7f7f7f 7f7f7f7f
7f20: 60632e75 622e6073 7f7f7f7f 7f7f7f7f 01010101 01010101 00000028 00000000
7f40: ffffffff ffffffff af8ed598 0000007f 0044bf10 00000000 af848824 0000007f
7f60: f1d5a570 0000007f 2dca8170 00000000 f1d5b870 0000007f 2dcc6ca0 00000000
7f80: f1d5bc70 0000007f 00000000 00000000 2dca8010 00000000 000001ed 00000000
7fa0: 004053b0 00000000 00423bb0 00000000 00423000 00000000 f1d5b710 0000007f
7fc0: 0041f460 00000000 f1d5b710 0000007f af848838 0000007f 80000000 00000000
7fe0: ffffff9c ffffffff ffffffff ffffffff dfdfdfcf cfdfdfdf dfdfdfcf cfdfdfdf
Call trace:
[<ffffffc00027b6c4>] clear_bit+0x14/0x30
[<ffffffc0002aeb78>] __xen_evtchn_do_upcall+0x9c/0x144
[<ffffffc0002aec28>] xen_hvm_evtchn_do_upcall+0x8/0x14
[<ffffffc000092ee8>] xen_arm_callback+0x8/0x18
[<ffffffc0000d79b8>] handle_percpu_devid_irq+0x90/0xb8
[<ffffffc0000d3cec>] generic_handle_irq+0x24/0x40
[<ffffffc0000848b8>] handle_IRQ+0x68/0xe0
[<ffffffc000081288>] gic_handle_irq+0x38/0x80
Exception stack(0xffffffc01cf07eb0 to 0xffffffc01cf07fd0)
7ea0:                                     2dca8170 00000000 f1d5b870 0000007f
7ec0: ffffffff ffffffff af848838 0000007f 00000000 00000000 f1d5b7a0 0000007f
7ee0: f1d5b720 0000007f 00000000 00000000 61642f76 632f6174 2e353a34 00706d74
7f00: f1d5b7ae 0000007f 00000000 00000000 0000004f 00000000 7f7f7f7f 7f7f7f7f
7f20: 60632e75 622e6073 7f7f7f7f 7f7f7f7f 01010101 01010101 00000028 00000000
7f40: ffffffff ffffffff af8ed598 0000007f 0044bf10 00000000 af848824 0000007f
7f60: f1d5a570 0000007f 2dca8170 00000000 f1d5b870 0000007f 2dcc6ca0 00000000
7f80: f1d5bc70 0000007f 00000000 00000000 2dca8010 00000000 000001ed 00000000
7fa0: 004053b0 00000000 00423bb0 00000000 00423000 00000000 f1d5b710 0000007f
7fc0: 0041f460 00000000 f1d5b710 0000007f
Code: 4a030000 d2800022 8b400c21 9ac32043 (c85f7c22)

Instruct compiler to keep local copy of "ready" world aligned.

Signed-off-by: Vladimir Murzin <murzin.v@gmail.com>
---
 I'd appreciate any thought how to fix it in the right way if suggest patch
 doesn't look appropriate ;) 

 drivers/xen/events/events_fifo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/xen/events/events_fifo.c b/drivers/xen/events/events_fifo.c
index 96109a9..291c4a8 100644
--- a/drivers/xen/events/events_fifo.c
+++ b/drivers/xen/events/events_fifo.c
@@ -285,7 +285,7 @@ static void consume_one_event(unsigned cpu,
 static void evtchn_fifo_handle_events(unsigned cpu)
 {
 	struct evtchn_fifo_control_block *control_block;
-	uint32_t ready;
+	uint32_t __aligned(sizeof(long)) ready;
 	unsigned q;
 
 	control_block = per_cpu(cpu_control_block, cpu);
-- 
1.8.3.2

Re: [PATCH] xen: fix alignment for bitops

[Jan Beulich]

>>> On 14.04.14 at 22:43, <murzin.v@gmail.com> wrote:
>  I'd appreciate any thought how to fix it in the right way if suggest patch
>  doesn't look appropriate ;) 

Clearly by making the bitops tolerate 32-bit aligned pointers rather
than modifying common code with ugly hacks that aren't even
necessary on x86 and arm32; I don't think this would remain the
only place you'd need to alter - we simply assume bitops on 32-bit
aligned quantities to work.

Jan

> --- a/drivers/xen/events/events_fifo.c
> +++ b/drivers/xen/events/events_fifo.c
> @@ -285,7 +285,7 @@ static void consume_one_event(unsigned cpu,
>  static void evtchn_fifo_handle_events(unsigned cpu)
>  {
>  	struct evtchn_fifo_control_block *control_block;
> -	uint32_t ready;
> +	uint32_t __aligned(sizeof(long)) ready;
>  	unsigned q;
>  
>  	control_block = per_cpu(cpu_control_block, cpu);

Re: [PATCH] xen: fix alignment for bitops

[Vladimir Murzin]

On Tue, Apr 15, 2014 at 8:35 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 14.04.14 at 22:43, <murzin.v@gmail.com> wrote:
>>  I'd appreciate any thought how to fix it in the right way if suggest patch
>>  doesn't look appropriate ;)
>
> Clearly by making the bitops tolerate 32-bit aligned pointers rather
> than modifying common code with ugly hacks that aren't even
> necessary on x86 and arm32; I don't think this would remain the

Agree it is not perfect, and it is why I asked about any other
reasonable ways to fix it.
In case of 32-bit arches effect of the patch is going to be nop.

> only place you'd need to alter - we simply assume bitops on 32-bit
> aligned quantities to work.
>

You do, but reality seems to be different, apart from arm64 looks like
ppc64 has the same alignment requirement, I'm not aware about other
64-bit implementations... but, is it really possible to convince all
these people to change the implementation? I guess the answer would be
"use unsigned long", or like that :)

Looking at the past "unsigned long vs u32" not a news for Xen project
[1], and I guess the only reason for BM is a simple workaround for
that.

Any other thought? Should we involve arch people in discussion?

[1] http://lists.xen.org/archives/html/xen-devel/2005-10/msg00242.html

Vladimir
> Jan
>
>> --- a/drivers/xen/events/events_fifo.c
>> +++ b/drivers/xen/events/events_fifo.c
>> @@ -285,7 +285,7 @@ static void consume_one_event(unsigned cpu,
>>  static void evtchn_fifo_handle_events(unsigned cpu)
>>  {
>>       struct evtchn_fifo_control_block *control_block;
>> -     uint32_t ready;
>> +     uint32_t __aligned(sizeof(long)) ready;
>>       unsigned q;
>>
>>       control_block = per_cpu(cpu_control_block, cpu);
>
>

Re: [PATCH] xen: fix alignment for bitops

[Jan Beulich]

>>> On 15.04.14 at 10:29, <murzin.v@gmail.com> wrote:
> On Tue, Apr 15, 2014 at 8:35 AM, Jan Beulich <JBeulich@suse.com> wrote:
>> only place you'd need to alter - we simply assume bitops on 32-bit
>> aligned quantities to work.
>>
> 
> You do, but reality seems to be different, apart from arm64 looks like
> ppc64 has the same alignment requirement, I'm not aware about other
> 64-bit implementations... but, is it really possible to convince all
> these people to change the implementation? I guess the answer would be
> "use unsigned long", or like that :)

That's no the point. The point is that if the arch has such
requirements, the arch-specific bitmap manipulation functions
should be written such that generic code works with the present
assumptions.

Jan

Re: [PATCH] xen: fix alignment for bitops

[Ian Campbell]

On Tue, 2014-04-15 at 09:49 +0100, Jan Beulich wrote:
> >>> On 15.04.14 at 10:29, <murzin.v@gmail.com> wrote:
> > On Tue, Apr 15, 2014 at 8:35 AM, Jan Beulich <JBeulich@suse.com> wrote:
> >> only place you'd need to alter - we simply assume bitops on 32-bit
> >> aligned quantities to work.
> >>
> > 
> > You do, but reality seems to be different, apart from arm64 looks like
> > ppc64 has the same alignment requirement, I'm not aware about other
> > 64-bit implementations... but, is it really possible to convince all
> > these people to change the implementation? I guess the answer would be
> > "use unsigned long", or like that :)
> 
> That's no the point. The point is that if the arch has such
> requirements, the arch-specific bitmap manipulation functions
> should be written such that generic code works with the present
> assumptions.

In this case the interface which arch-specific code has contracted to
expose to the generic code is:
        extern unsigned long find_first_bit(const unsigned long
        *addr, ...
i.e. the bitmask is unsigned long (and this is the case for the majority
of archs in Linux AFAICS, include asm-generic, note that this differs to
Xen which IIRC uses void * here)

So it is reasonable IMHO for the Linux arch code to except to be passed
something which obeys the alignment rules for an unsigned long.

I'm not convinced by the use of __aligned here -- I think it would be
better to just use unsigned long.

Ian.

Re: [PATCH] xen: fix alignment for bitops

[Vladimir Murzin]

On Tue, Apr 15, 2014 at 10:15 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Tue, 2014-04-15 at 09:49 +0100, Jan Beulich wrote:
>> >>> On 15.04.14 at 10:29, <murzin.v@gmail.com> wrote:
>> > On Tue, Apr 15, 2014 at 8:35 AM, Jan Beulich <JBeulich@suse.com> wrote:
>> >> only place you'd need to alter - we simply assume bitops on 32-bit
>> >> aligned quantities to work.
>> >>
>> >
>> > You do, but reality seems to be different, apart from arm64 looks like
>> > ppc64 has the same alignment requirement, I'm not aware about other
>> > 64-bit implementations... but, is it really possible to convince all
>> > these people to change the implementation? I guess the answer would be
>> > "use unsigned long", or like that :)
>>
>> That's no the point. The point is that if the arch has such
>> requirements, the arch-specific bitmap manipulation functions
>> should be written such that generic code works with the present
>> assumptions.
>
> In this case the interface which arch-specific code has contracted to
> expose to the generic code is:
>         extern unsigned long find_first_bit(const unsigned long
>         *addr, ...
> i.e. the bitmask is unsigned long (and this is the case for the majority
> of archs in Linux AFAICS, include asm-generic, note that this differs to
> Xen which IIRC uses void * here)
>
> So it is reasonable IMHO for the Linux arch code to except to be passed
> something which obeys the alignment rules for an unsigned long.
>
> I'm not convinced by the use of __aligned here -- I think it would be
> better to just use unsigned long.

It was original idea, but I was in doubt that it didn't break
interface... Should I send v2 or wait for other proposals?

Another option would be implement u32 bitops for Xen, may be based on
generic Linux implementation... so there would not be any dependencies
on arches.. don't have idea how bitops performance is critical for
Xen...

Vladimir

>
> Ian.
>

Re: [PATCH] xen: fix alignment for bitops

[Jan Beulich]

>>> On 15.04.14 at 11:15, <Ian.Campbell@citrix.com> wrote:
> On Tue, 2014-04-15 at 09:49 +0100, Jan Beulich wrote:
>> >>> On 15.04.14 at 10:29, <murzin.v@gmail.com> wrote:
>> > On Tue, Apr 15, 2014 at 8:35 AM, Jan Beulich <JBeulich@suse.com> wrote:
>> >> only place you'd need to alter - we simply assume bitops on 32-bit
>> >> aligned quantities to work.
>> >>
>> > 
>> > You do, but reality seems to be different, apart from arm64 looks like
>> > ppc64 has the same alignment requirement, I'm not aware about other
>> > 64-bit implementations... but, is it really possible to convince all
>> > these people to change the implementation? I guess the answer would be
>> > "use unsigned long", or like that :)
>> 
>> That's no the point. The point is that if the arch has such
>> requirements, the arch-specific bitmap manipulation functions
>> should be written such that generic code works with the present
>> assumptions.
> 
> In this case the interface which arch-specific code has contracted to
> expose to the generic code is:
>         extern unsigned long find_first_bit(const unsigned long
>         *addr, ...
> i.e. the bitmask is unsigned long (and this is the case for the majority
> of archs in Linux AFAICS, include asm-generic, note that this differs to
> Xen which IIRC uses void * here)
> 
> So it is reasonable IMHO for the Linux arch code to except to be passed
> something which obeys the alignment rules for an unsigned long.
> 
> I'm not convinced by the use of __aligned here -- I think it would be
> better to just use unsigned long.

Urgh - I thought this was about hypervisor code (due to the lack of a
Linux list being Cc-ed) - I fully agree with you for Linux side code.

Jan

Re: [PATCH] xen: fix alignment for bitops

[Ian Campbell]

On Tue, 2014-04-15 at 10:30 +0100, Vladimir Murzin wrote:
> On Tue, Apr 15, 2014 at 10:15 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Tue, 2014-04-15 at 09:49 +0100, Jan Beulich wrote:
> >> >>> On 15.04.14 at 10:29, <murzin.v@gmail.com> wrote:
> >> > On Tue, Apr 15, 2014 at 8:35 AM, Jan Beulich <JBeulich@suse.com> wrote:
> >> >> only place you'd need to alter - we simply assume bitops on 32-bit
> >> >> aligned quantities to work.
> >> >>
> >> >
> >> > You do, but reality seems to be different, apart from arm64 looks like
> >> > ppc64 has the same alignment requirement, I'm not aware about other
> >> > 64-bit implementations... but, is it really possible to convince all
> >> > these people to change the implementation? I guess the answer would be
> >> > "use unsigned long", or like that :)
> >>
> >> That's no the point. The point is that if the arch has such
> >> requirements, the arch-specific bitmap manipulation functions
> >> should be written such that generic code works with the present
> >> assumptions.
> >
> > In this case the interface which arch-specific code has contracted to
> > expose to the generic code is:
> >         extern unsigned long find_first_bit(const unsigned long
> >         *addr, ...
> > i.e. the bitmask is unsigned long (and this is the case for the majority
> > of archs in Linux AFAICS, include asm-generic, note that this differs to
> > Xen which IIRC uses void * here)
> >
> > So it is reasonable IMHO for the Linux arch code to except to be passed
> > something which obeys the alignment rules for an unsigned long.
> >
> > I'm not convinced by the use of __aligned here -- I think it would be
> > better to just use unsigned long.
> 
> It was original idea, but I was in doubt that it didn't break
> interface...

The usage is 
        ready = xchg(&control_block->ready, 0);
and
        ready |= xchg(&control_block->ready, 0);
where control_block->ready is a uint32_t, so  I don't think anything
will be broken (at least so long as sizeof(unsigned long) >=
sizeof(uint32_t), which I think we can rely on)

> Should I send v2 or wait for other proposals?

I'd wait for the Linux maintainers to speak up.

Ian.

Re: [PATCH] xen: fix alignment for bitops

[David Vrabel]

On 14/04/14 21:43, Vladimir Murzin wrote:
> Bitops operations like set/clear/change mandate world aligned pointer, mainly
> because architectures specific implementation.
> 
> Looks that DEFINE_PER_CPU does required alignment for cpu_control_block;
> however, local copy used for bitops might not be world aligned.
> 
> Instruct compiler to keep local copy of "ready" world aligned.
[...]
> --- a/drivers/xen/events/events_fifo.c
> +++ b/drivers/xen/events/events_fifo.c
> @@ -285,7 +285,7 @@ static void consume_one_event(unsigned cpu,
>  static void evtchn_fifo_handle_events(unsigned cpu)
>  {
>  	struct evtchn_fifo_control_block *control_block;
> -	uint32_t ready;
> +	uint32_t __aligned(sizeof(long)) ready;

This doesn't look sufficient to me.  The event words within the event
array are 32-bit sized/aligned and sync_set_bit() and sync_clear_bit()
etc. are done on these words.

I think arm64 is going to have to provide sync_clear_bit(),
sync_set_bit(), sync_test_bit() and sync_test_and_set_bit() that work
with 32-bit sized and aligned words.

David

Re: [PATCH] xen: fix alignment for bitops

[Vladimir Murzin]

On Tue, Apr 15, 2014 at 11:17 AM, David Vrabel <david.vrabel@citrix.com> wrote:
> On 14/04/14 21:43, Vladimir Murzin wrote:
>> Bitops operations like set/clear/change mandate world aligned pointer, mainly
>> because architectures specific implementation.
>>
>> Looks that DEFINE_PER_CPU does required alignment for cpu_control_block;
>> however, local copy used for bitops might not be world aligned.
>>
>> Instruct compiler to keep local copy of "ready" world aligned.
> [...]
>> --- a/drivers/xen/events/events_fifo.c
>> +++ b/drivers/xen/events/events_fifo.c
>> @@ -285,7 +285,7 @@ static void consume_one_event(unsigned cpu,
>>  static void evtchn_fifo_handle_events(unsigned cpu)
>>  {
>>       struct evtchn_fifo_control_block *control_block;
>> -     uint32_t ready;
>> +     uint32_t __aligned(sizeof(long)) ready;
>
> This doesn't look sufficient to me.  The event words within the event
> array are 32-bit sized/aligned and sync_set_bit() and sync_clear_bit()
> etc. are done on these words.
>
> I think arm64 is going to have to provide sync_clear_bit(),
> sync_set_bit(), sync_test_bit() and sync_test_and_set_bit() that work
> with 32-bit sized and aligned words.

Looks reasonable. If I understand correctly these sync_* functions are
used by Xen only (or at least was introduced because of Xen).
I might take rework them for arm64 to operate on 32-bit
size/alignment. Does this sounds reasonable?
... and what to do with this patch?

Vladimir

>
> David

Re: [PATCH] xen: fix alignment for bitops

[Vladimir Murzin]

On Tue, Apr 15, 2014 at 2:05 PM, Vladimir Murzin <murzin.v@gmail.com> wrote:
> On Tue, Apr 15, 2014 at 11:17 AM, David Vrabel <david.vrabel@citrix.com> wrote:
>> On 14/04/14 21:43, Vladimir Murzin wrote:
>>> Bitops operations like set/clear/change mandate world aligned pointer, mainly
>>> because architectures specific implementation.
>>>
>>> Looks that DEFINE_PER_CPU does required alignment for cpu_control_block;
>>> however, local copy used for bitops might not be world aligned.
>>>
>>> Instruct compiler to keep local copy of "ready" world aligned.
>> [...]
>>> --- a/drivers/xen/events/events_fifo.c
>>> +++ b/drivers/xen/events/events_fifo.c
>>> @@ -285,7 +285,7 @@ static void consume_one_event(unsigned cpu,
>>>  static void evtchn_fifo_handle_events(unsigned cpu)
>>>  {
>>>       struct evtchn_fifo_control_block *control_block;
>>> -     uint32_t ready;
>>> +     uint32_t __aligned(sizeof(long)) ready;
>>
>> This doesn't look sufficient to me.  The event words within the event
>> array are 32-bit sized/aligned and sync_set_bit() and sync_clear_bit()
>> etc. are done on these words.
>>
>> I think arm64 is going to have to provide sync_clear_bit(),
>> sync_set_bit(), sync_test_bit() and sync_test_and_set_bit() that work
>> with 32-bit sized and aligned words.
>
> Looks reasonable. If I understand correctly these sync_* functions are
> used by Xen only (or at least was introduced because of Xen).
> I might take rework them for arm64 to operate on 32-bit
> size/alignment. Does this sounds reasonable?
> ... and what to do with this patch?
>

I think it could be dropped if clear_bit(priority, BM(ready)) is
replaced with sync_ equivalent.

Vladimir

> Vladimir
>
>>
>> David

Re: [PATCH] xen: fix alignment for bitops

[David Vrabel]

On 15/04/14 14:13, Vladimir Murzin wrote:
> On Tue, Apr 15, 2014 at 2:05 PM, Vladimir Murzin <murzin.v@gmail.com> wrote:
>> On Tue, Apr 15, 2014 at 11:17 AM, David Vrabel <david.vrabel@citrix.com> wrote:
>>> On 14/04/14 21:43, Vladimir Murzin wrote:
>>>> Bitops operations like set/clear/change mandate world aligned pointer, mainly
>>>> because architectures specific implementation.
>>>>
>>>> Looks that DEFINE_PER_CPU does required alignment for cpu_control_block;
>>>> however, local copy used for bitops might not be world aligned.
>>>>
>>>> Instruct compiler to keep local copy of "ready" world aligned.
>>> [...]
>>>> --- a/drivers/xen/events/events_fifo.c
>>>> +++ b/drivers/xen/events/events_fifo.c
>>>> @@ -285,7 +285,7 @@ static void consume_one_event(unsigned cpu,
>>>>  static void evtchn_fifo_handle_events(unsigned cpu)
>>>>  {
>>>>       struct evtchn_fifo_control_block *control_block;
>>>> -     uint32_t ready;
>>>> +     uint32_t __aligned(sizeof(long)) ready;
>>>
>>> This doesn't look sufficient to me.  The event words within the event
>>> array are 32-bit sized/aligned and sync_set_bit() and sync_clear_bit()
>>> etc. are done on these words.
>>>
>>> I think arm64 is going to have to provide sync_clear_bit(),
>>> sync_set_bit(), sync_test_bit() and sync_test_and_set_bit() that work
>>> with 32-bit sized and aligned words.
>>
>> Looks reasonable. If I understand correctly these sync_* functions are
>> used by Xen only (or at least was introduced because of Xen).
>> I might take rework them for arm64 to operate on 32-bit
>> size/alignment. Does this sounds reasonable?
>> ... and what to do with this patch?
>>
> 
> I think it could be dropped if clear_bit(priority, BM(ready)) is
> replaced with sync_ equivalent.

ready can be unsigned long as Ian suggested.

David