[PATCH] ARM: Fix memblock_reserve() to include stext

[PATCH] ARM: Fix memblock_reserve() to include stext

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

This fix for the ARM architecture will include stext
in the memblock_reserve() call to make sure that the
following symbols are not overwritten (from System.map):

c0008000 T _text
c0008000 T stext
...
c0008138 T secondary_startup
...
c0009000 T _stext

With this patch applied CPU Hotplug starts working
again. Without this patch code in secondary_startup
never gets reached as expected.

This issue started triggering on kernels later than
v3.10 - perhaps a side effect of the CPU Hotplug init
section rework - so this is a fix for v3.11-rc.

Tested on the sh73a0 KZM9G board with CPU Hotplug:

 # echo 0 > /sys/devices/system/cpu/cpu1/online
 CPU1: shutdown
 # echo 1 > /sys/devices/system/cpu/cpu1/online
 CPU1: Booted secondary processor

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 arch/arm/mm/init.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- 0001/arch/arm/mm/init.c
+++ work/arch/arm/mm/init.c	2013-07-31 14:17:49.000000000 +0900
@@ -346,7 +346,7 @@ void __init arm_memblock_init(struct mem
 #ifdef CONFIG_XIP_KERNEL
 	memblock_reserve(__pa(_sdata), _end - _sdata);
 #else
-	memblock_reserve(__pa(_stext), _end - _stext);
+	memblock_reserve(__pa(_text), _end - _text);
 #endif
 #ifdef CONFIG_BLK_DEV_INITRD
 	if (phys_initrd_size &&

[PATCH] ARM: Fix memblock_reserve() to include stext

[Russell King - ARM Linux]

On Wed, Jul 31, 2013 at 02:58:07PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> This fix for the ARM architecture will include stext
> in the memblock_reserve() call to make sure that the
> following symbols are not overwritten (from System.map):
> 
> c0008000 T _text
> c0008000 T stext
> ...
> c0008138 T secondary_startup
> ...
> c0009000 T _stext
> 
> With this patch applied CPU Hotplug starts working
> again. Without this patch code in secondary_startup
> never gets reached as expected.
> 
> This issue started triggering on kernels later than
> v3.10 - perhaps a side effect of the CPU Hotplug init
> section rework - so this is a fix for v3.11-rc.

This is not the right fix.  The commit you previously pointed out (removal
of CPU hotplug) probably means there's a missing annotation somewhere in
the code.

And yes - the __CPUINIT just before ENTRY(secondary_startup) was removed
rather than being replaced with a .text (in both head.S and head-nommu.S).
Same goes for ENTRY(lookup_processor_type) in head-common.S.  All these
need to be replaced with .text to stop them falling into the head section.

[PATCH] arm: add .text at __CPUINIT places to prevent section grandfathering

[Paul Gortmaker]

In commit 22f0a27367742f65130c0fb25ef00f7297e032c1
("init.h: remove __cpuinit sections from the kernel") the macros
like __CPUINIT were converted from .section commands to no-ops.

This caused CPU hotplug regressions because the code following
the __CPUINIT lines was now grandfathered into whatever the
previous section happened to be.  In most files, __CPUINIT was
at the top of the file, so there was no previous section.

However in the files changed here, there was a previous section,
and so we'd inadvertently garbage collect CPU hotplug related
code.  This changeset inserts ".text" into the exact same spot
that the __CPUINIT section markers were found prior to commit
8bd26e3a7e49af2697449bbcb7187a39dc85d672 ("arm: delete
__cpuinit/__CPUINIT usage from all ARM users:) in the three affected
files, as suggested by Russell.

Bisected-by: Simon Horman <horms@verge.net.au>
Reported-by: Magnus Damm <magnus.damm@gmail.com>
Reported-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---

[NB: Untested - I've not got an SMP ARM handy at the moment]

 arch/arm/kernel/head-common.S | 1 +
 arch/arm/kernel/head-nommu.S  | 1 +
 arch/arm/kernel/head.S        | 1 +
 3 files changed, 3 insertions(+)

diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
index 47cd974..def853e 100644
--- a/arch/arm/kernel/head-common.S
+++ b/arch/arm/kernel/head-common.S
@@ -149,6 +149,7 @@ ENDPROC(lookup_processor_type)
  *	r5 = proc_info pointer in physical address space
  *	r9 = cpuid (preserved)
  */
+	.text
 __lookup_processor_type:
 	adr	r3, __lookup_processor_type_data
 	ldmia	r3, {r4 - r6}
diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
index b361de1..14235ba 100644
--- a/arch/arm/kernel/head-nommu.S
+++ b/arch/arm/kernel/head-nommu.S
@@ -87,6 +87,7 @@ ENTRY(stext)
 ENDPROC(stext)
 
 #ifdef CONFIG_SMP
+	.text
 ENTRY(secondary_startup)
 	/*
 	 * Common entry point for secondary CPUs.
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 9cf6063..2c7cc1e 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -343,6 +343,7 @@ __turn_mmu_on_loc:
 	.long	__turn_mmu_on_end
 
 #if defined(CONFIG_SMP)
+	.text
 ENTRY(secondary_startup)
 	/*
 	 * Common entry point for secondary CPUs.
-- 
1.8.1.2

[PATCH] arm: add .text at __CPUINIT places to prevent section grandfathering

[Russell King - ARM Linux]

On Wed, Jul 31, 2013 at 01:06:58PM -0400, Paul Gortmaker wrote:
> In commit 22f0a27367742f65130c0fb25ef00f7297e032c1
> ("init.h: remove __cpuinit sections from the kernel") the macros
> like __CPUINIT were converted from .section commands to no-ops.
> 
> This caused CPU hotplug regressions because the code following
> the __CPUINIT lines was now grandfathered into whatever the
> previous section happened to be.  In most files, __CPUINIT was
> at the top of the file, so there was no previous section.
> 
> However in the files changed here, there was a previous section,
> and so we'd inadvertently garbage collect CPU hotplug related
> code.  This changeset inserts ".text" into the exact same spot
> that the __CPUINIT section markers were found prior to commit
> 8bd26e3a7e49af2697449bbcb7187a39dc85d672 ("arm: delete
> __cpuinit/__CPUINIT usage from all ARM users:) in the three affected
> files, as suggested by Russell.
> 
> Bisected-by: Simon Horman <horms@verge.net.au>
> Reported-by: Magnus Damm <magnus.damm@gmail.com>
> Reported-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

I already have this in my git tree as of this morning, thanks anyway.

[PATCH] ARM: Fix memblock_reserve() to include stext

[Magnus Damm]

On Wed, Jul 31, 2013 at 6:43 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jul 31, 2013 at 02:58:07PM +0900, Magnus Damm wrote:
>> From: Magnus Damm <damm@opensource.se>
>>
>> This fix for the ARM architecture will include stext
>> in the memblock_reserve() call to make sure that the
>> following symbols are not overwritten (from System.map):
>>
>> c0008000 T _text
>> c0008000 T stext
>> ...
>> c0008138 T secondary_startup
>> ...
>> c0009000 T _stext
>>
>> With this patch applied CPU Hotplug starts working
>> again. Without this patch code in secondary_startup
>> never gets reached as expected.
>>
>> This issue started triggering on kernels later than
>> v3.10 - perhaps a side effect of the CPU Hotplug init
>> section rework - so this is a fix for v3.11-rc.
>
> This is not the right fix.  The commit you previously pointed out (removal
> of CPU hotplug) probably means there's a missing annotation somewhere in
> the code.
>
> And yes - the __CPUINIT just before ENTRY(secondary_startup) was removed
> rather than being replaced with a .text (in both head.S and head-nommu.S).
> Same goes for ENTRY(lookup_processor_type) in head-common.S.  All these
> need to be replaced with .text to stop them falling into the head section.

Thanks for your guidance, Russell. The right way to fix this now seems
pretty clear to me.

Are you aware of any existing fix for this or anyone working on
solving this issue? If not then I don't mind stepping up. I can
however think of quite a few other things to do if this has been
solved already though, so please let me know if you think I should
focus on fixing this.

Cheers,

/ magnus

[PATCH] ARM: Fix memblock_reserve() to include stext

[Russell King - ARM Linux]

On Thu, Aug 01, 2013 at 04:34:03AM +0900, Magnus Damm wrote:
> Are you aware of any existing fix for this or anyone working on
> solving this issue? If not then I don't mind stepping up. I can
> however think of quite a few other things to do if this has been
> solved already though, so please let me know if you think I should
> focus on fixing this.

I committed a fix for it this morning, so it should be in linux-next by
tomorrow.

[PATCH] ARM: Fix memblock_reserve() to include stext

[Simon Horman]

On Wed, Jul 31, 2013 at 08:39:30PM +0100, Russell King - ARM Linux wrote:
> On Thu, Aug 01, 2013 at 04:34:03AM +0900, Magnus Damm wrote:
> > Are you aware of any existing fix for this or anyone working on
> > solving this issue? If not then I don't mind stepping up. I can
> > however think of quite a few other things to do if this has been
> > solved already though, so please let me know if you think I should
> > focus on fixing this.
> 
> I committed a fix for it this morning, so it should be in linux-next by
> tomorrow.

Hi Russell,

Magnus has some outstanding patches for Renesas SoC boards which I believe
depend on this problem being resolved.  Could you indicate when you expect
this patch to be in a stable branch which I could use as a base for a
branch that will subsequently be pulled into arm-soc.

Thanks.

[PATCH] ARM: Fix memblock_reserve() to include stext

[Paul Gortmaker]

On 13-08-01 03:34 AM, Simon Horman wrote:
> On Wed, Jul 31, 2013 at 08:39:30PM +0100, Russell King - ARM Linux wrote:
>> On Thu, Aug 01, 2013 at 04:34:03AM +0900, Magnus Damm wrote:
>>> Are you aware of any existing fix for this or anyone working on
>>> solving this issue? If not then I don't mind stepping up. I can
>>> however think of quite a few other things to do if this has been
>>> solved already though, so please let me know if you think I should
>>> focus on fixing this.
>>
>> I committed a fix for it this morning, so it should be in linux-next by
>> tomorrow.
> 
> Hi Russell,
> 
> Magnus has some outstanding patches for Renesas SoC boards which I believe
> depend on this problem being resolved.  Could you indicate when you expect
> this patch to be in a stable branch which I could use as a base for a
> branch that will subsequently be pulled into arm-soc.

It will never be in a stable release, since the cpuinit removal was
added in the 3.11-rc1 merge window content, and the fix will be in
before 3.11 final hits the streets.  If you want stable stuff, you
should be using v3.10 at this point in time.

Paul.
--

> 
> Thanks.
> 

[PATCH] ARM: Fix memblock_reserve() to include stext

[Simon Horman]

On Thu, Aug 01, 2013 at 08:58:30AM -0400, Paul Gortmaker wrote:
> On 13-08-01 03:34 AM, Simon Horman wrote:
> > On Wed, Jul 31, 2013 at 08:39:30PM +0100, Russell King - ARM Linux wrote:
> >> On Thu, Aug 01, 2013 at 04:34:03AM +0900, Magnus Damm wrote:
> >>> Are you aware of any existing fix for this or anyone working on
> >>> solving this issue? If not then I don't mind stepping up. I can
> >>> however think of quite a few other things to do if this has been
> >>> solved already though, so please let me know if you think I should
> >>> focus on fixing this.
> >>
> >> I committed a fix for it this morning, so it should be in linux-next by
> >> tomorrow.
> > 
> > Hi Russell,
> > 
> > Magnus has some outstanding patches for Renesas SoC boards which I believe
> > depend on this problem being resolved.  Could you indicate when you expect
> > this patch to be in a stable branch which I could use as a base for a
> > branch that will subsequently be pulled into arm-soc.
> 
> It will never be in a stable release, since the cpuinit removal was
> added in the 3.11-rc1 merge window content, and the fix will be in
> before 3.11 final hits the streets.  If you want stable stuff, you
> should be using v3.10 at this point in time.

Sorry for my loose use of the word stable.

I meant a branch that will be incorporated into a future mainline release.
>From your response above I assume that I can use v3.11-rcX for some values
of X.

Can I confirm that the commit I should be looking for is
cd717a2a3db96af1e308d5bdb76c9d5f8154b4bd
("ARM: Add .text annotations where required after __CPUINIT removal")?

[PATCH] ARM: Fix memblock_reserve() to include stext

[Magnus Damm]

On Thu, Aug 1, 2013 at 4:39 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Aug 01, 2013 at 04:34:03AM +0900, Magnus Damm wrote:
>> Are you aware of any existing fix for this or anyone working on
>> solving this issue? If not then I don't mind stepping up. I can
>> however think of quite a few other things to do if this has been
>> solved already though, so please let me know if you think I should
>> focus on fixing this.
>
> I committed a fix for it this morning, so it should be in linux-next by
> tomorrow.

Thanks for writing a fix, Russell. I cherry picked the following
commit from linux-arm:

cd717a2 ARM: Add .text annotations where required after __CPUINIT removal

Applying it on top of kernel.org renesas git tag
renesas-devel-20130801v2 (v3.11-rc3 + v3.12 material) solves the
problem for me. I just tested this on KZM9G hardware without and with
the patch, so I can confirm that CPU Hotplug is now working again.

Tested-by: Magnus Damm <damm@opensource.se>

Cheers,

/ magnus

[PATCH] ARM: Fix memblock_reserve() to include stext

[Russell King - ARM Linux]

On Fri, Aug 02, 2013 at 12:24:17PM +0900, Magnus Damm wrote:
> On Thu, Aug 1, 2013 at 4:39 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Thu, Aug 01, 2013 at 04:34:03AM +0900, Magnus Damm wrote:
> >> Are you aware of any existing fix for this or anyone working on
> >> solving this issue? If not then I don't mind stepping up. I can
> >> however think of quite a few other things to do if this has been
> >> solved already though, so please let me know if you think I should
> >> focus on fixing this.
> >
> > I committed a fix for it this morning, so it should be in linux-next by
> > tomorrow.
> 
> Thanks for writing a fix, Russell. I cherry picked the following
> commit from linux-arm:
> 
> cd717a2 ARM: Add .text annotations where required after __CPUINIT removal
> 
> Applying it on top of kernel.org renesas git tag
> renesas-devel-20130801v2 (v3.11-rc3 + v3.12 material) solves the
> problem for me. I just tested this on KZM9G hardware without and with
> the patch, so I can confirm that CPU Hotplug is now working again.
> 
> Tested-by: Magnus Damm <damm@opensource.se>

It should've ended up in Linus' tree last night, but from the looks of
how quiet things seem to be, I guess everyone's away on August vacations
at the moment.