On 10/09/2020 13:38, Michal Simek wrote: > > > On 09. 09. 20 19:07, Edgar E. Iglesias wrote: >> From: "Edgar E. Iglesias" <edgar.igles...@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.igles...@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.
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 >
