[PATCH 0/3] ARM: move cpu_resume() to .text section

[PATCH 0/3] ARM: move cpu_resume() to .text section

[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.

Ard Biesheuvel (3):
  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/kernel/sleep.S       | 15 ++++++---------
 arch/arm/mach-exynos/sleep.S  |  9 +--------
 arch/arm/mach-s5pv210/sleep.S |  2 +-
 3 files changed, 8 insertions(+), 18 deletions(-)

-- 
1.8.3.2

[PATCH 1/3] 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.

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 31041b4a3f53..fb64c96cd801 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 1/3] ARM: move cpu_resume() to .text section

[Nicolas Pitre]

On Tue, 24 Mar 2015, Ard Biesheuvel wrote:

> 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.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

I'm mystified.  I'm sure I did convert this code away from .data at some 
point.  Oh well...

Reviewed-by: Nicolas Pitre <nico@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 31041b4a3f53..fb64c96cd801 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 1/3] ARM: move cpu_resume() to .text section

[Sudeep Holla]

On 24/03/15 16:10, Ard Biesheuvel wrote:
> 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.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

(Tested on Vexpress TC2 with CPUIdle enabled)
Tested-by: Sudeep Holla <sudeep.holla@arm.com>

Regards,
Sudeep

[PATCH 2/3] 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. Note that all the .data references in
the code are already open coded PC relative references.

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

diff --git a/arch/arm/mach-exynos/sleep.S b/arch/arm/mach-exynos/sleep.S
index d3583773785e..98fba486fbe5 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
 
 	/*
-- 
1.8.3.2

[PATCH 2/3] 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. Note that all the .data references in
> the code are already open coded PC relative references.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

You forgot to adjust the code in exynos_cpu_resume_ns referencing 
cp15_save_power and cp15_save_diag.


> ---
>  arch/arm/mach-exynos/sleep.S | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/sleep.S b/arch/arm/mach-exynos/sleep.S
> index d3583773785e..98fba486fbe5 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
>  
>  	/*
> -- 
> 1.8.3.2
> 
> 

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

[Ard Biesheuvel]

On 24 March 2015 at 17:33, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> 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. Note that all the .data references in
>> the code are already open coded PC relative references.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> You forgot to adjust the code in exynos_cpu_resume_ns referencing
> cp15_save_power and cp15_save_diag.
>

Oops, missed that ...

>
>> ---
>>  arch/arm/mach-exynos/sleep.S | 9 +--------
>>  1 file changed, 1 insertion(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/sleep.S b/arch/arm/mach-exynos/sleep.S
>> index d3583773785e..98fba486fbe5 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
>>
>>       /*
>> --
>> 1.8.3.2
>>
>>

[PATCH 3/3] 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.

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 3/3] ARM: s5pv210: 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.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Acked-by: Nicolas Pitre <nico@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
> 
>