[PATCH 17/30] arm-soc: tegra: make sleep asm code runtime relocatable

[Ard Biesheuvel]

On 14 August 2017 at 15:42, Dave Martin <Dave.Martin at arm.com> wrote:
> On Mon, Aug 14, 2017 at 01:53:58PM +0100, Ard Biesheuvel wrote:
>> The PIE kernel build does not allow absolute references encoded in
>> movw/movt instruction pairs, so use our mov_l macro instead (which
>> will still use such a pair unless CONFIG_RELOCATABLE is defined)
>>
>> Also, avoid 32-bit absolute literals to refer to absolute symbols.
>> Instead, use a 16 bit reference so that PIE linker cannot get
>> confused whether the symbol reference is subject to relocation at
>> runtime.
>
> This sounds like we're papering over something.
>
> If the linker is "confused", that sounds like we are either abusing
> it somehow, or the linker is broken.
>

There is some ambiguity in how SHN_ABS symbols are treated in shared
libraries and PIE executables.

https://sourceware.org/ml/binutils/2012-05/msg00019.html

I haven't confirmed whether it actually causes problems in this
particular case, but it is safer (and not entirely inappropriate) to
use a 16-bit field for a quantity that can easily fit one.


>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
>> ---
>>  arch/arm/mach-tegra/sleep-tegra20.S | 22 ++++++++++++--------
>>  arch/arm/mach-tegra/sleep-tegra30.S |  6 +++---
>>  arch/arm/mach-tegra/sleep.S         |  4 ++--
>>  3 files changed, 18 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S
>> index 5c8e638ee51a..cab95de5c8f1 100644
>> --- a/arch/arm/mach-tegra/sleep-tegra20.S
>> +++ b/arch/arm/mach-tegra/sleep-tegra20.S
>> @@ -99,7 +99,7 @@ ENTRY(tegra20_cpu_shutdown)
>>       cmp     r0, #0
>>       reteq   lr                      @ must not be called for CPU 0
>>       mov32   r1, TEGRA_IRAM_RESET_BASE_VIRT
>> -     ldr     r2, =__tegra20_cpu1_resettable_status_offset
>> +     ldrh    r2, 0f
>>       mov     r12, #CPU_RESETTABLE
>>       strb    r12, [r1, r2]
>>
>> @@ -121,6 +121,7 @@ ENTRY(tegra20_cpu_shutdown)
>>       beq     .
>>       ret     lr
>>  ENDPROC(tegra20_cpu_shutdown)
>> +0:   .short  __tegra20_cpu1_resettable_status_offset
>>  #endif
>>
>>  #ifdef CONFIG_PM_SLEEP
>> @@ -181,6 +182,9 @@ ENTRY(tegra_pen_unlock)
>>       ret     lr
>>  ENDPROC(tegra_pen_unlock)
>>
>> +.L__tegra20_cpu1_resettable_status_offset:
>> +     .short  __tegra20_cpu1_resettable_status_offset
>> +
>>  /*
>>   * tegra20_cpu_clear_resettable(void)
>>   *
>> @@ -189,7 +193,7 @@ ENDPROC(tegra_pen_unlock)
>>   */
>>  ENTRY(tegra20_cpu_clear_resettable)
>>       mov32   r1, TEGRA_IRAM_RESET_BASE_VIRT
>
> Can we simply port mov32 to mov_l?  Or do we hit a problem with
> multiplatform kernels where mov_l may involve a literal pool entry
> (and does it matter)?
>

The only place where it matters is in code that lives in idmap.text,
since the relative reference will point to the ID mapped alias of a
section that is not covered by the ID ID map. I think we should be
able to use mov_l everywhere else, and it should do the right thing
for ordinary and PIE builds