[PATCH 0/3] Try to make SMP booting slightly less fragile

[PATCH 0/3] Try to make SMP booting slightly less fragile

[Will Deacon]

Hi everyone,

After spending some time investigating SMP boot issues when using
(random?) configs from Qian Cai with lots of debug options enabled, I
hacked together these two patches which make SMP booting a little more
robust.

The whole thing is still a racy mess, but I'm not sure we can do much
about that without reworking it to use per-cpu data structures which
we can update atomically.

Will

--->8

Will Deacon (3):
  arm64: smp: Increase secondary CPU boot timeout value
  arm64: smp: Don't enter kernel with NULL stack pointer or task struct
  arm64: smp: Treat unknown boot failures as being 'stuck in kernel'

 arch/arm64/kernel/head.S | 8 ++++++++
 arch/arm64/kernel/smp.c  | 4 +++-
 2 files changed, 11 insertions(+), 1 deletion(-)

-- 
2.11.0


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

[PATCH 1/3] arm64: smp: Increase secondary CPU boot timeout value

[Will Deacon]

When many debug options are enabled simultaneously (e.g. PROVE_LOCKING,
KMEMLEAK, DEBUG_PAGE_ALLOC, KASAN etc), it is possible for us to timeout
when attempting to boot a secondary CPU and give up. Unfortunately, the
CPU will /eventually/ appear, and sit in the background happily stuck
in a recursive exception due to a NULL stack pointer.

Increase the timeout to 5s, which will of course be enough for anybody.

Signed-off-by: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/smp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 018a33e01b0e..63c7a7682e93 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -123,7 +123,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 		 * time out.
 		 */
 		wait_for_completion_timeout(&cpu_running,
-					    msecs_to_jiffies(1000));
+					    msecs_to_jiffies(5000));
 
 		if (!cpu_online(cpu)) {
 			pr_crit("CPU%u: failed to come online\n", cpu);
-- 
2.11.0


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

[PATCH 2/3] arm64: smp: Don't enter kernel with NULL stack pointer or task struct

[Will Deacon]

Although SMP bringup is inherently racy, we can significantly reduce
the window during which secondary CPUs can unexpectedly enter the
kernel by sanity checking the 'stack' and 'task' fields of the
'secondary_data' structure. If the booting CPU gave up waiting for us,
then they will have been cleared to NULL and we should spin in a WFE; WFI
loop instead.

Signed-off-by: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/head.S | 8 ++++++++
 arch/arm64/kernel/smp.c  | 1 +
 2 files changed, 9 insertions(+)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 2cdacd1c141b..0baadf335172 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -724,14 +724,22 @@ __secondary_switched:
 
 	adr_l	x0, secondary_data
 	ldr	x1, [x0, #CPU_BOOT_STACK]	// get secondary_data.stack
+	cbz	x1, __secondary_too_slow
 	mov	sp, x1
 	ldr	x2, [x0, #CPU_BOOT_TASK]
+	cbz	x2, __secondary_too_slow
 	msr	sp_el0, x2
 	mov	x29, #0
 	mov	x30, #0
 	b	secondary_start_kernel
 ENDPROC(__secondary_switched)
 
+__secondary_too_slow:
+	wfe
+	wfi
+	b	__secondary_too_slow
+ENDPROC(__secondary_too_slow)
+
 /*
  * The booting CPU updates the failed status @__early_cpu_boot_status,
  * with MMU turned off.
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 63c7a7682e93..1f8aeb77cba5 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -136,6 +136,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 
 	secondary_data.task = NULL;
 	secondary_data.stack = NULL;
+	__flush_dcache_area(&secondary_data, sizeof(secondary_data));
 	status = READ_ONCE(secondary_data.status);
 	if (ret && status) {
 
-- 
2.11.0


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

[PATCH 3/3] arm64: smp: Treat unknown boot failures as being 'stuck in kernel'

[Will Deacon]

When we fail to bring a secondary CPU online and it fails in an unknown
state, we should assume the worst and increment 'cpus_stuck_in_kernel'
so that things like kexec() are disabled.

Signed-off-by: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/smp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 1f8aeb77cba5..dc9fe879c279 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -147,6 +147,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 		default:
 			pr_err("CPU%u: failed in unknown state : 0x%lx\n",
 					cpu, status);
+			cpus_stuck_in_kernel++;
 			break;
 		case CPU_KILL_ME:
 			if (!op_cpu_kill(cpu)) {
-- 
2.11.0


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

Re: [PATCH 3/3] arm64: smp: Treat unknown boot failures as being 'stuck in kernel'

[Mark Rutland]

On Tue, Aug 27, 2019 at 04:18:15PM +0100, Will Deacon wrote:
> When we fail to bring a secondary CPU online and it fails in an unknown
> state, we should assume the worst and increment 'cpus_stuck_in_kernel'
> so that things like kexec() are disabled.

Definitely! I has assumed we already did this, but I see that we don't.

> Signed-off-by: Will Deacon <will@kernel.org>

I don't see a nicer way of doing this, so:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

> ---
>  arch/arm64/kernel/smp.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 1f8aeb77cba5..dc9fe879c279 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -147,6 +147,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
>  		default:
>  			pr_err("CPU%u: failed in unknown state : 0x%lx\n",
>  					cpu, status);
> +			cpus_stuck_in_kernel++;
>  			break;
>  		case CPU_KILL_ME:
>  			if (!op_cpu_kill(cpu)) {
> -- 
> 2.11.0
> 

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

Re: [PATCH 2/3] arm64: smp: Don't enter kernel with NULL stack pointer or task struct

[Mark Rutland]

On Tue, Aug 27, 2019 at 04:18:14PM +0100, Will Deacon wrote:
> Although SMP bringup is inherently racy, we can significantly reduce
> the window during which secondary CPUs can unexpectedly enter the
> kernel by sanity checking the 'stack' and 'task' fields of the
> 'secondary_data' structure. If the booting CPU gave up waiting for us,
> then they will have been cleared to NULL and we should spin in a WFE; WFI
> loop instead.
> 
> Signed-off-by: Will Deacon <will@kernel.org>

In future I think that we can handle this better using the MPIDR hash
and atomics to flip the values and cater for races, but this is better
than what we have, so FWIW:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

> ---
>  arch/arm64/kernel/head.S | 8 ++++++++
>  arch/arm64/kernel/smp.c  | 1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 2cdacd1c141b..0baadf335172 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -724,14 +724,22 @@ __secondary_switched:
>  
>  	adr_l	x0, secondary_data
>  	ldr	x1, [x0, #CPU_BOOT_STACK]	// get secondary_data.stack
> +	cbz	x1, __secondary_too_slow
>  	mov	sp, x1
>  	ldr	x2, [x0, #CPU_BOOT_TASK]
> +	cbz	x2, __secondary_too_slow
>  	msr	sp_el0, x2
>  	mov	x29, #0
>  	mov	x30, #0
>  	b	secondary_start_kernel
>  ENDPROC(__secondary_switched)
>  
> +__secondary_too_slow:
> +	wfe
> +	wfi
> +	b	__secondary_too_slow
> +ENDPROC(__secondary_too_slow)
> +
>  /*
>   * The booting CPU updates the failed status @__early_cpu_boot_status,
>   * with MMU turned off.
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 63c7a7682e93..1f8aeb77cba5 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -136,6 +136,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
>  
>  	secondary_data.task = NULL;
>  	secondary_data.stack = NULL;
> +	__flush_dcache_area(&secondary_data, sizeof(secondary_data));
>  	status = READ_ONCE(secondary_data.status);
>  	if (ret && status) {
>  
> -- 
> 2.11.0
> 

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

Re: [PATCH 1/3] arm64: smp: Increase secondary CPU boot timeout value

[Mark Rutland]

On Tue, Aug 27, 2019 at 04:18:13PM +0100, Will Deacon wrote:
> When many debug options are enabled simultaneously (e.g. PROVE_LOCKING,
> KMEMLEAK, DEBUG_PAGE_ALLOC, KASAN etc), it is possible for us to timeout
> when attempting to boot a secondary CPU and give up. Unfortunately, the
> CPU will /eventually/ appear, and sit in the background happily stuck
> in a recursive exception due to a NULL stack pointer.
> 
> Increase the timeout to 5s, which will of course be enough for anybody.
> 
> Signed-off-by: Will Deacon <will@kernel.org>

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

> ---
>  arch/arm64/kernel/smp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 018a33e01b0e..63c7a7682e93 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -123,7 +123,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
>  		 * time out.
>  		 */
>  		wait_for_completion_timeout(&cpu_running,
> -					    msecs_to_jiffies(1000));
> +					    msecs_to_jiffies(5000));
>  
>  		if (!cpu_online(cpu)) {
>  			pr_crit("CPU%u: failed to come online\n", cpu);
> -- 
> 2.11.0
> 

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