[PATCH v3 0/4] arm64: Large PIE fixes
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
This fixes some build issues with large (> 1MB) PIE U-Boot setups. We also document the 4K aligned load address requirement and add an early run-time check for it.
As requested by reviewers, I've also added a runtime check for non-PIE builds to trap the startup sequence early if the start address doesn't match between run-time and link-time.
Cheers, Edgar
ChangeLog: v2 -> v3: * Add non-PIE start address (run vs link-time) check * Move 4K aligment Kconfig help description * Fix load of __bss_start * Use x0 rather than x1 in PIE load-address check * Add missing tabs * Add load-address check for non-PIE * U-boot -> U-Boot in a few places * Tweak commit messages
v1 -> v2: * Fix adr of _start in arch/arm/lib/crt0_64.S * Add early check and bail out into a WFI loop when not 4K aligned * Document the 4K alignement requirement in Kconfig
Edgar E. Iglesias (4): arm64: Mention 4K aligned load addresses in the PIE Kconfig help arm64: Trap PIE builds early if load address is not 4K aligned arm64: Add support for larger PIE U-Boot arm64: Trap non-PIE builds early if starting from wrong address
arch/arm/Kconfig | 4 ++-- arch/arm/cpu/armv8/start.S | 36 ++++++++++++++++++++++++++++++++++-- arch/arm/lib/crt0_64.S | 8 +++++++- 3 files changed, 43 insertions(+), 5 deletions(-)
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
Mention the requirement of 4K aligned load addresses in the help section for the POSITION_INDEPENDENT option.
Suggested-by: Michal Simek michal.simek@xilinx.com Signed-off-by: Edgar E. Iglesias edgar.iglesias@xilinx.com --- arch/arm/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 80702c23d3..67286e8b5d 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -16,8 +16,8 @@ config POSITION_INDEPENDENT help U-Boot expects to be linked to a specific hard-coded address, and to be loaded to and run from that address. This option lifts that - restriction, thus allowing the code to be loaded to and executed - from almost any address. This logic relies on the relocation + restriction, thus allowing the code to be loaded to and executed from + almost any 4K aligned address. This logic relies on the relocation information that is embedded in the binary to support U-Boot relocating itself to the top-of-RAM later during execution.
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
PIE requires a 4K aligned load address. If this is not met, trap the startup sequence in a WFI loop rather than running into obscure failures.
Tested-by: Michal Simek michal.simek@xilinx.com Suggested-by: André Przywara andre.przywara@arm.com Signed-off-by: Edgar E. Iglesias edgar.iglesias@xilinx.com --- arch/arm/cpu/armv8/start.S | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index 002698b501..85baebc5f7 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -59,6 +59,23 @@ reset: save_boot_params_ret:
#if CONFIG_POSITION_INDEPENDENT + /* Verify that we're 4K aligned. */ + adr x0, _start + ands x0, x0, #0xfff + b.eq 1f +0: + /* + * FATAL, can't continue. + * U-Boot needs to be loaded at a 4K aligned address. + * + * We use ADRP and ADD to load some symbol addresses during startup. + * The ADD uses an absolute (non pc-relative) lo12 relocation + * thus requiring 4K alignment. + */ + wfi + b 0b +1: + /* * Fix .rela.dyn relocations. This allows U-Boot to be loaded to and * executed at a different address than it was linked at.
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
Linking a U-Boot larger than 1MB fails with PIE enabled: u-boot/arch/arm/cpu/armv8/start.S:71:(.text+0x3c): relocation truncated to fit: R_AARCH64_ADR_PREL_LO21 against symbol `__rel_dyn_end' defined in .bss_start section in u-boot.
This extends the supported range by using adrp & add to load symbols early while starting up.
Signed-off-by: Edgar E. Iglesias edgar.iglesias@xilinx.com --- arch/arm/cpu/armv8/start.S | 6 ++++-- arch/arm/lib/crt0_64.S | 8 +++++++- 2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index 85baebc5f7..e5c2856cf5 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -84,8 +84,10 @@ pie_fixup: adr x0, _start /* x0 <- Runtime value of _start */ ldr x1, _TEXT_BASE /* x1 <- Linked value of _start */ sub x9, x0, x1 /* x9 <- Run-vs-link offset */ - adr x2, __rel_dyn_start /* x2 <- Runtime &__rel_dyn_start */ - adr x3, __rel_dyn_end /* x3 <- Runtime &__rel_dyn_end */ + adrp x2, __rel_dyn_start /* x2 <- Runtime &__rel_dyn_start */ + add x2, x2, #:lo12:__rel_dyn_start + adrp x3, __rel_dyn_end /* x3 <- Runtime &__rel_dyn_end */ + add x3, x3, #:lo12:__rel_dyn_end pie_fix_loop: ldp x0, x1, [x2], #16 /* (x0, x1) <- (Link location, fixup) */ ldr x4, [x2], #8 /* x4 <- addend */ diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S index 04afa518ac..9d2319c0e8 100644 --- a/arch/arm/lib/crt0_64.S +++ b/arch/arm/lib/crt0_64.S @@ -73,7 +73,12 @@ ENTRY(_main) #elif defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK) ldr x0, =(CONFIG_SPL_STACK) #elif defined(CONFIG_INIT_SP_RELATIVE) +#if CONFIG_POSITION_INDEPENDENT + adrp x0, __bss_start /* x0 <- Runtime &__bss_start */ + add x0, x0, #:lo12:__bss_start +#else adr x0, __bss_start +#endif add x0, x0, #CONFIG_SYS_INIT_SP_BSS_OFFSET #else ldr x0, =(CONFIG_SYS_INIT_SP_ADDR) @@ -102,7 +107,8 @@ ENTRY(_main) adr lr, relocation_return #if CONFIG_POSITION_INDEPENDENT /* Add in link-vs-runtime offset */ - adr x0, _start /* x0 <- Runtime value of _start */ + adrp x0, _start /* x0 <- Runtime value of _start */ + add x0, x0, #:lo12:_start ldr x9, _TEXT_BASE /* x9 <- Linked value of _start */ sub x9, x9, x0 /* x9 <- Run-vs-link offset */ add lr, lr, x9
On 09. 09. 20 19:07, Edgar E. Iglesias wrote:
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
Linking a U-Boot larger than 1MB fails with PIE enabled: u-boot/arch/arm/cpu/armv8/start.S:71:(.text+0x3c): relocation truncated to fit: R_AARCH64_ADR_PREL_LO21 against symbol `__rel_dyn_end' defined in .bss_start section in u-boot.
This extends the supported range by using adrp & add to load symbols early while starting up.
Signed-off-by: Edgar E. Iglesias edgar.iglesias@xilinx.com
arch/arm/cpu/armv8/start.S | 6 ++++-- arch/arm/lib/crt0_64.S | 8 +++++++- 2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index 85baebc5f7..e5c2856cf5 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -84,8 +84,10 @@ pie_fixup: adr x0, _start /* x0 <- Runtime value of _start */ ldr x1, _TEXT_BASE /* x1 <- Linked value of _start */ sub x9, x0, x1 /* x9 <- Run-vs-link offset */
- adr x2, __rel_dyn_start /* x2 <- Runtime &__rel_dyn_start */
- adr x3, __rel_dyn_end /* x3 <- Runtime &__rel_dyn_end */
- adrp x2, __rel_dyn_start /* x2 <- Runtime &__rel_dyn_start */
- add x2, x2, #:lo12:__rel_dyn_start
- adrp x3, __rel_dyn_end /* x3 <- Runtime &__rel_dyn_end */
- add x3, x3, #:lo12:__rel_dyn_end
pie_fix_loop: ldp x0, x1, [x2], #16 /* (x0, x1) <- (Link location, fixup) */ ldr x4, [x2], #8 /* x4 <- addend */ diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S index 04afa518ac..9d2319c0e8 100644 --- a/arch/arm/lib/crt0_64.S +++ b/arch/arm/lib/crt0_64.S @@ -73,7 +73,12 @@ ENTRY(_main) #elif defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK) ldr x0, =(CONFIG_SPL_STACK) #elif defined(CONFIG_INIT_SP_RELATIVE) +#if CONFIG_POSITION_INDEPENDENT
- adrp x0, __bss_start /* x0 <- Runtime &__bss_start */
- add x0, x0, #:lo12:__bss_start
+#else adr x0, __bss_start +#endif add x0, x0, #CONFIG_SYS_INIT_SP_BSS_OFFSET #else ldr x0, =(CONFIG_SYS_INIT_SP_ADDR) @@ -102,7 +107,8 @@ ENTRY(_main) adr lr, relocation_return #if CONFIG_POSITION_INDEPENDENT /* Add in link-vs-runtime offset */
- adr x0, _start /* x0 <- Runtime value of _start */
- adrp x0, _start /* x0 <- Runtime value of _start */
- add x0, x0, #:lo12:_start ldr x9, _TEXT_BASE /* x9 <- Linked value of _start */ sub x9, x9, x0 /* x9 <- Run-vs-link offset */ add lr, lr, x9
Signed-off-by: Michal Simek michal.simek@xilinx.com Tested-by: Michal Simek michal.simek@xilinx.com
Thanks, Michal
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
Trap non-PIE builds early if the start address doesn't match between run-time and link-time. This will trap the startup sequence rather than letting it run into obscure errors.
Signed-off-by: Edgar E. Iglesias edgar.iglesias@xilinx.com --- arch/arm/cpu/armv8/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index e5c2856cf5..39e1b842c4 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -101,6 +101,19 @@ pie_skip_reloc: cmp x2, x3 b.lo pie_fix_loop pie_fixup_done: +#else + adr x0, _start + ldr x1, _TEXT_BASE + cmp x0, x1 + beq 1f +0: + /* + * FATAL, can't continue. + * U-Boot needs to start executing at CONFIG_SYS_TEXT_BASE. + */ + wfi + b 0b +1: #endif
#ifdef CONFIG_SYS_RESET_SCTRL
On 09. 09. 20 19:07, Edgar E. Iglesias wrote:
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
Trap non-PIE builds early if the start address doesn't match between run-time and link-time. This will trap the startup sequence rather than letting it run into obscure errors.
Signed-off-by: Edgar E. Iglesias edgar.iglesias@xilinx.com
arch/arm/cpu/armv8/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index e5c2856cf5..39e1b842c4 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -101,6 +101,19 @@ pie_skip_reloc: cmp x2, x3 b.lo pie_fix_loop pie_fixup_done: +#else
- adr x0, _start
- ldr x1, _TEXT_BASE
- cmp x0, x1
- beq 1f
+0:
- /*
* FATAL, can't continue.* U-Boot needs to start executing at CONFIG_SYS_TEXT_BASE.*/- wfi
- b 0b
+1: #endif
#ifdef CONFIG_SYS_RESET_SCTRL
NACK for this.
1. It breaks SPL flow because CONFIG_SYS_TEXT_BASE is text base for U-Boot proper 2. It likely also breaks TPL flow for the same reason
3. And last thing is that this code is used only for U-Boot proper. .globl _TEXT_BASE _TEXT_BASE: .quad CONFIG_SYS_TEXT_BASE
The fixes are below. Point 3 should be likely be in separate patch because it is unrelated.
Thanks, Michal
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index b37dbabf4d42..cdc609e873fc 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -33,10 +33,11 @@ _start:
.align 3
+#if !(defined(CONFIG_SPL_BUILD) || defined(CONFIG_TPL_BUILD)) .globl _TEXT_BASE _TEXT_BASE: .quad CONFIG_SYS_TEXT_BASE - +#endif /* * These are defined in the linker script. */ @@ -102,6 +103,7 @@ pie_skip_reloc: b.lo pie_fix_loop pie_fixup_done: #else +#if !(defined(CONFIG_SPL_BUILD) || defined(CONFIG_TPL_BUILD)) adr x0, _start ldr x1, _TEXT_BASE cmp x0, x1 @@ -115,6 +117,7 @@ pie_fixup_done: b 0b 1: #endif +#endif
#ifdef CONFIG_SYS_RESET_SCTRL bl reset_sctrl
On 10/09/2020 13:38, Michal Simek wrote:
On 09. 09. 20 19:07, Edgar E. Iglesias wrote:
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
Trap non-PIE builds early if the start address doesn't match between run-time and link-time. This will trap the startup sequence rather than letting it run into obscure errors.
Signed-off-by: Edgar E. Iglesias edgar.iglesias@xilinx.com
arch/arm/cpu/armv8/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index e5c2856cf5..39e1b842c4 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -101,6 +101,19 @@ pie_skip_reloc: cmp x2, x3 b.lo pie_fix_loop pie_fixup_done: +#else
- adr x0, _start
- ldr x1, _TEXT_BASE
- cmp x0, x1
- beq 1f
+0:
- /*
* FATAL, can't continue.* U-Boot needs to start executing at CONFIG_SYS_TEXT_BASE.*/- wfi
- b 0b
+1: #endif
#ifdef CONFIG_SYS_RESET_SCTRL
NACK for this.
- It breaks SPL flow because CONFIG_SYS_TEXT_BASE is text base for
U-Boot proper 2. It likely also breaks TPL flow for the same reason
- And last thing is that this code is used only for U-Boot proper.
.globl _TEXT_BASE _TEXT_BASE: .quad CONFIG_SYS_TEXT_BASE
The fixes are below. Point 3 should be likely be in separate patch because it is unrelated.
So if this patch causes issues, can't we just drop it? I mean right now you will probably just crash anyway if you load it at the wrong address, but maybe late enough that you get more hints or even some output.
Now this patch makes sure that you don't get anything, so I don't see how this is really improving the situation. It seems like a case of "don't fix things that ain't broken".
Cheers, Andre
Thanks, Michal
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index b37dbabf4d42..cdc609e873fc 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -33,10 +33,11 @@ _start:
.align 3+#if !(defined(CONFIG_SPL_BUILD) || defined(CONFIG_TPL_BUILD)) .globl _TEXT_BASE _TEXT_BASE: .quad CONFIG_SYS_TEXT_BASE
+#endif /*
- These are defined in the linker script.
*/ @@ -102,6 +103,7 @@ pie_skip_reloc: b.lo pie_fix_loop pie_fixup_done: #else +#if !(defined(CONFIG_SPL_BUILD) || defined(CONFIG_TPL_BUILD)) adr x0, _start ldr x1, _TEXT_BASE cmp x0, x1 @@ -115,6 +117,7 @@ pie_fixup_done: b 0b 1: #endif +#endif
#ifdef CONFIG_SYS_RESET_SCTRL bl reset_sctrl
On 10. 09. 20 15:06, André Przywara wrote:
On 10/09/2020 13:38, Michal Simek wrote:
On 09. 09. 20 19:07, Edgar E. Iglesias wrote:
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
Trap non-PIE builds early if the start address doesn't match between run-time and link-time. This will trap the startup sequence rather than letting it run into obscure errors.
Signed-off-by: Edgar E. Iglesias edgar.iglesias@xilinx.com
arch/arm/cpu/armv8/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index e5c2856cf5..39e1b842c4 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -101,6 +101,19 @@ pie_skip_reloc: cmp x2, x3 b.lo pie_fix_loop pie_fixup_done: +#else
- adr x0, _start
- ldr x1, _TEXT_BASE
- cmp x0, x1
- beq 1f
+0:
- /*
* FATAL, can't continue.* U-Boot needs to start executing at CONFIG_SYS_TEXT_BASE.*/- wfi
- b 0b
+1: #endif
#ifdef CONFIG_SYS_RESET_SCTRL
NACK for this.
- It breaks SPL flow because CONFIG_SYS_TEXT_BASE is text base for
U-Boot proper 2. It likely also breaks TPL flow for the same reason
- And last thing is that this code is used only for U-Boot proper.
.globl _TEXT_BASE _TEXT_BASE: .quad CONFIG_SYS_TEXT_BASE
The fixes are below. Point 3 should be likely be in separate patch because it is unrelated.
So if this patch causes issues, can't we just drop it? I mean right now you will probably just crash anyway if you load it at the wrong address, but maybe late enough that you get more hints or even some output.
Now this patch makes sure that you don't get anything, so I don't see how this is really improving the situation. It seems like a case of "don't fix things that ain't broken".
I am fine with dropping it. Tom: What do you think?
Thanks, Michal
On Thu, Sep 10, 2020 at 03:38:25PM +0200, Michal Simek wrote:
On 10. 09. 20 15:06, André Przywara wrote:
On 10/09/2020 13:38, Michal Simek wrote:
On 09. 09. 20 19:07, Edgar E. Iglesias wrote:
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
Trap non-PIE builds early if the start address doesn't match between run-time and link-time. This will trap the startup sequence rather than letting it run into obscure errors.
Signed-off-by: Edgar E. Iglesias edgar.iglesias@xilinx.com
arch/arm/cpu/armv8/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index e5c2856cf5..39e1b842c4 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -101,6 +101,19 @@ pie_skip_reloc: cmp x2, x3 b.lo pie_fix_loop pie_fixup_done: +#else
- adr x0, _start
- ldr x1, _TEXT_BASE
- cmp x0, x1
- beq 1f
+0:
- /*
* FATAL, can't continue.* U-Boot needs to start executing at CONFIG_SYS_TEXT_BASE.*/- wfi
- b 0b
+1: #endif
#ifdef CONFIG_SYS_RESET_SCTRL
NACK for this.
- It breaks SPL flow because CONFIG_SYS_TEXT_BASE is text base for
U-Boot proper 2. It likely also breaks TPL flow for the same reason
- And last thing is that this code is used only for U-Boot proper.
.globl _TEXT_BASE _TEXT_BASE: .quad CONFIG_SYS_TEXT_BASE
The fixes are below. Point 3 should be likely be in separate patch because it is unrelated.
So if this patch causes issues, can't we just drop it? I mean right now you will probably just crash anyway if you load it at the wrong address, but maybe late enough that you get more hints or even some output.
Now this patch makes sure that you don't get anything, so I don't see how this is really improving the situation. It seems like a case of "don't fix things that ain't broken".
I am fine with dropping it. Tom: What do you think?
OK, yes, we can set this aside for now at least. I assume this is all for v2021.01 anyhow?
On 10. 09. 20 15:50, Tom Rini wrote:
On Thu, Sep 10, 2020 at 03:38:25PM +0200, Michal Simek wrote:
On 10. 09. 20 15:06, André Przywara wrote:
On 10/09/2020 13:38, Michal Simek wrote:
On 09. 09. 20 19:07, Edgar E. Iglesias wrote:
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
Trap non-PIE builds early if the start address doesn't match between run-time and link-time. This will trap the startup sequence rather than letting it run into obscure errors.
Signed-off-by: Edgar E. Iglesias edgar.iglesias@xilinx.com --- arch/arm/cpu/armv8/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index e5c2856cf5..39e1b842c4 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -101,6 +101,19 @@ pie_skip_reloc: cmp x2, x3 b.lo pie_fix_loop pie_fixup_done: +#else + adr x0, _start + ldr x1, _TEXT_BASE + cmp x0, x1 + beq 1f +0: + /* + * FATAL, can't continue. + * U-Boot needs to start executing at CONFIG_SYS_TEXT_BASE. + */ + wfi + b 0b +1: #endif
#ifdef CONFIG_SYS_RESET_SCTRL
NACK for this.
- It breaks SPL flow because CONFIG_SYS_TEXT_BASE is text
base for U-Boot proper 2. It likely also breaks TPL flow for the same reason
- And last thing is that this code is used only for U-Boot
proper. .globl _TEXT_BASE _TEXT_BASE: .quad CONFIG_SYS_TEXT_BASE
The fixes are below. Point 3 should be likely be in separate patch because it is unrelated.
So if this patch causes issues, can't we just drop it? I mean right now you will probably just crash anyway if you load it at the wrong address, but maybe late enough that you get more hints or even some output.
Now this patch makes sure that you don't get anything, so I don't see how this is really improving the situation. It seems like a case of "don't fix things that ain't broken".
I am fine with dropping it. Tom: What do you think?
OK, yes, we can set this aside for now at least. I assume this is all for v2021.01 anyhow?
I would target it for 2021.01.
Thanks, Michal
On Thu, Sep 10, 2020 at 05:02:56PM +0200, Michal Simek wrote:
On 10. 09. 20 15:50, Tom Rini wrote:
On Thu, Sep 10, 2020 at 03:38:25PM +0200, Michal Simek wrote:
On 10. 09. 20 15:06, André Przywara wrote:
On 10/09/2020 13:38, Michal Simek wrote:
On 09. 09. 20 19:07, Edgar E. Iglesias wrote:
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
Trap non-PIE builds early if the start address doesn't match between run-time and link-time. This will trap the startup sequence rather than letting it run into obscure errors.
Signed-off-by: Edgar E. Iglesias edgar.iglesias@xilinx.com --- arch/arm/cpu/armv8/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index e5c2856cf5..39e1b842c4 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -101,6 +101,19 @@ pie_skip_reloc: cmp x2, x3 b.lo pie_fix_loop pie_fixup_done: +#else + adr x0, _start + ldr x1, _TEXT_BASE + cmp x0, x1 + beq 1f +0: + /* + * FATAL, can't continue. + * U-Boot needs to start executing at CONFIG_SYS_TEXT_BASE. + */ + wfi + b 0b +1: #endif
#ifdef CONFIG_SYS_RESET_SCTRL
NACK for this.
- It breaks SPL flow because CONFIG_SYS_TEXT_BASE is text
base for U-Boot proper 2. It likely also breaks TPL flow for the same reason
- And last thing is that this code is used only for U-Boot
proper. .globl _TEXT_BASE _TEXT_BASE: .quad CONFIG_SYS_TEXT_BASE
The fixes are below. Point 3 should be likely be in separate patch because it is unrelated.
So if this patch causes issues, can't we just drop it? I mean right now you will probably just crash anyway if you load it at the wrong address, but maybe late enough that you get more hints or even some output.
Now this patch makes sure that you don't get anything, so I don't see how this is really improving the situation. It seems like a case of "don't fix things that ain't broken".
I am fine with dropping it. Tom: What do you think?
OK, yes, we can set this aside for now at least. I assume this is all for v2021.01 anyhow?
I would target it for 2021.01.
Dropping #4 and queueing the rest for 2021.01 sounds good to me too. We can revisit a possible check for non-PIE later.
Cheers, Edgar
On 9/9/20 11:07 AM, Edgar E. Iglesias wrote:
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
This fixes some build issues with large (> 1MB) PIE U-Boot setups. We also document the 4K aligned load address requirement and add an early run-time check for it.
As requested by reviewers, I've also added a runtime check for non-PIE builds to trap the startup sequence early if the start address doesn't match between run-time and link-time.
The series,
Reviewed-by: Stephen Warren swarren@nvidia.com
Hi,
On 09. 09. 20 19:07, Edgar E. Iglesias wrote:
From: "Edgar E. Iglesias" edgar.iglesias@xilinx.com
This fixes some build issues with large (> 1MB) PIE U-Boot setups. We also document the 4K aligned load address requirement and add an early run-time check for it.
As requested by reviewers, I've also added a runtime check for non-PIE builds to trap the startup sequence early if the start address doesn't match between run-time and link-time.
Cheers, Edgar
ChangeLog: v2 -> v3:
- Add non-PIE start address (run vs link-time) check
- Move 4K aligment Kconfig help description
- Fix load of __bss_start
- Use x0 rather than x1 in PIE load-address check
- Add missing tabs
- Add load-address check for non-PIE
- U-boot -> U-Boot in a few places
- Tweak commit messages
v1 -> v2:
- Fix adr of _start in arch/arm/lib/crt0_64.S
- Add early check and bail out into a WFI loop when not 4K aligned
- Document the 4K alignement requirement in Kconfig
Edgar E. Iglesias (4): arm64: Mention 4K aligned load addresses in the PIE Kconfig help arm64: Trap PIE builds early if load address is not 4K aligned arm64: Add support for larger PIE U-Boot arm64: Trap non-PIE builds early if starting from wrong address
arch/arm/Kconfig | 4 ++-- arch/arm/cpu/armv8/start.S | 36 ++++++++++++++++++++++++++++++++++-- arch/arm/lib/crt0_64.S | 8 +++++++- 3 files changed, 43 insertions(+), 5 deletions(-)
Because these changes are made for supporting Xilinx SOCs 1-3 applied to my tree. Patch 4 is dropped as was agreed.
Thanks, Michal
participants (7)
-
André Przywara -
Edgar E. Iglesias -
Edgar E. Iglesias -
Michal Simek -
Michal Simek -
Stephen Warren -
Tom Rini