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

[Edgar E. Iglesias]

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 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?
> >
> 
> 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