Re: [Xen-devel] [PATCH v2] xen: fix alignment for bitops

[Vladimir Murzin <murzin.v@xxxxxxxxx>]

On Thu, Apr 17, 2014 at 11:22 AM, David Vrabel <david.vrabel@xxxxxxxxxx> wrote:
> On 16/04/14 08:55, 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.
>>
>> For arm64 it ends up with unaligned access trap...
>
> Thanks.  Does this version work for you?
>
> David
>
> 8<----------------------
> xen/events/fifo: fix alignment for bitops on local ready word
>
> Bitops operations like set_bit(), clear_bit(), and test_bit() require
> word aligned pointers, because of architectures specific
> implementations.
>
> Looks that DEFINE_PER_CPU does required alignment for
> cpu_control_block; however, local copy of the ready word might not be
> word aligned.
>
> For arm64 it ends up with unaligned access trap at:
>
>     if (head == 0)
>         clear_bit(priority, BM(ready));
>
> Use unsigned long for "ready" to make sure it is word aligned.
>
> Signed-off-by: Vladimir Murzin <murzin.v@xxxxxxxxx>
> Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx
> ---
>  drivers/xen/events/events_fifo.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/events/events_fifo.c 
> b/drivers/xen/events/events_fifo.c
> index 96109a9..475d967 100644
> --- a/drivers/xen/events/events_fifo.c
> +++ b/drivers/xen/events/events_fifo.c
> @@ -243,7 +243,7 @@ static void handle_irq_for_port(unsigned port)
>
>  static void consume_one_event(unsigned cpu,
>                               struct evtchn_fifo_control_block *control_block,
> -                             unsigned priority, uint32_t *ready)
> +                             unsigned priority, unsigned long *ready)
>  {
>         struct evtchn_fifo_queue *q = &per_cpu(cpu_queue, cpu);
>         uint32_t head;
> @@ -273,7 +273,7 @@ static void consume_one_event(unsigned cpu,
>          * copy of the ready word.
>          */
>         if (head == 0)
> -               clear_bit(priority, BM(ready));
> +               clear_bit(priority, ready);
>
>         if (sync_test_bit(EVTCHN_FIFO_PENDING, BM(word))
>             && !sync_test_bit(EVTCHN_FIFO_MASKED, BM(word)))
> @@ -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;
> +       unsigned long ready;
>         unsigned q;
>
>         control_block = per_cpu(cpu_control_block, cpu);

Yes. It fixes warning and I'm able to boot Dom0.

Vladimir

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