| Message ID | 20241031220605.31684-1-semen.protsenko@linaro.org |
|---|---|
| State | New |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | armv8: cpu: Implement allow_unaligned() | expand |
On 10/31/24 23:06, Sam Protsenko wrote: > Usually ARMv8 platforms allow unaligned access for Normal memory. But > some chips might not allow it by default, having SCTLR.A bit set to 1 > before U-Boot execution. One such example is Exynos850 SoC. As > allow_unaligned() is not implemented for ARMv8 at the moment, its __weak > implementation is used, which does nothing. That might lead to unaligned > access abort, for example when running EFI selftest. Fix that by > implementing allow_unaligned() for ARMv8. > > The issue was found when running EFI selftest on E850-96 board > (Exynos850 based): > > => bootefi selftest $fdtcontroladdr > > ... > Executing 'HII database protocols' > "Synchronous Abort" handler, esr 0x96000021, far 0xbaac0991 > ... > resetting ... > > Unaligned abort happens in u16_strnlen(), which is called from > efi_hii_sibt_string_ucs2_block_next(): > > u16_strlen(blk->string_text) > > where 'blk' type is struct efi_hii_sibt_string_ucs2_block. Because this > struct is packed, doing "->string_text" makes 'blk' address incremented > by 1 byte, which makes it unaligned. Although allow_unaligned() was > called in efi_init_early() before EFI selftest execution, it wasn't > implemented for ARMv8 CPUs, so data abort happened. > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > --- > arch/arm/cpu/armv8/cpu.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/arm/cpu/armv8/cpu.c b/arch/arm/cpu/armv8/cpu.c > index d568efa427ab..82ecf02f4b03 100644 > --- a/arch/arm/cpu/armv8/cpu.c > +++ b/arch/arm/cpu/armv8/cpu.c > @@ -94,3 +94,8 @@ void armv8_setup_psci(void) > secure_ram_addr(psci_arch_init)(); > } > #endif > + > +void allow_unaligned(void) > +{ > + set_sctlr(get_sctlr() & ~CR_A); Looking at the ARM documentation SCTLR_EL2 is a 64bit register. https://developer.arm.com/documentation/ddi0595/2021-06/AArch64-Registers/SCTLR-EL2--System-Control-Register--EL2-?lang=en But get_sctlr() is defined as arch/arm/include/asm/system.h:174: static inline unsigned int get_sctlr(void) Your code might not work as expected. Should we adjust the definition of get_sctlr() to return a long value? Best regards Heinrich > +}
On Fri, Nov 1, 2024 at 6:46 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > [snip] > > +void allow_unaligned(void) > > +{ > > + set_sctlr(get_sctlr() & ~CR_A); > > Looking at the ARM documentation SCTLR_EL2 is a 64bit register. > > https://developer.arm.com/documentation/ddi0595/2021-06/AArch64-Registers/SCTLR-EL2--System-Control-Register--EL2-?lang=en > > But get_sctlr() is defined as > > arch/arm/include/asm/system.h:174: > static inline unsigned int get_sctlr(void) > > Your code might not work as expected. Should we adjust the definition of > get_sctlr() to return a long value? > Hi Heinrich, Good catch! I totally overlooked it. And it's already used in a similar way in a couple of different places. I'll handle that in my v2 soon. Thanks! > Best regards > > Heinrich > > > +} >
diff --git a/arch/arm/cpu/armv8/cpu.c b/arch/arm/cpu/armv8/cpu.c index d568efa427ab..82ecf02f4b03 100644 --- a/arch/arm/cpu/armv8/cpu.c +++ b/arch/arm/cpu/armv8/cpu.c @@ -94,3 +94,8 @@ void armv8_setup_psci(void) secure_ram_addr(psci_arch_init)(); } #endif + +void allow_unaligned(void) +{ + set_sctlr(get_sctlr() & ~CR_A); +}
Usually ARMv8 platforms allow unaligned access for Normal memory. But some chips might not allow it by default, having SCTLR.A bit set to 1 before U-Boot execution. One such example is Exynos850 SoC. As allow_unaligned() is not implemented for ARMv8 at the moment, its __weak implementation is used, which does nothing. That might lead to unaligned access abort, for example when running EFI selftest. Fix that by implementing allow_unaligned() for ARMv8. The issue was found when running EFI selftest on E850-96 board (Exynos850 based): => bootefi selftest $fdtcontroladdr ... Executing 'HII database protocols' "Synchronous Abort" handler, esr 0x96000021, far 0xbaac0991 ... resetting ... Unaligned abort happens in u16_strnlen(), which is called from efi_hii_sibt_string_ucs2_block_next(): u16_strlen(blk->string_text) where 'blk' type is struct efi_hii_sibt_string_ucs2_block. Because this struct is packed, doing "->string_text" makes 'blk' address incremented by 1 byte, which makes it unaligned. Although allow_unaligned() was called in efi_init_early() before EFI selftest execution, it wasn't implemented for ARMv8 CPUs, so data abort happened. Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> --- arch/arm/cpu/armv8/cpu.c | 5 +++++ 1 file changed, 5 insertions(+)