All of lore.kernel.org
 help / color / mirror / Atom feed
From: ard.biesheuvel@linaro.org (Ard Biesheuvel)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 0/8] ARM: clean up PC-relative arithmetic
Date: Thu, 4 Aug 2016 09:17:04 +0200	[thread overview]
Message-ID: <CAKv+Gu-+ib-XuQfg8ZL4D7nSk5YRSYaCQyf+UdZPmg+5CxPmJw@mail.gmail.com> (raw)
In-Reply-To: <20160803181739.GL1041@n2100.armlinux.org.uk>

On 3 August 2016 at 20:17, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Wed, Aug 03, 2016 at 05:38:42PM +0200, Ard Biesheuvel wrote:
>> There are various places in the ARM kernel where the following pattern
>> is used to create a PC-relative reference that is valid even before the
>> MMU is on:
>>
>>      adr    rX, 1f
>>      ldr    rY, [rX]
>>      add    rX, rX, rY
>>      ...
>>   1: .long  <symbol> - .
>>
>> or
>>      adr    rX, 1f
>>      ldmia  rX, {rY .. rY+n}
>>      sub    rX, rX, rY
>>      add    rY+1, rY+1, rX
>>      add    rY+2, rY+2, rX
>>      ...
>>   1: .long  .
>>      .long  <symbolY>
>>      .long  <symbolY+1>
>>      ...
>>
>> Both cases can be greatly simplified by letting the linker do the
>> calculations for us. This series implements adr_l, ldr_l and str_l
>> macros, and uses them to simplify a couple of instances of the above
>> patterns.
>
> I don't buy that argument, sorry, and the argument is actually wrong.
> No, we're _not_ letting the linker do the calculations for us, we're
> letting the linker do _some_ of the calculation, but not all.
>
> What you're replacing the above with is stuff like (I guess, because
> I've no idea what this :pc_g0: notation is):
>
>         add     rX, pc, #(sym - . - 8) & 0xff
>         add     rX, rX, #(sym - . - 4) & 0xff00
>         add     rX, rX, #(sym - .) & 0xff0000
>
> which I think is a more complex (and less obvious) way to calculate it.
> It's also buggy when we end up with a relative offset greater than 16MB,
> which we have in multi-zImage kernels.
>

Even if you think this is a more complex way to calculate it, at least
it is encapsulated in a single macro instead of having similar but not
identical open coded instances all over the place.

As for the range: the ldr/str variants have 28 bits of range (2x
scaled 8 bit immediate for the adds and a single unscaled 12 bit
immediate for the ldr/str). The adr variant has 26 bits (3x scaled
immediate counting from bit 2) range for word aligned symbols, which
gives us +/- 64 MB, which should be plenty. The only pathological
outlier is allyesconfig, but that uses Thumb2 anyway.

The relocations documented here
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0044f/IHI0044F_aaelf.pdf

> So no, I don't like this at all
>

Noted

  reply	other threads:[~2016-08-04  7:17 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-03 15:38 [PATCH 0/8] ARM: clean up PC-relative arithmetic Ard Biesheuvel
2016-08-03 15:38 ` [PATCH 1/8] ARM: assembler: introduce adr_l, ldr_l and str_l macros Ard Biesheuvel
2016-08-03 16:49   ` Dave Martin
2016-08-04  7:40     ` Ard Biesheuvel
2016-08-04  9:44       ` Dave Martin
2016-08-04 10:34         ` Ard Biesheuvel
2016-08-04 11:22           ` Dave Martin
2016-08-04 12:16             ` Russell King - ARM Linux
2016-08-04 12:55               ` Dave Martin
2016-08-04 13:58               ` Ard Biesheuvel
2016-08-04  7:43     ` Ard Biesheuvel
2016-08-03 18:09   ` Russell King - ARM Linux
2016-08-04  7:40     ` Ard Biesheuvel
2016-08-04  9:38       ` Dave Martin
2016-08-04 10:12         ` Ard Biesheuvel
2016-08-04 11:08           ` Dave Martin
2016-08-04 11:10             ` Ard Biesheuvel
2016-08-04 11:30               ` Dave Martin
2016-08-04 11:34                 ` Ard Biesheuvel
2016-08-04 13:31                   ` Dave Martin
2016-08-04 13:46                     ` Ard Biesheuvel
2016-08-04 15:38                       ` Dave Martin
2016-08-03 15:38 ` [PATCH 2/8] ARM: head-common.S: use PC-relative insn sequence for __proc_info Ard Biesheuvel
2016-08-03 15:38 ` [PATCH 3/8] ARM: head-common.S: use PC-relative insn sequence for __turn_mmu_on Ard Biesheuvel
2016-08-03 15:38 ` [PATCH 4/8] ARM: head.S: use PC-relative insn sequence for secondary_data Ard Biesheuvel
2016-08-03 18:06   ` Russell King - ARM Linux
2016-08-03 15:38 ` [PATCH 5/8] ARM: head: use PC-relative insn sequence for __smp_alt Ard Biesheuvel
2016-08-03 15:38 ` [PATCH 6/8] ARM: sleep.S: use PC-relative insn sequence for sleep_save_sp/mpidr_hash Ard Biesheuvel
2016-08-03 15:38 ` [PATCH 7/8] ARM: head.S: use PC-relative insn sequences for __fixup_pv_table Ard Biesheuvel
2016-08-03 15:38 ` [PATCH 8/8] ARM: head.S: use PC relative insn sequence to calculate PHYS_OFFSET Ard Biesheuvel
2016-08-03 18:17 ` [PATCH 0/8] ARM: clean up PC-relative arithmetic Russell King - ARM Linux
2016-08-04  7:17   ` Ard Biesheuvel [this message]
2016-08-04  9:49     ` Russell King - ARM Linux
2016-08-04  9:54       ` Ard Biesheuvel
2016-08-04 10:03         ` Russell King - ARM Linux
2016-08-04 10:11           ` Ard Biesheuvel
2016-08-04 10:14             ` Russell King - ARM Linux
2016-08-03 18:27 ` Nicolas Pitre
2016-08-04 14:11   ` Ard Biesheuvel

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=CAKv+Gu-+ib-XuQfg8ZL4D7nSk5YRSYaCQyf+UdZPmg+5CxPmJw@mail.gmail.com \
    --to=ard.biesheuvel@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.