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

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

Re: [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

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

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

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

Re: [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")?

Re: [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

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