[PATCH v3 4/4] arm64: Trap non-PIE builds early if starting from wrong address

[Tom Rini]

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 at 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 at 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".
> 
> 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?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200910/f310e4ad/attachment.sig>