ARM: shmobile: Avoid writing to .text

Hi Simon, Magnus,

When CONFIG_DEBUG_RODATA=y, the kernel crashes during system suspend:

Freezing user space processes ... (elapsed 0.004 seconds) done. Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done.
PM: suspend of devices complete after 111.948 msecs
PM: late suspend of devices complete after 1.086 msecs
PM: noirq suspend of devices complete after 11.576 msecs Disabling non-boot CPUs ...
Kernel panic - not syncing: Attempted to kill the idle task! 1014ec ---[ end Kernel panic - not syncing: Attempted to kill the idle task!
CPU0: stopping

This happens because the shmobile assembler sources have several variables that are written to in the .text section, while .text is mapped read-only after kernel bootup if CONFIG_DEBUG_RODATA=y.

This series fixes this by moving variables from .text to .bss, or just removing them.
Note that there's still an issue with shmobile_boot_fn in arch/arm/mach-shmobile/headsmp.S. So far I didn't manage to fix this (the code and data are copied to SRAM on some SoCs). However, currently this is mostly harmless, as this is written to during early kernel boot up only, before .text is marked read-only. It does matter for XIP (anyone using that with SMP?), so we do want to fix that in the long run, too.

These issues were uncovered by "[PATCH v2] ARM: mm: flip priority of CONFIG_DEBUG_RODATA". As that patch is already queued in arm/for-next, I think all these fixes should be queued for v4.5, to avoid a dependency with the arm tree.

Changes compared to v2:
- Use .bss instead of .data, as requested by Nicolas Pitre <[email protected]>,
- Added Acked-by and Reviewed-by.

Changes compared to v1:
- Dropped "ARM: shmobile: Move shmobile_scu_base from .text to .data", which Simon has applied, and queued in soc-for-v4.6 (should be soc-for-v4.5?),
- Added Reviewed-by,
- Store offsets instead of pointers, as suggested by Nicolas Pitre <[email protected]>,
- Added "ARM: shmobile: Remove shmobile_boot_arg".

Tested hard on sh73a0/kzm9g, r8a7791/koelsch.
Tested lighter on emev2/kzm9d[*], r8a73a4/ape6evm, r8a7740/armadillo, and r8a7779/marzen[*] ([*] = no remote resume).

Thanks!

Geert Uytterhoeven (2):
ARM: shmobile: Move shmobile_smp_{mpidr,fn,arg}[] from .text to .bss
ARM: shmobile: Remove shmobile_boot_arg

 arch/arm/mach-shmobile/common.h       |  1 -
 arch/arm/mach-shmobile/headsmp.S      | 28 ++++++++++++++++------------
 arch/arm/mach-shmobile/platsmp-apmu.c |  1 -
 arch/arm/mach-shmobile/platsmp-scu.c  |  1 -
 4 files changed, 16 insertions(+), 15 deletions(-)

1.9.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

If CONFIG_DEBUG_RODATA=y, the kernel crashes during system suspend:

Freezing user space processes ... (elapsed 0.004 seconds) done. Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done.
PM: suspend of devices complete after 111.948 msecs
PM: late suspend of devices complete after 1.086 msecs
PM: noirq suspend of devices complete after 11.576 msecs Disabling non-boot CPUs ...
Kernel panic - not syncing: Attempted to kill the idle task! 1014ec ---[ end Kernel panic - not syncing: Attempted to kill the idle task!
CPU0: stopping

This happens because the .text section is marked read-only, while the arrays shmobile_smp_mpidr[], shmobile_smp_fn[], and shmobile_smp_arg[] are being written to.

Fix this by moving these arrays from the .text to the .bss section. This requires accessing them through PC-relative offsets.

---
v3:
- Use .bss instead of .data, as requested by Nicolas Pitre <[email protected]>,
- Add Reviewed-by,

v2:
- Add Reviewed-by,
- Store offsets instead of pointers, as suggested by Nicolas Pitre <[email protected]>.

---
 arch/arm/mach-shmobile/headsmp.S | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

index 330c1fc63197df89..94d86ed164144cdf 100644
@@ -50,9 +50,11 @@ ENTRY(shmobile_smp_boot)
 	mrc	p15, 0, r1, c0, c0, 5		@ r1 = MPIDR
 	and	r0, r1, r0			@ r0 = cpu_logical_map() value
 	mov	r1, #0				@ r1 = CPU index
-	adr	r5, 1f				@ array of per-cpu mpidr values
-	adr	r6, 2f				@ array of per-cpu functions
-	adr	r7, 3f				@ array of per-cpu arguments
+	adr	r2, 1f
+	ldmia	r2, {r5, r6, r7}
+	add	r5, r5, r2			@ array of per-cpu mpidr values
+	add	r6, r6, r2			@ array of per-cpu functions
+	add	r7, r7, r2			@ array of per-cpu arguments
 
 shmobile_smp_boot_find_mpidr:
 	ldr	r8, [r5, r1, lsl #2]

@@ -80,12 +82,18 @@ ENTRY(shmobile_smp_sleep)
 	b	shmobile_smp_boot
 ENDPROC(shmobile_smp_sleep)
 
+	.align	2
+1:	.long	shmobile_smp_mpidr - .
+	.long	shmobile_smp_fn - 1b
+	.long	shmobile_smp_arg - 1b
+
+	.bss
 	.globl	shmobile_smp_mpidr
 shmobile_smp_mpidr:
-1:	.space	NR_CPUS * 4
+	.space	NR_CPUS * 4
 	.globl	shmobile_smp_fn
 shmobile_smp_fn:
-2:	.space	NR_CPUS * 4
+	.space	NR_CPUS * 4
 	.globl	shmobile_smp_arg
 shmobile_smp_arg:
-3:	.space	NR_CPUS * 4
+	.space	NR_CPUS * 4

1.9.1

CPU boot configuration writes to shmobile_boot_arg, which is located in the .text section, and thus should not be written to.

As of commit 1d33a354bbb618ba ("ARM: shmobile: Per-CPU SMP boot / sleep code for SCU SoCs"), and ignoring accidental remainings, shmobile_boot_arg is always set to MPIDR_HWID_BITMASK by C code. Hence we can just hardcode this in the assembler code, and remove the variable, and thus also remove the need to write to this variable.

---
v3:
- Add Acked-by,

v2:
- New.

---
 arch/arm/mach-shmobile/common.h       | 1 -
 arch/arm/mach-shmobile/headsmp.S      | 8 ++------
 arch/arm/mach-shmobile/platsmp-apmu.c | 1 -
 arch/arm/mach-shmobile/platsmp-scu.c  | 1 -
 4 files changed, 2 insertions(+), 9 deletions(-)

index 225c12bb3de91e76..5464b7a75e3028a7 100644
@@ -4,7 +4,6 @@
 extern void shmobile_init_delay(void);
 extern void shmobile_boot_vector(void);
 extern unsigned long shmobile_boot_fn;
-extern unsigned long shmobile_boot_arg;
 extern unsigned long shmobile_boot_size;
 extern void shmobile_smp_boot(void);
 extern void shmobile_smp_sleep(void);

index 94d86ed164144cdf..32e0bf6e3ccb9bd3 100644
@@ -24,7 +24,6 @@
 	.arm
 	.align  12
 ENTRY(shmobile_boot_vector)
-	ldr     r0, 2f
 	ldr     r1, 1f
 	bx	r1
 

@@ -34,9 +33,6 @@ ENDPROC(shmobile_boot_vector)
 	.globl	shmobile_boot_fn
 shmobile_boot_fn:
 1:	.space	4
-	.globl	shmobile_boot_arg
-shmobile_boot_arg:
-2:	.space	4
 	.globl	shmobile_boot_size
 shmobile_boot_size:
 	.long	. - shmobile_boot_vector

@@ -46,9 +42,9 @@ shmobile_boot_size:
  */
 
 ENTRY(shmobile_smp_boot)
-						@ r0 = MPIDR_HWID_BITMASK
 	mrc	p15, 0, r1, c0, c0, 5		@ r1 = MPIDR
-	and	r0, r1, r0			@ r0 = cpu_logical_map() value
+	and	r0, r1, #0xffffff		@ MPIDR_HWID_BITMASK
+						@ r0 = cpu_logical_map() value
 	mov	r1, #0				@ r1 = CPU index
 	adr	r2, 1f
 	ldmia	r2, {r5, r6, r7}

index 911884f7e28b174c..aba75c89f9c1c5eb 100644
@@ -123,7 +123,6 @@ void __init shmobile_smp_apmu_prepare_cpus(unsigned int max_cpus,
 {
 	/* install boot code shared by all CPUs */
 	shmobile_boot_fn = virt_to_phys(shmobile_smp_boot);
-	shmobile_boot_arg = MPIDR_HWID_BITMASK;
 
 	/* perform per-cpu setup */
 	apmu_parse_cfg(apmu_init_cpu, apmu_config, num);

index 4c7d2caf3730f644..8d478f1da265d55e 100644
@@ -46,7 +46,6 @@ void __init shmobile_smp_scu_prepare_cpus(phys_addr_t scu_base_phys,
 {
 	/* install boot code shared by all CPUs */
 	shmobile_boot_fn = virt_to_phys(shmobile_smp_boot);
-	shmobile_boot_arg = MPIDR_HWID_BITMASK;
 
 	/* enable SCU and cache coherency on booting CPU */
 	shmobile_scu_base_phys = scu_base_phys;

1.9.1

Thanks Geert,

I have queued these up.