[PATCH v4 0/8] MMU disabling code and kexec fixes

[PATCH v4 0/8] MMU disabling code and kexec fixes

[Will Deacon]

Hello,

This is version 4 of the patches originally posted here:

v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052157.html
v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052559.html
v3: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/053252.html

There are many changes since v3. Notably:
	- Support for Thumb-2
	- Based on 3.1-rc3 (now that some of the previous patches have
	  been merged)
	- The reserved stack page now sits below swapper and is reserved
	  by memblock during memory init. This has the advantage of being
	  at a known physical address for a platform, which will be helpful
	  for non-hotplug SMP booting.

Currently, SMP kexec relies on platform_cpu_kill (a CPU hotplug callback)
doing the right thing. I'm working on a generic SMP soft reboot implementation
but this isn't quite working yet (and my git repo is predictably offline
at the moment so I can't provide a link to the patches).

All feedback welcome.

Thanks,

Will


Will Deacon (8):
  ARM: proc-v7: disable SCTLR.TE when disabling MMU
  ARM: kexec: ensure new kernel is entered in ARM state
  ARM: lib: add switch_stack function for safely changing stack
  ARM: idmap: add header file for identity mapping functions
  ARM: reset: allow kernelspace mappings to be flat mapped during reset
  ARM: reset: add reset functionality for jumping to a physical address
  ARM: kexec: use arm_machine_reset for branching to the reboot buffer
  ARM: stop: execute platform callback from cpu_stop code

 arch/arm/Kconfig                  |    2 +-
 arch/arm/include/asm/idmap.h      |   15 +++++++
 arch/arm/include/asm/pgtable.h    |    3 -
 arch/arm/include/asm/system.h     |    1 +
 arch/arm/kernel/machine_kexec.c   |   14 +------
 arch/arm/kernel/process.c         |   76 ++++++++++++++++++++++++++++++++++--
 arch/arm/kernel/relocate_kernel.S |    2 +-
 arch/arm/kernel/smp.c             |    5 ++
 arch/arm/lib/Makefile             |    3 +-
 arch/arm/lib/switch_stack.S       |   44 +++++++++++++++++++++
 arch/arm/mm/idmap.c               |   43 ++++++++++++++++-----
 arch/arm/mm/mmu.c                 |   13 +++++-
 arch/arm/mm/proc-v7.S             |    1 +
 13 files changed, 187 insertions(+), 35 deletions(-)
 create mode 100644 arch/arm/include/asm/idmap.h
 create mode 100644 arch/arm/lib/switch_stack.S

[PATCH v4 1/8] ARM: proc-v7: disable SCTLR.TE when disabling MMU

[Will Deacon]

cpu_v7_reset disables the MMU and then branches to the provided address.
On Thumb-2 kernels, we should take care to clear the Thumb Exception
enable bit in the System Control Register, otherwise this may wreak
havok in the code to which we are branching (for example, an ARM kernel
image via kexec).

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/mm/proc-v7.S |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index a30e785..96b872c 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -66,6 +66,7 @@ ENDPROC(cpu_v7_proc_fin)
 ENTRY(cpu_v7_reset)
 	mrc	p15, 0, r1, c1, c0, 0		@ ctrl register
 	bic	r1, r1, #0x1			@ ...............m
+ THUMB(	bic	r1, r1, #1 << 30 )		@ Thumb exceptions
 	mcr	p15, 0, r1, c1, c0, 0		@ disable MMU
 	isb
 	mov	pc, r0
-- 
1.7.0.4

[PATCH v4 1/8] ARM: proc-v7: disable SCTLR.TE when disabling MMU

[Dave Martin]

On Tue, Aug 23, 2011 at 10:46:45PM +0100, Will Deacon wrote:
> cpu_v7_reset disables the MMU and then branches to the provided address.
> On Thumb-2 kernels, we should take care to clear the Thumb Exception
> enable bit in the System Control Register, otherwise this may wreak
> havok in the code to which we are branching (for example, an ARM kernel
> image via kexec).
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm/mm/proc-v7.S |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index a30e785..96b872c 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -66,6 +66,7 @@ ENDPROC(cpu_v7_proc_fin)
>  ENTRY(cpu_v7_reset)
>  	mrc	p15, 0, r1, c1, c0, 0		@ ctrl register
>  	bic	r1, r1, #0x1			@ ...............m
> + THUMB(	bic	r1, r1, #1 << 30 )		@ Thumb exceptions

Can we give the architectural name for the bit here?

If we must have magic numbers, we can at least minimise the scope for
confusion.

What that,
Reviewed-by: Dave Martin <dave.martin@linaro.org>

Cheers
---Dave

[PATCH v4 1/8] ARM: proc-v7: disable SCTLR.TE when disabling MMU

[Will Deacon]

On Wed, Aug 24, 2011 at 11:15:07AM +0100, Dave Martin wrote:
> On Tue, Aug 23, 2011 at 10:46:45PM +0100, Will Deacon wrote:
> > cpu_v7_reset disables the MMU and then branches to the provided address.
> > On Thumb-2 kernels, we should take care to clear the Thumb Exception
> > enable bit in the System Control Register, otherwise this may wreak
> > havok in the code to which we are branching (for example, an ARM kernel
> > image via kexec).
> > 
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm/mm/proc-v7.S |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> > index a30e785..96b872c 100644
> > --- a/arch/arm/mm/proc-v7.S
> > +++ b/arch/arm/mm/proc-v7.S
> > @@ -66,6 +66,7 @@ ENDPROC(cpu_v7_proc_fin)
> >  ENTRY(cpu_v7_reset)
> >  	mrc	p15, 0, r1, c1, c0, 0		@ ctrl register
> >  	bic	r1, r1, #0x1			@ ...............m
> > + THUMB(	bic	r1, r1, #1 << 30 )		@ Thumb exceptions
> 
> Can we give the architectural name for the bit here?

Ok. It's SCTLR.TE so representing it as we do for SCTLR.M looks a bit odd
unless you use two lines. Perhaps if I just add @ SCTLR.TE (Thumb
exceptions) ?

> What that,
> Reviewed-by: Dave Martin <dave.martin@linaro.org>

Ta,

Will

[PATCH v4 1/8] ARM: proc-v7: disable SCTLR.TE when disabling MMU

[Dave Martin]

On Wed, Aug 24, 2011 at 12:01:50PM +0100, Will Deacon wrote:
> On Wed, Aug 24, 2011 at 11:15:07AM +0100, Dave Martin wrote:
> > On Tue, Aug 23, 2011 at 10:46:45PM +0100, Will Deacon wrote:
> > > cpu_v7_reset disables the MMU and then branches to the provided address.
> > > On Thumb-2 kernels, we should take care to clear the Thumb Exception
> > > enable bit in the System Control Register, otherwise this may wreak
> > > havok in the code to which we are branching (for example, an ARM kernel
> > > image via kexec).
> > > 
> > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > ---
> > >  arch/arm/mm/proc-v7.S |    1 +
> > >  1 files changed, 1 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> > > index a30e785..96b872c 100644
> > > --- a/arch/arm/mm/proc-v7.S
> > > +++ b/arch/arm/mm/proc-v7.S
> > > @@ -66,6 +66,7 @@ ENDPROC(cpu_v7_proc_fin)
> > >  ENTRY(cpu_v7_reset)
> > >  	mrc	p15, 0, r1, c1, c0, 0		@ ctrl register
> > >  	bic	r1, r1, #0x1			@ ...............m
> > > + THUMB(	bic	r1, r1, #1 << 30 )		@ Thumb exceptions
> > 
> > Can we give the architectural name for the bit here?
> 
> Ok. It's SCTLR.TE so representing it as we do for SCTLR.M looks a bit odd
> unless you use two lines. Perhaps if I just add @ SCTLR.TE (Thumb
> exceptions) ?

That seems fine -- exactly how it looks is not so important as the
information itself IMHO.

It's a pretty minor nit in any case.

Cheers
---Dave

[PATCH v4 2/8] ARM: kexec: ensure new kernel is entered in ARM state

[Will Deacon]

Commit 540b5738 ("ARM: 6999/1: head, zImage: Always Enter the kernel in
ARM state") mandates that the kernel should be entered in ARM state.

If a Thumb-2 kernel kexecs a new kernel image, we need to ensure that
we change state when branching to the new code. This patch replaces a
mov pc, lr with a bx lr so that we transition to ARM state if need be.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/relocate_kernel.S |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S
index 9cf4cbf..0908633 100644
--- a/arch/arm/kernel/relocate_kernel.S
+++ b/arch/arm/kernel/relocate_kernel.S
@@ -57,7 +57,7 @@ relocate_new_kernel:
 	mov r0,#0
 	ldr r1,kexec_mach_type
 	ldr r2,kexec_boot_atags
-	mov pc,lr
+	bx lr
 
 	.align
 
-- 
1.7.0.4

[PATCH v4 2/8] ARM: kexec: ensure new kernel is entered in ARM state

[Nicolas Pitre]

On Tue, 23 Aug 2011, Will Deacon wrote:

> Commit 540b5738 ("ARM: 6999/1: head, zImage: Always Enter the kernel in
> ARM state") mandates that the kernel should be entered in ARM state.
> 
> If a Thumb-2 kernel kexecs a new kernel image, we need to ensure that
> we change state when branching to the new code. This patch replaces a
> mov pc, lr with a bx lr so that we transition to ARM state if need be.

This will break kexec support on ARMv4 targets.  No idea if any ARMv4 
target uses kexec though.

> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm/kernel/relocate_kernel.S |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S
> index 9cf4cbf..0908633 100644
> --- a/arch/arm/kernel/relocate_kernel.S
> +++ b/arch/arm/kernel/relocate_kernel.S
> @@ -57,7 +57,7 @@ relocate_new_kernel:
>  	mov r0,#0
>  	ldr r1,kexec_mach_type
>  	ldr r2,kexec_boot_atags
> -	mov pc,lr
> +	bx lr
>  
>  	.align
>  
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH v4 2/8] ARM: kexec: ensure new kernel is entered in ARM state

[Dave Martin]

On Tue, Aug 23, 2011 at 09:28:12PM -0400, Nicolas Pitre wrote:
> On Tue, 23 Aug 2011, Will Deacon wrote:
> 
> > Commit 540b5738 ("ARM: 6999/1: head, zImage: Always Enter the kernel in
> > ARM state") mandates that the kernel should be entered in ARM state.
> > 
> > If a Thumb-2 kernel kexecs a new kernel image, we need to ensure that
> > we change state when branching to the new code. This patch replaces a
> > mov pc, lr with a bx lr so that we transition to ARM state if need be.
> 
> This will break kexec support on ARMv4 targets.  No idea if any ARMv4 
> target uses kexec though.

Huh, good point -- I was too hasty.

We should use ARM() THUMB() here.

Cheers
---Dave

> 
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm/kernel/relocate_kernel.S |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S
> > index 9cf4cbf..0908633 100644
> > --- a/arch/arm/kernel/relocate_kernel.S
> > +++ b/arch/arm/kernel/relocate_kernel.S
> > @@ -57,7 +57,7 @@ relocate_new_kernel:
> >  	mov r0,#0
> >  	ldr r1,kexec_mach_type
> >  	ldr r2,kexec_boot_atags
> > -	mov pc,lr
> > +	bx lr
> >  
> >  	.align
> >  
> > -- 
> > 1.7.0.4
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 2/8] ARM: kexec: ensure new kernel is entered in ARM state

[Will Deacon]

On Wed, Aug 24, 2011 at 02:28:12AM +0100, Nicolas Pitre wrote:
> On Tue, 23 Aug 2011, Will Deacon wrote:
> 
> > Commit 540b5738 ("ARM: 6999/1: head, zImage: Always Enter the kernel in
> > ARM state") mandates that the kernel should be entered in ARM state.
> > 
> > If a Thumb-2 kernel kexecs a new kernel image, we need to ensure that
> > we change state when branching to the new code. This patch replaces a
> > mov pc, lr with a bx lr so that we transition to ARM state if need be.
> 
> This will break kexec support on ARMv4 targets.  No idea if any ARMv4 
> target uses kexec though.

It's easy enough to sort out though, so I may as well fix the code in case
anybody wants to try kexec on ARMv4!

Thanks,

Will

[PATCH v4 2/8] ARM: kexec: ensure new kernel is entered in ARM state

[Dave Martin]

On Tue, Aug 23, 2011 at 10:46:46PM +0100, Will Deacon wrote:

> Commit 540b5738 ("ARM: 6999/1: head, zImage: Always Enter the kernel in
> ARM state") mandates that the kernel should be entered in ARM state.
> 
> If a Thumb-2 kernel kexecs a new kernel image, we need to ensure that
> we change state when branching to the new code. This patch replaces a
> mov pc, lr with a bx lr so that we transition to ARM state if need be.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm/kernel/relocate_kernel.S |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S
> index 9cf4cbf..0908633 100644
> --- a/arch/arm/kernel/relocate_kernel.S
> +++ b/arch/arm/kernel/relocate_kernel.S
> @@ -57,7 +57,7 @@ relocate_new_kernel:
>  	mov r0,#0
>  	ldr r1,kexec_mach_type
>  	ldr r2,kexec_boot_atags
> -	mov pc,lr
> +	bx lr
>  
>  	.align
>  

Reviewed-by: Dave Martin <dave.martin@linaro.org>

> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 3/8] ARM: lib: add switch_stack function for safely changing stack

[Will Deacon]

When disabling the MMU, it is necessary to take out a 1:1 identity map
of the reset code so that it can safely be executed with and without
the MMU active. To avoid the situation where the physical address of the
reset code aliases with the virtual address of the active stack (which
cannot be included in the 1:1 mapping), it is desirable to change to a
new stack at a location which is less likely to alias.

This code adds a new lib function, switch_stack:

void switch_stack(void (*fn)(void *), void *arg, void *sp);

which changes the stack to point at the sp parameter, before invoking
fn(arg) with the new stack selected.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/lib/Makefile       |    3 +-
 arch/arm/lib/switch_stack.S |   44 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/lib/switch_stack.S

diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index cf73a7f..cbb1bc1 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -13,7 +13,8 @@ lib-y		:= backtrace.o changebit.o csumipv6.o csumpartial.o   \
 		   testchangebit.o testclearbit.o testsetbit.o        \
 		   ashldi3.o ashrdi3.o lshrdi3.o muldi3.o             \
 		   ucmpdi2.o lib1funcs.o div64.o                      \
-		   io-readsb.o io-writesb.o io-readsl.o io-writesl.o
+		   io-readsb.o io-writesb.o io-readsl.o io-writesl.o  \
+		   switch_stack.o
 
 mmu-y	:= clear_user.o copy_page.o getuser.o putuser.o
 
diff --git a/arch/arm/lib/switch_stack.S b/arch/arm/lib/switch_stack.S
new file mode 100644
index 0000000..552090d
--- /dev/null
+++ b/arch/arm/lib/switch_stack.S
@@ -0,0 +1,44 @@
+/*
+ * arch/arm/lib/switch_stack.S
+ *
+ * Copyright (C) 2011 ARM Ltd.
+ * Written by Will Deacon <will.deacon@arm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+
+/*
+ * void switch_stack(void (*fn)(void *), void *arg, void *sp)
+ *
+ * Change the stack to that pointed at by sp, then invoke fn(arg) with
+ * the new stack.
+ */
+ENTRY(switch_stack)
+	str	sp, [r2, #-4]!
+	str	lr, [r2, #-4]!
+
+	mov	sp, r2
+	mov	r2, r0
+	mov	r0, r1
+
+	adr	lr, BSYM(1f)
+	mov	pc, r2
+
+1:	ldr	lr, [sp]
+	ldr	sp, [sp, #4]
+	mov	pc, lr
+ENDPROC(switch_stack)
-- 
1.7.0.4

[PATCH v4 3/8] ARM: lib: add switch_stack function for safely changing stack

[Nicolas Pitre]

On Tue, 23 Aug 2011, Will Deacon wrote:

> When disabling the MMU, it is necessary to take out a 1:1 identity map
> of the reset code so that it can safely be executed with and without
> the MMU active. To avoid the situation where the physical address of the
> reset code aliases with the virtual address of the active stack (which
> cannot be included in the 1:1 mapping), it is desirable to change to a
> new stack at a location which is less likely to alias.
> 
> This code adds a new lib function, switch_stack:
> 
> void switch_stack(void (*fn)(void *), void *arg, void *sp);
> 
> which changes the stack to point at the sp parameter, before invoking
> fn(arg) with the new stack selected.
> 
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

For the implementation:

Reviewed-by: Nicolas Pitre <nicolas.pitre@linaro.org>

However I think switch_stack() is too vague a name for what this 
actually does.  Maybe something like call_with_stack(fn, arg, sp) is 
more precise.

> ---
>  arch/arm/lib/Makefile       |    3 +-
>  arch/arm/lib/switch_stack.S |   44 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+), 1 deletions(-)
>  create mode 100644 arch/arm/lib/switch_stack.S
> 
> diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
> index cf73a7f..cbb1bc1 100644
> --- a/arch/arm/lib/Makefile
> +++ b/arch/arm/lib/Makefile
> @@ -13,7 +13,8 @@ lib-y		:= backtrace.o changebit.o csumipv6.o csumpartial.o   \
>  		   testchangebit.o testclearbit.o testsetbit.o        \
>  		   ashldi3.o ashrdi3.o lshrdi3.o muldi3.o             \
>  		   ucmpdi2.o lib1funcs.o div64.o                      \
> -		   io-readsb.o io-writesb.o io-readsl.o io-writesl.o
> +		   io-readsb.o io-writesb.o io-readsl.o io-writesl.o  \
> +		   switch_stack.o
>  
>  mmu-y	:= clear_user.o copy_page.o getuser.o putuser.o
>  
> diff --git a/arch/arm/lib/switch_stack.S b/arch/arm/lib/switch_stack.S
> new file mode 100644
> index 0000000..552090d
> --- /dev/null
> +++ b/arch/arm/lib/switch_stack.S
> @@ -0,0 +1,44 @@
> +/*
> + * arch/arm/lib/switch_stack.S
> + *
> + * Copyright (C) 2011 ARM Ltd.
> + * Written by Will Deacon <will.deacon@arm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/assembler.h>
> +
> +/*
> + * void switch_stack(void (*fn)(void *), void *arg, void *sp)
> + *
> + * Change the stack to that pointed at by sp, then invoke fn(arg) with
> + * the new stack.
> + */
> +ENTRY(switch_stack)
> +	str	sp, [r2, #-4]!
> +	str	lr, [r2, #-4]!
> +
> +	mov	sp, r2
> +	mov	r2, r0
> +	mov	r0, r1
> +
> +	adr	lr, BSYM(1f)
> +	mov	pc, r2
> +
> +1:	ldr	lr, [sp]
> +	ldr	sp, [sp, #4]
> +	mov	pc, lr
> +ENDPROC(switch_stack)
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH v4 3/8] ARM: lib: add switch_stack function for safely changing stack

[Dave Martin]

On Tue, Aug 23, 2011 at 09:07:29PM -0400, Nicolas Pitre wrote:
> On Tue, 23 Aug 2011, Will Deacon wrote:
> 
> > When disabling the MMU, it is necessary to take out a 1:1 identity map
> > of the reset code so that it can safely be executed with and without
> > the MMU active. To avoid the situation where the physical address of the
> > reset code aliases with the virtual address of the active stack (which
> > cannot be included in the 1:1 mapping), it is desirable to change to a
> > new stack at a location which is less likely to alias.
> > 
> > This code adds a new lib function, switch_stack:
> > 
> > void switch_stack(void (*fn)(void *), void *arg, void *sp);
> > 
> > which changes the stack to point at the sp parameter, before invoking
> > fn(arg) with the new stack selected.
> > 
> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> 
> For the implementation:
> 
> Reviewed-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> 
> However I think switch_stack() is too vague a name for what this 
> actually does.  Maybe something like call_with_stack(fn, arg, sp) is 
> more precise.

call_with_stack() soundslike a good name to me.

---Dave

[PATCH v4 3/8] ARM: lib: add switch_stack function for safely changing stack

[Will Deacon]

On Wed, Aug 24, 2011 at 02:07:29AM +0100, Nicolas Pitre wrote:
> On Tue, 23 Aug 2011, Will Deacon wrote:
> 
> > When disabling the MMU, it is necessary to take out a 1:1 identity map
> > of the reset code so that it can safely be executed with and without
> > the MMU active. To avoid the situation where the physical address of the
> > reset code aliases with the virtual address of the active stack (which
> > cannot be included in the 1:1 mapping), it is desirable to change to a
> > new stack at a location which is less likely to alias.
> > 
> > This code adds a new lib function, switch_stack:
> > 
> > void switch_stack(void (*fn)(void *), void *arg, void *sp);
> > 
> > which changes the stack to point at the sp parameter, before invoking
> > fn(arg) with the new stack selected.
> > 
> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> 
> For the implementation:
> 
> Reviewed-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> 
> However I think switch_stack() is too vague a name for what this 
> actually does.  Maybe something like call_with_stack(fn, arg, sp) is 
> more precise.

That's a much better name, thanks.

Will

[PATCH v4 4/8] ARM: idmap: add header file for identity mapping functions

[Will Deacon]

The identity mappings functions are useful outside of SMP booting, so
expose them through their own header file.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/idmap.h   |   11 +++++++++++
 arch/arm/include/asm/pgtable.h |    3 ---
 arch/arm/kernel/process.c      |    3 +--
 arch/arm/kernel/smp.c          |    1 +
 arch/arm/mm/idmap.c            |    5 +++++
 5 files changed, 18 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/include/asm/idmap.h

diff --git a/arch/arm/include/asm/idmap.h b/arch/arm/include/asm/idmap.h
new file mode 100644
index 0000000..fe7ea86
--- /dev/null
+++ b/arch/arm/include/asm/idmap.h
@@ -0,0 +1,11 @@
+#ifndef _ARM_IDMAP_H
+#define _ARM_IDMAP_H
+
+#include <asm/page.h>
+
+void identity_mapping_add(pgd_t *pgd, unsigned long addr, unsigned long end);
+void identity_mapping_del(pgd_t *pgd, unsigned long addr, unsigned long end);
+
+void setup_mm_for_reboot(char mode);
+
+#endif	/* _ARM_IDMAP_H */
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 5750704..9d559a8 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -474,9 +474,6 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
 
 #define pgtable_cache_init() do { } while (0)
 
-void identity_mapping_add(pgd_t *, unsigned long, unsigned long);
-void identity_mapping_del(pgd_t *, unsigned long, unsigned long);
-
 #endif /* !__ASSEMBLY__ */
 
 #endif /* CONFIG_MMU */
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 1a347f4..89fe6ee 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -33,6 +33,7 @@
 #include <linux/cpuidle.h>
 
 #include <asm/cacheflush.h>
+#include <asm/idmap.h>
 #include <asm/leds.h>
 #include <asm/processor.h>
 #include <asm/system.h>
@@ -57,8 +58,6 @@ static const char *isa_modes[] = {
   "ARM" , "Thumb" , "Jazelle", "ThumbEE"
 };
 
-extern void setup_mm_for_reboot(char mode);
-
 static volatile int hlt_counter;
 
 #include <mach/system.h>
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index d88ff02..96a7956 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -39,6 +39,7 @@
 #include <asm/tlbflush.h>
 #include <asm/ptrace.h>
 #include <asm/localtimer.h>
+#include <asm/idmap.h>
 
 /*
  * as from 2.5, kernels no longer have an init_tasks structure
diff --git a/arch/arm/mm/idmap.c b/arch/arm/mm/idmap.c
index 2be9139..e99dc38 100644
--- a/arch/arm/mm/idmap.c
+++ b/arch/arm/mm/idmap.c
@@ -1,6 +1,7 @@
 #include <linux/kernel.h>
 
 #include <asm/cputype.h>
+#include <asm/idmap.h>
 #include <asm/pgalloc.h>
 #include <asm/pgtable.h>
 
@@ -71,6 +72,10 @@ void identity_mapping_del(pgd_t *pgd, unsigned long addr, unsigned long end)
 		idmap_del_pud(pgd, addr, next);
 	} while (pgd++, addr = next, addr != end);
 }
+#else
+void identity_mapping_del(pgd_t *pgd, unsigned long addr, unsigned long end)
+{
+}
 #endif
 
 /*
-- 
1.7.0.4

[PATCH v4 5/8] ARM: reset: allow kernelspace mappings to be flat mapped during reset

[Will Deacon]

Currently, switch_mm_for_reboot only takes out a 1:1 mapping from 0x0
to TASK_SIZE during reboot. For situations where we actually want to
turn off the MMU (e.g. kexec, hibernate, CPU hotplug) we want to map
as much memory as possible using the identity mapping so that we
increase the chance of mapping our reset code.

This patch extends setup_mm_for_reboot to take a set of page tables as
an optional parameter. If this is NULL then the behaviour is as before,
otherwise the new tables are used to remap all of memory apart from a
small window around the kernel image (TASK_SIZE - _end). The page
immediately below swapper_pg_dir is reserved during boot and can be used
as a temporary stack whilst the flat mapping is in place.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/idmap.h    |    6 +++++-
 arch/arm/kernel/machine_kexec.c |    4 ++--
 arch/arm/kernel/process.c       |    2 +-
 arch/arm/mm/idmap.c             |   38 ++++++++++++++++++++++++++++----------
 arch/arm/mm/mmu.c               |   13 +++++++++++--
 5 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/arch/arm/include/asm/idmap.h b/arch/arm/include/asm/idmap.h
index fe7ea86..c2c0576 100644
--- a/arch/arm/include/asm/idmap.h
+++ b/arch/arm/include/asm/idmap.h
@@ -2,10 +2,14 @@
 #define _ARM_IDMAP_H
 
 #include <asm/page.h>
+#include <asm/sections.h>
 
 void identity_mapping_add(pgd_t *pgd, unsigned long addr, unsigned long end);
 void identity_mapping_del(pgd_t *pgd, unsigned long addr, unsigned long end);
 
-void setup_mm_for_reboot(char mode);
+/* Page reserved below swapper. */
+#define RESERVE_STACK_PAGE	(unsigned long)swapper_pg_dir
+
+void setup_mm_for_reboot(char mode, pgd_t *pgd);
 
 #endif	/* _ARM_IDMAP_H */
diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c
index e59bbd4..dfc1784 100644
--- a/arch/arm/kernel/machine_kexec.c
+++ b/arch/arm/kernel/machine_kexec.c
@@ -16,7 +16,7 @@
 extern const unsigned char relocate_new_kernel[];
 extern const unsigned int relocate_new_kernel_size;
 
-extern void setup_mm_for_reboot(char mode);
+extern void setup_mm_for_reboot(char mode, pgd_t *pgd);
 
 extern unsigned long kexec_start_address;
 extern unsigned long kexec_indirection_page;
@@ -113,7 +113,7 @@ void machine_kexec(struct kimage *image)
 		kexec_reinit();
 	local_irq_disable();
 	local_fiq_disable();
-	setup_mm_for_reboot(0); /* mode is not used, so just pass 0*/
+	setup_mm_for_reboot(0, NULL); /* mode is not used, so just pass 0*/
 	flush_cache_all();
 	outer_flush_all();
 	outer_disable();
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 89fe6ee..84228ec 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -102,7 +102,7 @@ void arm_machine_restart(char mode, const char *cmd)
 	 * we may need it to insert some 1:1 mappings so that
 	 * soft boot works.
 	 */
-	setup_mm_for_reboot(mode);
+	setup_mm_for_reboot(mode, NULL);
 
 	/* Clean and invalidate caches */
 	flush_cache_all();
diff --git a/arch/arm/mm/idmap.c b/arch/arm/mm/idmap.c
index e99dc38..3d83ce4 100644
--- a/arch/arm/mm/idmap.c
+++ b/arch/arm/mm/idmap.c
@@ -79,17 +79,35 @@ void identity_mapping_del(pgd_t *pgd, unsigned long addr, unsigned long end)
 #endif
 
 /*
- * In order to soft-boot, we need to insert a 1:1 mapping in place of
- * the user-mode pages.  This will then ensure that we have predictable
- * results when turning the mmu off
+ * In order to soft-boot, we need to insert a 1:1 mapping of memory.
+ * This will then ensure that we have predictable results when turning
+ * the mmu off.
  */
-void setup_mm_for_reboot(char mode)
+void setup_mm_for_reboot(char mode, pgd_t *pgd)
 {
-	/*
-	 * We need to access to user-mode page tables here. For kernel threads
-	 * we don't have any user-mode mappings so we use the context that we
-	 * "borrowed".
-	 */
-	identity_mapping_add(current->active_mm->pgd, 0, TASK_SIZE);
+	unsigned long kernel_end;
+
+	/* If we don't have a pgd, hijack the current task. */
+	if (pgd == NULL) {
+		pgd = current->active_mm->pgd;
+		identity_mapping_add(pgd, 0, TASK_SIZE);
+	} else {
+		identity_mapping_add(pgd, 0, TASK_SIZE);
+		/*
+		 * Extend the flat mapping into kernelspace.
+		 * We leave room for the kernel image and the reserved
+		 * page below swapper.
+		 */
+		kernel_end = ALIGN((unsigned long)_end, PMD_SIZE);
+		identity_mapping_add(pgd, kernel_end, 0);
+	}
+
+	/* Clean and invalidate L1. */
+	flush_cache_all();
+
+	/* Switch exclusively to kernel mappings. */
+	cpu_switch_mm(pgd, &init_mm);
+
+	/* Flush the TLB. */
 	local_flush_tlb_all();
 }
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 594d677..e664cbd 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -894,18 +894,27 @@ static inline void prepare_page_table(void)
  */
 void __init arm_mm_memblock_reserve(void)
 {
+	phys_addr_t swapper_pa = __pa(swapper_pg_dir);
+
 	/*
 	 * Reserve the page tables.  These are already in use,
 	 * and can only be in node 0.
 	 */
-	memblock_reserve(__pa(swapper_pg_dir), PTRS_PER_PGD * sizeof(pgd_t));
+	memblock_reserve(swapper_pa, PTRS_PER_PGD * sizeof(pgd_t));
 
 #ifdef CONFIG_SA1111
 	/*
 	 * Because of the SA1111 DMA bug, we want to preserve our
 	 * precious DMA-able memory...
 	 */
-	memblock_reserve(PHYS_OFFSET, __pa(swapper_pg_dir) - PHYS_OFFSET);
+	memblock_reserve(PHYS_OFFSET, swapper_pa - PHYS_OFFSET);
+#else
+	/*
+	 * Reserve the page immediately below swapper to use as a
+	 * temporary stack and a holding area for secondary CPUs when we
+	 * are kexec'd.
+	 */
+	memblock_reserve(swapper_pa - PAGE_SIZE, PAGE_SIZE);
 #endif
 }
 
-- 
1.7.0.4

[PATCH v4 6/8] ARM: reset: add reset functionality for jumping to a physical address

[Will Deacon]

Tools such as kexec and CPU hotplug require a way to reset the processor
and branch to some code in physical space. This requires various bits of
jiggery pokery with the caches and MMU which, when it goes wrong, tends
to lock up the system.

This patch implements a new function, arm_machine_reset, for
consolidating this code in one place where it can be used by multiple
subsystems.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/system.h |    1 +
 arch/arm/kernel/process.c     |   71 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 832888d..cd2a3cd 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -108,6 +108,7 @@ extern int cpu_architecture(void);
 extern void cpu_init(void);
 
 void arm_machine_restart(char mode, const char *cmd);
+void arm_machine_reset(unsigned long reset_code_phys);
 extern void (*arm_pm_restart)(char str, const char *cmd);
 
 #define UDBG_UNDEFINED	(1 << 0)
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 84228ec..cda91a2 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -39,6 +39,7 @@
 #include <asm/system.h>
 #include <asm/thread_notify.h>
 #include <asm/stacktrace.h>
+#include <asm/pgalloc.h>
 #include <asm/mach/time.h>
 
 #ifdef CONFIG_CC_STACKPROTECTOR
@@ -91,6 +92,8 @@ static int __init hlt_setup(char *__unused)
 __setup("nohlt", nohlt_setup);
 __setup("hlt", hlt_setup);
 
+static char reboot_mode = 'h';
+
 void arm_machine_restart(char mode, const char *cmd)
 {
 	/* Disable interrupts first */
@@ -127,6 +130,72 @@ void arm_machine_restart(char mode, const char *cmd)
 	while (1);
 }
 
+extern void switch_stack(void (*fn)(void *), void *arg, void *sp);
+typedef void (*phys_reset_t)(unsigned long);
+
+struct arm_machine_reset_args {
+	unsigned long reset_code_phys;
+	pgd_t *reboot_tables;
+};
+
+void __arm_machine_reset(void *args)
+{
+	struct arm_machine_reset_args reset_args;
+	unsigned long reset_code_phys;
+	phys_reset_t phys_reset;
+
+	/* Copy our arguments onto the new stack. */
+	memcpy(&reset_args, args, sizeof(reset_args));
+	reset_code_phys = reset_args.reset_code_phys;
+
+	/* Take out a flat memory mapping. */
+	setup_mm_for_reboot(reboot_mode, reset_args.reboot_tables);
+
+	/* Clean and invalidate caches */
+	flush_cache_all();
+
+	/* Turn off caching */
+	cpu_proc_fin();
+
+	/* Push out any further dirty data, and ensure cache is empty */
+	flush_cache_all();
+
+	/* Switch to the identity mapping. */
+	phys_reset = (phys_reset_t)virt_to_phys(cpu_reset);
+	phys_reset(reset_code_phys);
+
+	/* Should never get here. */
+	BUG();
+}
+
+void arm_machine_reset(unsigned long reset_code_phys)
+{
+	phys_addr_t cpu_reset_end_phys;
+	void *cpu_reset_end;
+	struct arm_machine_reset_args reset_args;
+
+	cpu_reset_end = (void *)PAGE_ALIGN((unsigned long)cpu_reset);
+	cpu_reset_end_phys = virt_to_phys(cpu_reset_end);
+
+	/* Disable interrupts first. */
+	local_irq_disable();
+	local_fiq_disable();
+
+	/* Disable the L2. */
+	outer_disable();
+
+	/* Populate the reset args. */
+	reset_args.reset_code_phys = reset_code_phys;
+	reset_args.reboot_tables = pgd_alloc(&init_mm);
+
+	/* Change to the new stack and continue with the reset. */
+	switch_stack(__arm_machine_reset, (void *)&reset_args,
+		     (void *)RESERVE_STACK_PAGE);
+
+	/* Should never get here. */
+	BUG();
+}
+
 /*
  * Function pointers to optional machine specific functions
  */
@@ -216,8 +285,6 @@ void cpu_idle(void)
 	}
 }
 
-static char reboot_mode = 'h';
-
 int __init reboot_setup(char *str)
 {
 	reboot_mode = str[0];
-- 
1.7.0.4

[PATCH v4 7/8] ARM: kexec: use arm_machine_reset for branching to the reboot buffer

[Will Deacon]

Now that there is a common way to reset the machine, let's use it
instead of reinventing the wheel in the kexec backend.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/machine_kexec.c |   14 ++------------
 1 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c
index dfc1784..f84567c 100644
--- a/arch/arm/kernel/machine_kexec.c
+++ b/arch/arm/kernel/machine_kexec.c
@@ -16,8 +16,6 @@
 extern const unsigned char relocate_new_kernel[];
 extern const unsigned int relocate_new_kernel_size;
 
-extern void setup_mm_for_reboot(char mode, pgd_t *pgd);
-
 extern unsigned long kexec_start_address;
 extern unsigned long kexec_indirection_page;
 extern unsigned long kexec_mach_type;
@@ -111,14 +109,6 @@ void machine_kexec(struct kimage *image)
 
 	if (kexec_reinit)
 		kexec_reinit();
-	local_irq_disable();
-	local_fiq_disable();
-	setup_mm_for_reboot(0, NULL); /* mode is not used, so just pass 0*/
-	flush_cache_all();
-	outer_flush_all();
-	outer_disable();
-	cpu_proc_fin();
-	outer_inv_all();
-	flush_cache_all();
-	cpu_reset(reboot_code_buffer_phys);
+
+	arm_machine_reset(reboot_code_buffer_phys);
 }
-- 
1.7.0.4

[PATCH v4 8/8] ARM: stop: execute platform callback from cpu_stop code

[Will Deacon]

Sending IPI_CPU_STOP to a CPU causes it to execute a busy cpu_relax
loop forever. This makes it impossible to kexec successfully on an SMP
system since the secondary CPUs do not reset.

This patch adds a callback to platform_cpu_kill, defined when
CONFIG_HOTPLUG_CPU=y, from the ipi_cpu_stop handling code. This function
currently just returns 1 on all platforms that define it but allows them
to do something more sophisticated in the future.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/Kconfig      |    2 +-
 arch/arm/kernel/smp.c |    4 ++++
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5ebc5d9..d2e9d42 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1849,7 +1849,7 @@ config XIP_PHYS_ADDR
 
 config KEXEC
 	bool "Kexec system call (EXPERIMENTAL)"
-	depends on EXPERIMENTAL
+	depends on EXPERIMENTAL && (!SMP || HOTPLUG_CPU)
 	help
 	  kexec is a system call that implements the ability to shutdown your
 	  current kernel, and to start another kernel.  It is like a reboot
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 96a7956..3f6e8d1 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -559,6 +559,10 @@ static void ipi_cpu_stop(unsigned int cpu)
 	local_fiq_disable();
 	local_irq_disable();
 
+#ifdef CONFIG_HOTPLUG_CPU
+	platform_cpu_kill(cpu);
+#endif
+
 	while (1)
 		cpu_relax();
 }
-- 
1.7.0.4

[PATCH v4 0/8] MMU disabling code and kexec fixes

[Nicolas Pitre]

On Tue, 23 Aug 2011, Will Deacon wrote:

> Hello,
> 
> This is version 4 of the patches originally posted here:
> 
> v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052157.html
> v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052559.html
> v3: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/053252.html
> 
> There are many changes since v3. Notably:
> 	- Support for Thumb-2
> 	- Based on 3.1-rc3 (now that some of the previous patches have
> 	  been merged)
> 	- The reserved stack page now sits below swapper and is reserved
> 	  by memblock during memory init. This has the advantage of being
> 	  at a known physical address for a platform, which will be helpful
> 	  for non-hotplug SMP booting.

Is this really necessary?  Relying on "known physical address" is often 
going to lead us in situations where that physical address get encoded 
outside the kernel proper and then we're stuck with it forever.  This 
would be more "reliable" if the interface would be passed that address 
and if the page was dynamically allocated/freed.


Nicolas

[PATCH v4 0/8] MMU disabling code and kexec fixes

[Will Deacon]

Hi Nicolas,

Thanks for looking at this lot.

On Wed, Aug 24, 2011 at 02:25:36AM +0100, Nicolas Pitre wrote:
> On Tue, 23 Aug 2011, Will Deacon wrote:
> 
> > Hello,
> > 
> > This is version 4 of the patches originally posted here:
> > 
> > v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052157.html
> > v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052559.html
> > v3: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/053252.html
> > 
> > There are many changes since v3. Notably:
> > 	- Support for Thumb-2
> > 	- Based on 3.1-rc3 (now that some of the previous patches have
> > 	  been merged)
> > 	- The reserved stack page now sits below swapper and is reserved
> > 	  by memblock during memory init. This has the advantage of being
> > 	  at a known physical address for a platform, which will be helpful
> > 	  for non-hotplug SMP booting.
> 
> Is this really necessary?  Relying on "known physical address" is often 
> going to lead us in situations where that physical address get encoded 
> outside the kernel proper and then we're stuck with it forever.  This 
> would be more "reliable" if the interface would be passed that address 
> and if the page was dynamically allocated/freed.

We don't want to allocate this dynamically because we need to avoid the
situation where the virtual address of this page aliases with the physical
address of the reset code. By keeping the page close to the start of
physical memory, we avoid this scenario.

On top of that, I'd like to use the bottom of this page as a holding pen
for secondary CPUs when doing an SMP kexec. In this case, the new kernel
needs to take care not to allocate the page for general use. I suppose we
could fix this with a new ATAG / FDT property but that seems a bit overkill.

I was also thinking that it might be useful if bootloaders want to throw all
of the CPUs at the kernel during boot, rather than implementing their own
SMP pen. In this case, they could make use of this reserved page (I was
planning to version the pen implementation).

The latter idea might simply be crazy, but I think the other points are
valid :)

Will

[PATCH v4 0/8] MMU disabling code and kexec fixes

[Nicolas Pitre]

On Wed, 24 Aug 2011, Will Deacon wrote:

> Hi Nicolas,
> 
> Thanks for looking at this lot.
> 
> On Wed, Aug 24, 2011 at 02:25:36AM +0100, Nicolas Pitre wrote:
> > On Tue, 23 Aug 2011, Will Deacon wrote:
> > 
> > > Hello,
> > > 
> > > This is version 4 of the patches originally posted here:
> > > 
> > > v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052157.html
> > > v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052559.html
> > > v3: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/053252.html
> > > 
> > > There are many changes since v3. Notably:
> > > 	- Support for Thumb-2
> > > 	- Based on 3.1-rc3 (now that some of the previous patches have
> > > 	  been merged)
> > > 	- The reserved stack page now sits below swapper and is reserved
> > > 	  by memblock during memory init. This has the advantage of being
> > > 	  at a known physical address for a platform, which will be helpful
> > > 	  for non-hotplug SMP booting.
> > 
> > Is this really necessary?  Relying on "known physical address" is often 
> > going to lead us in situations where that physical address get encoded 
> > outside the kernel proper and then we're stuck with it forever.  This 
> > would be more "reliable" if the interface would be passed that address 
> > and if the page was dynamically allocated/freed.
> 
> We don't want to allocate this dynamically because we need to avoid the
> situation where the virtual address of this page aliases with the physical
> address of the reset code. By keeping the page close to the start of
> physical memory, we avoid this scenario.

Well... how probable might this be the case in practice?

And wouldn't this issue easily being worked around simply by allocating 
a second page if the first one is found to alias?  Surely the reset code 
is much smaller than a page.

> On top of that, I'd like to use the bottom of this page as a holding pen
> for secondary CPUs when doing an SMP kexec. In this case, the new kernel
> needs to take care not to allocate the page for general use. I suppose we
> could fix this with a new ATAG / FDT property but that seems a bit overkill.

Is it?

> I was also thinking that it might be useful if bootloaders want to throw all
> of the CPUs at the kernel during boot, rather than implementing their own
> SMP pen. In this case, they could make use of this reserved page (I was
> planning to version the pen implementation).

I get really uncomfortable at the prospect of having more _implicit_ 
dependencies between the kernel and bootloaders.  If ever we want to 
move things around for whatever reasons in the future then such fixed 
legacy addresses might really get in the way.  So I don't see the 
passing of that info using an ATAG or a DT node as something not bad at 
all.  This becomes just another resource provided to the kernel, like 
the ramdisk image, that can be located almost anywhere. And then you 
have no backward compatibility issues either as the page validity 
becomes explicit.


Nicolas

[PATCH v4 0/8] MMU disabling code and kexec fixes

[Will Deacon]

On Wed, Aug 24, 2011 at 03:53:44PM +0100, Nicolas Pitre wrote:
> On Wed, 24 Aug 2011, Will Deacon wrote:
> > On Wed, Aug 24, 2011 at 02:25:36AM +0100, Nicolas Pitre wrote:
> > > Is this really necessary?  Relying on "known physical address" is often 
> > > going to lead us in situations where that physical address get encoded 
> > > outside the kernel proper and then we're stuck with it forever.  This 
> > > would be more "reliable" if the interface would be passed that address 
> > > and if the page was dynamically allocated/freed.
> > 
> > We don't want to allocate this dynamically because we need to avoid the
> > situation where the virtual address of this page aliases with the physical
> > address of the reset code. By keeping the page close to the start of
> > physical memory, we avoid this scenario.
> 
> Well... how probable might this be the case in practice?
> 
> And wouldn't this issue easily being worked around simply by allocating 
> a second page if the first one is found to alias?  Surely the reset code 
> is much smaller than a page.

Ok, so we can probably get around this corner case by checking for the alias
and reallocating in the next section (the ID mapping is done at the section
granularity).

> > On top of that, I'd like to use the bottom of this page as a holding pen
> > for secondary CPUs when doing an SMP kexec. In this case, the new kernel
> > needs to take care not to allocate the page for general use. I suppose we
> > could fix this with a new ATAG / FDT property but that seems a bit overkill.
> 
> Is it?

Well it means re-encoding a DT blob from within the kernel so that we can
tell the next kernel not to allocate from a given page of physical memory.
If we already have this infrastructure, then great, but it looks to me like
it only lives within dtc (as libfdt) at the moment.

> > I was also thinking that it might be useful if bootloaders want to throw all
> > of the CPUs at the kernel during boot, rather than implementing their own
> > SMP pen. In this case, they could make use of this reserved page (I was
> > planning to version the pen implementation).
> 
> I get really uncomfortable at the prospect of having more _implicit_ 
> dependencies between the kernel and bootloaders.  If ever we want to 
> move things around for whatever reasons in the future then such fixed 
> legacy addresses might really get in the way.  So I don't see the 
> passing of that info using an ATAG or a DT node as something not bad at 
> all.  This becomes just another resource provided to the kernel, like 
> the ramdisk image, that can be located almost anywhere. And then you 
> have no backward compatibility issues either as the page validity 
> becomes explicit.

Ok, point taken on the bootloader interface, I'd just rather have a way to
make this unreliable to bootloaders without having to re-encode the .dtb in
the kexec code.

Can you think of any clever tricks so that we only look at this pen when
we're being kexec'd?

Cheers,

Will

[PATCH v4 0/8] MMU disabling code and kexec fixes

[Nicolas Pitre]

On Wed, 24 Aug 2011, Will Deacon wrote:

> On Wed, Aug 24, 2011 at 03:53:44PM +0100, Nicolas Pitre wrote:
> > On Wed, 24 Aug 2011, Will Deacon wrote:
> > > On top of that, I'd like to use the bottom of this page as a holding pen
> > > for secondary CPUs when doing an SMP kexec. In this case, the new kernel
> > > needs to take care not to allocate the page for general use. I suppose we
> > > could fix this with a new ATAG / FDT property but that seems a bit overkill.
> > 
> > Is it?
> 
> Well it means re-encoding a DT blob from within the kernel so that we can
> tell the next kernel not to allocate from a given page of physical memory.
> If we already have this infrastructure, then great, but it looks to me like
> it only lives within dtc (as libfdt) at the moment.

Hmmm... The kexec user space tools know how to manipulate a device tree 
for the next kernel, right?  So it's only a matter for it to ask the 
current kernel what the physical address for that page is and stuff it 
into the FDT it is preparing already.


Nicolas

[PATCH v4 0/8] MMU disabling code and kexec fixes

[Will Deacon]

On Wed, Aug 24, 2011 at 06:30:07PM +0100, Nicolas Pitre wrote:
> On Wed, 24 Aug 2011, Will Deacon wrote:
> 
> > On Wed, Aug 24, 2011 at 03:53:44PM +0100, Nicolas Pitre wrote:
> > > On Wed, 24 Aug 2011, Will Deacon wrote:
> > > > On top of that, I'd like to use the bottom of this page as a holding pen
> > > > for secondary CPUs when doing an SMP kexec. In this case, the new kernel
> > > > needs to take care not to allocate the page for general use. I suppose we
> > > > could fix this with a new ATAG / FDT property but that seems a bit overkill.
> > > 
> > > Is it?
> > 
> > Well it means re-encoding a DT blob from within the kernel so that we can
> > tell the next kernel not to allocate from a given page of physical memory.
> > If we already have this infrastructure, then great, but it looks to me like
> > it only lives within dtc (as libfdt) at the moment.
> 
> Hmmm... The kexec user space tools know how to manipulate a device tree 
> for the next kernel, right?  So it's only a matter for it to ask the 
> current kernel what the physical address for that page is and stuff it 
> into the FDT it is preparing already.

oo-er, it seems a bit odd telling userspace about that address but it does
fix the problem. I can't think of a better idea, so I'll start looking for a
sane interface for passing this address back.

Thanks for the help,

Will

[PATCH v4 0/8] MMU disabling code and kexec fixes

[Will Deacon]

Hi Nicolas,

On Wed, Aug 24, 2011 at 06:45:37PM +0100, Will Deacon wrote:
> On Wed, Aug 24, 2011 at 06:30:07PM +0100, Nicolas Pitre wrote:
> > Hmmm... The kexec user space tools know how to manipulate a device tree 
> > for the next kernel, right?  So it's only a matter for it to ask the 
> > current kernel what the physical address for that page is and stuff it 
> > into the FDT it is preparing already.
> 
> oo-er, it seems a bit odd telling userspace about that address but it does
> fix the problem. I can't think of a better idea, so I'll start looking for a
> sane interface for passing this address back.

I had a look at the kexec-tools source for PPC (unfortunately all the fdt
stuff is under there, so adding this for ARM is going to be a pain) and
here's what they do:

  If a .dtb file is passed to kexec, that is passed to the new kernel. It is
  fixed up by the kexec tool so that things like the command-line can be
  modified. There is a device tree property called "cpu-release-addr" which
  specifies the pen address for each CPU and this property is actually read
  from the device tree and added to the list of reserved regions so that the
  next kernel doesn't clobber it.

  If a .dtb file is not passed, kexec looks at /proc/device-tree/ and
  constructs a new blob from that.

Note that PPC doesn't make an attempt to allocate the pen dynamically. If we
want to do that, I guess we could emit a dummy "cpu-release-addr" property
via /proc/, but this has a couple of downsides:

  1.) People might try and use this property (and it would allow bootloaders
      to specify pens to non-kexec'd kernels, which would likely end in
      disaster)

  2.) We would have to allocate the pen during boot and leave it allocated
      for any CONFIG_KEXEC kernels.

Thinking about the concerns you had about bootloaders using the pen if it's
at a known physical location, I realised that we don't actually need to
populate the pen until a kexec is in progress. Although the pen page will
always be reserved, we can just copy the pen code into it on the
kexec_prepare path and that way prevent it being abused by anybody else.

So, do you still reckon we should push forward with the dynamic allocation
and ATAG/FDT interface or lazily populate at a known address instead?

Thanks,

Will

[PATCH v4 0/8] MMU disabling code and kexec fixes

[Jamie Iles]

Hi Will,

On Tue, Aug 23, 2011 at 10:46:44PM +0100, Will Deacon wrote:
> Hello,
> 
> This is version 4 of the patches originally posted here:
> 
> v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052157.html
> v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052559.html
> v3: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/053252.html
> 
> There are many changes since v3. Notably:
> 	- Support for Thumb-2
> 	- Based on 3.1-rc3 (now that some of the previous patches have
> 	  been merged)
> 	- The reserved stack page now sits below swapper and is reserved
> 	  by memblock during memory init. This has the advantage of being
> 	  at a known physical address for a platform, which will be helpful
> 	  for non-hotplug SMP booting.
> 
> Currently, SMP kexec relies on platform_cpu_kill (a CPU hotplug callback)
> doing the right thing. I'm working on a generic SMP soft reboot implementation
> but this isn't quite working yet (and my git repo is predictably offline
> at the moment so I can't provide a link to the patches).

I've tried this on picoxcell and reboot with arm_machine_reset() works 
nicely, but I couldn't get kexec to work (though it didn't work without 
your patches...).

With your patches applied I get the following output on booting the new 
kernel:

[    0.000000] Initializing cgroup subsys cpu
[    0.000000] Linux version 3.1.0-rc3+ (jamiei at pulham) (gcc version 4.5.3 20110223 (prerelease) (GCC) ) #40 PREEMPT Wed Aug 24 13:21:06 BST 2011
[    0.000000] CPU: ARMv6-compatible processor [410fb767] revision 7 (ARMv7), cr=00c5387f
[    0.000000] CPU: VIPT aliasing data cache, unknown instruction cache
[    0.000000] Machine: Picochip picoXcell
[    0.000000] bootconsole [earlycon0] enabled
[    0.000000] Memory policy: ECC disabled, Data cache writeback
[    0.000000] Kernel panic - not syncing: ERROR: Failed to allocate 0x1000 bytes below 0x0.
[    0.000000]
[    0.000000] [<c001368c>] (unwind_backtrace+0x0/0xe0) from [<c02c875c>] (panic+0x68/0x1a0)
[    0.000000] [<c02c875c>] (panic+0x68/0x1a0) from [<c03c5180>] (memblock_alloc_base+0x2c/0x34)
[    0.000000] [<c03c5180>] (memblock_alloc_base+0x2c/0x34) from [<c03be894>] (early_alloc.clone.1+0x10/0x28)
[    0.000000] [<c03be894>] (early_alloc.clone.1+0x10/0x28) from [<c03bf2b4>] (paging_init+0x4c4/0x6d0)
[    0.000000] [<c03bf2b4>] (paging_init+0x4c4/0x6d0) from [<c03bca54>] (setup_arch+0x424/0x780)
[    0.000000] [<c03bca54>] (setup_arch+0x424/0x780) from [<c03bb510>] (start_kernel+0xac/0x2e4)
[    0.000000] [<c03bb510>] (start_kernel+0xac/0x2e4) from [<00008040>] (0x8040)

and the allocation that fails appears to be allocating the vectors page.  
Without your patches I don't get any output though so it's a definite 
improvement!

Note that this is a device tree enabled platform so there is no 
/proc/atags.  I'm happy to test any related patches on our platform, but 
kexec isn't something we really use.

Jamie

[PATCH v4 0/8] MMU disabling code and kexec fixes

[Will Deacon]

On Wed, Aug 24, 2011 at 01:34:31PM +0100, Jamie Iles wrote:
> Hi Will,

Hello Jamie,

> On Tue, Aug 23, 2011 at 10:46:44PM +0100, Will Deacon wrote:
> > Currently, SMP kexec relies on platform_cpu_kill (a CPU hotplug callback)
> > doing the right thing. I'm working on a generic SMP soft reboot implementation
> > but this isn't quite working yet (and my git repo is predictably offline
> > at the moment so I can't provide a link to the patches).
> 
> I've tried this on picoxcell and reboot with arm_machine_reset() works 
> nicely, but I couldn't get kexec to work (though it didn't work without 
> your patches...).

Ok, thanks for giving it a go though!

> With your patches applied I get the following output on booting the new 
> kernel:
> 
> [    0.000000] Initializing cgroup subsys cpu
> [    0.000000] Linux version 3.1.0-rc3+ (jamiei at pulham) (gcc version 4.5.3 20110223 (prerelease) (GCC) ) #40 PREEMPT Wed Aug 24 13:21:06 BST 2011
> [    0.000000] CPU: ARMv6-compatible processor [410fb767] revision 7 (ARMv7), cr=00c5387f
> [    0.000000] CPU: VIPT aliasing data cache, unknown instruction cache
> [    0.000000] Machine: Picochip picoXcell
> [    0.000000] bootconsole [earlycon0] enabled
> [    0.000000] Memory policy: ECC disabled, Data cache writeback
> [    0.000000] Kernel panic - not syncing: ERROR: Failed to allocate 0x1000 bytes below 0x0.
> [    0.000000]
> [    0.000000] [<c001368c>] (unwind_backtrace+0x0/0xe0) from [<c02c875c>] (panic+0x68/0x1a0)
> [    0.000000] [<c02c875c>] (panic+0x68/0x1a0) from [<c03c5180>] (memblock_alloc_base+0x2c/0x34)
> [    0.000000] [<c03c5180>] (memblock_alloc_base+0x2c/0x34) from [<c03be894>] (early_alloc.clone.1+0x10/0x28)
> [    0.000000] [<c03be894>] (early_alloc.clone.1+0x10/0x28) from [<c03bf2b4>] (paging_init+0x4c4/0x6d0)
> [    0.000000] [<c03bf2b4>] (paging_init+0x4c4/0x6d0) from [<c03bca54>] (setup_arch+0x424/0x780)
> [    0.000000] [<c03bca54>] (setup_arch+0x424/0x780) from [<c03bb510>] (start_kernel+0xac/0x2e4)
> [    0.000000] [<c03bb510>] (start_kernel+0xac/0x2e4) from [<00008040>] (0x8040)
> 
> and the allocation that fails appears to be allocating the vectors page.  
> Without your patches I don't get any output though so it's a definite 
> improvement!

Wahey, that's not bad. If you're starting to boot the new kernel then lots
of the tricky stuff is out of the way.

> Note that this is a device tree enabled platform so there is no 
> /proc/atags.  I'm happy to test any related patches on our platform, but 
> kexec isn't something we really use.

This is almost certainly the problem as the ARM kexec code doesn't have FDT
support afaict, so the new kernel doesn't think it has any memory. Can you
try hardcoding a command line into the kernel image which specifies the
memory layout?

I think the way forward with the kexec work is for me to get it working
solidly on UP platforms (including Thumb-2 and FDT) and then add SMP support
later on.

Will

[PATCH v4 0/8] MMU disabling code and kexec fixes

[Jamie Iles]

On Wed, Aug 24, 2011 at 01:42:01PM +0100, Will Deacon wrote:
> On Wed, Aug 24, 2011 at 01:34:31PM +0100, Jamie Iles wrote:
[...]
> > Note that this is a device tree enabled platform so there is no 
> > /proc/atags.  I'm happy to test any related patches on our platform, but 
> > kexec isn't something we really use.
> 
> This is almost certainly the problem as the ARM kexec code doesn't have FDT
> support afaict, so the new kernel doesn't think it has any memory. Can you
> try hardcoding a command line into the kernel image which specifies the
> memory layout?

That does the trick.  It now gets to the point where the platform code 
starts looking for platform specific parts of the device tree (which it 
can't find) so I'd call that a success.

Feel free to add a Tested-by: Jamie Iles <jamie@jamieiles.com> if you 
like!

Jamie