Re: [PATCH v2] xen: fix alignment for bitops

[David Vrabel <david.vrabel@citrix.com>]

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@gmail.com>
Signed-off-by: David Vrabel <david.vrabel@citrix.com
---
 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);