* [PATCH 1/3] ARM: implement ENTRY_FUNCTION_WITHSTACK
@ 2021-12-02 7:42 Ahmad Fatoum
2021-12-02 7:42 ` [PATCH 2/3] ARM: turn arm_mem_stack_top into a macro Ahmad Fatoum
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2021-12-02 7:42 UTC (permalink / raw)
To: barebox; +Cc: ukl, Ahmad Fatoum
The point of ENTRY_FUNCTION is to write the entry point in C. Due to
lack of __naked on ARM64, the start of the entry point will have prologue
using stack and it's not possible to set up the stack safely without
branching into non-inline assembly[0]. On ARM32, where we got __naked, we
have the potential for a different problem: If BootROM sets up stack for
us and we branch to a naked function, which doesn't set up its own
stack, compiler may decide to spill local variables overwriting
instructions it had already run[1].
For code reuse between ARM and ARM64, it would be nice to use the same
entry point structure for both. Currently, the only way is to write it
in non-inline assembly using the ENTRY_PROC macro.
This introduces another way: the ARM64 barebox header has enough space
for 8 instructions of which 5 are unused (2 instructions compiler prologue
+ 1 instruction to jump after the header), we could place a stack setup
routine there to avoid having to write a separate assembly file.
For ARM32, we just call arm_setup_stack and branch out directly after,
freeing board porters of the burden of getting it right.
Add a new ENTRY_FUNCTION_WITHSTACK to realize this.
[0]: 76bced6fe146 ("ARM: document arm_setup_stack() pitfalls"),
[1]: b51b15ba1738 ("RISC-V: board-dt-2nd: move low level init into nonnaked function")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/include/asm/barebox-arm-head.h | 6 +---
arch/arm/include/asm/barebox-arm.h | 45 +++++++++++++++++++++++++
arch/arm/lib/pbl.lds.S | 1 +
3 files changed, 47 insertions(+), 5 deletions(-)
diff --git a/arch/arm/include/asm/barebox-arm-head.h b/arch/arm/include/asm/barebox-arm-head.h
index 187d12c9fc8d..3e8da0da7c0a 100644
--- a/arch/arm/include/asm/barebox-arm-head.h
+++ b/arch/arm/include/asm/barebox-arm-head.h
@@ -44,14 +44,10 @@ static inline void __barebox_arm_head(void)
"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"
- "nop\n"
- "nop\n"
- "nop\n"
- "nop\n"
- "nop\n"
#endif
".asciz \"barebox\"\n"
#ifdef CONFIG_CPU_32
diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index cfb5943f33d6..d70de6d5e6e6 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -161,6 +161,51 @@ static inline unsigned long arm_mem_barebox_image(unsigned long membase,
}
}
+#ifdef CONFIG_CPU_64
+
+#define ____emit_entry_prologue(instr, ...) do { \
+ static __attribute__ ((unused,section(".text_head_prologue"))) \
+ const u32 __entry_prologue[] = {(instr), ##__VA_ARGS__}; \
+ barrier_data(__entry_prologue); \
+} while(0)
+
+#define __emit_entry_prologue(instr1, instr2, instr3, instr4, instr5) \
+ ____emit_entry_prologue(instr1, instr2, instr3, instr4, instr5)
+
+#define __ARM_SETUP_STACK(stack_top) \
+ __emit_entry_prologue(0x14000002 /* b pc+0x8 */, \
+ stack_top /* 32-bit literal */, \
+ 0x18ffffe9 /* ldr w9, top */, \
+ 0xb4000049 /* cbz x9, pc+0x8 */, \
+ 0x9100013f /* mov sp, x9 */)
+#else
+#define __ARM_SETUP_STACK(stack_top) if (stack_top) arm_setup_stack(stack_top)
+#endif
+
+/*
+ * Unlike ENTRY_FUNCTION, this can be used to setup stack for a C entry
+ * point on both ARM32 and ARM64. ENTRY_FUNCTION on ARM64 can only be used
+ * if preceding boot stage has initialized the stack pointer.
+ *
+ * Stack top of 0 means stack is already set up. In that case, the follow-up
+ * code block will not be inlined and may spill to stack right away.
+ */
+#define ENTRY_FUNCTION_WITHSTACK(name, stack_top, arg0, arg1, arg2) \
+ void name(ulong r0, ulong r1, ulong r2); \
+ \
+ static void __##name(ulong, ulong, ulong); \
+ \
+ void NAKED __section(.text_head_entry_##name) name \
+ (ulong r0, ulong r1, ulong r2) \
+ { \
+ __barebox_arm_head(); \
+ __ARM_SETUP_STACK(stack_top); \
+ __##name(r0, r1, r2); \
+ } \
+ static void noinline __##name \
+ (ulong arg0, ulong arg1, ulong arg2)
+
+
#define ENTRY_FUNCTION(name, arg0, arg1, arg2) \
void name(ulong r0, ulong r1, ulong r2); \
\
diff --git a/arch/arm/lib/pbl.lds.S b/arch/arm/lib/pbl.lds.S
index 0a0fb8b5ac13..e77b3220fcb0 100644
--- a/arch/arm/lib/pbl.lds.S
+++ b/arch/arm/lib/pbl.lds.S
@@ -31,6 +31,7 @@ SECTIONS
.text :
{
_stext = .;
+ *(.text_head_prologue*)
*(.text_head_entry*)
__bare_init_start = .;
*(.text_bare_init*)
--
2.30.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] ARM: turn arm_mem_stack_top into a macro
2021-12-02 7:42 [PATCH 1/3] ARM: implement ENTRY_FUNCTION_WITHSTACK Ahmad Fatoum
@ 2021-12-02 7:42 ` Ahmad Fatoum
2021-12-02 8:01 ` Ahmad Fatoum
2021-12-02 7:42 ` [PATCH 3/3] ARM: rpi: use ENTRY_FUNCTION_WITHSTACK to prepare for ARM64 support Ahmad Fatoum
2021-12-02 7:56 ` [PATCH] fixup! ARM: implement ENTRY_FUNCTION_WITHSTACK Ahmad Fatoum
2 siblings, 1 reply; 6+ messages in thread
From: Ahmad Fatoum @ 2021-12-02 7:42 UTC (permalink / raw)
To: barebox; +Cc: ukl, Ahmad Fatoum
This makes it possible to use for static initialization, like done in a
follow-up commit.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/include/asm/barebox-arm.h | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index d70de6d5e6e6..0481d3391c26 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -97,14 +97,13 @@ static inline void arm_fixup_vectors(void)
void *barebox_arm_boot_dtb(void);
-static inline unsigned long arm_mem_stack_top(unsigned long membase,
- unsigned long endmem)
-{
- if (IS_ENABLED(CONFIG_BOOTM_OPTEE) || IS_ENABLED(CONFIG_PBL_OPTEE))
- endmem -= OPTEE_SIZE;
+#define __arm_mem_stack_top(membase, endmem) ((endmem) - SZ_64K)
- return endmem - SZ_64K;
-}
+#if defined(CONFIG_BOOTM_OPTEE) || defined(CONFIG_PBL_OPTEE)
+#define arm_mem_stack_top(membase, endmem) (__arm_mem_stack_top(membase, endmem) - OPTEE_SIZE)
+#else
+#define arm_mem_stack_top(membase, endmem) (__arm_mem_stack_top(membase, endmem) - OPTEE_SIZE)
+#endif
static inline unsigned long arm_mem_stack(unsigned long membase,
unsigned long endmem)
--
2.30.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] ARM: turn arm_mem_stack_top into a macro
2021-12-02 7:42 ` [PATCH 2/3] ARM: turn arm_mem_stack_top into a macro Ahmad Fatoum
@ 2021-12-02 8:01 ` Ahmad Fatoum
2021-12-07 9:26 ` Sascha Hauer
0 siblings, 1 reply; 6+ messages in thread
From: Ahmad Fatoum @ 2021-12-02 8:01 UTC (permalink / raw)
To: barebox; +Cc: ukl
On 02.12.21 08:42, Ahmad Fatoum wrote:
> This makes it possible to use for static initialization, like done in a
> follow-up commit.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> arch/arm/include/asm/barebox-arm.h | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
> index d70de6d5e6e6..0481d3391c26 100644
> --- a/arch/arm/include/asm/barebox-arm.h
> +++ b/arch/arm/include/asm/barebox-arm.h
> @@ -97,14 +97,13 @@ static inline void arm_fixup_vectors(void)
>
> void *barebox_arm_boot_dtb(void);
>
> -static inline unsigned long arm_mem_stack_top(unsigned long membase,
> - unsigned long endmem)
> -{
> - if (IS_ENABLED(CONFIG_BOOTM_OPTEE) || IS_ENABLED(CONFIG_PBL_OPTEE))
> - endmem -= OPTEE_SIZE;
> +#define __arm_mem_stack_top(membase, endmem) ((endmem) - SZ_64K)
>
> - return endmem - SZ_64K;
> -}
> +#if defined(CONFIG_BOOTM_OPTEE) || defined(CONFIG_PBL_OPTEE)
> +#define arm_mem_stack_top(membase, endmem) (__arm_mem_stack_top(membase, endmem) - OPTEE_SIZE)
> +#else
> +#define arm_mem_stack_top(membase, endmem) (__arm_mem_stack_top(membase, endmem) - OPTEE_SIZE)
Argh. Should have looked more closely before sending out. I'll respin later,
but looking forward to feedback in the mean time.
> +#endif
>
> static inline unsigned long arm_mem_stack(unsigned long membase,
> unsigned long endmem)
>
--
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 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] ARM: turn arm_mem_stack_top into a macro
2021-12-02 8:01 ` Ahmad Fatoum
@ 2021-12-07 9:26 ` Sascha Hauer
0 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2021-12-07 9:26 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox, ukl
On Thu, Dec 02, 2021 at 09:01:10AM +0100, Ahmad Fatoum wrote:
> On 02.12.21 08:42, Ahmad Fatoum wrote:
> > This makes it possible to use for static initialization, like done in a
> > follow-up commit.
> >
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > ---
> > arch/arm/include/asm/barebox-arm.h | 13 ++++++-------
> > 1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
> > index d70de6d5e6e6..0481d3391c26 100644
> > --- a/arch/arm/include/asm/barebox-arm.h
> > +++ b/arch/arm/include/asm/barebox-arm.h
> > @@ -97,14 +97,13 @@ static inline void arm_fixup_vectors(void)
> >
> > void *barebox_arm_boot_dtb(void);
> >
> > -static inline unsigned long arm_mem_stack_top(unsigned long membase,
> > - unsigned long endmem)
> > -{
> > - if (IS_ENABLED(CONFIG_BOOTM_OPTEE) || IS_ENABLED(CONFIG_PBL_OPTEE))
> > - endmem -= OPTEE_SIZE;
> > +#define __arm_mem_stack_top(membase, endmem) ((endmem) - SZ_64K)
> >
> > - return endmem - SZ_64K;
> > -}
> > +#if defined(CONFIG_BOOTM_OPTEE) || defined(CONFIG_PBL_OPTEE)
> > +#define arm_mem_stack_top(membase, endmem) (__arm_mem_stack_top(membase, endmem) - OPTEE_SIZE)
> > +#else
> > +#define arm_mem_stack_top(membase, endmem) (__arm_mem_stack_top(membase, endmem) - OPTEE_SIZE)
>
> Argh. Should have looked more closely before sending out. I'll respin later,
> but looking forward to feedback in the mean time.
Apart from this error this series looks good to me (at least in the
range of how good such a preprocessor/assembly wasteland can look).
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 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] ARM: rpi: use ENTRY_FUNCTION_WITHSTACK to prepare for ARM64 support
2021-12-02 7:42 [PATCH 1/3] ARM: implement ENTRY_FUNCTION_WITHSTACK Ahmad Fatoum
2021-12-02 7:42 ` [PATCH 2/3] ARM: turn arm_mem_stack_top into a macro Ahmad Fatoum
@ 2021-12-02 7:42 ` Ahmad Fatoum
2021-12-02 7:56 ` [PATCH] fixup! ARM: implement ENTRY_FUNCTION_WITHSTACK Ahmad Fatoum
2 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2021-12-02 7:42 UTC (permalink / raw)
To: barebox; +Cc: ukl, Ahmad Fatoum
arm_setup_stack used in start_raspberry_pi is not safe on ARM64, due to
lack of __naked support. since 76bced6fe146 ("ARM: document
arm_setup_stack() pitfalls"), its use in ARM64 code is a compile-time
error.
The Raspberry Pi 4 support will be ARM64. To allow it to reuse
the existing low-level code, use the new ENTRY_FUNCTION_WITHSTACK,
which will set up stack appropriately for both ARM32 and ARM64.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/boards/raspberry-pi/lowlevel.c | 35 ++++++++++++-------------
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/arch/arm/boards/raspberry-pi/lowlevel.c b/arch/arm/boards/raspberry-pi/lowlevel.c
index d58beb605255..ab5b47051926 100644
--- a/arch/arm/boards/raspberry-pi/lowlevel.c
+++ b/arch/arm/boards/raspberry-pi/lowlevel.c
@@ -31,52 +31,51 @@ static void copy_vc_fdt(void *dest, void *src, unsigned long max_size)
memmove(dest, src, size);
}
+/* A pointer to the FDT created by VideoCore was passed to us in r2. We
+ * reserve some memory just above the region used for Barebox and copy
+ * this FDT there. We fetch it from there later in rpi_devices_init().
+ */
+#define rpi_stack_top(memsize) \
+ arm_mem_stack_top(BCM2835_SDRAM_BASE, BCM2835_SDRAM_BASE + memsize - VIDEOCORE_FDT_SZ)
+
/* Must be inline since stack isn't setup yet. */
static inline void start_raspberry_pi(unsigned long memsize, void *fdt,
void *vc_fdt)
{
- void *saved_vc_fdt;
- unsigned long membase = BCM2835_SDRAM_BASE;
-
- /* A pointer to the FDT created by VideoCore was passed to us in r2. We
- * reserve some memory just above the region used for Basebox and copy
- * this FDT there. We fetch it from there later in rpi_devices_init().*/
- memsize -= VIDEOCORE_FDT_SZ;
+ unsigned long endmem = rpi_stack_top(memsize);
arm_cpu_lowlevel_init();
- /* Copied from barebox_arm_entry(). We need stack here early
- * for normal function calls to work. */
- arm_setup_stack(arm_mem_stack_top(membase, membase + memsize));
+ copy_vc_fdt((void *)endmem, vc_fdt, VIDEOCORE_FDT_SZ);
fdt += get_runtime_offset();
- saved_vc_fdt = (void *)(membase + memsize);
- copy_vc_fdt(saved_vc_fdt, vc_fdt, VIDEOCORE_FDT_SZ);
-
- barebox_arm_entry(membase, memsize, fdt);
+ barebox_arm_entry(BCM2835_SDRAM_BASE, endmem - BCM2835_SDRAM_BASE, fdt);
}
+#define RPI_ENTRY_FUNCTION(name, memsize, r2) \
+ ENTRY_FUNCTION_WITHSTACK(name, rpi_stack_top(memsize), __r0, __r1, r2)
+
extern char __dtb_bcm2835_rpi_start[];
-ENTRY_FUNCTION(start_raspberry_pi1, r0, r1, r2)
+RPI_ENTRY_FUNCTION(start_raspberry_pi1, SZ_128M, r2)
{
start_raspberry_pi(SZ_128M, __dtb_bcm2835_rpi_start, (void *)r2);
}
extern char __dtb_bcm2836_rpi_2_start[];
-ENTRY_FUNCTION(start_raspberry_pi2, r0, r1, r2)
+RPI_ENTRY_FUNCTION(start_raspberry_pi2, SZ_512M, r2)
{
start_raspberry_pi(SZ_512M, __dtb_bcm2836_rpi_2_start, (void *)r2);
}
extern char __dtb_bcm2837_rpi_3_start[];
-ENTRY_FUNCTION(start_raspberry_pi3, r0, r1, r2)
+RPI_ENTRY_FUNCTION(start_raspberry_pi3, SZ_512M, r2)
{
start_raspberry_pi(SZ_512M, __dtb_bcm2837_rpi_3_start, (void *)r2);
}
extern char __dtb_bcm2837_rpi_cm3_start[];
-ENTRY_FUNCTION(start_raspberry_pi_cm3, r0, r1, r2)
+RPI_ENTRY_FUNCTION(start_raspberry_pi_cm3, SZ_512M, r2)
{
start_raspberry_pi(SZ_512M, __dtb_bcm2837_rpi_cm3_start, (void *)r2);
}
--
2.30.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] fixup! ARM: implement ENTRY_FUNCTION_WITHSTACK
2021-12-02 7:42 [PATCH 1/3] ARM: implement ENTRY_FUNCTION_WITHSTACK Ahmad Fatoum
2021-12-02 7:42 ` [PATCH 2/3] ARM: turn arm_mem_stack_top into a macro Ahmad Fatoum
2021-12-02 7:42 ` [PATCH 3/3] ARM: rpi: use ENTRY_FUNCTION_WITHSTACK to prepare for ARM64 support Ahmad Fatoum
@ 2021-12-02 7:56 ` Ahmad Fatoum
2 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2021-12-02 7:56 UTC (permalink / raw)
To: barebox; +Cc: ukl, Ahmad Fatoum
We want same prologue for both. Otherwise header is shifted.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/include/asm/barebox-arm.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index d70de6d5e6e6..5018b3e2f57a 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -215,6 +215,7 @@ static inline unsigned long arm_mem_barebox_image(unsigned long membase,
(ulong r0, ulong r1, ulong r2) \
{ \
__barebox_arm_head(); \
+ __ARM_SETUP_STACK(0); \
__##name(r0, r1, r2); \
} \
static void NAKED noinline __##name \
--
2.30.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-07 9:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-02 7:42 [PATCH 1/3] ARM: implement ENTRY_FUNCTION_WITHSTACK Ahmad Fatoum
2021-12-02 7:42 ` [PATCH 2/3] ARM: turn arm_mem_stack_top into a macro Ahmad Fatoum
2021-12-02 8:01 ` Ahmad Fatoum
2021-12-07 9:26 ` Sascha Hauer
2021-12-02 7:42 ` [PATCH 3/3] ARM: rpi: use ENTRY_FUNCTION_WITHSTACK to prepare for ARM64 support Ahmad Fatoum
2021-12-02 7:56 ` [PATCH] fixup! ARM: implement ENTRY_FUNCTION_WITHSTACK Ahmad Fatoum
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox