GCC9 now produces the following warning: common.h:51:2: warning: listing the stack pointer register ‘sp’ in a clobber list is deprecated [-Wdeprecated] 51 | __asm__ __volatile__("mov sp, %0" | ^~~~~~~ common.h:51:2: note: the value of the stack pointer after an ‘asm’ statement must be the same as it was before the statement Stack pointer was added to clobber list in commit f9fc8254b2 ("ARM: Mark SP as being clobbered in arm_setup_stack()") to prevent GCC from generating code that would corrupt 'boarddata' pointer by trying to restore it from invalid stack frame. Interestingly enough, seemingly unrelated change in commit 64d95896cf ("ARM: aarch64: compile with general-regs-only") changed generated code such that adding SP to clobber list became no longer necessary. While the above can probably be a fix by itself, it seems a better and more future proof approach would be to address the problem at its root and re-implement offending startup sequence in assembly. Signed-off-by: Andrey Smirnov <andrew.smirnov@xxxxxxxxx> --- Changes since v1: - Switched both ARM and ARM64 to use assembly arch/arm/cpu/Makefile | 4 ++-- arch/arm/cpu/entry.c | 23 +++++++++++++---------- arch/arm/cpu/entry_ll.S | 27 +++++++++++++++++++++++++++ arch/arm/cpu/entry_ll_64.S | 25 +++++++++++++++++++++++++ arch/arm/include/asm/common.h | 3 +-- 5 files changed, 68 insertions(+), 14 deletions(-) create mode 100644 arch/arm/cpu/entry_ll.S create mode 100644 arch/arm/cpu/entry_ll_64.S diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile index 97e4eb52e3..0a0cd91107 100644 --- a/arch/arm/cpu/Makefile +++ b/arch/arm/cpu/Makefile @@ -8,7 +8,7 @@ obj-pbl-$(CONFIG_CPU_32v7) += hyp.o AFLAGS_hyp.o :=-Wa,-march=armv7-a -Wa,-mcpu=all AFLAGS_pbl-hyp.o :=-Wa,-march=armv7-a -Wa,-mcpu=all -obj-y += start.o entry.o +obj-y += start.o entry.o entry_ll$(S64).o pbl-$(CONFIG_BOARD_ARM_GENERIC_DT) += board-dt-2nd.o pbl-$(CONFIG_BOARD_ARM_GENERIC_DT_AARCH64) += board-dt-2nd-aarch64.o @@ -47,7 +47,7 @@ AFLAGS_cache-armv8.o :=-Wa,-march=armv8-a obj-pbl-$(CONFIG_CPU_64v8) += cache-armv8.o AFLAGS_pbl-cache-armv8.o :=-Wa,-march=armv8-a -pbl-y += entry.o +pbl-y += entry.o entry_ll$(S64).o pbl-$(CONFIG_PBL_SINGLE_IMAGE) += start-pbl.o pbl-$(CONFIG_PBL_MULTI_IMAGES) += uncompress.o diff --git a/arch/arm/cpu/entry.c b/arch/arm/cpu/entry.c index 30df95f078..0b447de801 100644 --- a/arch/arm/cpu/entry.c +++ b/arch/arm/cpu/entry.c @@ -24,16 +24,19 @@ * be fine. */ +/* + * It can be hard to convince GCC to not use old stack pointer after + * we modify it with arm_setup_stack() on ARM64, so we implement the + * low level details in assembly + */ +void __noreturn __barebox_arm_entry(unsigned long membase, + unsigned long memsize, + void *boarddata, + unsigned long sp); + void NAKED __noreturn barebox_arm_entry(unsigned long membase, - unsigned long memsize, void *boarddata) + unsigned long memsize, void *boarddata) { - arm_setup_stack(arm_mem_stack_top(membase, membase + memsize)); - arm_early_mmu_cache_invalidate(); - - if (IS_ENABLED(CONFIG_PBL_MULTI_IMAGES)) - barebox_multi_pbl_start(membase, memsize, boarddata); - else if (IS_ENABLED(CONFIG_PBL_SINGLE_IMAGE)) - barebox_single_pbl_start(membase, memsize, boarddata); - else - barebox_non_pbl_start(membase, memsize, boarddata); + __barebox_arm_entry(membase, memsize, boarddata, + arm_mem_stack_top(membase, membase + memsize)); } diff --git a/arch/arm/cpu/entry_ll.S b/arch/arm/cpu/entry_ll.S new file mode 100644 index 0000000000..ad9c703cd7 --- /dev/null +++ b/arch/arm/cpu/entry_ll.S @@ -0,0 +1,27 @@ +#include <linux/linkage.h> +#include <asm/sections.h> + +/* + * r0: memory base + * r1: memory size + * r2: board data + * r3: new value for SP + */ +.section .text.__barebox_arm_entry +ENTRY(__barebox_arm_entry) + mov sp, r3 + mov r4, r0 + mov r5, r1 + mov r6, r2 + bl arm_early_mmu_cache_invalidate + mov r0, r4 + mov r1, r5 + mov r2, r6 +#if IS_ENABLED(CONFIG_PBL_MULTI_IMAGES) + b barebox_multi_pbl_start +#elif IS_ENABLED(CONFIG_PBL_SINGLE_IMAGE) + b barebox_single_pbl_start +#else + b barebox_non_pbl_start +#endif +ENDPROC(__barebox_arm_entry) diff --git a/arch/arm/cpu/entry_ll_64.S b/arch/arm/cpu/entry_ll_64.S new file mode 100644 index 0000000000..c829bd6efd --- /dev/null +++ b/arch/arm/cpu/entry_ll_64.S @@ -0,0 +1,25 @@ +#include <linux/linkage.h> +#include <asm/sections.h> + +/* + * x0: memory base + * x1: memory size + * x2: board data + * x3: new value for SP + */ +.section .text.__barebox_arm_entry +ENTRY(__barebox_arm_entry) + mov sp, x3 + /* + * arm_early_mmu_cache_invalidate is jsut a call to + * v8_invalidate_icache_all() which doesn't clobber x0, x1 or x2 + */ + bl arm_early_mmu_cache_invalidate +#if IS_ENABLED(CONFIG_PBL_MULTI_IMAGES) + b barebox_multi_pbl_start +#elif IS_ENABLED(CONFIG_PBL_SINGLE_IMAGE) + b barebox_single_pbl_start +#else + b barebox_non_pbl_start +#endif +ENDPROC(__barebox_arm_entry) \ No newline at end of file diff --git a/arch/arm/include/asm/common.h b/arch/arm/include/asm/common.h index c32cdfe5ec..d03ee6273f 100644 --- a/arch/arm/include/asm/common.h +++ b/arch/arm/include/asm/common.h @@ -50,8 +50,7 @@ static inline void arm_setup_stack(unsigned long top) { __asm__ __volatile__("mov sp, %0" : - : "r"(top) - : "sp"); + : "r"(top)); } #endif /* __ASM_ARM_COMMON_H */ -- 2.21.0 _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox