Re: Unaligned access on arm32

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Julien,

> On 7 Sep 2022, at 16:30, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi all,
> 
> I think mentioned it privately a while a go, but never sent an e-mail about 
> it.
> 
> While testing arm32 with IOREQ, I noticed Xen is crashing because an 
> alignment fault:
> 
> (XEN) Data Abort Trap. Syndrome=0x1800061
> (XEN) Walking Hypervisor VA 0x40027ebb on CPU0 via TTBR 0x000000004012f000
> (XEN) 1ST[0x001] = 0x00400000bbffff7f
> (XEN) 2ND[0x000] = 0x00500000bc000f7d
> (XEN) CPU0: Unexpected Trap: Data Abort
> (XEN) ----[ Xen-4.17-unstable  arm32  debug=n  Tainted:   C    ]----
> (XEN) CPU:    0
> (XEN) PC:     002613b8 try_fwd_ioserv+0x44/0x1bc
> (XEN) CPSR:   6000005a MODE:Hypervisor
> (XEN)      R0: 00000000 R1: 00000001 R2: 0022a748 R3: 00000006
> (XEN)      R4: 40027f20 R5: 40027f58 R6: 40028000 R7: 00000000
> (XEN)      R8: 40027f20 R9: 4003a438 R10:002f0044 R11:40027edc R12:00000002
> (XEN) HYP: SP: 40027e94 LR: 00260edc
> (XEN)
> (XEN)   VTCR_EL2: 80003558
> (XEN)  VTTBR_EL2: 00010000bbff8000
> (XEN)
> (XEN)  SCTLR_EL2: 30cd187f
> (XEN)    HCR_EL2: 007c663f
> (XEN)  TTBR0_EL2: 000000004012f000
> (XEN)
> (XEN)    ESR_EL2: 97800061
> (XEN)  HPFAR_EL2: 0067faf0
> (XEN)      HDFAR: 40027ebb
> (XEN)      HIFAR: 67600000
> (XEN)
> (XEN) Xen stack trace from sp=40027e94:
> (XEN)    97800061 0022a748 00000001 00000000 8000005a 00800000 4003a000 
> 00000001
> (XEN)    4003a180 00000000 bbff47ff 00000000 67faf200 00000000 4003a000 
> 40027f20
> (XEN)    4003a438 40027f1c 00260edc 002f0110 40027f58 40028000 4003a000 
> 0000013b
> (XEN)    40028000 002f0280 00000090 40027f58 67faf200 93820006 67faf200 
> 00000000
> (XEN)    00000000 40027f54 0026b6ac 93820006 0022a748 00000001 00000004 
> 67faf200
> (XEN)    00000000 00000000 00000000 00000000 ffffffff 68000000 400001d3 
> 40027f58
> (XEN)    00201870 60000000 67601324 67faf200 00000000 00000013 00000000 
> 00000000
> (XEN)    ffffffff 68000000 400001d3 00000000 00000000 00000000 ffffffff 
> 00000000
> (XEN)    67601074 000001d3 93820006 00000000 00000000 00000000 00000000 
> 67601008
> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 
> 00000000
> (XEN)    00000000 00000000 00000000 400001d3 00000000 00000000 00000000 
> 00000000
> (XEN)    00000000 00000000 00000001
> (XEN) Xen call trace:
> (XEN)    [<002613b8>] try_fwd_ioserv+0x44/0x1bc (PC)
> (XEN)    [<00260edc>] try_handle_mmio+0x2b0/0x2f4 (LR)
> (XEN)    [<00260edc>] try_handle_mmio+0x2b0/0x2f4
> (XEN)    [<0026b6ac>] arch/arm/traps.c#do_trap_stage2_abort_guest+0x18c/0x34c
> (XEN)    [<00201870>] entry.o#return_from_trap+0/0x4
> (XEN)
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) CPU0: Unexpected Trap: Data Abort
> (XEN) ****************************************
> 
> The disassembled code is:
> 
> 00261374 <try_fwd_ioserv>:
>  261374:       e16d42f0        strd    r4, [sp, #-32]! ; 0xffffffe0
>  261378:       e1a04002        mov     r4, r2
>  26137c:       e1a05000        mov     r5, r0
>  261380:       e1cd60f8        strd    r6, [sp, #8]
>  261384:       e3a00000        mov     r0, #0
>  261388:       e1a06001        mov     r6, r1
>  26138c:       e1cd81f0        strd    r8, [sp, #16]
>  261390:       e3a01001        mov     r1, #1
>  261394:       e58db018        str     fp, [sp, #24]
>  261398:       e28db01c        add     fp, sp, #28
>  26139c:       e58de01c        str     lr, [sp, #28]
>  2613a0:       e24dd028        sub     sp, sp, #40     ; 0x28
>  2613a4:       e1c220d4        ldrd    r2, [r2, #4]
>  2613a8:       e50b0024        str     r0, [fp, #-36]  ; 0xffffffdc
>  2613ac:       e5d67a26        ldrb    r7, [r6, #2598] ; 0xa26
>  2613b0:       e14b24f4        strd    r2, [fp, #-68]  ; 0xffffffbc
>  2613b4:       e5d43000        ldrb    r3, [r4]
> * 2613b8:       e50b0021        str     r0, [fp, #-33]  ; 0xffffffdf
> 
> The problem is GCC [1] decided to use 'str' for accessing an address that is 
> not 32-bit aligned (fp - 33 = 0x40027e73). On arm32, we are forbidding 
> aligned access, hence why it crashed.
> 
> Looking online, it looks like GCC has an option to turned on/off the use of 
> unaligned access [2] and it is enabled by default on ARMv7 (I am not sure why 
> we didn't notice this before...).
> 
> I have rebuilt Xen with the option turn off and dom0 is now booting fine.
> 
> However, I am a bit puzzled because the C code didn't contain unaligned 
> access. It was all introduced by the compiler itself. This is breaking our 
> assumption that the compiler will not break down correctly naturally aligned 
> access.

Strictly speaking without the option the compiler does not say that it will not 
generate unaligned access so nothing forbids it to generate some.
So I doubt we could argue that the compiler did something wrong here (there 
might be a debate on why it did so as the C code is not doing one).

> 
> When using the new option, Xen size is increasing by 0.40% (about ~3KB) in my 
> setup. Nothing too concerning, but I would like to have some opinion first.
> 
> The alternative would be to allow unaligned access. That said, I think this 
> is much saner to have a strict enforcement as we can catch GCC trying to do 
> unaligned access when it is not expected.
> 
> There is also the argument that unaligned access can be slower on some 
> platform. So it is better to avoid them.
> 
> Any thoughts?

I think we need to be coherent: if we are not allowing unaligned access then we 
should explicitly ask the compiler not generate some, so here one way or the 
other we need to add the compiler option.

Now on arm32, unaligned access might be slower and seeing the size increase I 
would vote to stay as is to detect them and pass the compiler option to not do 
any.

Cheers
Bertrand

> 
> Cheers,
> 
> [1] gcc (GCC) 7.3.1 20180712 (Red Hat 7.3.1-15)
> [2] https://gcc.gnu.org/onlinedocs/gcc-4.8.5/gcc/ARM-Options.html
> 
> -- 
> Julien Grall