[PATCH 0/1] Temporary fix for data abort on armv6z EFI boot

[PATCH 0/1] Temporary fix for data abort on armv6z EFI boot

[Cristian Ciocaltea]

I'm trying to boot the Linux kernel on RaspberryPi Zero via GRUB2,
which in turn is executed by U-Boot as an UEFI binary.

However, I'm facing data abort issues in efi_get_memory_map()
that seem to be related to some ldrd instructions generated by the
compiler.

To simplify the investigation, I temporarily gave up on GRUB2 and
started directly the Linux kernel via U-Boot bootefi command.
The result is an immediate crash at PC 0x92c8:

data abort
pc : [<1c6b12c8>]          lr : [<1c6b1558>]
reloc pc : [<fe76a2c8>]    lr : [<fe76a558>]
sp : 1db43b30  ip : 00000000     fp : 1db43cc0
r10: 20494249  r9 : ffffffff     r8 : 00000000
r7 : 1db43b3c  r6 : 00000000     r5 : 1db43bfa  r4 : 1db43bb0
r3 : 1db43b9c  r2 : 00000028     r1 : 00000000  r0 : 1df4f828
Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
Code: e3a02028 e3a01000 e527100c e5832000 (e1c420d4)
UEFI image [0x1c6a8000:0x1cb2ffff] pc=0x92c8 '/boot\zImage'
Resetting CPU ...

The related disassembled section shows the *ldrd* instruction
in the context of the following statement:
*map->map_size =	*map->desc_size * 32;

drivers/firmware/efi/libstub/efi-stub-helper.c:90
	*map->desc_size =	sizeof(*m);
    92ac:	e5913008 	ldr	r3, [r1, #8]
drivers/firmware/efi/libstub/efi-stub-helper.c:84
{
    92b0:	e1a04001 	mov	r4, r1
drivers/firmware/efi/libstub/efi-stub-helper.c:85
	efi_memory_desc_t *m = NULL;
    92b4:	e28d7018 	add	r7, sp, #24
linux/drivers/firmware/efi/libstub/efi-stub-helper.c:90
	*map->desc_size =	sizeof(*m);
    92b8:	e3a02028 	mov	r2, #40	; 0x28
    92c4:	e5832000 	str	r2, [r3]
linux/drivers/firmware/efi/libstub/efi-stub-helper.c:91
	*map->map_size =	*map->desc_size * 32;
    92c8:	e1c420d4 	ldrd	r2, [r4, #4]

I changed the code to avoid the memory access and eventually get
rid of the ldrd instruction:
*map->map_size =	sizeof(*m) * 32;

A subsequent data abort was caused by a similar ldrd instruction
generated in the context of the statement:
*map->map_size += *map->desc_size * EFI_MMAP_NR_SLACK_SLOTS;

The workaround to avoid the ldrd instruction was trickier in that case,
as you may see in the patch, but eventually the kernel was able to boot
successfully, with or without GRUB2 chain-loading. The system looks
stable for the moment, but most probably there are many other similar
statements which were not enabled for this particular use case and still
have the potential to trigger data aborts.

Unfortunately I'm not sure how to further investigate this issue and,
therefore, I would kindly ask for some feedback or suggestions.

Some additional notes:
 - Used GCC 7.3.0 and GCC 8.2.0 based armv6-eabihf toolchains:
   https://toolchains.bootlin.com/releases_armv6-eabihf.html
 - Kernel and root filesystem build process handled by buildroot
 - Tested with kernels: 4.2, 4.3, 4.4

-- 
2.17.1

[PATCH 1/1] efi/libstub: Workaround for data abort on armv6z architecture

[Cristian Ciocaltea]

The armv6-eabihf toolchains generate some ldrd instructions
that trigger data abort on RaspberryPi Zero (ARM1176JZF-S CPU):

drivers/firmware/efi/libstub/efi-stub-helper.c:91
	*map->map_size =	*map->desc_size * 32;
    92c8:	e1c420d4 	ldrd	r2, [r4, #4]

This patch is a hack to instruct the compiler to avoid
using the problematic ldrd instructions.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 35dbc2791c97..8d7d27b8b9c2 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -86,9 +86,10 @@ efi_status_t efi_get_memory_map(efi_system_table_t *sys_table_arg,
 	efi_status_t status;
 	unsigned long key;
 	u32 desc_version;
+	unsigned long hack;
 
 	*map->desc_size =	sizeof(*m);
-	*map->map_size =	*map->desc_size * 32;
+	*map->map_size =	sizeof(*m) * 32;
 	*map->buff_size =	*map->map_size;
 again:
 	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
@@ -111,7 +112,9 @@ efi_status_t efi_get_memory_map(efi_system_table_t *sys_table_arg,
 		 * exceed this headroom once we are ready to trigger
 		 * ExitBootServices()
 		 */
-		*map->map_size += *map->desc_size * EFI_MMAP_NR_SLACK_SLOTS;
+		hack = *map->desc_size * EFI_MMAP_NR_SLACK_SLOTS + 1;
+		*map->map_size += hack;
+		--(*map->map_size);
 		*map->buff_size = *map->map_size;
 		goto again;
 	}
-- 
2.17.1

Re: [PATCH 0/1] Temporary fix for data abort on armv6z EFI boot

[Ard Biesheuvel]

(adding some people that may be able to help, and removing some others)

On Mon, 18 Nov 2019 at 18:42, Cristian Ciocaltea
<cristian.ciocaltea@gmail.com> wrote:
>
> I'm trying to boot the Linux kernel on RaspberryPi Zero via GRUB2,
> which in turn is executed by U-Boot as an UEFI binary.
>
> However, I'm facing data abort issues in efi_get_memory_map()
> that seem to be related to some ldrd instructions generated by the
> compiler.
>
> To simplify the investigation, I temporarily gave up on GRUB2 and
> started directly the Linux kernel via U-Boot bootefi command.
> The result is an immediate crash at PC 0x92c8:
>
> data abort
> pc : [<1c6b12c8>]          lr : [<1c6b1558>]
> reloc pc : [<fe76a2c8>]    lr : [<fe76a558>]
> sp : 1db43b30  ip : 00000000     fp : 1db43cc0
> r10: 20494249  r9 : ffffffff     r8 : 00000000
> r7 : 1db43b3c  r6 : 00000000     r5 : 1db43bfa  r4 : 1db43bb0
> r3 : 1db43b9c  r2 : 00000028     r1 : 00000000  r0 : 1df4f828
> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> Code: e3a02028 e3a01000 e527100c e5832000 (e1c420d4)
> UEFI image [0x1c6a8000:0x1cb2ffff] pc=0x92c8 '/boot\zImage'
> Resetting CPU ...
>
> The related disassembled section shows the *ldrd* instruction
> in the context of the following statement:
> *map->map_size =        *map->desc_size * 32;
>
> drivers/firmware/efi/libstub/efi-stub-helper.c:90
>         *map->desc_size =       sizeof(*m);
>     92ac:       e5913008        ldr     r3, [r1, #8]
> drivers/firmware/efi/libstub/efi-stub-helper.c:84
> {
>     92b0:       e1a04001        mov     r4, r1
> drivers/firmware/efi/libstub/efi-stub-helper.c:85
>         efi_memory_desc_t *m = NULL;
>     92b4:       e28d7018        add     r7, sp, #24
> linux/drivers/firmware/efi/libstub/efi-stub-helper.c:90
>         *map->desc_size =       sizeof(*m);
>     92b8:       e3a02028        mov     r2, #40 ; 0x28
>     92c4:       e5832000        str     r2, [r3]
> linux/drivers/firmware/efi/libstub/efi-stub-helper.c:91
>         *map->map_size =        *map->desc_size * 32;
>     92c8:       e1c420d4        ldrd    r2, [r4, #4]
>

This instruction looks absolutely fine, so I don't know why this is aborting.
0x1db43bb0+4 is word aligned, as required, and the address seems to be mapped.



> I changed the code to avoid the memory access and eventually get
> rid of the ldrd instruction:
> *map->map_size =        sizeof(*m) * 32;
>
> A subsequent data abort was caused by a similar ldrd instruction
> generated in the context of the statement:
> *map->map_size += *map->desc_size * EFI_MMAP_NR_SLACK_SLOTS;
>
> The workaround to avoid the ldrd instruction was trickier in that case,
> as you may see in the patch, but eventually the kernel was able to boot
> successfully, with or without GRUB2 chain-loading. The system looks
> stable for the moment, but most probably there are many other similar
> statements which were not enabled for this particular use case and still
> have the potential to trigger data aborts.
>
> Unfortunately I'm not sure how to further investigate this issue and,
> therefore, I would kindly ask for some feedback or suggestions.
>
> Some additional notes:
>  - Used GCC 7.3.0 and GCC 8.2.0 based armv6-eabihf toolchains:
>    https://toolchains.bootlin.com/releases_armv6-eabihf.html
>  - Kernel and root filesystem build process handled by buildroot
>  - Tested with kernels: 4.2, 4.3, 4.4
>
> --
> 2.17.1
>

Re: [PATCH 0/1] Temporary fix for data abort on armv6z EFI boot

[Heinrich Schuchardt]

On 11/18/19 7:02 PM, Ard Biesheuvel wrote:
> (adding some people that may be able to help, and removing some others)
>
> On Mon, 18 Nov 2019 at 18:42, Cristian Ciocaltea
> <cristian.ciocaltea@gmail.com> wrote:
>>
>> I'm trying to boot the Linux kernel on RaspberryPi Zero via GRUB2,
>> which in turn is executed by U-Boot as an UEFI binary.
>>
>> However, I'm facing data abort issues in efi_get_memory_map()
>> that seem to be related to some ldrd instructions generated by the
>> compiler.
>>
>> To simplify the investigation, I temporarily gave up on GRUB2 and
>> started directly the Linux kernel via U-Boot bootefi command.
>> The result is an immediate crash at PC 0x92c8:
>>
>> data abort
>> pc : [<1c6b12c8>]          lr : [<1c6b1558>]
>> reloc pc : [<fe76a2c8>]    lr : [<fe76a558>]
>> sp : 1db43b30  ip : 00000000     fp : 1db43cc0
>> r10: 20494249  r9 : ffffffff     r8 : 00000000
>> r7 : 1db43b3c  r6 : 00000000     r5 : 1db43bfa  r4 : 1db43bb0
>> r3 : 1db43b9c  r2 : 00000028     r1 : 00000000  r0 : 1df4f828
>> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
>> Code: e3a02028 e3a01000 e527100c e5832000 (e1c420d4)
>> UEFI image [0x1c6a8000:0x1cb2ffff] pc=0x92c8 '/boot\zImage'
>> Resetting CPU ...
>>
>> The related disassembled section shows the *ldrd* instruction
>> in the context of the following statement:
>> *map->map_size =        *map->desc_size * 32;
>>
>> drivers/firmware/efi/libstub/efi-stub-helper.c:90
>>          *map->desc_size =       sizeof(*m);
>>      92ac:       e5913008        ldr     r3, [r1, #8]
>> drivers/firmware/efi/libstub/efi-stub-helper.c:84
>> {
>>      92b0:       e1a04001        mov     r4, r1
>> drivers/firmware/efi/libstub/efi-stub-helper.c:85
>>          efi_memory_desc_t *m = NULL;
>>      92b4:       e28d7018        add     r7, sp, #24
>> linux/drivers/firmware/efi/libstub/efi-stub-helper.c:90
>>          *map->desc_size =       sizeof(*m);
>>      92b8:       e3a02028        mov     r2, #40 ; 0x28
>>      92c4:       e5832000        str     r2, [r3]
>> linux/drivers/firmware/efi/libstub/efi-stub-helper.c:91
>>          *map->map_size =        *map->desc_size * 32;
>>      92c8:       e1c420d4        ldrd    r2, [r4, #4]
>>
>
> This instruction looks absolutely fine, so I don't know why this is aborting.
> 0x1db43bb0+4 is word aligned, as required, and the address seems to be mapped.
>
>
>
>> I changed the code to avoid the memory access and eventually get
>> rid of the ldrd instruction:
>> *map->map_size =        sizeof(*m) * 32;
>>
>> A subsequent data abort was caused by a similar ldrd instruction
>> generated in the context of the statement:
>> *map->map_size += *map->desc_size * EFI_MMAP_NR_SLACK_SLOTS;
>>
>> The workaround to avoid the ldrd instruction was trickier in that case,
>> as you may see in the patch, but eventually the kernel was able to boot
>> successfully, with or without GRUB2 chain-loading. The system looks
>> stable for the moment, but most probably there are many other similar
>> statements which were not enabled for this particular use case and still
>> have the potential to trigger data aborts.
>>
>> Unfortunately I'm not sure how to further investigate this issue and,
>> therefore, I would kindly ask for some feedback or suggestions.
>>
>> Some additional notes:
>>   - Used GCC 7.3.0 and GCC 8.2.0 based armv6-eabihf toolchains:
>>     https://toolchains.bootlin.com/releases_armv6-eabihf.html
>>   - Kernel and root filesystem build process handled by buildroot
>>   - Tested with kernels: 4.2, 4.3, 4.4
>>
>> --
>> 2.17.1
>>
>

As described in

efi_loader: restrict EFI_LOADER to armv7 and armv8 on ARM
https://lists.denx.de/pipermail/u-boot/2019-November/390613.html

booting ARMv5 and ARMv6 via UEFI is not supported by U-Boot.

What would be needed is an implementation of function allow_unaligned()
in arch/arm/cpu/arm1176/sctlr.S (cf. arch/arm/cpu/armv7/sctlr.S).

Best regards

Heinrich

Re: [PATCH 0/1] Temporary fix for data abort on armv6z EFI boot

[Mark Rutland]

On Mon, Nov 18, 2019 at 07:42:51PM +0200, Cristian Ciocaltea wrote:
> I'm trying to boot the Linux kernel on RaspberryPi Zero via GRUB2,
> which in turn is executed by U-Boot as an UEFI binary.
> 
> However, I'm facing data abort issues in efi_get_memory_map()
> that seem to be related to some ldrd instructions generated by the
> compiler.
> 
> To simplify the investigation, I temporarily gave up on GRUB2 and
> started directly the Linux kernel via U-Boot bootefi command.
> The result is an immediate crash at PC 0x92c8:
> 
> data abort
> pc : [<1c6b12c8>]          lr : [<1c6b1558>]
> reloc pc : [<fe76a2c8>]    lr : [<fe76a558>]
> sp : 1db43b30  ip : 00000000     fp : 1db43cc0
> r10: 20494249  r9 : ffffffff     r8 : 00000000
> r7 : 1db43b3c  r6 : 00000000     r5 : 1db43bfa  r4 : 1db43bb0
> r3 : 1db43b9c  r2 : 00000028     r1 : 00000000  r0 : 1df4f828
> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> Code: e3a02028 e3a01000 e527100c e5832000 (e1c420d4)
> UEFI image [0x1c6a8000:0x1cb2ffff] pc=0x92c8 '/boot\zImage'
> Resetting CPU ...
> 
> The related disassembled section shows the *ldrd* instruction
> in the context of the following statement:
> *map->map_size =	*map->desc_size * 32;
> 
> drivers/firmware/efi/libstub/efi-stub-helper.c:90
> 	*map->desc_size =	sizeof(*m);
>     92ac:	e5913008 	ldr	r3, [r1, #8]
> drivers/firmware/efi/libstub/efi-stub-helper.c:84
> {
>     92b0:	e1a04001 	mov	r4, r1
> drivers/firmware/efi/libstub/efi-stub-helper.c:85
> 	efi_memory_desc_t *m = NULL;
>     92b4:	e28d7018 	add	r7, sp, #24
> linux/drivers/firmware/efi/libstub/efi-stub-helper.c:90
> 	*map->desc_size =	sizeof(*m);
>     92b8:	e3a02028 	mov	r2, #40	; 0x28
>     92c4:	e5832000 	str	r2, [r3]
> linux/drivers/firmware/efi/libstub/efi-stub-helper.c:91
> 	*map->map_size =	*map->desc_size * 32;
>     92c8:	e1c420d4 	ldrd	r2, [r4, #4]

At this point, r4 is 16-byte aligned, so that's a misaligned LDRD.

Looking at version 2.8of the UEFI spec, in section 2.3.5 "AArch32
Platforms", it states that SCTLR should be configured:

| A=0, U=1 on ARMv6 and ARMv7

... which IIUC should permit a misaligned LDRD. Is U-Boot definitely
configuring SCTLR that way? Or has it perhaps set A=1, U=0?

Thanks,
Mark.

Re: [PATCH 0/1] Temporary fix for data abort on armv6z EFI boot

[Ard Biesheuvel]

On Mon, 18 Nov 2019 at 19:10, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Mon, Nov 18, 2019 at 07:42:51PM +0200, Cristian Ciocaltea wrote:
> > I'm trying to boot the Linux kernel on RaspberryPi Zero via GRUB2,
> > which in turn is executed by U-Boot as an UEFI binary.
> >
> > However, I'm facing data abort issues in efi_get_memory_map()
> > that seem to be related to some ldrd instructions generated by the
> > compiler.
> >
> > To simplify the investigation, I temporarily gave up on GRUB2 and
> > started directly the Linux kernel via U-Boot bootefi command.
> > The result is an immediate crash at PC 0x92c8:
> >
> > data abort
> > pc : [<1c6b12c8>]          lr : [<1c6b1558>]
> > reloc pc : [<fe76a2c8>]    lr : [<fe76a558>]
> > sp : 1db43b30  ip : 00000000     fp : 1db43cc0
> > r10: 20494249  r9 : ffffffff     r8 : 00000000
> > r7 : 1db43b3c  r6 : 00000000     r5 : 1db43bfa  r4 : 1db43bb0
> > r3 : 1db43b9c  r2 : 00000028     r1 : 00000000  r0 : 1df4f828
> > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > Code: e3a02028 e3a01000 e527100c e5832000 (e1c420d4)
> > UEFI image [0x1c6a8000:0x1cb2ffff] pc=0x92c8 '/boot\zImage'
> > Resetting CPU ...
> >
> > The related disassembled section shows the *ldrd* instruction
> > in the context of the following statement:
> > *map->map_size =      *map->desc_size * 32;
> >
> > drivers/firmware/efi/libstub/efi-stub-helper.c:90
> >       *map->desc_size =       sizeof(*m);
> >     92ac:     e5913008        ldr     r3, [r1, #8]
> > drivers/firmware/efi/libstub/efi-stub-helper.c:84
> > {
> >     92b0:     e1a04001        mov     r4, r1
> > drivers/firmware/efi/libstub/efi-stub-helper.c:85
> >       efi_memory_desc_t *m = NULL;
> >     92b4:     e28d7018        add     r7, sp, #24
> > linux/drivers/firmware/efi/libstub/efi-stub-helper.c:90
> >       *map->desc_size =       sizeof(*m);
> >     92b8:     e3a02028        mov     r2, #40 ; 0x28
> >     92c4:     e5832000        str     r2, [r3]
> > linux/drivers/firmware/efi/libstub/efi-stub-helper.c:91
> >       *map->map_size =        *map->desc_size * 32;
> >     92c8:     e1c420d4        ldrd    r2, [r4, #4]
>
> At this point, r4 is 16-byte aligned, so that's a misaligned LDRD.
>
> Looking at version 2.8of the UEFI spec, in section 2.3.5 "AArch32
> Platforms", it states that SCTLR should be configured:
>
> | A=0, U=1 on ARMv6 and ARMv7
>
> ... which IIUC should permit a misaligned LDRD. Is U-Boot definitely
> configuring SCTLR that way? Or has it perhaps set A=1, U=0?
>

LDRD requires word alignment only, no?

Re: [PATCH 0/1] Temporary fix for data abort on armv6z EFI boot

[Mark Rutland]

On Mon, Nov 18, 2019 at 07:15:04PM +0100, Ard Biesheuvel wrote:
> On Mon, 18 Nov 2019 at 19:10, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Mon, Nov 18, 2019 at 07:42:51PM +0200, Cristian Ciocaltea wrote:
> > > I'm trying to boot the Linux kernel on RaspberryPi Zero via GRUB2,
> > > which in turn is executed by U-Boot as an UEFI binary.
> > >
> > > However, I'm facing data abort issues in efi_get_memory_map()
> > > that seem to be related to some ldrd instructions generated by the
> > > compiler.
> > >
> > > To simplify the investigation, I temporarily gave up on GRUB2 and
> > > started directly the Linux kernel via U-Boot bootefi command.
> > > The result is an immediate crash at PC 0x92c8:
> > >
> > > data abort
> > > pc : [<1c6b12c8>]          lr : [<1c6b1558>]
> > > reloc pc : [<fe76a2c8>]    lr : [<fe76a558>]
> > > sp : 1db43b30  ip : 00000000     fp : 1db43cc0
> > > r10: 20494249  r9 : ffffffff     r8 : 00000000
> > > r7 : 1db43b3c  r6 : 00000000     r5 : 1db43bfa  r4 : 1db43bb0
> > > r3 : 1db43b9c  r2 : 00000028     r1 : 00000000  r0 : 1df4f828
> > > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > > Code: e3a02028 e3a01000 e527100c e5832000 (e1c420d4)
> > > UEFI image [0x1c6a8000:0x1cb2ffff] pc=0x92c8 '/boot\zImage'
> > > Resetting CPU ...
> > >
> > > The related disassembled section shows the *ldrd* instruction
> > > in the context of the following statement:
> > > *map->map_size =      *map->desc_size * 32;
> > >
> > > drivers/firmware/efi/libstub/efi-stub-helper.c:90
> > >       *map->desc_size =       sizeof(*m);
> > >     92ac:     e5913008        ldr     r3, [r1, #8]
> > > drivers/firmware/efi/libstub/efi-stub-helper.c:84
> > > {
> > >     92b0:     e1a04001        mov     r4, r1
> > > drivers/firmware/efi/libstub/efi-stub-helper.c:85
> > >       efi_memory_desc_t *m = NULL;
> > >     92b4:     e28d7018        add     r7, sp, #24
> > > linux/drivers/firmware/efi/libstub/efi-stub-helper.c:90
> > >       *map->desc_size =       sizeof(*m);
> > >     92b8:     e3a02028        mov     r2, #40 ; 0x28
> > >     92c4:     e5832000        str     r2, [r3]
> > > linux/drivers/firmware/efi/libstub/efi-stub-helper.c:91
> > >       *map->map_size =        *map->desc_size * 32;
> > >     92c8:     e1c420d4        ldrd    r2, [r4, #4]
> >
> > At this point, r4 is 16-byte aligned, so that's a misaligned LDRD.
> >
> > Looking at version 2.8of the UEFI spec, in section 2.3.5 "AArch32
> > Platforms", it states that SCTLR should be configured:
> >
> > | A=0, U=1 on ARMv6 and ARMv7
> >
> > ... which IIUC should permit a misaligned LDRD. Is U-Boot definitely
> > configuring SCTLR that way? Or has it perhaps set A=1, U=0?
> 
> LDRD requires word alignment only, no?

Looking at the ARMv6 ARM ARM, I see mention of modulo-8 alignemnt
checking (specifically for LDRD/STRD) when A=1, U=0. Unless I've
misunder

See A2.7.3 "Endian configuration and control" in ARM DDI 0100I. Table
A2-6 and A2-8 both suggest that, with A2-8 saying that a Data Abort
would result.

Thanks,
Mark.

Re: [PATCH 0/1] Temporary fix for data abort on armv6z EFI boot

[Ard Biesheuvel]

(adding Heinrich and Guillaume again)

On Mon, 18 Nov 2019 at 19:51, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Mon, Nov 18, 2019 at 07:15:04PM +0100, Ard Biesheuvel wrote:
> > On Mon, 18 Nov 2019 at 19:10, Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > On Mon, Nov 18, 2019 at 07:42:51PM +0200, Cristian Ciocaltea wrote:
> > > > I'm trying to boot the Linux kernel on RaspberryPi Zero via GRUB2,
> > > > which in turn is executed by U-Boot as an UEFI binary.
> > > >
> > > > However, I'm facing data abort issues in efi_get_memory_map()
> > > > that seem to be related to some ldrd instructions generated by the
> > > > compiler.
> > > >
> > > > To simplify the investigation, I temporarily gave up on GRUB2 and
> > > > started directly the Linux kernel via U-Boot bootefi command.
> > > > The result is an immediate crash at PC 0x92c8:
> > > >
> > > > data abort
> > > > pc : [<1c6b12c8>]          lr : [<1c6b1558>]
> > > > reloc pc : [<fe76a2c8>]    lr : [<fe76a558>]
> > > > sp : 1db43b30  ip : 00000000     fp : 1db43cc0
> > > > r10: 20494249  r9 : ffffffff     r8 : 00000000
> > > > r7 : 1db43b3c  r6 : 00000000     r5 : 1db43bfa  r4 : 1db43bb0
> > > > r3 : 1db43b9c  r2 : 00000028     r1 : 00000000  r0 : 1df4f828
> > > > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > > > Code: e3a02028 e3a01000 e527100c e5832000 (e1c420d4)
> > > > UEFI image [0x1c6a8000:0x1cb2ffff] pc=0x92c8 '/boot\zImage'
> > > > Resetting CPU ...
> > > >
> > > > The related disassembled section shows the *ldrd* instruction
> > > > in the context of the following statement:
> > > > *map->map_size =      *map->desc_size * 32;
> > > >
> > > > drivers/firmware/efi/libstub/efi-stub-helper.c:90
> > > >       *map->desc_size =       sizeof(*m);
> > > >     92ac:     e5913008        ldr     r3, [r1, #8]
> > > > drivers/firmware/efi/libstub/efi-stub-helper.c:84
> > > > {
> > > >     92b0:     e1a04001        mov     r4, r1
> > > > drivers/firmware/efi/libstub/efi-stub-helper.c:85
> > > >       efi_memory_desc_t *m = NULL;
> > > >     92b4:     e28d7018        add     r7, sp, #24
> > > > linux/drivers/firmware/efi/libstub/efi-stub-helper.c:90
> > > >       *map->desc_size =       sizeof(*m);
> > > >     92b8:     e3a02028        mov     r2, #40 ; 0x28
> > > >     92c4:     e5832000        str     r2, [r3]
> > > > linux/drivers/firmware/efi/libstub/efi-stub-helper.c:91
> > > >       *map->map_size =        *map->desc_size * 32;
> > > >     92c8:     e1c420d4        ldrd    r2, [r4, #4]
> > >
> > > At this point, r4 is 16-byte aligned, so that's a misaligned LDRD.
> > >
> > > Looking at version 2.8of the UEFI spec, in section 2.3.5 "AArch32
> > > Platforms", it states that SCTLR should be configured:
> > >
> > > | A=0, U=1 on ARMv6 and ARMv7
> > >
> > > ... which IIUC should permit a misaligned LDRD. Is U-Boot definitely
> > > configuring SCTLR that way? Or has it perhaps set A=1, U=0?
> >
> > LDRD requires word alignment only, no?
>
> Looking at the ARMv6 ARM ARM, I see mention of modulo-8 alignemnt
> checking (specifically for LDRD/STRD) when A=1, U=0. Unless I've
> misunder
>
> See A2.7.3 "Endian configuration and control" in ARM DDI 0100I. Table
> A2-6 and A2-8 both suggest that, with A2-8 saying that a Data Abort
> would result.
>

You are right.

I have added Heinrich to cc [again] - perhaps he knows whether u-boot
always adheres to this UEFI requirement?

Re: [PATCH 0/1] Temporary fix for data abort on armv6z EFI boot

[Heinrich Schuchardt]

On 11/18/19 8:41 PM, Ard Biesheuvel wrote:
> (adding Heinrich and Guillaume again)
>
> On Mon, 18 Nov 2019 at 19:51, Mark Rutland <mark.rutland@arm.com> wrote:
>>
>> On Mon, Nov 18, 2019 at 07:15:04PM +0100, Ard Biesheuvel wrote:
>>> On Mon, 18 Nov 2019 at 19:10, Mark Rutland <mark.rutland@arm.com> wrote:
>>>>
>>>> On Mon, Nov 18, 2019 at 07:42:51PM +0200, Cristian Ciocaltea wrote:
>>>>> I'm trying to boot the Linux kernel on RaspberryPi Zero via GRUB2,
>>>>> which in turn is executed by U-Boot as an UEFI binary.
>>>>>
>>>>> However, I'm facing data abort issues in efi_get_memory_map()
>>>>> that seem to be related to some ldrd instructions generated by the
>>>>> compiler.
>>>>>
>>>>> To simplify the investigation, I temporarily gave up on GRUB2 and
>>>>> started directly the Linux kernel via U-Boot bootefi command.
>>>>> The result is an immediate crash at PC 0x92c8:

Hello Christian,

could you, please, check if this patch applied to U-Boot v2019.10
resolves your problem:

https://github.com/xypron/u-boot-patches/blob/efi-next/0001-arm-arm11-allow-unaligned-memory-access.patch

Currently I do not have an ARMv6 board available for testing. (But I
just ordered one.)

Best regards

Heinrich Schuchardt

>>>>>
>>>>> data abort
>>>>> pc : [<1c6b12c8>]          lr : [<1c6b1558>]
>>>>> reloc pc : [<fe76a2c8>]    lr : [<fe76a558>]
>>>>> sp : 1db43b30  ip : 00000000     fp : 1db43cc0
>>>>> r10: 20494249  r9 : ffffffff     r8 : 00000000
>>>>> r7 : 1db43b3c  r6 : 00000000     r5 : 1db43bfa  r4 : 1db43bb0
>>>>> r3 : 1db43b9c  r2 : 00000028     r1 : 00000000  r0 : 1df4f828
>>>>> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
>>>>> Code: e3a02028 e3a01000 e527100c e5832000 (e1c420d4)
>>>>> UEFI image [0x1c6a8000:0x1cb2ffff] pc=0x92c8 '/boot\zImage'
>>>>> Resetting CPU ...
>>>>>
>>>>> The related disassembled section shows the *ldrd* instruction
>>>>> in the context of the following statement:
>>>>> *map->map_size =      *map->desc_size * 32;
>>>>>
>>>>> drivers/firmware/efi/libstub/efi-stub-helper.c:90
>>>>>        *map->desc_size =       sizeof(*m);
>>>>>      92ac:     e5913008        ldr     r3, [r1, #8]
>>>>> drivers/firmware/efi/libstub/efi-stub-helper.c:84
>>>>> {
>>>>>      92b0:     e1a04001        mov     r4, r1
>>>>> drivers/firmware/efi/libstub/efi-stub-helper.c:85
>>>>>        efi_memory_desc_t *m = NULL;
>>>>>      92b4:     e28d7018        add     r7, sp, #24
>>>>> linux/drivers/firmware/efi/libstub/efi-stub-helper.c:90
>>>>>        *map->desc_size =       sizeof(*m);
>>>>>      92b8:     e3a02028        mov     r2, #40 ; 0x28
>>>>>      92c4:     e5832000        str     r2, [r3]
>>>>> linux/drivers/firmware/efi/libstub/efi-stub-helper.c:91
>>>>>        *map->map_size =        *map->desc_size * 32;
>>>>>      92c8:     e1c420d4        ldrd    r2, [r4, #4]
>>>>
>>>> At this point, r4 is 16-byte aligned, so that's a misaligned LDRD.
>>>>
>>>> Looking at version 2.8of the UEFI spec, in section 2.3.5 "AArch32
>>>> Platforms", it states that SCTLR should be configured:
>>>>
>>>> | A=0, U=1 on ARMv6 and ARMv7
>>>>
>>>> ... which IIUC should permit a misaligned LDRD. Is U-Boot definitely
>>>> configuring SCTLR that way? Or has it perhaps set A=1, U=0?
>>>
>>> LDRD requires word alignment only, no?
>>
>> Looking at the ARMv6 ARM ARM, I see mention of modulo-8 alignemnt
>> checking (specifically for LDRD/STRD) when A=1, U=0. Unless I've
>> misunder
>>
>> See A2.7.3 "Endian configuration and control" in ARM DDI 0100I. Table
>> A2-6 and A2-8 both suggest that, with A2-8 saying that a Data Abort
>> would result.
>>
>
> You are right.
>
> I have added Heinrich to cc [again] - perhaps he knows whether u-boot
> always adheres to this UEFI requirement?
>

RE: [PATCH 0/1] Temporary fix for data abort on armv6z EFI boot

[Guillaume Gardet]

Hi,

> -----Original Message-----
> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Sent: 19 November 2019 04:16
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Mark Rutland
> <Mark.Rutland@arm.com>; Guillaume Gardet <Guillaume.Gardet@arm.com>
> Cc: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>; Ingo Molnar
> <mingo@kernel.org>; Kairui Song <kasong@redhat.com>; Hans de Goede
> <hdegoede@redhat.com>; Matthew Garrett <matthewgarrett@google.com>;
> linux-efi <linux-efi@vger.kernel.org>
> Subject: Re: [PATCH 0/1] Temporary fix for data abort on armv6z EFI boot
>
> On 11/18/19 8:41 PM, Ard Biesheuvel wrote:
> > (adding Heinrich and Guillaume again)
> >
> > On Mon, 18 Nov 2019 at 19:51, Mark Rutland <mark.rutland@arm.com> wrote:
> >>
> >> On Mon, Nov 18, 2019 at 07:15:04PM +0100, Ard Biesheuvel wrote:
> >>> On Mon, 18 Nov 2019 at 19:10, Mark Rutland <mark.rutland@arm.com>
> wrote:
> >>>>
> >>>> On Mon, Nov 18, 2019 at 07:42:51PM +0200, Cristian Ciocaltea wrote:
> >>>>> I'm trying to boot the Linux kernel on RaspberryPi Zero via GRUB2,
> >>>>> which in turn is executed by U-Boot as an UEFI binary.
> >>>>>
> >>>>> However, I'm facing data abort issues in efi_get_memory_map() that
> >>>>> seem to be related to some ldrd instructions generated by the
> >>>>> compiler.
> >>>>>
> >>>>> To simplify the investigation, I temporarily gave up on GRUB2 and
> >>>>> started directly the Linux kernel via U-Boot bootefi command.
> >>>>> The result is an immediate crash at PC 0x92c8:
>
> Hello Christian,
>
> could you, please, check if this patch applied to U-Boot v2019.10 resolves your
> problem:
>
> https://github.com/xypron/u-boot-patches/blob/efi-next/0001-arm-arm11-
> allow-unaligned-memory-access.patch
>
> Currently I do not have an ARMv6 board available for testing. (But I just ordered
> one.)

I tested your patch on top of u-boot v2019.10 and it fixes the armv6 data abort.

Thanks a lot!
Guillaume

>
> Best regards
>
> Heinrich Schuchardt
>
> >>>>>
> >>>>> data abort
> >>>>> pc : [<1c6b12c8>]          lr : [<1c6b1558>]
> >>>>> reloc pc : [<fe76a2c8>]    lr : [<fe76a558>]
> >>>>> sp : 1db43b30  ip : 00000000     fp : 1db43cc0
> >>>>> r10: 20494249  r9 : ffffffff     r8 : 00000000
> >>>>> r7 : 1db43b3c  r6 : 00000000     r5 : 1db43bfa  r4 : 1db43bb0
> >>>>> r3 : 1db43b9c  r2 : 00000028     r1 : 00000000  r0 : 1df4f828
> >>>>> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> >>>>> Code: e3a02028 e3a01000 e527100c e5832000 (e1c420d4) UEFI image
> >>>>> [0x1c6a8000:0x1cb2ffff] pc=0x92c8 '/boot\zImage'
> >>>>> Resetting CPU ...
> >>>>>
> >>>>> The related disassembled section shows the *ldrd* instruction in
> >>>>> the context of the following statement:
> >>>>> *map->map_size =      *map->desc_size * 32;
> >>>>>
> >>>>> drivers/firmware/efi/libstub/efi-stub-helper.c:90
> >>>>>        *map->desc_size =       sizeof(*m);
> >>>>>      92ac:     e5913008        ldr     r3, [r1, #8]
> >>>>> drivers/firmware/efi/libstub/efi-stub-helper.c:84
> >>>>> {
> >>>>>      92b0:     e1a04001        mov     r4, r1
> >>>>> drivers/firmware/efi/libstub/efi-stub-helper.c:85
> >>>>>        efi_memory_desc_t *m = NULL;
> >>>>>      92b4:     e28d7018        add     r7, sp, #24
> >>>>> linux/drivers/firmware/efi/libstub/efi-stub-helper.c:90
> >>>>>        *map->desc_size =       sizeof(*m);
> >>>>>      92b8:     e3a02028        mov     r2, #40 ; 0x28
> >>>>>      92c4:     e5832000        str     r2, [r3]
> >>>>> linux/drivers/firmware/efi/libstub/efi-stub-helper.c:91
> >>>>>        *map->map_size =        *map->desc_size * 32;
> >>>>>      92c8:     e1c420d4        ldrd    r2, [r4, #4]
> >>>>
> >>>> At this point, r4 is 16-byte aligned, so that's a misaligned LDRD.
> >>>>
> >>>> Looking at version 2.8of the UEFI spec, in section 2.3.5 "AArch32
> >>>> Platforms", it states that SCTLR should be configured:
> >>>>
> >>>> | A=0, U=1 on ARMv6 and ARMv7
> >>>>
> >>>> ... which IIUC should permit a misaligned LDRD. Is U-Boot
> >>>> definitely configuring SCTLR that way? Or has it perhaps set A=1, U=0?
> >>>
> >>> LDRD requires word alignment only, no?
> >>
> >> Looking at the ARMv6 ARM ARM, I see mention of modulo-8 alignemnt
> >> checking (specifically for LDRD/STRD) when A=1, U=0. Unless I've
> >> misunder
> >>
> >> See A2.7.3 "Endian configuration and control" in ARM DDI 0100I. Table
> >> A2-6 and A2-8 both suggest that, with A2-8 saying that a Data Abort
> >> would result.
> >>
> >
> > You are right.
> >
> > I have added Heinrich to cc [again] - perhaps he knows whether u-boot
> > always adheres to this UEFI requirement?
> >

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.