[PATCH 1/2] ARM: armv7: Add C wrapper for allow_unaligned()

Rename current assembler implementation of allow_unaligned() to v7_arch_cp15_allow_unaligned() and add it into armv7.h header, then add C wrapper of allow_unaligned().
This fixes misbehavior when linking U-Boot on ARMv7a i.MX6Q, where the CPU specific allow_unaligned() implementation was ignored and instead the __weak allow_unaligned() implementation from lib/efi_loader/efi_setup.c was used, which led to "data abort" just before booting Linux via tftp, in efi_dp_from_file() -> path_to_uefi() -> utf16_put() .
Adding the wrapper fixes the problem.
Fixes: c7c0ca37673 ("efi_loader: fix efi_dp_from_file()") Signed-off-by: Marek Vasut marex@denx.de --- Cc: Fabio Estevam festevam@denx.de Cc: Heinrich Schuchardt xypron.glpk@gmx.de Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com --- arch/arm/cpu/armv7/cpu.c | 5 +++++ arch/arm/cpu/armv7/sctlr.S | 6 +++--- arch/arm/include/asm/armv7.h | 1 + 3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c index 68807d20997..6259ffa5108 100644 --- a/arch/arm/cpu/armv7/cpu.c +++ b/arch/arm/cpu/armv7/cpu.c @@ -83,3 +83,8 @@ int cleanup_before_linux(void) { return cleanup_before_linux_select(CBL_ALL); } + +void allow_unaligned(void) +{ + v7_arch_cp15_allow_unaligned(); +} diff --git a/arch/arm/cpu/armv7/sctlr.S b/arch/arm/cpu/armv7/sctlr.S index bd56e41afe1..d44b21498f6 100644 --- a/arch/arm/cpu/armv7/sctlr.S +++ b/arch/arm/cpu/armv7/sctlr.S @@ -8,15 +8,15 @@ #include <linux/linkage.h>
/* - * void allow_unaligned(void) - allow unaligned access + * void v7_arch_cp15_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) +ENTRY(v7_arch_cp15_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) +ENDPROC(v7_arch_cp15_allow_unaligned) diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h index 2fb824b69e2..c002998ac0b 100644 --- a/arch/arm/include/asm/armv7.h +++ b/arch/arm/include/asm/armv7.h @@ -156,6 +156,7 @@ void v7_arch_cp15_set_l2aux_ctrl(u32 l2auxctrl, u32 cpu_midr, u32 cpu_rev); void v7_arch_cp15_set_acr(u32 acr, u32 cpu_midr, u32 cpu_rev_comb, u32 cpu_variant, u32 cpu_rev); +void v7_arch_cp15_allow_unaligned(void); #endif /* ! __ASSEMBLY__ */
#endif

Rename current assembler implementation of allow_unaligned() to arm11_arch_cp15_allow_unaligned() and add it into arm11.h header, then add C wrapper of allow_unaligned().
This fixes misbehavior when linking U-Boot, where the CPU specific allow_unaligned() implementation was ignored and instead the __weak allow_unaligned() implementation from lib/efi_loader/efi_setup.c was used, which led to "data abort" just before booting Linux via tftp, in efi_dp_from_file() -> path_to_uefi() -> utf16_put() .
Adding the wrapper fixes the problem.
Fixes: c7c0ca37673 ("efi_loader: fix efi_dp_from_file()") Signed-off-by: Marek Vasut marex@denx.de --- Cc: Fabio Estevam festevam@denx.de Cc: Heinrich Schuchardt xypron.glpk@gmx.de Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com --- arch/arm/cpu/arm11/cpu.c | 6 ++++++ arch/arm/cpu/arm11/sctlr.S | 6 +++--- arch/arm/include/asm/arm11.h | 12 ++++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) create mode 100644 arch/arm/include/asm/arm11.h
diff --git a/arch/arm/cpu/arm11/cpu.c b/arch/arm/cpu/arm11/cpu.c index ffe35111d58..1e16b89d006 100644 --- a/arch/arm/cpu/arm11/cpu.c +++ b/arch/arm/cpu/arm11/cpu.c @@ -20,6 +20,7 @@ #include <irq_func.h> #include <asm/cache.h> #include <asm/system.h> +#include <asm/arm11.h>
static void cache_flush(void);
@@ -43,6 +44,11 @@ int cleanup_before_linux (void) return 0; }
+void allow_unaligned(void) +{ + arm11_arch_cp15_allow_unaligned(); +} + static void cache_flush(void) { unsigned long i = 0; diff --git a/arch/arm/cpu/arm11/sctlr.S b/arch/arm/cpu/arm11/sctlr.S index 74a7fc4a25b..8722f8380c8 100644 --- a/arch/arm/cpu/arm11/sctlr.S +++ b/arch/arm/cpu/arm11/sctlr.S @@ -8,7 +8,7 @@ #include <linux/linkage.h>
/* - * void allow_unaligned(void) - allow unaligned access + * void arm11_arch_cp15_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. @@ -16,10 +16,10 @@ * data abort or undefined behavior but is handled by the CPU. * For details see the "ARM Architecture Reference Manual" for ARMv6. */ -ENTRY(allow_unaligned) +ENTRY(arm11_arch_cp15_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) +ENDPROC(arm11_arch_cp15_allow_unaligned) diff --git a/arch/arm/include/asm/arm11.h b/arch/arm/include/asm/arm11.h new file mode 100644 index 00000000000..5276f735ef9 --- /dev/null +++ b/arch/arm/include/asm/arm11.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (C) 2023 Marek Vasut marex@denx.de + */ +#ifndef ARM11_H +#define ARM11_H + +#ifndef __ASSEMBLY__ +void arm11_arch_cp15_allow_unaligned(void); +#endif /* ! __ASSEMBLY__ */ + +#endif /* ARM11_H */

On 7/1/23 15:00, Marek Vasut wrote:
Rename current assembler implementation of allow_unaligned() to v7_arch_cp15_allow_unaligned() and add it into armv7.h header, then add C wrapper of allow_unaligned().
This fixes misbehavior when linking U-Boot on ARMv7a i.MX6Q, where the CPU specific allow_unaligned() implementation was ignored and instead the __weak allow_unaligned() implementation from lib/efi_loader/efi_setup.c was used, which led to "data abort" just before booting Linux via tftp, in efi_dp_from_file() -> path_to_uefi() -> utf16_put() .
Adding the wrapper fixes the problem.
Fixes: c7c0ca37673 ("efi_loader: fix efi_dp_from_file()")
Hello Marek,
Thank you for investigating this issue.
Looking at https://lore.kernel.org/u-boot/20230320171335.1368308-1-ilias.apalodimas@lin... the problem predates c7c0ca37673. So this fixes line seems wrong.
Your series has a fix for armv7 but misses arm11. So we would be better of with Ilias patch. Somehow Ilias never followed up on my questions and we lost track.
Could you, please, test Ilias patch.
Best regards
Heinrich
Signed-off-by: Marek Vasut marex@denx.de
Cc: Fabio Estevam festevam@denx.de Cc: Heinrich Schuchardt xypron.glpk@gmx.de Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
arch/arm/cpu/armv7/cpu.c | 5 +++++ arch/arm/cpu/armv7/sctlr.S | 6 +++--- arch/arm/include/asm/armv7.h | 1 + 3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c index 68807d20997..6259ffa5108 100644 --- a/arch/arm/cpu/armv7/cpu.c +++ b/arch/arm/cpu/armv7/cpu.c @@ -83,3 +83,8 @@ int cleanup_before_linux(void) { return cleanup_before_linux_select(CBL_ALL); }
+void allow_unaligned(void) +{
- v7_arch_cp15_allow_unaligned();
+} diff --git a/arch/arm/cpu/armv7/sctlr.S b/arch/arm/cpu/armv7/sctlr.S index bd56e41afe1..d44b21498f6 100644 --- a/arch/arm/cpu/armv7/sctlr.S +++ b/arch/arm/cpu/armv7/sctlr.S @@ -8,15 +8,15 @@ #include <linux/linkage.h>
/*
- void allow_unaligned(void) - allow unaligned access
*/
- void v7_arch_cp15_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) +ENTRY(v7_arch_cp15_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) +ENDPROC(v7_arch_cp15_allow_unaligned) diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h index 2fb824b69e2..c002998ac0b 100644 --- a/arch/arm/include/asm/armv7.h +++ b/arch/arm/include/asm/armv7.h @@ -156,6 +156,7 @@ void v7_arch_cp15_set_l2aux_ctrl(u32 l2auxctrl, u32 cpu_midr, u32 cpu_rev); void v7_arch_cp15_set_acr(u32 acr, u32 cpu_midr, u32 cpu_rev_comb, u32 cpu_variant, u32 cpu_rev); +void v7_arch_cp15_allow_unaligned(void); #endif /* ! __ASSEMBLY__ */
#endif

On 7/1/23 16:57, Heinrich Schuchardt wrote:
On 7/1/23 15:00, Marek Vasut wrote:
Rename current assembler implementation of allow_unaligned() to v7_arch_cp15_allow_unaligned() and add it into armv7.h header, then add C wrapper of allow_unaligned().
This fixes misbehavior when linking U-Boot on ARMv7a i.MX6Q, where the CPU specific allow_unaligned() implementation was ignored and instead the __weak allow_unaligned() implementation from lib/efi_loader/efi_setup.c was used, which led to "data abort" just before booting Linux via tftp, in efi_dp_from_file() -> path_to_uefi() -> utf16_put() .
Adding the wrapper fixes the problem.
Fixes: c7c0ca37673 ("efi_loader: fix efi_dp_from_file()")
Hello Marek,
Thank you for investigating this issue.
Looking at https://lore.kernel.org/u-boot/20230320171335.1368308-1-ilias.apalodimas@lin... the problem predates c7c0ca37673. So this fixes line seems wrong.
That commit does trigger the error however . Maybe this was broken ever since allow_unaligned() was introduced ?
Your series has a fix for armv7 but misses arm11.
That's exactly what 2/2 in this series does ?
So we would be better of with Ilias patch. Somehow Ilias never followed up on my questions and we lost track.
Could you, please, test Ilias patch.
That patch corrupts r0 register in both cases in the asm volatile(), since it does not push r0 on stack or use %0 temporary or whatever .
participants (2)
-
Heinrich Schuchardt
-
Marek Vasut