Re: [PATCH] riscv: smp: fix non-SPINWAIT secondary hart rendezvous for fw_dynamic platforms

Hi,

On 2026-07-29 3:34 AM, Paul Sherman wrote:
> On platforms where firmware (e.g. fw_dynamic) releases all harts to the
> same Linux entry point simultaneously, the hart selected by OpenSBI as
> Domain0 Boot HART is already in SBI_HSM_STATE_STARTED when Linux later
> attempts to bring it online as a secondary CPU via SBI HART_START.
> OpenSBI correctly returns SBI_ERR_ALREADY_STARTED, but Linux has no
> recovery path: with CONFIG_RISCV_BOOT_SPINWAIT=n, there is no secondary
> wait path in _start_kernel for harts that entered Linux directly from
> firmware, and sbi_cpu_start() has no handler for SBI_ERR_ALREADY_STARTED.

There is no need to handle SBI_ERR_ALREADY_STARTED because the contract for the
"ordered booting" entry method (see
https://kernel.org/doc/html/latest/arch/riscv/boot.html#kernel-entry) requires
that only one hart is in the STARTED state when the kernel begins executing.

It sounds like your firmware implements something closer to the
RISCV_BOOT_SPINWAIT entry method, so if you disable CONFIG_RISCV_BOOT_SPINWAIT,
it is unsurprising that there would be problems.

Now, the kernel assumes that if the SBI HSM extension is present, the firmware
implements the "ordered booting" entry method. This seems like a reasonable
assumption. Why doesn't your firmware do that? Can you fix your firmware?

Regards,
Samuel

> This causes one CPU to be permanently dropped per boot. The missing CPU
> is always the OpenSBI Domain0 Boot HART, which varies between boots on
> Sophgo SG2042 (hart 1, 2, or 3), explaining the apparent 'moving victim'.
> 
> Fix with three cooperating changes:
> 
> 1. Initialize boot_cpu_hartid to INVALID_HARTID instead of relying on
>    BSS zero-initialization. Without this, boot_cpu_hartid aliases with
>    hart 0, causing hart 0 to always appear to win the boot CPU race
>    regardless of which hart actually stored its hartid first.
> 
> 2. Add a non-SPINWAIT secondary wait path in _start_kernel. When
>    firmware releases multiple harts to the same entry point, non-primary
>    harts divert into the existing spinwait rendezvous arrays (previously
>    used only by CONFIG_RISCV_BOOT_SPINWAIT) and wait for cpu_start() to
>    provide boot data before proceeding to secondary startup.
> 
> 3. Handle SBI_ERR_ALREADY_STARTED (-EALREADY) in sbi_cpu_start(). When
>    HART_START returns -EALREADY, the hart is already executing in Linux
>    and spinning in .Lwait_for_cpu_up_sbi. Write the spinwait rendezvous
>    arrays to release it into secondary startup, matching the approach
>    used by cpu_ops_spinwait.c.
> 
> The arrays __cpu_spinwait_stack_pointer and __cpu_spinwait_task_pointer
> are defined unconditionally in cpu_ops_spinwait.c but their extern
> declarations in head.h were guarded by CONFIG_RISCV_BOOT_SPINWAIT.
> Move the declarations outside the guard since the arrays are always
> present and now used by both boot paths.
> 
> Note: The NR_CPUS bound check mirrors the identical pattern in
> cpu_ops_spinwait.c:32 which guards the same arrays against out-of-range
> hartids on platforms with discontiguous hart numbering.
> 
> Link: https://lore.kernel.org/linux-riscv/[email protected]/
> Tested-on: Milk-V Pioneer (Sophgo SG2042, 64-hart RISC-V, 4-NUMA nodes,
>            128GB DDR4, OpenSBI v1.5, Linux v7.2-rc5)
> Result: boot_cpu_hartid correctly reflects Domain0 Boot HART, all 64
>         CPUs online in 2.7 seconds (was 63 CPUs, boot always on hart 0)
> Signed-off-by: Paul Sherman <[email protected]>
> ---
>  arch/riscv/kernel/cpu_ops_sbi.c | 41 ++++++++++++++++++++++++++++++++-
>  arch/riscv/kernel/head.S        | 25 ++++++++++++++++++++
>  arch/riscv/kernel/head.h        |  2 --
>  arch/riscv/kernel/setup.c       |  9 +++++++-
>  4 files changed, 73 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/kernel/cpu_ops_sbi.c b/arch/riscv/kernel/cpu_ops_sbi.c
> index ee6e4b5cc39e9..3268fda5be35f 100644
> --- a/arch/riscv/kernel/cpu_ops_sbi.c
> +++ b/arch/riscv/kernel/cpu_ops_sbi.c
> @@ -12,6 +12,7 @@
>  #include <asm/cpu_ops_sbi.h>
>  #include <asm/sbi.h>
>  #include <asm/smp.h>
> +#include "head.h"
>  
>  extern char secondary_start_sbi[];
>  const struct cpu_operations cpu_ops_sbi;
> @@ -23,6 +24,18 @@ const struct cpu_operations cpu_ops_sbi;
>   */
>  static struct sbi_hart_boot_data boot_data[NR_CPUS];
>  
> +#ifndef CONFIG_RISCV_BOOT_SPINWAIT
> +/*
> + * Secondary hart rendezvous arrays, shared with head.S.
> + * These arrays are named for historical reasons after the spinwait
> + * boot protocol, but serve a generic purpose: holding per-hart boot
> + * data until a secondary hart is ready to proceed. Defined here when
> + * CONFIG_RISCV_BOOT_SPINWAIT=n; otherwise defined in cpu_ops_spinwait.c.
> + */
> +void *__cpu_spinwait_stack_pointer[NR_CPUS] __section(".data");
> +void *__cpu_spinwait_task_pointer[NR_CPUS] __section(".data");
> +#endif
> +
>  static int sbi_hsm_hart_start(unsigned long hartid, unsigned long saddr,
>  			      unsigned long priv)
>  {
> @@ -62,6 +75,7 @@ static int sbi_cpu_start(unsigned int cpuid, struct task_struct *tidle)
>  	unsigned long boot_addr = __pa_symbol(secondary_start_sbi);
>  	unsigned long hartid = cpuid_to_hartid_map(cpuid);
>  	unsigned long hsm_data;
> +	int ret;
>  	struct sbi_hart_boot_data *bdata = &boot_data[cpuid];
>  
>  	/* Make sure tidle is updated */
> @@ -71,7 +85,32 @@ static int sbi_cpu_start(unsigned int cpuid, struct task_struct *tidle)
>  	/* Make sure boot data is updated */
>  	smp_mb();
>  	hsm_data = __pa(bdata);
> -	return sbi_hsm_hart_start(hartid, boot_addr, hsm_data);
> +
> +	ret = sbi_hsm_hart_start(hartid, boot_addr, hsm_data);
> +
> +	/*
> +	 * The firmware boot hart enters Linux directly from the bootloader
> +	 * and is already in SBI_HSM_STATE_STARTED when Linux attempts to
> +	 * bring it online as a secondary CPU. HART_START correctly returns
> +	 * SBI_ERR_ALREADY_STARTED in this case. The hart is spinning in
> +	 * .Lwait_for_cpu_up_sbi waiting for boot data - write the spinwait
> +	 * rendezvous arrays to release it into secondary startup.
> +	 *
> +	 * Guard against invalid or out-of-range hartids, matching the
> +	 * same constraint enforced in cpu_ops_spinwait.c.
> +	 */
> +	if (ret == -EALREADY) {
> +		if (hartid != INVALID_HARTID &&
> +		    hartid < (unsigned long)NR_CPUS) { /* array bound */
> +			/* Ensure bdata writes visible before spinwait arrays */
> +			smp_wmb();
> +			WRITE_ONCE(__cpu_spinwait_stack_pointer[hartid],
> +				task_pt_regs(tidle));
> +			WRITE_ONCE(__cpu_spinwait_task_pointer[hartid], tidle);
> +		}
> +		ret = 0;
> +	}
> +	return ret;
>  }
>  
>  #ifdef CONFIG_HOTPLUG_CPU
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index f6a8ca49e6277..da48f5efd928b 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -281,6 +281,31 @@ SYM_CODE_START(_start_kernel)
>  	la a2, boot_cpu_hartid
>  	REG_S a0, (a2)
>  
> +#ifndef CONFIG_RISCV_BOOT_SPINWAIT
> +	/*
> +	 * On platforms where firmware releases all harts to the same
> +	 * entry point (e.g. fw_dynamic), non-primary harts must divert
> +	 * here before MMU setup. Wait in the spinwait rendezvous arrays
> +	 * until cpu_start() provides boot data. a0 = hartid.
> +	 */
> +	REG_L a3, (a2)
> +	beq a0, a3, .Lprimary_hart
> +	slli a3, a0, LGREG
> +	la a1, __cpu_spinwait_stack_pointer
> +	la a2, __cpu_spinwait_task_pointer
> +	add a1, a3, a1
> +	add a2, a3, a2
> +.Lwait_for_cpu_up_sbi:
> +	fence r, r
> +	REG_L sp, (a1)
> +	REG_L tp, (a2)
> +	beqz sp, .Lwait_for_cpu_up_sbi
> +	beqz tp, .Lwait_for_cpu_up_sbi
> +	fence
> +	tail .Lsecondary_start_common
> +.Lprimary_hart:
> +#endif /* !CONFIG_RISCV_BOOT_SPINWAIT */
> +
>  	/* Initialize page tables and relocate to virtual addresses */
>  	la tp, init_task
>  	la sp, init_thread_union + THREAD_SIZE
> diff --git a/arch/riscv/kernel/head.h b/arch/riscv/kernel/head.h
> index 05a04bef442b1..1b34f8e655581 100644
> --- a/arch/riscv/kernel/head.h
> +++ b/arch/riscv/kernel/head.h
> @@ -12,9 +12,7 @@ extern atomic_t hart_lottery;
>  
>  asmlinkage void __init setup_vm(uintptr_t dtb_pa);
>  
> -#ifdef CONFIG_RISCV_BOOT_SPINWAIT
>  extern void *__cpu_spinwait_stack_pointer[];
>  extern void *__cpu_spinwait_task_pointer[];
> -#endif
>  
>  #endif /* __ASM_HEAD_H */
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index 52d1d2b8f338b..d2eecb7ad4965 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -47,7 +47,14 @@
>   * BSS.
>   */
>  atomic_t hart_lottery __section(".sdata");
> -unsigned long boot_cpu_hartid;
> +/*
> + * Initialize to INVALID_HARTID so that the first hart to store its
> + * hartid in head.S is unambiguously the boot CPU. Without this,
> + * boot_cpu_hartid starts as 0 (BSS), which aliases with hart 0 and
> + * causes hart 0 to always appear to win the boot CPU race regardless
> + * of which hart actually wrote first.
> + */
> +unsigned long boot_cpu_hartid = INVALID_HARTID;
>  EXPORT_SYMBOL_GPL(boot_cpu_hartid);
>  
>  /*


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv