[PATCH] arm64: Add support for bigger u-boot when CONFIG_POSITION_INDEPENDENT=y

[Edgar E. Iglesias <edgar.iglesias@xilinx.com>]

On Thu, Sep 03, 2020 at 01:10:48PM -0600, Stephen Warren wrote:
> On 9/3/20 1:07 PM, Edgar E. Iglesias wrote:
> > On Thu, Sep 03, 2020 at 09:45:39AM -0600, Stephen Warren wrote:
> >> On 9/3/20 7:40 AM, Andr? Przywara wrote:
> >>> On 03/09/2020 14:35, Michal Simek wrote:
> >>>>
> >>>>
> >>>> On 02. 09. 20 18:34, Stephen Warren wrote:
> >>>>> On 9/2/20 5:15 AM, Michal Simek wrote:
> >>>>>> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >>>>>>
> >>>>>> When U-Boot binary exceeds 1MB with CONFIG_POSITION_INDEPENDENT=y
> >>>>>> compilation error is shown:
> >>>>>> /mnt/disk/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.
> >>>>>>
> >>>>>> It is caused by adr instruction which permits the calculation of any byte
> >>>>>> address within +- 1MB of the current PC.
> >>>>>> Because U-Boot is bigger then 1MB calculation is failing.
> >>>>>>
> >>>>>> The patch is using adrp/add instructions where adrp shifts a signed, 21-bit
> >>>>>> immediate left by 12 bits (4k page), adds it to the value of the program
> >>>>>> counter with the bottom 12 bits cleared to zero. Then add instruction
> >>>>>> provides the lower 12 bits which is offset within 4k page.
> >>>>>> These two instructions together compose full 32bit offset which should be
> >>>>>> more then enough to cover the whole u-boot size.
> >>>>>
> >>>>>> diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
> >>>>>
> >>>>>> @@ -67,8 +67,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 */
> >>>>>
> >>>>> There are likely a bunch of other places in the code that need updating
> >>>>> too; take a look at commit 49e93875a62f "arm64: support running at addr
> >>>>> other than linked to" (which introduced the code above) to find other
> >>>>> places with similar instruction sequences that almost certainly need
> >>>>> updating.
> >>>>>
> >>>>
> >>>> I didn't hit any issue to have a need to update them. Definitely worth
> >>>> to check that locations too.
> >>>
> >>> So I thought the same, so I checked at least this file. And while there
> >>> are other "adr" instructions, they only go to nearby labels, so don't
> >>> need to be pumped up.
> >>> But I will try to grep for more cases as well.
> >>
> >>
> >> So in the patch I linked to, what about the added ard instructions in
> >> arch/arm/lib/crt0_64.S and arch/arm/lib/relocate_64.S?
> >>
> >> Perhaps that code gets linked more towards the middle of U-Boot than the
> >> code you're fixing in start.S, so the max 1M offset just happens to
> >> reach all the relevant symbols and relocations that are in your current
> >> binary, but if your binary gets a little larger (e.g. goes from 1.05M to
> >> 2M say) that code will fail in the same way?
> > 
> > Yes, those were apparently already corrected by Ibai:
> > https://github.com/u-boot/u-boot/commit/98ffbb78e12646a1d06236ad6a1893217f255aae#diff-4f864f65dc6b6f2535a5d252b7c9fcc7
> 
> Ah OK.
> 
> So I guess this means that in practice, U-Boot already has the
> limitation that the load (and relocation target?) address must be
> 4K-aligned, since it uses the same instruction sequence we're
> discusssing here.

Apparently, yes.

Anyway, we'll submit a patch tomorrow with the early trapping if not
aligned.

Cheers,
Edgar