Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] ARM: smp: set current pointer in assembly before jumping into C
@ 2026-08-01 16:35 Karl Mehltretter
  2026-08-02  9:34 ` Ard Biesheuvel
  0 siblings, 1 reply; 2+ messages in thread
From: Karl Mehltretter @ 2026-08-01 16:35 UTC (permalink / raw)
  To: Russell King
  Cc: Karl Mehltretter, Ard Biesheuvel, linux-arm-kernel, linux-kernel,
	stable

With CONFIG_KCOV_INSTRUMENT_ALL=y, secondary CPUs fail to come online
on ARMv6K/ARMv7 SMP:

  CPU1: failed to boot: -38

The compiler inserts a call to __sanitizer_cov_trace_pc() at the entry
of secondary_start_kernel(), before set_current(task). The KCOV hook
dereferences current while TPIDRURO is still uninitialized, causing a
data abort before cpu_init() has installed the exception stacks. The
secondary CPU consequently never completes startup.

Initialize current in the secondary assembly handoff, as
__mmap_switched already does for the boot CPU.

With current set in assembly, the set_current() call in
secondary_start_kernel() is redundant. Drop it together with the
function and the task argument. The hotplug resuscitate path in
arch_cpu_idle_dead() needs no hand-off either, as smp_ops.cpu_die()
can only return if the CPU kept its register state, TPIDRURO included.

Fixes: 50596b7559bf ("ARM: smp: Store current pointer in TPIDRURO register if available")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
v2: drop the redundant set_current() call and helper (Ard), along with
the now-unused task argument to secondary_start_kernel()
v1: https://lore.kernel.org/r/20260718091600.92021-1-kmehltretter@gmail.com/

Tested on QEMU vexpress-a15 (v6.16) with CONFIG_KCOV_INSTRUMENT_ALL=y,
covering SMP bringup and 10 CPU1 offline/online cycles through the
arch_cpu_idle_dead() resuscitate path.

The head-nommu.S change is untested because no currently reachable
Kconfig combination enables non-MMU SMP.

 arch/arm/include/asm/smp.h   |  2 +-
 arch/arm/kernel/head-nommu.S |  1 +
 arch/arm/kernel/head.S       |  1 +
 arch/arm/kernel/smp.c        | 15 ++-------------
 4 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index 8c05a7f374d8..84aa6a663e81 100644
--- a/arch/arm/include/asm/smp.h
+++ b/arch/arm/include/asm/smp.h
@@ -43,7 +43,7 @@ extern void set_smp_ipi_range(int ipi_base, int nr_ipi);
  * Called from platform specific assembly code, this is the
  * secondary CPU entry point.
  */
-asmlinkage void secondary_start_kernel(struct task_struct *task);
+asmlinkage void secondary_start_kernel(void);
 
 
 /*
diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
index b9d6818f1ee1..f80bbfbd3a6f 100644
--- a/arch/arm/kernel/head-nommu.S
+++ b/arch/arm/kernel/head-nommu.S
@@ -116,6 +116,7 @@ ENTRY(secondary_startup)
 	ldr	r7, __secondary_data		@ reload r7
 	ldr	sp, [r7, #12]			@ set up the stack pointer
 	ldr	r0, [r7, #16]			@ set up task pointer
+	set_current r0, r1
 	mov	fp, #0
 	b	secondary_start_kernel
 ENDPROC(secondary_startup)
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index f22c50d4bd41..a22403a629ef 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -442,6 +442,7 @@ ENTRY(__secondary_switched)
 	adr_l	r7, secondary_data + 12		@ get secondary_data.stack
 	ldr	sp, [r7]
 	ldr	r0, [r7, #4]			@ get secondary_data.task
+	set_current r0, r1
 	mov	fp, #0
 	b	secondary_start_kernel
 ENDPROC(__secondary_switched)
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 50999886a8b5..e30206365877 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -371,12 +371,9 @@ void __noreturn arch_cpu_idle_dead(void)
 	 */
 	__asm__("mov	sp, %0\n"
 	"	mov	fp, #0\n"
-	"	mov	r0, %1\n"
 	"	b	secondary_start_kernel"
 		:
-		: "r" (task_stack_page(current) + THREAD_SIZE - 8),
-		  "r" (current)
-		: "r0");
+		: "r" (task_stack_page(current) + THREAD_SIZE - 8));
 
 	unreachable();
 }
@@ -397,23 +394,15 @@ static void smp_store_cpu_info(unsigned int cpuid)
 	check_cpu_icache_size(cpuid);
 }
 
-static void set_current(struct task_struct *cur)
-{
-	/* Set TPIDRURO */
-	asm("mcr p15, 0, %0, c13, c0, 3" :: "r"(cur) : "memory");
-}
-
 /*
  * This is the secondary CPU boot entry.  We're using this CPUs
  * idle thread stack, but a set of temporary page tables.
  */
-asmlinkage void secondary_start_kernel(struct task_struct *task)
+asmlinkage void secondary_start_kernel(void)
 {
 	struct mm_struct *mm = &init_mm;
 	unsigned int cpu;
 
-	set_current(task);
-
 	secondary_biglittle_init();
 
 	/*
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] ARM: smp: set current pointer in assembly before jumping into C
  2026-08-01 16:35 [PATCH v2] ARM: smp: set current pointer in assembly before jumping into C Karl Mehltretter
@ 2026-08-02  9:34 ` Ard Biesheuvel
  0 siblings, 0 replies; 2+ messages in thread
From: Ard Biesheuvel @ 2026-08-02  9:34 UTC (permalink / raw)
  To: Karl Mehltretter, Russell King; +Cc: linux-arm-kernel, linux-kernel, stable



On Sat, 1 Aug 2026, at 18:35, Karl Mehltretter wrote:
> With CONFIG_KCOV_INSTRUMENT_ALL=y, secondary CPUs fail to come online
> on ARMv6K/ARMv7 SMP:
>
>   CPU1: failed to boot: -38
>
> The compiler inserts a call to __sanitizer_cov_trace_pc() at the entry
> of secondary_start_kernel(), before set_current(task). The KCOV hook
> dereferences current while TPIDRURO is still uninitialized, causing a
> data abort before cpu_init() has installed the exception stacks. The
> secondary CPU consequently never completes startup.
>
> Initialize current in the secondary assembly handoff, as
> __mmap_switched already does for the boot CPU.
>
> With current set in assembly, the set_current() call in
> secondary_start_kernel() is redundant. Drop it together with the
> function and the task argument. The hotplug resuscitate path in
> arch_cpu_idle_dead() needs no hand-off either, as smp_ops.cpu_die()
> can only return if the CPU kept its register state, TPIDRURO included.
>
> Fixes: 50596b7559bf ("ARM: smp: Store current pointer in TPIDRURO 
> register if available")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-02  9:35 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-01 16:35 [PATCH v2] ARM: smp: set current pointer in assembly before jumping into C Karl Mehltretter
2026-08-02  9:34 ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox