On 14 August 2017 at 15:42, Dave Martin [off-list ref] wrote:

On Mon, Aug 14, 2017 at 01:53:58PM +0100, Ard Biesheuvel wrote:

quoted

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.

quoted

Signed-off-by: Ard Biesheuvel <redacted>
---
 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