[PATCH v2 0/4] ARM veneer and .text/.data fixes for large kernels

[PATCH v2 0/4] ARM veneer and .text/.data fixes for large kernels

[Ard Biesheuvel]

A couple of the ARM sleep.S files put the resume code in the data
section. This was originally done for convenience, so that the
data it touches is always in range of a simple 'adr' PC relative
reference. However, code in the data section is problematic for
large kernels, where branched go out of range and the linker
is unaware that the contents of .data are in fact executable code
for which it could emit veneers.

So instead, move everything back to .text, and fix up the one
remaining straight adr reference by changing it into an explicit
PC relative reference.

Note that we could probably drop the b_far/bl_far patches if we
merge these patches instead.

Now includes the PIC veneer patch with Nico's ack added.

Ard Biesheuvel (4):
  ARM: force linker to use PIC veneers
  ARM: move cpu_resume() to .text section
  ARM: exynos: move resume code to .text section
  ARM: s5pv210: move resume code to .text section

 arch/arm/Makefile             |  2 +-
 arch/arm/kernel/sleep.S       | 15 ++++++---------
 arch/arm/mach-exynos/sleep.S  | 23 +++++++++++++----------
 arch/arm/mach-s5pv210/sleep.S |  2 +-
 4 files changed, 21 insertions(+), 21 deletions(-)

-- 
1.8.3.2

[PATCH v2 1/4] ARM: force linker to use PIC veneers

[Ard Biesheuvel]

When building a very large kernel, it is up to the linker to decide
when and where to insert stubs to allow calls to functions that are
out of range for the ordinary b/bl instructions.

However, since the kernel is built as a position dependent binary,
these stubs (aka veneers) may contain absolute addresses, which will
break far calls performed with the MMU off.

For instance, the call from __enable_mmu() in the .head.text section
to __turn_mmu_on() in the .idmap.text section may be turned into
something like this:

c0008168 <__enable_mmu>:
c0008168:       f020 0002       bic.w   r0, r0, #2
c000816c:       f420 5080       bic.w   r0, r0, #4096
c0008170:       f000 b846       b.w     c0008200 <____turn_mmu_on_veneer>
[...]
c0008200 <____turn_mmu_on_veneer>:
c0008200:       4778            bx      pc
c0008202:       46c0            nop
c0008204:       e59fc000        ldr     ip, [pc]
c0008208:       e12fff1c        bx      ip
c000820c:       c13dfae1        teqgt   sp, r1, ror #21
[...]
c13dfae0 <__turn_mmu_on>:
c13dfae0:       4600            mov     r0, r0
[...]

After adding --pic-veneer to the LDFLAGS, the veneer is emitted like
this instead:

c0008200 <____turn_mmu_on_veneer>:
c0008200:       4778            bx      pc
c0008202:       46c0            nop
c0008204:       e59fc004        ldr     ip, [pc, #4]
c0008208:       e08fc00c        add     ip, pc, ip
c000820c:       e12fff1c        bx      ip
c0008210:       013d7d31        teqeq   sp, r1, lsr sp
c0008214:       00000000        andeq   r0, r0, r0

Note that this particular example is best addressed by moving
.head.text and .idmap.text closer together, but this issue could
potentially affect any code that needs to execute with the
MMU off.

Acked-by: Nicolas Pitre <nico@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index eb7bb511f853..ae5b33527f32 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -13,7 +13,7 @@
 # Ensure linker flags are correct
 LDFLAGS		:=
 
-LDFLAGS_vmlinux	:=-p --no-undefined -X
+LDFLAGS_vmlinux	:=-p --no-undefined -X --pic-veneer
 ifeq ($(CONFIG_CPU_ENDIAN_BE8),y)
 LDFLAGS_vmlinux	+= --be8
 LDFLAGS_MODULE	+= --be8
-- 
1.8.3.2

[PATCH v2 2/4] ARM: move cpu_resume() to .text section

[Ard Biesheuvel]

Move cpu_resume() to the .text section where it belongs. Change
the adr reference to sleep_save_sp to an explicit PC relative
reference so sleep_save_sp itself can remain in .data.

This helps prevent linker failure on large kernels, as the code
in the .data section may be too far away to be in range for normal
b/bl instructions.

Reviewed-by: Nicolas Pitre <nico@linaro.org>
Tested-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/kernel/sleep.S | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/arch/arm/kernel/sleep.S b/arch/arm/kernel/sleep.S
index 0ea3813fedce..fabdb14e88b8 100644
--- a/arch/arm/kernel/sleep.S
+++ b/arch/arm/kernel/sleep.S
@@ -116,14 +116,7 @@ cpu_resume_after_mmu:
 	ldmfd	sp!, {r4 - r11, pc}
 ENDPROC(cpu_resume_after_mmu)
 
-/*
- * Note: Yes, part of the following code is located into the .data section.
- *       This is to allow sleep_save_sp to be accessed with a relative load
- *       while we can't rely on any MMU translation.  We could have put
- *       sleep_save_sp in the .text section as well, but some setups might
- *       insist on it to be truly read-only.
- */
-	.data
+	.text
 	.align
 ENTRY(cpu_resume)
 ARM_BE8(setend be)			@ ensure we are in BE mode
@@ -145,6 +138,8 @@ ARM_BE8(setend be)			@ ensure we are in BE mode
 	compute_mpidr_hash	r1, r4, r5, r6, r0, r3
 1:
 	adr	r0, _sleep_save_sp
+	ldr	r2, [r0]
+	add	r0, r0, r2
 	ldr	r0, [r0, #SLEEP_SAVE_SP_PHYS]
 	ldr	r0, [r0, r1, lsl #2]
 
@@ -156,10 +151,12 @@ THUMB(	bx	r3			)
 ENDPROC(cpu_resume)
 
 	.align 2
+_sleep_save_sp:
+	.long	sleep_save_sp - .
 mpidr_hash_ptr:
 	.long	mpidr_hash - .			@ mpidr_hash struct offset
 
+	.data
 	.type	sleep_save_sp, #object
 ENTRY(sleep_save_sp)
-_sleep_save_sp:
 	.space	SLEEP_SAVE_SP_SZ		@ struct sleep_save_sp
-- 
1.8.3.2

[PATCH v2 3/4] ARM: exynos: move resume code to .text section

[Ard Biesheuvel]

This code calls cpu_resume() using a straight branch (b), so
it has no business being in the .data section, especially now
we moved cpu_resume() back to .text as well.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/mach-exynos/sleep.S | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-exynos/sleep.S b/arch/arm/mach-exynos/sleep.S
index 31d25834b9c4..fe5d9ed24ac2 100644
--- a/arch/arm/mach-exynos/sleep.S
+++ b/arch/arm/mach-exynos/sleep.S
@@ -23,14 +23,7 @@
 #define CPU_MASK	0xff0ffff0
 #define CPU_CORTEX_A9	0x410fc090
 
-	/*
-	 * The following code is located into the .data section. This is to
-	 * allow l2x0_regs_phys to be accessed with a relative load while we
-	 * can't rely on any MMU translation. We could have put l2x0_regs_phys
-	 * in the .text section as well, but some setups might insist on it to
-	 * be truly read-only. (Reference from: arch/arm/kernel/sleep.S)
-	 */
-	.data
+	.text
 	.align
 
 	/*
@@ -69,10 +62,12 @@ ENTRY(exynos_cpu_resume_ns)
 	cmp	r0, r1
 	bne	skip_cp15
 
-	adr	r0, cp15_save_power
+	adr	r0, _cp15_save_power
 	ldr	r1, [r0]
-	adr	r0, cp15_save_diag
+	ldr	r1, [r0, r1]
+	adr	r0, _cp15_save_diag
 	ldr	r2, [r0]
+	ldr	r2, [r0, r2]
 	mov	r0, #SMC_CMD_C15RESUME
 	dsb
 	smc	#0
@@ -118,6 +113,13 @@ skip_l2x0:
 skip_cp15:
 	b	cpu_resume
 ENDPROC(exynos_cpu_resume_ns)
+
+_cp15_save_power:
+	.long	cp15_save_power - .
+_cp15_save_diag:
+	.long	cp15_save_diag - .
+
+	.data
 	.globl cp15_save_diag
 cp15_save_diag:
 	.long	0	@ cp15 diagnostic
@@ -126,6 +128,7 @@ cp15_save_power:
 	.long	0	@ cp15 power control
 
 #ifdef CONFIG_CACHE_L2X0
+	.text
 	.align
 1:	.long	l2x0_saved_regs - .
 #endif /* CONFIG_CACHE_L2X0 */
-- 
1.8.3.2

[PATCH v2 4/4] ARM: s5pv210: move resume code to .text section

[Ard Biesheuvel]

This code calls cpu_resume() using a straight branch (b), so
now that we have moved cpu_resume() back to .text, this should
be moved there as well.

Acked-by: Nicolas Pitre <nico@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/mach-s5pv210/sleep.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-s5pv210/sleep.S b/arch/arm/mach-s5pv210/sleep.S
index 7c43ddd33ba8..dfbfc0f7f8b8 100644
--- a/arch/arm/mach-s5pv210/sleep.S
+++ b/arch/arm/mach-s5pv210/sleep.S
@@ -14,7 +14,7 @@
 
 #include <linux/linkage.h>
 
-	.data
+	.text
 	.align
 
 	/*
-- 
1.8.3.2

[PATCH v2 3/4] ARM: exynos: move resume code to .text section

[Nicolas Pitre]

On Tue, 24 Mar 2015, Ard Biesheuvel wrote:

> This code calls cpu_resume() using a straight branch (b), so
> it has no business being in the .data section, especially now
> we moved cpu_resume() back to .text as well.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm/mach-exynos/sleep.S | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/sleep.S b/arch/arm/mach-exynos/sleep.S
> index 31d25834b9c4..fe5d9ed24ac2 100644
> --- a/arch/arm/mach-exynos/sleep.S
> +++ b/arch/arm/mach-exynos/sleep.S
> @@ -23,14 +23,7 @@
>  #define CPU_MASK	0xff0ffff0
>  #define CPU_CORTEX_A9	0x410fc090
>  
> -	/*
> -	 * The following code is located into the .data section. This is to
> -	 * allow l2x0_regs_phys to be accessed with a relative load while we
> -	 * can't rely on any MMU translation. We could have put l2x0_regs_phys
> -	 * in the .text section as well, but some setups might insist on it to
> -	 * be truly read-only. (Reference from: arch/arm/kernel/sleep.S)
> -	 */
> -	.data
> +	.text
>  	.align
>  
>  	/*
> @@ -69,10 +62,12 @@ ENTRY(exynos_cpu_resume_ns)
>  	cmp	r0, r1
>  	bne	skip_cp15
>  
> -	adr	r0, cp15_save_power
> +	adr	r0, _cp15_save_power
>  	ldr	r1, [r0]
> -	adr	r0, cp15_save_diag
> +	ldr	r1, [r0, r1]
> +	adr	r0, _cp15_save_diag
>  	ldr	r2, [r0]
> +	ldr	r2, [r0, r2]
>  	mov	r0, #SMC_CMD_C15RESUME
>  	dsb
>  	smc	#0
> @@ -118,6 +113,13 @@ skip_l2x0:
>  skip_cp15:
>  	b	cpu_resume
>  ENDPROC(exynos_cpu_resume_ns)
> +

A .align would be needed here in case this is compiled as Thumb2.

> +_cp15_save_power:
> +	.long	cp15_save_power - .
> +_cp15_save_diag:
> +	.long	cp15_save_diag - .
> +
> +	.data
>  	.globl cp15_save_diag
>  cp15_save_diag:
>  	.long	0	@ cp15 diagnostic
> @@ -126,6 +128,7 @@ cp15_save_power:
>  	.long	0	@ cp15 power control
>  
>  #ifdef CONFIG_CACHE_L2X0
> +	.text
>  	.align
>  1:	.long	l2x0_saved_regs - .
>  #endif /* CONFIG_CACHE_L2X0 */

This looks rather weird. I'd suggest you move this before the added 
.data instead.


Nicolas

[PATCH v3] ARM: exynos: move resume code to .text section

[Ard Biesheuvel]

This code calls cpu_resume() using a straight branch (b), so
now that we have moved cpu_resume() back to .text, this should
be moved there as well. Any direct references to symbols that will
remain in the .data section are replaced with explicit PC-relative
references.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v3: add missing .align, regroup changes to be more logical
v2: keep cp15_save_power and cp15_save_diag in the .data section

 arch/arm/mach-exynos/sleep.S | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-exynos/sleep.S b/arch/arm/mach-exynos/sleep.S
index 31d25834b9c4..cf950790fbdc 100644
--- a/arch/arm/mach-exynos/sleep.S
+++ b/arch/arm/mach-exynos/sleep.S
@@ -23,14 +23,7 @@
 #define CPU_MASK	0xff0ffff0
 #define CPU_CORTEX_A9	0x410fc090
 
-	/*
-	 * The following code is located into the .data section. This is to
-	 * allow l2x0_regs_phys to be accessed with a relative load while we
-	 * can't rely on any MMU translation. We could have put l2x0_regs_phys
-	 * in the .text section as well, but some setups might insist on it to
-	 * be truly read-only. (Reference from: arch/arm/kernel/sleep.S)
-	 */
-	.data
+	.text
 	.align
 
 	/*
@@ -69,10 +62,12 @@ ENTRY(exynos_cpu_resume_ns)
 	cmp	r0, r1
 	bne	skip_cp15
 
-	adr	r0, cp15_save_power
+	adr	r0, _cp15_save_power
 	ldr	r1, [r0]
-	adr	r0, cp15_save_diag
+	ldr	r1, [r0, r1]
+	adr	r0, _cp15_save_diag
 	ldr	r2, [r0]
+	ldr	r2, [r0, r2]
 	mov	r0, #SMC_CMD_C15RESUME
 	dsb
 	smc	#0
@@ -118,14 +113,20 @@ skip_l2x0:
 skip_cp15:
 	b	cpu_resume
 ENDPROC(exynos_cpu_resume_ns)
+
+	.align
+_cp15_save_power:
+	.long	cp15_save_power - .
+_cp15_save_diag:
+	.long	cp15_save_diag - .
+#ifdef CONFIG_CACHE_L2X0
+1:	.long	l2x0_saved_regs - .
+#endif /* CONFIG_CACHE_L2X0 */
+
+	.data
 	.globl cp15_save_diag
 cp15_save_diag:
 	.long	0	@ cp15 diagnostic
 	.globl cp15_save_power
 cp15_save_power:
 	.long	0	@ cp15 power control
-
-#ifdef CONFIG_CACHE_L2X0
-	.align
-1:	.long	l2x0_saved_regs - .
-#endif /* CONFIG_CACHE_L2X0 */
-- 
1.8.3.2

[PATCH v3] ARM: exynos: move resume code to .text section

[Nicolas Pitre]

On Tue, 24 Mar 2015, Ard Biesheuvel wrote:

> This code calls cpu_resume() using a straight branch (b), so
> now that we have moved cpu_resume() back to .text, this should
> be moved there as well. Any direct references to symbols that will
> remain in the .data section are replaced with explicit PC-relative
> references.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Acked-by: Nicolas Pitre <nico@linaro.org>

> ---
> v3: add missing .align, regroup changes to be more logical
> v2: keep cp15_save_power and cp15_save_diag in the .data section
> 
>  arch/arm/mach-exynos/sleep.S | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/sleep.S b/arch/arm/mach-exynos/sleep.S
> index 31d25834b9c4..cf950790fbdc 100644
> --- a/arch/arm/mach-exynos/sleep.S
> +++ b/arch/arm/mach-exynos/sleep.S
> @@ -23,14 +23,7 @@
>  #define CPU_MASK	0xff0ffff0
>  #define CPU_CORTEX_A9	0x410fc090
>  
> -	/*
> -	 * The following code is located into the .data section. This is to
> -	 * allow l2x0_regs_phys to be accessed with a relative load while we
> -	 * can't rely on any MMU translation. We could have put l2x0_regs_phys
> -	 * in the .text section as well, but some setups might insist on it to
> -	 * be truly read-only. (Reference from: arch/arm/kernel/sleep.S)
> -	 */
> -	.data
> +	.text
>  	.align
>  
>  	/*
> @@ -69,10 +62,12 @@ ENTRY(exynos_cpu_resume_ns)
>  	cmp	r0, r1
>  	bne	skip_cp15
>  
> -	adr	r0, cp15_save_power
> +	adr	r0, _cp15_save_power
>  	ldr	r1, [r0]
> -	adr	r0, cp15_save_diag
> +	ldr	r1, [r0, r1]
> +	adr	r0, _cp15_save_diag
>  	ldr	r2, [r0]
> +	ldr	r2, [r0, r2]
>  	mov	r0, #SMC_CMD_C15RESUME
>  	dsb
>  	smc	#0
> @@ -118,14 +113,20 @@ skip_l2x0:
>  skip_cp15:
>  	b	cpu_resume
>  ENDPROC(exynos_cpu_resume_ns)
> +
> +	.align
> +_cp15_save_power:
> +	.long	cp15_save_power - .
> +_cp15_save_diag:
> +	.long	cp15_save_diag - .
> +#ifdef CONFIG_CACHE_L2X0
> +1:	.long	l2x0_saved_regs - .
> +#endif /* CONFIG_CACHE_L2X0 */
> +
> +	.data
>  	.globl cp15_save_diag
>  cp15_save_diag:
>  	.long	0	@ cp15 diagnostic
>  	.globl cp15_save_power
>  cp15_save_power:
>  	.long	0	@ cp15 power control
> -
> -#ifdef CONFIG_CACHE_L2X0
> -	.align
> -1:	.long	l2x0_saved_regs - .
> -#endif /* CONFIG_CACHE_L2X0 */
> -- 
> 1.8.3.2
> 
>