[PATCH] arm: enable unaligned accesses by default if EFI is configured
Heinrich Schuchardt
heinrich.schuchardt at canonical.com
Fri Mar 17 14:56:02 CET 2023
On 3/17/23 14:42, Ilias Apalodimas wrote:
> Heinrich reports that compiling with LTO & EFI breaks armv7 and arm11*
> builds. The reason is that LTO (maybe a binutils bug?) replaces the
> asm version of allow_unaligned() with the __weak definition of the
> function. As a result EFI code ends up running with unaligned accesses
> disabled and eventually crashes.
>
> So let's enable unaligned access for those architectures during
> our start.S routines and avoid the linker issues.
>
> Reported-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
The problem was originally reported by Tom. My contribution was to track
it down to missing enabling of unaligned access due to a linker problem.
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
> arch/arm/cpu/arm11/Makefile | 4 ----
> arch/arm/cpu/arm11/sctlr.S | 25 -------------------------
> arch/arm/cpu/arm1136/start.S | 3 +++
> arch/arm/cpu/arm1176/start.S | 3 +++
> arch/arm/cpu/armv7/Makefile | 1 -
> arch/arm/cpu/armv7/sctlr.S | 22 ----------------------
> arch/arm/cpu/armv7/start.S | 3 +++
> include/asm-generic/unaligned.h | 4 ----
> lib/efi_loader/efi_device_path.c | 7 -------
> lib/efi_loader/efi_setup.c | 12 ------------
> 10 files changed, 9 insertions(+), 75 deletions(-)
> delete mode 100644 arch/arm/cpu/arm11/sctlr.S
> delete mode 100644 arch/arm/cpu/armv7/sctlr.S
>
> diff --git a/arch/arm/cpu/arm11/Makefile b/arch/arm/cpu/arm11/Makefile
> index 5dfa01ae8d05..5d721fce12b5 100644
> --- a/arch/arm/cpu/arm11/Makefile
> +++ b/arch/arm/cpu/arm11/Makefile
> @@ -4,7 +4,3 @@
> # Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>
> obj-y = cpu.o
> -
> -ifneq ($(CONFIG_SPL_BUILD),y)
> -obj-$(CONFIG_EFI_LOADER) += sctlr.o
> -endif
> diff --git a/arch/arm/cpu/arm11/sctlr.S b/arch/arm/cpu/arm11/sctlr.S
> deleted file mode 100644
> index 74a7fc4a25b6..000000000000
> --- a/arch/arm/cpu/arm11/sctlr.S
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0+ */
> -/*
> - * Routines to access the system control register
> - *
> - * Copyright (c) 2019 Heinrich Schuchardt
> - */
> -
> -#include <linux/linkage.h>
> -
> -/*
> - * void allow_unaligned(void) - allow unaligned access
> - *
> - * This routine sets the enable unaligned data support flag and clears the
> - * aligned flag in the system control register.
> - * After calling this routine unaligned access does no longer leads to a
> - * data abort or undefined behavior but is handled by the CPU.
> - * For details see the "ARM Architecture Reference Manual" for ARMv6.
> - */
> -ENTRY(allow_unaligned)
> - mrc p15, 0, r0, c1, c0, 0 @ load system control register
> - orr r0, r0, #1 << 22 @ set unaligned data support flag
> - bic r0, r0, #2 @ clear aligned flag
> - mcr p15, 0, r0, c1, c0, 0 @ write system control register
> - bx lr @ return
> -ENDPROC(allow_unaligned)
> diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S
> index 4bc27f637366..f2a18d8f3423 100644
> --- a/arch/arm/cpu/arm1136/start.S
> +++ b/arch/arm/cpu/arm1136/start.S
> @@ -77,7 +77,10 @@ cpu_init_crit:
> mrc p15, 0, r0, c1, c0, 0
> bic r0, r0, #0x00002300 @ clear bits 13, 9:8 (--V- --RS)
> bic r0, r0, #0x00000087 @ clear bits 7, 2:0 (B--- -CAM)
> +#if !CONFIG_IS_ENABLED(EFI_LOADER)
> + /* allow unaligned accesses since EFI requires it */
This comment line is only reached without UEFI support. So here you
should explain why we forbid unaligned access without UEFI.
Best regards
Heinrich
> orr r0, r0, #0x00000002 @ set bit 1 (A) Align
> +#endif
> orr r0, r0, #0x00001000 @ set bit 12 (I) I-Cache
> mcr p15, 0, r0, c1, c0, 0
>
> diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S
> index 78a9cc173a39..0c80160702ba 100644
> --- a/arch/arm/cpu/arm1176/start.S
> +++ b/arch/arm/cpu/arm1176/start.S
> @@ -79,7 +79,10 @@ cpu_init_crit:
> mrc p15, 0, r0, c1, c0, 0
> bic r0, r0, #0x00002300 @ clear bits 13, 9:8 (--V- --RS)
> bic r0, r0, #0x00000087 @ clear bits 7, 2:0 (B--- -CAM)
> +#if !CONFIG_IS_ENABLED(EFI_LOADER)
> + /* allow unailgned accesses since EFI requires it */
> orr r0, r0, #0x00000002 @ set bit 1 (A) Align
> +#endif
> orr r0, r0, #0x00001000 @ set bit 12 (I) I-Cache
>
> /* Prepare to disable the MMU */
> diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
> index 653eef8ad79e..ca50f6e93e10 100644
> --- a/arch/arm/cpu/armv7/Makefile
> +++ b/arch/arm/cpu/armv7/Makefile
> @@ -13,7 +13,6 @@ obj-y += syslib.o
> obj-$(CONFIG_SYS_ARM_MPU) += mpu_v7r.o
>
> ifneq ($(CONFIG_SPL_BUILD),y)
> -obj-$(CONFIG_EFI_LOADER) += sctlr.o
> obj-$(CONFIG_ARMV7_NONSEC) += exception_level.o
> endif
>
> diff --git a/arch/arm/cpu/armv7/sctlr.S b/arch/arm/cpu/armv7/sctlr.S
> deleted file mode 100644
> index bd56e41afe18..000000000000
> --- a/arch/arm/cpu/armv7/sctlr.S
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0+ */
> -/*
> - * Routines to access the system control register
> - *
> - * Copyright (c) 2018 Heinrich Schuchardt
> - */
> -
> -#include <linux/linkage.h>
> -
> -/*
> - * void allow_unaligned(void) - allow unaligned access
> - *
> - * This routine clears the aligned flag in the system control register.
> - * After calling this routine unaligned access does no longer lead to a
> - * data abort but is handled by the CPU.
> - */
> -ENTRY(allow_unaligned)
> - mrc p15, 0, r0, c1, c0, 0 @ load system control register
> - bic r0, r0, #2 @ clear aligned flag
> - mcr p15, 0, r0, c1, c0, 0 @ write system control register
> - bx lr @ return
> -ENDPROC(allow_unaligned)
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index 7d7aac021e22..abb89efb9ed5 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -197,7 +197,10 @@ ENTRY(cpu_init_cp15)
> mrc p15, 0, r0, c1, c0, 0
> bic r0, r0, #0x00002000 @ clear bits 13 (--V-)
> bic r0, r0, #0x00000007 @ clear bits 2:0 (-CAM)
> +#if !CONFIG_IS_ENABLED(EFI_LOADER)
> + /* allow unaligned accesses since EFI requires it */
> orr r0, r0, #0x00000002 @ set bit 1 (--A-) Align
> +#endif
> orr r0, r0, #0x00000800 @ set bit 11 (Z---) BTB
> #if CONFIG_IS_ENABLED(SYS_ICACHE_OFF)
> bic r0, r0, #0x00001000 @ clear bit 12 (I) I-cache
> diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h
> index 3d33a5a063e8..cf032ce1d4cc 100644
> --- a/include/asm-generic/unaligned.h
> +++ b/include/asm-generic/unaligned.h
> @@ -19,8 +19,4 @@
> #else
> #error invalid endian
> #endif
> -
> -/* Allow unaligned memory access */
> -void allow_unaligned(void);
> -
> #endif
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index 3b267b713e98..bbaaf626d9cb 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -954,13 +954,6 @@ static void path_to_uefi(void *uefi, const char *src)
> {
> u16 *pos = uefi;
>
> - /*
> - * efi_set_bootdev() calls this routine indirectly before the UEFI
> - * subsystem is initialized. So we cannot assume unaligned access to be
> - * enabled.
> - */
> - allow_unaligned();
> -
> while (*src) {
> s32 code = utf8_get(&src);
>
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 58d4e1340233..bd17db61c697 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -17,15 +17,6 @@
>
> efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED;
>
> -/*
> - * Allow unaligned memory access.
> - *
> - * This routine is overridden by architectures providing this feature.
> - */
> -void __weak allow_unaligned(void)
> -{
> -}
> -
> /**
> * efi_init_platform_lang() - define supported languages
> *
> @@ -191,9 +182,6 @@ int efi_init_early(void)
> {
> efi_status_t ret;
>
> - /* Allow unaligned memory access */
> - allow_unaligned();
> -
> /* Initialize root node */
> ret = efi_root_node_register();
> if (ret != EFI_SUCCESS)
More information about the U-Boot
mailing list