| Message ID | 20190827151815.2160-3-will@kernel.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 5b1cfe3a0ba74c1f2b83b607712a217b9f9463a2 |
| Headers | show |
| Series | Try to make SMP booting slightly less fragile | expand |
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 >
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) {
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(+)