* [PATCH 1/8] ARM64: cpu: select HAVE_PBL_MULTI_IMAGES globally
2022-10-24 6:57 [PATCH 0/8] ARM64: rewrite ENTRY_FUNCTION_WITHSTACK in assembly Ahmad Fatoum
@ 2022-10-24 6:57 ` Ahmad Fatoum
2022-10-24 6:57 ` [PATCH 2/8] ARM64: asm: define ENTRY_FUNCTION in terms of ENTRY_FUNCTION_WITHSTACK Ahmad Fatoum
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-10-24 6:57 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
Incoming changes to the macro definition of ENTRY_FUNCTION will result
in removal of barebox_arm_head for ARM64. Besides ENTRY_FUNCTION itself,
only remaining user would be the non-PBL and single-PBL case. We already
mandate PBL for all ARM64 boards, so mandate them to be multi-image
capable as well.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/cpu/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/cpu/Kconfig b/arch/arm/cpu/Kconfig
index 9b5a833abfff..3ef360473765 100644
--- a/arch/arm/cpu/Kconfig
+++ b/arch/arm/cpu/Kconfig
@@ -16,6 +16,7 @@ config CPU_64
bool
select PHYS_ADDR_T_64BIT
select HAVE_PBL_IMAGE
+ select HAVE_PBL_MULTI_IMAGES
select HAS_DMA
# Select CPU types depending on the architecture selected. This selects
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/8] ARM64: asm: define ENTRY_FUNCTION in terms of ENTRY_FUNCTION_WITHSTACK
2022-10-24 6:57 [PATCH 0/8] ARM64: rewrite ENTRY_FUNCTION_WITHSTACK in assembly Ahmad Fatoum
2022-10-24 6:57 ` [PATCH 1/8] ARM64: cpu: select HAVE_PBL_MULTI_IMAGES globally Ahmad Fatoum
@ 2022-10-24 6:57 ` Ahmad Fatoum
2022-10-24 6:57 ` [PATCH 3/8] pbl: have linker define __pbl_board_entry alias Ahmad Fatoum
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-10-24 6:57 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
We can't do the same for arm32, as the entry point there must be naked
for proper operation, but for ARM64, ENTRY_FUNCTION(name, ...) is
already equivalent to ENTRY_FUNCTION_WITHSTACK(name, 0, ...), so
consolidate them.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/include/asm/barebox-arm.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index a34f77f2abf3..47d20b6b04ea 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -180,6 +180,10 @@ static inline unsigned long arm_mem_barebox_image(unsigned long membase,
} \
static void noinline __##name \
(ulong arg0, ulong arg1, ulong arg2)
+
+#define ENTRY_FUNCTION(name, arg0, arg1, arg2) \
+ ENTRY_FUNCTION_WITHSTACK(name, 0, arg0, arg1, arg2)
+
#else
#define ENTRY_FUNCTION_WITHSTACK(name, stack_top, arg0, arg1, arg2) \
static void ____##name(ulong, ulong, ulong); \
@@ -190,8 +194,6 @@ static inline unsigned long arm_mem_barebox_image(unsigned long membase,
} \
static void noinline ____##name \
(ulong arg0, ulong arg1, ulong arg2)
-#endif
-
#define ENTRY_FUNCTION(name, arg0, arg1, arg2) \
void name(ulong r0, ulong r1, ulong r2); \
@@ -207,6 +209,7 @@ static inline unsigned long arm_mem_barebox_image(unsigned long membase,
} \
static void NAKED noinline __##name \
(ulong arg0, ulong arg1, ulong arg2)
+#endif
/*
* When using compressed images in conjunction with relocatable images
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/8] pbl: have linker define __pbl_board_entry alias
2022-10-24 6:57 [PATCH 0/8] ARM64: rewrite ENTRY_FUNCTION_WITHSTACK in assembly Ahmad Fatoum
2022-10-24 6:57 ` [PATCH 1/8] ARM64: cpu: select HAVE_PBL_MULTI_IMAGES globally Ahmad Fatoum
2022-10-24 6:57 ` [PATCH 2/8] ARM64: asm: define ENTRY_FUNCTION in terms of ENTRY_FUNCTION_WITHSTACK Ahmad Fatoum
@ 2022-10-24 6:57 ` Ahmad Fatoum
2022-10-24 6:57 ` [PATCH 4/8] asm-generic: memory_layout: define __keep_symbolref() Ahmad Fatoum
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-10-24 6:57 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
We want the board-specific entry points to have control over what code
comes first. So far, we solved this by setting the entry point on the
linker command line and doing the early setup in inline assembly.
As __attribute__((naked)) is not supported for arm64, we'll move there
to out-of-line assembly and thus we'll need a way to portably reference
the board-specific entry point. Define a new alias that points at it.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
images/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/images/Makefile b/images/Makefile
index 218a24ff1ddd..aa5814710f28 100644
--- a/images/Makefile
+++ b/images/Makefile
@@ -59,6 +59,7 @@ $(pbl-lds): $(obj)/../arch/$(SRCARCH)/lib/pbl.lds.S FORCE
quiet_cmd_elf__ ?= LD $@
cmd_elf__ ?= $(LD) $(LDFLAGS_pbl) --gc-sections \
-e $(2) -Map $@.map $(LDFLAGS_$(@F)) -o $@ \
+ --defsym=__pbl_board_entry=$(2) \
-T $(pbl-lds) \
--whole-archive $(BAREBOX_PBL_OBJS) $(obj)/piggy.o \
$(obj)/sha_sum.o
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/8] asm-generic: memory_layout: define __keep_symbolref()
2022-10-24 6:57 [PATCH 0/8] ARM64: rewrite ENTRY_FUNCTION_WITHSTACK in assembly Ahmad Fatoum
` (2 preceding siblings ...)
2022-10-24 6:57 ` [PATCH 3/8] pbl: have linker define __pbl_board_entry alias Ahmad Fatoum
@ 2022-10-24 6:57 ` Ahmad Fatoum
2022-10-24 6:57 ` [PATCH 5/8] ARM64: asm: rewrite ENTRY_FUNCTION(_WITHSTACK) fully in assembly Ahmad Fatoum
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-10-24 6:57 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
When multiple PBL entry points are built as part of a multi-image build,
the link process will be conducted multiple times, each with a different
entry point and then linker garbage collection will take care to remove
any unreferenced sections. Sometimes, we want to keep sections around,
even if they are unreferenced in code, because the linker will place
them at a location, where they can fulfill their purpose without being
referenced explicitly.
Examples are the barebox IMD entries, which attach image meta-data to
barebox or the stack setup prologue which is inserted at the start of
the image to work around lack of __attribute__((naked)) support on
ARM64.
So far, these sections were kept around by either a call to an external
function, which worked because we don't employ LTO or via
barrier_data(ptr), which in its <linux/compiler-gcc.h> implementation
generates a load from the supplied ptr. Improve readability, by defining
a new __keep_symbolref() that describes what it is supposed to do and
does just that: Keep a symbol reference alive as long as the code
surrounding it is not eliminated as dead code.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/include/asm/barebox-arm.h | 2 +-
include/asm-generic/memory_layout.h | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index 47d20b6b04ea..68a9610398cc 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -141,7 +141,7 @@ static inline unsigned long arm_mem_barebox_image(unsigned long membase,
#define ____emit_entry_prologue(name, instr, ...) do { \
static __attribute__ ((unused,section(".text_head_prologue_" __stringify(name)))) \
const u32 __entry_prologue[] = {(instr), ##__VA_ARGS__}; \
- barrier_data(__entry_prologue); \
+ __keep_symbolref(__entry_prologue); \
} while(0)
#define __emit_entry_prologue(name, instr1, instr2, instr3, instr4, instr5) \
diff --git a/include/asm-generic/memory_layout.h b/include/asm-generic/memory_layout.h
index 5cfd2a43a056..7593e18da151 100644
--- a/include/asm-generic/memory_layout.h
+++ b/include/asm-generic/memory_layout.h
@@ -23,4 +23,11 @@
#define MALLOC_SIZE CONFIG_MALLOC_SIZE
#define STACK_SIZE CONFIG_STACK_SIZE
+/*
+ * This generates a useless load from the specified symbol
+ * to ensure linker garbage collection doesn't delete it
+ */
+#define __keep_symbolref(sym) \
+ __asm__ __volatile__("": :"r"(&sym) :)
+
#endif /* __ASM_GENERIC_MEMORY_LAYOUT_H */
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/8] ARM64: asm: rewrite ENTRY_FUNCTION(_WITHSTACK) fully in assembly
2022-10-24 6:57 [PATCH 0/8] ARM64: rewrite ENTRY_FUNCTION_WITHSTACK in assembly Ahmad Fatoum
` (3 preceding siblings ...)
2022-10-24 6:57 ` [PATCH 4/8] asm-generic: memory_layout: define __keep_symbolref() Ahmad Fatoum
@ 2022-10-24 6:57 ` Ahmad Fatoum
2022-10-24 8:55 ` Sascha Hauer
2022-10-24 12:11 ` [PATCH] fixup! " Ahmad Fatoum
2022-10-24 6:57 ` [PATCH 6/8] ARM64: asm: drop __barebox_arm_head Ahmad Fatoum
` (2 subsequent siblings)
7 siblings, 2 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-10-24 6:57 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
Recent episode with pointer authentication showed again that for
platforms without __attribute__((naked)), we are better off writing
the early header in assembly. We still want to keep the board specific
entry points in C for ease of use, so we have ENTRY_FUNCTION_WITHSTACK
generate two symbols:
- A 32-bit stack top value that's placed in .rodata
- An entry point with the normal C code, including stack-using
prologues
The new common assembly head code will access the stack pointer in a
position-independent manner and set it up, before continuing with the
C code. The barebox header is part of the common assembly head code
ensuring it's not moved around due to compiler code generation.
The common code will need access to board-specific entry point and stack
top. The former is readily available as the alias __pbl_board_entry.
The latter is a bit more complicated, as the symbol may not exist for
boards not using the common header in a multi-image build.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Makefile | 3 +--
arch/arm/cpu/Makefile | 2 ++
arch/arm/cpu/head_64.S | 36 ++++++++++++++++++++++++++++++
arch/arm/include/asm/barebox-arm.h | 28 +++++++----------------
arch/arm/lib/pbl.lds.S | 9 ++++++++
5 files changed, 56 insertions(+), 22 deletions(-)
create mode 100644 arch/arm/cpu/head_64.S
diff --git a/Makefile b/Makefile
index 81ef44122367..a4b9f3c021d7 100644
--- a/Makefile
+++ b/Makefile
@@ -660,8 +660,7 @@ KBUILD_CFLAGS += $(call cc-option,-fno-stack-check)
KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none)
# We don't have the necessary infrastructure to benefit from ARMv8.3+ pointer
-# authentication. On older CPUs, they are interpreted as NOPs and blot the
-# code and break less portable code that expects a very specific code layout
+# authentication. On older CPUs, they are interpreted as NOPs bloating the code
KBUILD_CFLAGS += $(call cc-option,-mbranch-protection=none)
KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
index c0993c1abe5d..7674c1464c1f 100644
--- a/arch/arm/cpu/Makefile
+++ b/arch/arm/cpu/Makefile
@@ -13,6 +13,8 @@ AFLAGS_hyp.pbl.o :=-Wa,-march=armv7-a -Wa,-mcpu=all
obj-y += start.o entry.o entry_ll$(S64).o
KASAN_SANITIZE_start.o := n
+pbl-$(CONFIG_CPU_64) += head_64.o
+
pbl-$(CONFIG_BOARD_ARM_GENERIC_DT) += board-dt-2nd.o
pbl-$(CONFIG_BOARD_ARM_GENERIC_DT_AARCH64) += board-dt-2nd-aarch64.o
diff --git a/arch/arm/cpu/head_64.S b/arch/arm/cpu/head_64.S
new file mode 100644
index 000000000000..4ed4ffb05250
--- /dev/null
+++ b/arch/arm/cpu/head_64.S
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include <linux/linkage.h>
+#include <asm/barebox-arm64.h>
+#include <asm/image.h>
+
+/* Linker will point these at board-specific symbols */
+.globl __pbl_board_stack_top
+.globl __pbl_board_entry
+
+.section .text_head_prologue_common, "x"
+ENTRY(__barebox_arm64_head)
+ nop
+ nop
+ nop
+ adr x9, __pbl_board_stack_top
+ ldr w9, [x9]
+ cbz x9, 1f
+ mov sp, x9
+ b 1f
+ .org 0x20
+ .asciz "barebox"
+ .word 0xffffffff
+ .word _barebox_image_size /* image size to copy */
+ .rept 8
+ .word 0x55555555
+ .endr
+1:
+#ifdef CONFIG_PBL_BREAK
+ brk #17
+ nop
+#else
+ nop
+ nop
+#endif
+ b __pbl_board_entry
+END(__barebox_arm64_head)
diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index 68a9610398cc..89b4a89755a6 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -136,24 +136,7 @@ static inline unsigned long arm_mem_barebox_image(unsigned long membase,
}
}
-#ifdef CONFIG_CPU_64
-
-#define ____emit_entry_prologue(name, instr, ...) do { \
- static __attribute__ ((unused,section(".text_head_prologue_" __stringify(name)))) \
- const u32 __entry_prologue[] = {(instr), ##__VA_ARGS__}; \
- __keep_symbolref(__entry_prologue); \
-} while(0)
-
-#define __emit_entry_prologue(name, instr1, instr2, instr3, instr4, instr5) \
- ____emit_entry_prologue(name, instr1, instr2, instr3, instr4, instr5)
-
-#define __ARM_SETUP_STACK(name, stack_top) \
- __emit_entry_prologue(name, 0x14000002 /* b pc+0x8 */, \
- stack_top /* 32-bit literal */, \
- 0x18ffffe9 /* ldr w9, top */, \
- 0xb4000049 /* cbz x9, pc+0x8 */, \
- 0x9100013f /* mov sp, x9 */)
-#else
+#ifndef CONFIG_CPU_64
#define __ARM_SETUP_STACK(name, stack_top) if (stack_top) arm_setup_stack(stack_top)
#endif
@@ -166,6 +149,9 @@ static inline unsigned long arm_mem_barebox_image(unsigned long membase,
* code block will not be inlined and may spill to stack right away.
*/
#ifdef CONFIG_CPU_64
+
+void __barebox_arm64_head(ulong x0, ulong x1, ulong x2);
+
#define ENTRY_FUNCTION_WITHSTACK(name, stack_top, arg0, arg1, arg2) \
void name(ulong r0, ulong r1, ulong r2); \
\
@@ -174,8 +160,10 @@ static inline unsigned long arm_mem_barebox_image(unsigned long membase,
void __section(.text_head_entry_##name) name \
(ulong r0, ulong r1, ulong r2) \
{ \
- __barebox_arm_head(); \
- __ARM_SETUP_STACK(name, stack_top); \
+ static __section(.pbl_board_stack_top_##name) \
+ const u32 __stack_top = (stack_top); \
+ __keep_symbolref(__barebox_arm64_head); \
+ __keep_symbolref(__stack_top); \
__##name(r0, r1, r2); \
} \
static void noinline __##name \
diff --git a/arch/arm/lib/pbl.lds.S b/arch/arm/lib/pbl.lds.S
index ae1babdcfd27..114ec7bc8195 100644
--- a/arch/arm/lib/pbl.lds.S
+++ b/arch/arm/lib/pbl.lds.S
@@ -44,6 +44,15 @@ SECTIONS
. = ALIGN(4);
.rodata : { *(.rodata*) }
+ . = ALIGN(4);
+ __pbl_board_stack_top = .;
+ .rodata.pbl_board_stack_top : {
+ *(.pbl_board_stack_top_*)
+ /* Dummy for when BootROM sets up usable stack */
+ LONG(0x00000000);
+ }
+ ASSERT(. - __pbl_board_stack_top <= 8, "Only One PBL per Image allowed")
+
.barebox_imd : { BAREBOX_IMD }
_etext = .; /* End of text and rodata section */
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 5/8] ARM64: asm: rewrite ENTRY_FUNCTION(_WITHSTACK) fully in assembly
2022-10-24 6:57 ` [PATCH 5/8] ARM64: asm: rewrite ENTRY_FUNCTION(_WITHSTACK) fully in assembly Ahmad Fatoum
@ 2022-10-24 8:55 ` Sascha Hauer
2022-10-24 10:02 ` Ahmad Fatoum
2022-10-24 12:11 ` [PATCH] fixup! " Ahmad Fatoum
1 sibling, 1 reply; 12+ messages in thread
From: Sascha Hauer @ 2022-10-24 8:55 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox
On Mon, Oct 24, 2022 at 08:57:13AM +0200, Ahmad Fatoum wrote:
> Recent episode with pointer authentication showed again that for
> platforms without __attribute__((naked)), we are better off writing
> the early header in assembly. We still want to keep the board specific
> entry points in C for ease of use, so we have ENTRY_FUNCTION_WITHSTACK
> generate two symbols:
>
> - A 32-bit stack top value that's placed in .rodata
>
> - An entry point with the normal C code, including stack-using
> prologues
>
> The new common assembly head code will access the stack pointer in a
> position-independent manner and set it up, before continuing with the
> C code. The barebox header is part of the common assembly head code
> ensuring it's not moved around due to compiler code generation.
>
> The common code will need access to board-specific entry point and stack
> top. The former is readily available as the alias __pbl_board_entry.
> The latter is a bit more complicated, as the symbol may not exist for
> boards not using the common header in a multi-image build.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> Makefile | 3 +--
> arch/arm/cpu/Makefile | 2 ++
> arch/arm/cpu/head_64.S | 36 ++++++++++++++++++++++++++++++
> arch/arm/include/asm/barebox-arm.h | 28 +++++++----------------
> arch/arm/lib/pbl.lds.S | 9 ++++++++
> 5 files changed, 56 insertions(+), 22 deletions(-)
> create mode 100644 arch/arm/cpu/head_64.S
>
> diff --git a/Makefile b/Makefile
> index 81ef44122367..a4b9f3c021d7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -660,8 +660,7 @@ KBUILD_CFLAGS += $(call cc-option,-fno-stack-check)
> KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none)
>
> # We don't have the necessary infrastructure to benefit from ARMv8.3+ pointer
> -# authentication. On older CPUs, they are interpreted as NOPs and blot the
> -# code and break less portable code that expects a very specific code layout
> +# authentication. On older CPUs, they are interpreted as NOPs bloating the code
> KBUILD_CFLAGS += $(call cc-option,-mbranch-protection=none)
>
> KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
> diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
> index c0993c1abe5d..7674c1464c1f 100644
> --- a/arch/arm/cpu/Makefile
> +++ b/arch/arm/cpu/Makefile
> @@ -13,6 +13,8 @@ AFLAGS_hyp.pbl.o :=-Wa,-march=armv7-a -Wa,-mcpu=all
> obj-y += start.o entry.o entry_ll$(S64).o
> KASAN_SANITIZE_start.o := n
>
> +pbl-$(CONFIG_CPU_64) += head_64.o
> +
> pbl-$(CONFIG_BOARD_ARM_GENERIC_DT) += board-dt-2nd.o
> pbl-$(CONFIG_BOARD_ARM_GENERIC_DT_AARCH64) += board-dt-2nd-aarch64.o
>
> diff --git a/arch/arm/cpu/head_64.S b/arch/arm/cpu/head_64.S
> new file mode 100644
> index 000000000000..4ed4ffb05250
> --- /dev/null
> +++ b/arch/arm/cpu/head_64.S
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#include <linux/linkage.h>
> +#include <asm/barebox-arm64.h>
> +#include <asm/image.h>
> +
> +/* Linker will point these at board-specific symbols */
> +.globl __pbl_board_stack_top
> +.globl __pbl_board_entry
> +
> +.section .text_head_prologue_common, "x"
> +ENTRY(__barebox_arm64_head)
> + nop
> + nop
> + nop
Why these nops here?
> + adr x9, __pbl_board_stack_top
> + ldr w9, [x9]
> + cbz x9, 1f
> + mov sp, x9
> + b 1f
> + .org 0x20
> + .asciz "barebox"
> + .word 0xffffffff
> + .word _barebox_image_size /* image size to copy */
> + .rept 8
> + .word 0x55555555
> + .endr
> +1:
> +#ifdef CONFIG_PBL_BREAK
> + brk #17
> + nop
> +#else
> + nop
> + nop
> +#endif
Why do you jump over the barebox header? You could put all the code
before it.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 5/8] ARM64: asm: rewrite ENTRY_FUNCTION(_WITHSTACK) fully in assembly
2022-10-24 8:55 ` Sascha Hauer
@ 2022-10-24 10:02 ` Ahmad Fatoum
0 siblings, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-10-24 10:02 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
Hello Sascha,
On 24.10.22 10:55, Sascha Hauer wrote:
> On Mon, Oct 24, 2022 at 08:57:13AM +0200, Ahmad Fatoum wrote:
>> Recent episode with pointer authentication showed again that for
>> platforms without __attribute__((naked)), we are better off writing
>> the early header in assembly. We still want to keep the board specific
>> entry points in C for ease of use, so we have ENTRY_FUNCTION_WITHSTACK
>> generate two symbols:
>>
>> - A 32-bit stack top value that's placed in .rodata
>>
>> - An entry point with the normal C code, including stack-using
>> prologues
>>
>> The new common assembly head code will access the stack pointer in a
>> position-independent manner and set it up, before continuing with the
>> C code. The barebox header is part of the common assembly head code
>> ensuring it's not moved around due to compiler code generation.
>>
>> The common code will need access to board-specific entry point and stack
>> top. The former is readily available as the alias __pbl_board_entry.
>> The latter is a bit more complicated, as the symbol may not exist for
>> boards not using the common header in a multi-image build.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> Makefile | 3 +--
>> arch/arm/cpu/Makefile | 2 ++
>> arch/arm/cpu/head_64.S | 36 ++++++++++++++++++++++++++++++
>> arch/arm/include/asm/barebox-arm.h | 28 +++++++----------------
>> arch/arm/lib/pbl.lds.S | 9 ++++++++
>> 5 files changed, 56 insertions(+), 22 deletions(-)
>> create mode 100644 arch/arm/cpu/head_64.S
>>
>> diff --git a/Makefile b/Makefile
>> index 81ef44122367..a4b9f3c021d7 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -660,8 +660,7 @@ KBUILD_CFLAGS += $(call cc-option,-fno-stack-check)
>> KBUILD_CFLAGS += $(call cc-option,-fcf-protection=none)
>>
>> # We don't have the necessary infrastructure to benefit from ARMv8.3+ pointer
>> -# authentication. On older CPUs, they are interpreted as NOPs and blot the
>> -# code and break less portable code that expects a very specific code layout
>> +# authentication. On older CPUs, they are interpreted as NOPs bloating the code
>> KBUILD_CFLAGS += $(call cc-option,-mbranch-protection=none)
>>
>> KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
>> diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
>> index c0993c1abe5d..7674c1464c1f 100644
>> --- a/arch/arm/cpu/Makefile
>> +++ b/arch/arm/cpu/Makefile
>> @@ -13,6 +13,8 @@ AFLAGS_hyp.pbl.o :=-Wa,-march=armv7-a -Wa,-mcpu=all
>> obj-y += start.o entry.o entry_ll$(S64).o
>> KASAN_SANITIZE_start.o := n
>>
>> +pbl-$(CONFIG_CPU_64) += head_64.o
>> +
>> pbl-$(CONFIG_BOARD_ARM_GENERIC_DT) += board-dt-2nd.o
>> pbl-$(CONFIG_BOARD_ARM_GENERIC_DT_AARCH64) += board-dt-2nd-aarch64.o
>>
>> diff --git a/arch/arm/cpu/head_64.S b/arch/arm/cpu/head_64.S
>> new file mode 100644
>> index 000000000000..4ed4ffb05250
>> --- /dev/null
>> +++ b/arch/arm/cpu/head_64.S
>> @@ -0,0 +1,36 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +#include <linux/linkage.h>
>> +#include <asm/barebox-arm64.h>
>> +#include <asm/image.h>
>> +
>> +/* Linker will point these at board-specific symbols */
>> +.globl __pbl_board_stack_top
>> +.globl __pbl_board_entry
>> +
>> +.section .text_head_prologue_common, "x"
>> +ENTRY(__barebox_arm64_head)
>> + nop
>> + nop
>> + nop
>
> Why these nops here?
Perhaps we decide to place something there in future.
For example, `ccmp x18, #0, #0xd, pl` is a fancy nop
whose first 2 bytes are the ELF/PE magic 'MZ' when read in
little-endian.
For now, I left it as normal nops. The .org 20 below is
not (only) for alignment, but as a sanity check, if we
add too much code here.
>> + adr x9, __pbl_board_stack_top
>> + ldr w9, [x9]
>> + cbz x9, 1f
>> + mov sp, x9
>> + b 1f
>> + .org 0x20
>> + .asciz "barebox"
>> + .word 0xffffffff
>> + .word _barebox_image_size /* image size to copy */
>> + .rept 8
>> + .word 0x55555555
>> + .endr
>> +1:
>> +#ifdef CONFIG_PBL_BREAK
>> + brk #17
>> + nop
>> +#else
>> + nop
>> + nop
>> +#endif
>
> Why do you jump over the barebox header? You could put all the code
> before it.
I want to keep at the same location as ARM32.
Cheers,
Ahmad
>
> Sascha
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] fixup! ARM64: asm: rewrite ENTRY_FUNCTION(_WITHSTACK) fully in assembly
2022-10-24 6:57 ` [PATCH 5/8] ARM64: asm: rewrite ENTRY_FUNCTION(_WITHSTACK) fully in assembly Ahmad Fatoum
2022-10-24 8:55 ` Sascha Hauer
@ 2022-10-24 12:11 ` Ahmad Fatoum
1 sibling, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-10-24 12:11 UTC (permalink / raw)
To: barebox; +Cc: sha, Ahmad Fatoum
We have enough space before the barebox header to set up stack, insert
the optional break point and branch to the board-specific entry point.
Do that instead of jumping over the header and continuing directly
afterwards.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/cpu/head_64.S | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/arch/arm/cpu/head_64.S b/arch/arm/cpu/head_64.S
index 4ed4ffb05250..f934e96c6eaf 100644
--- a/arch/arm/cpu/head_64.S
+++ b/arch/arm/cpu/head_64.S
@@ -9,21 +9,11 @@
.section .text_head_prologue_common, "x"
ENTRY(__barebox_arm64_head)
- nop
- nop
nop
adr x9, __pbl_board_stack_top
ldr w9, [x9]
cbz x9, 1f
mov sp, x9
- b 1f
- .org 0x20
- .asciz "barebox"
- .word 0xffffffff
- .word _barebox_image_size /* image size to copy */
- .rept 8
- .word 0x55555555
- .endr
1:
#ifdef CONFIG_PBL_BREAK
brk #17
@@ -33,4 +23,11 @@ ENTRY(__barebox_arm64_head)
nop
#endif
b __pbl_board_entry
+ .org 0x20
+ .asciz "barebox"
+ .word 0xffffffff
+ .word _barebox_image_size /* image size to copy */
+ .rept 8
+ .word 0x55555555
+ .endr
END(__barebox_arm64_head)
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 6/8] ARM64: asm: drop __barebox_arm_head
2022-10-24 6:57 [PATCH 0/8] ARM64: rewrite ENTRY_FUNCTION_WITHSTACK in assembly Ahmad Fatoum
` (4 preceding siblings ...)
2022-10-24 6:57 ` [PATCH 5/8] ARM64: asm: rewrite ENTRY_FUNCTION(_WITHSTACK) fully in assembly Ahmad Fatoum
@ 2022-10-24 6:57 ` Ahmad Fatoum
2022-10-24 6:57 ` [PATCH 7/8] ARM: asm: cleanup 32-bit entry points Ahmad Fatoum
2022-10-24 6:57 ` [PATCH 8/8] Documentation: devel: porting: bring it up-to-date Ahmad Fatoum
7 siblings, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-10-24 6:57 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
Now that we have the common PBL head written in assembly, the 64-bit
inline assembly version remains unused. Drop it.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/include/asm/barebox-arm-head.h | 17 ++---------------
.../mach-mvebu/include/mach/barebox-arm-head.h | 17 -----------------
2 files changed, 2 insertions(+), 32 deletions(-)
diff --git a/arch/arm/include/asm/barebox-arm-head.h b/arch/arm/include/asm/barebox-arm-head.h
index 099e1bef3cb8..1a1d58c1aa35 100644
--- a/arch/arm/include/asm/barebox-arm-head.h
+++ b/arch/arm/include/asm/barebox-arm-head.h
@@ -18,13 +18,13 @@ void barebox_arm_reset_vector(uint32_t r0, uint32_t r1, uint32_t r2);
#define ARM_HEAD_SPARE_OFS 0x30
#define ARM_HEAD_SPARE_MARKER 0x55555555
+#ifdef CONFIG_CPU_32
#ifdef CONFIG_HAVE_MACH_ARM_HEAD
#include <mach/barebox-arm-head.h>
#else
static inline void __barebox_arm_head(void)
{
__asm__ __volatile__ (
-#ifdef CONFIG_CPU_32
#ifdef CONFIG_THUMB2_BAREBOX
".arm\n"
"adr r9, 1f + 1\n"
@@ -44,32 +44,18 @@ static inline void __barebox_arm_head(void)
"1: b 1b\n"
"1: b 1b\n"
"1: b 1b\n"
-#endif
-#else
- /* 5 instructions added by ENTRY_FUNCTION */
- /* two instruction long function prologue */
- /* only use if stack is initialized! */
- "b 2f\n"
#endif
".asciz \"barebox\"\n"
-#ifdef CONFIG_CPU_32
".word _text\n" /* text base. If copied there,
* barebox can skip relocation
*/
-#else
- ".word 0xffffffff\n"
-#endif
".word _barebox_image_size\n" /* image size to copy */
".rept 8\n"
".word 0x55555555\n"
".endr\n"
"2:\n"
#ifdef CONFIG_PBL_BREAK
-#ifdef CONFIG_CPU_V8
- "brk #17\n"
-#else
"bkpt #17\n"
-#endif
"nop\n"
#else
"nop\n"
@@ -85,6 +71,7 @@ static inline void barebox_arm_head(void)
);
}
#endif
+#endif
#endif /* __ASSEMBLY__ */
diff --git a/arch/arm/mach-mvebu/include/mach/barebox-arm-head.h b/arch/arm/mach-mvebu/include/mach/barebox-arm-head.h
index 12f8cfc5a029..2ef3377402f1 100644
--- a/arch/arm/mach-mvebu/include/mach/barebox-arm-head.h
+++ b/arch/arm/mach-mvebu/include/mach/barebox-arm-head.h
@@ -6,7 +6,6 @@
static inline void __barebox_arm_head(void)
{
__asm__ __volatile__ (
-#ifdef CONFIG_CPU_32
#ifdef CONFIG_THUMB2_BAREBOX
".arm\n"
"adr r9, 1f + 1\n"
@@ -26,23 +25,11 @@ static inline void __barebox_arm_head(void)
"1: b 1b\n"
"1: b 1b\n"
"1: b 1b\n"
-#endif
-#else
- "b 2f\n"
- "nop\n"
- "nop\n"
- "nop\n"
- "nop\n"
- "nop\n"
#endif
".asciz \"barebox\"\n"
-#ifdef CONFIG_CPU_32
".word _text\n" /* text base. If copied there,
* barebox can skip relocation
*/
-#else
- ".word 0xffffffff\n"
-#endif
".word _barebox_image_size\n" /* image size to copy */
/*
@@ -58,11 +45,7 @@ static inline void __barebox_arm_head(void)
".endr\n"
"2:\n"
#ifdef CONFIG_PBL_BREAK
-#ifdef CONFIG_CPU_V8
- "brk #17\n"
-#else
"bkpt #17\n"
-#endif
"nop\n"
#else
"nop\n"
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 7/8] ARM: asm: cleanup 32-bit entry points
2022-10-24 6:57 [PATCH 0/8] ARM64: rewrite ENTRY_FUNCTION_WITHSTACK in assembly Ahmad Fatoum
` (5 preceding siblings ...)
2022-10-24 6:57 ` [PATCH 6/8] ARM64: asm: drop __barebox_arm_head Ahmad Fatoum
@ 2022-10-24 6:57 ` Ahmad Fatoum
2022-10-24 6:57 ` [PATCH 8/8] Documentation: devel: porting: bring it up-to-date Ahmad Fatoum
7 siblings, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-10-24 6:57 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum
The experiment of using a common definition between ARM32 and ARM64
for ENTRY_FUNCTION_WITHSTACK was not fruitful and ARM64 no longer
uses __ARM_SETUP_STACK. Thus remove the definition and open code it for
ARM32. No functional change.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/include/asm/barebox-arm.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index 89b4a89755a6..dd12f642d993 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -177,7 +177,8 @@ void __barebox_arm64_head(ulong x0, ulong x1, ulong x2);
static void ____##name(ulong, ulong, ulong); \
ENTRY_FUNCTION(name, arg0, arg1, arg2) \
{ \
- __ARM_SETUP_STACK(name, stack_top); \
+ if (stack_top) \
+ arm_setup_stack(stack_top); \
____##name(arg0, arg1, arg2); \
} \
static void noinline ____##name \
@@ -188,15 +189,14 @@ void __barebox_arm64_head(ulong x0, ulong x1, ulong x2);
\
static void __##name(ulong, ulong, ulong); \
\
- void NAKED __section(.text_head_entry_##name) name \
+ void __naked __section(.text_head_entry_##name) name \
(ulong r0, ulong r1, ulong r2) \
{ \
__barebox_arm_head(); \
- __ARM_SETUP_STACK(name, 0); \
__##name(r0, r1, r2); \
} \
- static void NAKED noinline __##name \
- (ulong arg0, ulong arg1, ulong arg2)
+ static void __naked noinline __##name \
+ (ulong arg0, ulong arg1, ulong arg2)
#endif
/*
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 8/8] Documentation: devel: porting: bring it up-to-date
2022-10-24 6:57 [PATCH 0/8] ARM64: rewrite ENTRY_FUNCTION_WITHSTACK in assembly Ahmad Fatoum
` (6 preceding siblings ...)
2022-10-24 6:57 ` [PATCH 7/8] ARM: asm: cleanup 32-bit entry points Ahmad Fatoum
@ 2022-10-24 6:57 ` Ahmad Fatoum
7 siblings, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-10-24 6:57 UTC (permalink / raw)
To: barebox
A number of changes happened since this guide was first drafted:
- We no longer use inline assembly for the header on ARM64, which
lacks __attribute__((naked))
- FDT compression algorithm is no longer limited to LZO
Update the guide to reflect this and while at it, fix some typos.
---
Documentation/devel/porting.rst | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/Documentation/devel/porting.rst b/Documentation/devel/porting.rst
index dea5ebd1c511..f95e8cbba3e4 100644
--- a/Documentation/devel/porting.rst
+++ b/Documentation/devel/porting.rst
@@ -195,14 +195,15 @@ Looking at other boards you might see some different patterns:
by using ``ENTRY_FUNCTION_WITHSTACK``, which will take care to initialize the
stack beforehand. If either a barebox assembly entry point,
``ENTRY_FUNCTION_WITHSTACK`` or earlier firmware has set up the stack, there is
- no reason to use ``__naked``, just use ``ENTRY_FNCTION_WITHSTACK`` with a zero
+ no reason to use ``__naked``, just use ``ENTRY_FUNCTION_WITHSTACK`` with a zero
stack top.
``noinline``
Compiler code inlining is oblivious to stack manipulation in
inline assembly. If you want to ensure a new function has its own stack frame
(e.g. after setting up the stack in a ``__naked`` function), you must jump to
- a ``__noreturn noinline`` function.
+ a ``__noreturn noinline`` function. This is already handled by
+ ``ENTRY_FUNCTION_WITHSTACK``.
``arm_setup_stack``
For 32-bit ARM, ``arm_setup_stack`` initializes the stack
@@ -214,7 +215,7 @@ Looking at other boards you might see some different patterns:
``__dtb_z_my_board_start[];``
Because the PBL normally doesn't parse anything out
of the device tree blob, boards can benefit from keeping the device tree blob
- compressed and only unpack it in barebox proper. Such LZO-compressed device trees
+ compressed and only unpack it in barebox proper. Such compressed device trees
are prefixed with ``__dtb_z_``. It's usually a good idea to use this.
``imx6q_barebox_entry(...);``
@@ -232,7 +233,7 @@ Looking at other boards you might see some different patterns:
``*_start_image(...)/*_load_image(...)/*_xload_*(...)``
If the SRAM couldn't fit both PBL and the compressed barebox proper, PBL
- will need to chainload full barebox binary from disk.
+ will need to chainload full barebox binary from the boot medium.
Repeating previous advice: The specifics about how different SoCs handle
things can vary widely. You're best served by mimicking a similar recently
@@ -404,9 +405,18 @@ New header format
=================
Your loader may require a specific header or format. If the header is meant
-to be executable, it should preferably be added as inline assembly to
-the start of the PBL entry points. See ``__barebox_arm_head`` and
-``__barebox_riscv_header``. Otherwise, add a new tool to ``scripts/``
+to be executable, it should be written in assembly.
+If the C compiler for that platform supports ``__attribute__((naked))``, it
+can be written in inline assembly inside such a naked function. See for
+example ``__barebox_arm_head`` for ARM32 or ``__barebox_riscv_header`` for RISC-V.
+
+For platforms, without naked function support, inline assembly may not be used
+and the entry point should be written in a dedicated assembly file.
+This is the case with ARM64, see for example ``__barebox_arm64_head`` and the
+``ENTRY_PROC`` macro.
+
+Another way, which is often used for non-executable headers with extra
+meta-information like a checksum, is adding a new tool to ``scripts/``
and have it run as part the image build process. ``images/`` contains
various examples.
@@ -434,7 +444,7 @@ well as its prerequisites like clocks, resets or pin multiplexers.
Examples for this are the i.MX xload functions. Some BootROMs boot from
a FAT file system. There is vfat support in the PBL. Refer to the sama5d2
-baord support for an example.
+board support for an example.
Core drivers
============
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread