Skip to contentSkip to conversation

ARM: shmobile: Move written variables from .text to .data

PATCH New Geert Uytterhoeven 7 participants 9 messages Jan 28, 2016

Subsystems (3)
lore
Share & tools
lore

Geert Uytterhoeven

author maintainer
#0 lore raw

cover ARM: shmobile: Move written variables from .text to .data

Hi Simon, Magnus,

When CONFIG_ARM_KERNMEM_PERMS=y and 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 .data. Note that there's still an issue with shmobile_boot_fn and shmobile_boot_arg 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 are harmless, as these are written during early kernel boot up only, before .text is marked read-only.

These issues were uncovered by "[PATCH v2] ARM: mm: flip priority of CONFIG_DEBUG_RODATA". As that patch is planned for queue up (again) soon, it may be a good idea to queue these fixes for v4.5, to avoid a dependency with the arm tree.

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_scu_base from .text to .data
ARM: shmobile: Move shmobile_smp_{mpidr,fn,arg}[] from .text to .data

 arch/arm/mach-shmobile/headsmp-scu.S |  6 ------
 arch/arm/mach-shmobile/headsmp.S     | 22 ++++++++++++++++------
 arch/arm/mach-shmobile/platsmp-scu.c |  3 +++
 3 files changed, 19 insertions(+), 12 deletions(-)
Signature · 12 lines
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
author maintainer
#1 lore raw

PATCH 1/2 ARM: shmobile: Move shmobile_scu_base from .text to .data

shmobile_scu_base is being written to, so it doesn't belong in the .text section. Fix this by moving it from asm .text to C .data, as it's no longer used from asm code since commit 4f6da36f7edd5790 ("ARM: shmobile:
Remove old SCU boot code").

---
 arch/arm/mach-shmobile/headsmp-scu.S | 6 ------
 arch/arm/mach-shmobile/platsmp-scu.c | 3 +++
 2 files changed, 3 insertions(+), 6 deletions(-)
arch/arm/mach-shmobile/headsmp-scu.S +0 -6
index fa5248c52399c9b5..5e503d91ad70ddf2 100644
@@ -38,9 +38,3 @@ ENTRY(shmobile_boot_scu)
 
 	b	secondary_startup
 ENDPROC(shmobile_boot_scu)
-
-	.text
-	.align	2
-	.globl	shmobile_scu_base
-shmobile_scu_base:
-	.space	4
arch/arm/mach-shmobile/platsmp-scu.c +3 -0
index 64663110ab6ca0e1..f2198c9c74359736 100644
@@ -17,6 +17,9 @@
 #include <asm/smp_scu.h>
 #include "common.h"
 
+
+void __iomem *shmobile_scu_base;
+
 static int shmobile_smp_scu_notifier_call(struct notifier_block *nfb,
 					  unsigned long action, void *hcpu)
 {
Signature · 1 line
1.9.1
#2 lore raw

PATCH 1/2 ARM: shmobile: Move shmobile_scu_base from .text to .data

Hello.

Quoted 6 lines · Geert Uytterhoeven

shmobile_scu_base is being written to, so it doesn't belong in the .text section. Fix this by moving it from asm .text to C .data, as it's no longer used from asm code since commit 4f6da36f7edd5790 ("ARM: shmobile:
Remove old SCU boot code").

[...]

arch/arm/mach-shmobile/platsmp-scu.c line 20

Quoted 9 lines
arch/arm/mach-shmobile/platsmp-scu.c +1 -0
index 64663110ab6ca0e1..f2198c9c74359736 100644
@@ -17,6 +17,9 @@
  #include <asm/smp_scu.h>
  #include "common.h"
 
+

Why add 2nd empty line?

arch/arm/mach-shmobile/platsmp-scu.c line 25

Quoted 5 lines
arch/arm/mach-shmobile/platsmp-scu.c +2 -0
+void __iomem *shmobile_scu_base;
+
  static int shmobile_smp_scu_notifier_call(struct notifier_block *nfb,
  					  unsigned long action, void *hcpu)
  {

MBR, Sergei

#3 lore raw

PATCH 1/2 ARM: shmobile: Move shmobile_scu_base from .text to .data

Quoted 9 lines · Sergei Shtylyov
Quoted 9 lines
arch/arm/mach-shmobile/platsmp-scu.c +0 -0
index 64663110ab6ca0e1..f2198c9c74359736 100644
@@ -17,6 +17,9 @@
  #include <asm/smp_scu.h>
  #include "common.h"
 
Quoted 3 lines

arch/arm/mach-shmobile/platsmp-scu.c line 20

Quoted 1 line
arch/arm/mach-shmobile/platsmp-scu.c +1 -0
+

Why add 2nd empty line?

I always leave two blank lines between different sections, and one blank line between function or data blocks.

Is that a problem?

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

author maintainer
#4 lore raw

PATCH 2/2 ARM: shmobile: Move shmobile_smp_{mpidr, fn, arg}[] from .text to .data

In reply to Geert Uytterhoeven

When CONFIG_ARM_KERNMEM_PERMS=y and 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 .data section. This requires accessing them through a pointer after virtual to physical translation.

---
 arch/arm/mach-shmobile/headsmp.S | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)
arch/arm/mach-shmobile/headsmp.S +16 -6
index 330c1fc63197df89..f924273ceaf476b4 100644
@@ -50,9 +50,12 @@ 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, {r4, r5, r6, r7}
+	sub	r2, r2, r4			@ virt->phys offset
+	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 +83,19 @@ ENTRY(shmobile_smp_sleep)
 	b	shmobile_smp_boot
 ENDPROC(shmobile_smp_sleep)
 
+	.align	2
+1:	.long	.
+	.long	shmobile_smp_mpidr
+	.long	shmobile_smp_fn
+	.long	shmobile_smp_arg
+
+	.data
 	.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
Signature · 1 line
1.9.1
#5 lore raw

PATCH 2/2 ARM: shmobile: Move shmobile_smp_{mpidr,fn,arg}[] from .text to .data

Quoted 34 lines · Geert Uytterhoeven

When CONFIG_ARM_KERNMEM_PERMS=y and 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 .data section. This requires accessing them through a pointer after virtual to physical translation.

---
 arch/arm/mach-shmobile/headsmp.S | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)
arch/arm/mach-shmobile/headsmp.S +0 -0
index 330c1fc63197df89..f924273ceaf476b4 100644
@@ -50,9 +50,12 @@ ENTRY(shmobile_smp_boot)
 	mrc	p15, 0, r1, c0, c0, 5		@ r1 = MPIDR
 	and	r0, r1, r0			@ r0 = cpu_logical_map() value

arch/arm/mach-shmobile/headsmp.S ENTRY(shmobile_smp_boot) · line 58

Quoted 10 lines
arch/arm/mach-shmobile/headsmp.S +6 -3
 	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, {r4, r5, r6, r7}
+	sub	r2, r2, r4			@ virt->phys offset
+	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

You could do something slightly more efficient like this:

	adr	r2, 1f
	ldmia	r2, {r5, r6, r7}
	add	r5, r5, r2
	add	r6, r6, r2
	add	r7, r7, r2

[...]

1: .long shmobile_smp_mpidr - .
.long shmobile_smp_fn - 1b
.long shmobile_smp_arg - 1b

Nicolas

#6 lore raw

cover ARM: shmobile: Move written variables from .text to .data

In reply to Geert Uytterhoeven
Quoted 39 lines · Geert Uytterhoeven

Hi Simon, Magnus,

When CONFIG_ARM_KERNMEM_PERMS=y and 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 .data. Note that there's still an issue with shmobile_boot_fn and shmobile_boot_arg 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 are harmless, as these are written during early kernel boot up only, before .text is marked read-only.

These issues were uncovered by "[PATCH v2] ARM: mm: flip priority of CONFIG_DEBUG_RODATA". As that patch is planned for queue up (again) soon, it may be a good idea to queue these fixes for v4.5, to avoid a dependency with the arm tree.

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

Thanks!

Quoted 8 lines

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

 arch/arm/mach-shmobile/headsmp-scu.S |  6 ------
 arch/arm/mach-shmobile/headsmp.S     | 22 ++++++++++++++++------
 arch/arm/mach-shmobile/platsmp-scu.c |  3 +++
 3 files changed, 19 insertions(+), 12 deletions(-)

Thanks!

And FWIW, I've now got the RODATA patch back in the patch queue, since we're at -rc1:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8501/1

-Kees

Quoted 14 lines

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

Signature · 2 lines
Kees Cook
Chrome OS & Brillo Security
#7 lore raw

cover ARM: shmobile: Move written variables from .text to .data

Quoted 50 lines · Kees Cook
Quoted 39 lines · Geert Uytterhoeven

Hi Simon, Magnus,

When CONFIG_ARM_KERNMEM_PERMS=y and 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 .data. Note that there's still an issue with shmobile_boot_fn and shmobile_boot_arg 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 are harmless, as these are written during early kernel boot up only, before .text is marked read-only.

These issues were uncovered by "[PATCH v2] ARM: mm: flip priority of CONFIG_DEBUG_RODATA". As that patch is planned for queue up (again) soon, it may be a good idea to queue these fixes for v4.5, to avoid a dependency with the arm tree.

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

Thanks!

Quoted 8 lines

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

 arch/arm/mach-shmobile/headsmp-scu.S |  6 ------
 arch/arm/mach-shmobile/headsmp.S     | 22 ++++++++++++++++------
 arch/arm/mach-shmobile/platsmp-scu.c |  3 +++
 3 files changed, 19 insertions(+), 12 deletions(-)
Quoted 7 lines

Thanks!

And FWIW, I've now got the RODATA patch back in the patch queue, since we're at -rc1:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8501/1

And I'll be dropping it into linux-next for next week (iow, I'll commit it tomorrow.)

Unfortunately, -rc1 broke some platforms, so Olof's builder isn't going to get full coverage of it while building only my tree.

Signature · 3 lines
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
#8 lore raw

cover ARM: shmobile: Move written variables from .text to .data

Quoted 11 lines · Russell King - ARM Linux
Quoted 4 lines · Kees Cook

And FWIW, I've now got the RODATA patch back in the patch queue, since we're at -rc1:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8501/1

And I'll be dropping it into linux-next for next week (iow, I'll commit it tomorrow.)

Unfortunately, -rc1 broke some platforms, so Olof's builder isn't going to get full coverage of it while building only my tree.

FYI I sent a pull request for the omap related fixes earlier today. So we should be good to go from that point of view for enabling rodata by default for v4.6.

Regards,

Tony