All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: u-boot@lists.denx.de
Subject: [PATCH] arm64: Add support for bigger u-boot when CONFIG_POSITION_INDEPENDENT=y
Date: Thu, 3 Sep 2020 13:10:48 -0600	[thread overview]
Message-ID: <a397cf7c-d17e-aab8-adf2-dbf144de5aef@wwwdotorg.org> (raw)
In-Reply-To: <20200903190746.GD14249@toto>

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.

  reply	other threads:[~2020-09-03 19:10 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-02 11:15 [PATCH] arm64: Add support for bigger u-boot when CONFIG_POSITION_INDEPENDENT=y Michal Simek
2020-09-02 14:43 ` André Przywara
2020-09-02 14:51   ` Michal Simek
2020-09-02 14:53   ` Edgar E. Iglesias
2020-09-02 15:18     ` André Przywara
2020-09-02 15:25       ` Edgar E. Iglesias
2020-09-02 18:59         ` André Przywara
2020-09-03 13:41           ` Michal Simek
2020-09-03 13:52             ` André Przywara
2020-09-03 13:59               ` Edgar E. Iglesias
2020-09-03 14:03                 ` Michal Simek
2020-09-02 16:34 ` Stephen Warren
2020-09-03 13:35   ` Michal Simek
2020-09-03 13:40     ` André Przywara
2020-09-03 15:45       ` Stephen Warren
2020-09-03 19:07         ` Edgar E. Iglesias
2020-09-03 19:10           ` Stephen Warren [this message]
2020-09-03 19:12             ` Edgar E. Iglesias

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a397cf7c-d17e-aab8-adf2-dbf144de5aef@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.