Skip to contentSkip to conversation

ARM: move cpu_resume() to .text section

PATCH New Ard Biesheuvel 3 participants 9 messages Mar 24, 2015

Subsystems (3)
Arm (32-bit) Arm sub-architectures Arm Samsung Exynos
lore
Share & tools
lore

Ard Biesheuvel

author
#0 lore raw

cover ARM: move cpu_resume() to .text section

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(-)
Signature · 1 line
1.8.3.2
author
#1 lore raw

PATCH 1/3 ARM: move cpu_resume() to .text section

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 <[email protected]>
---
 arch/arm/kernel/sleep.S | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)
arch/arm/kernel/sleep.S +6 -9
index 31041b4a3f53..fb64c96cd801 100644
@@ -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
Signature · 1 line
1.8.3.2
#2 lore raw

PATCH 1/3 ARM: move cpu_resume() to .text section

Quoted 9 lines · 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 <[email protected]>

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

Reviewed-by Nicolas Pitre <[email protected]>
Quoted 51 lines
---
 arch/arm/kernel/sleep.S | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)
arch/arm/kernel/sleep.S +6 -9
index 31041b4a3f53..fb64c96cd801 100644
@@ -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
Signature · 1 line
1.8.3.2
#3 lore raw

PATCH 1/3 ARM: move cpu_resume() to .text section

In reply to Ard Biesheuvel
Quoted 9 lines · 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 <[email protected]>

(Tested on Vexpress TC2 with CPUIdle enabled)

Tested-by Sudeep Holla <[email protected]>

Regards,
Sudeep

author
#4 lore raw

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

In reply to 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 <[email protected]>
---
 arch/arm/mach-exynos/sleep.S | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)
arch/arm/mach-exynos/sleep.S +1 -8
index d3583773785e..98fba486fbe5 100644
@@ -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
 
 	/*
Signature · 1 line
1.8.3.2
#5 lore raw

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

Quoted 6 lines · 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 <[email protected]>

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

Quoted 28 lines
---
 arch/arm/mach-exynos/sleep.S | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)
arch/arm/mach-exynos/sleep.S +1 -8
index d3583773785e..98fba486fbe5 100644
@@ -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
 
 	/*
Signature · 1 line
1.8.3.2
author
#6 lore raw

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

Quoted 7 lines · Nicolas Pitre
Quoted 5 lines · 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.

Quoted 5 lines
Quoted 1 line
Signed-off-by Ard Biesheuvel <[email protected]>

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

Oops, missed that ...

Quoted 29 lines
Quoted 29 lines
---
 arch/arm/mach-exynos/sleep.S | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)
arch/arm/mach-exynos/sleep.S +1 -9
index d3583773785e..98fba486fbe5 100644
@@ -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

author
#7 lore raw

PATCH 3/3 ARM: s5pv210: move resume code to .text section

In reply to 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 <[email protected]>
---
 arch/arm/mach-s5pv210/sleep.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/mach-s5pv210/sleep.S +1 -1
index 7c43ddd33ba8..dfbfc0f7f8b8 100644
@@ -14,7 +14,7 @@
 
 #include <linux/linkage.h>
 
-	.data
+	.text
 	.align
 
 	/*
Signature · 1 line
1.8.3.2
#8 lore raw

PATCH 3/3 ARM: s5pv210: move resume code to .text section

Quoted 5 lines · 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 <[email protected]>
Acked-by Nicolas Pitre <[email protected]>
Quoted 21 lines
---
 arch/arm/mach-s5pv210/sleep.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/mach-s5pv210/sleep.S +1 -1
index 7c43ddd33ba8..dfbfc0f7f8b8 100644
@@ -14,7 +14,7 @@
 
 #include <linux/linkage.h>
 
-	.data
+	.text
 	.align
 
 	/*
Signature · 1 line
1.8.3.2