Re: [PATCH] riscv: Add WFI to secondary hart spinwait loop

[litaliano00]

Hi Bill,

Thank you for reviewing the patch. You are completely right on both points.

Nam Cao pointed out the missing wake-up interrupt earlier in the thread,
and I realized that a bare WFI here would cause a deadlock, exactly as
you described for HSM-capable platforms (or any setup lacking an explicit
IPI after the pointers are written).

Thank you also for clarifying the intent of `.Lsecondary_park`. It makes
sense that an infinite sleep is acceptable there but fatal here in the
boot path.

I have already withdrawn this patch based on the earlier discussion with
Nam. Fixing this FIXME properly would require a larger redesign to ensure
a wake-up signal is guaranteed across all boot methods, rather than just
a single-instruction change.

Best regards,
Adriano


On Tue, Apr 7, 2026 at 1:57 PM BillXiang
<xiangwencheng@xxxxxxxxxxxxxxxxxxx> wrote:
>
> Hi,
>
> On 4/7/2026 3:45 PM, Adriano Vero wrote:
>  > The .Lwait_for_cpu_up loop in the RISCV_BOOT_SPINWAIT path busy-polls
>  > __cpu_spinwait_stack_pointer and __cpu_spinwait_task_pointer, burning
>  > power on all non-boot harts while they wait for the primary hart to
>  > complete early boot setup.
>  >
>  > Add a WFI instruction before each polling iteration to allow the
>  > hardware to enter a low-power state while waiting. Per the RISC-V
>  > privileged specification, WFI wakes on any pending interrupt even
>  > with global interrupts disabled (SIE=0), and implementations are
>  > permitted to treat it as a NOP, so this is safe in all contexts.
>  >
>  > The same pattern is already used in .Lsecondary_park in the same
>  > file.
>
> The secondary_park is just an infinite loop for debug and it may not be
> a good example to follow here.
>
>  >
>  > Signed-off-by: Adriano Vero <litaliano00.contact@xxxxxxxxx>
>  > ---
>  >   arch/riscv/kernel/head.S | 9 ++++++++-
>  >   1 file changed, 8 insertions(+), 1 deletion(-)
>  >
>  > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>  > index 9c99c5ad6..ca208da7c 100644
>  > --- a/arch/riscv/kernel/head.S
>  > +++ b/arch/riscv/kernel/head.S
>  > @@ -385,7 +385,14 @@ SYM_CODE_START(_start_kernel)
>  >       * get far enough along the boot process that it should continue.
>  >       */
>  >   .Lwait_for_cpu_up:
>  > -    /* FIXME: We should WFI to save some energy here. */
>  > +    /*
>  > +     * Wait for the boot hart to populate the stack and task pointers.
>  > +     * Use WFI to avoid burning power in a busy-wait loop. Per the
>  > +     * RISC-V privileged spec, WFI wakes on a pending interrupt even
>  > +     * with global interrupts disabled (e.g. SIE=0), and implementations
>  > +     * are permitted to treat it as a NOP, so this is always safe.
>  > +     */
>  > +    wfi
>
> I think this will work for implementations that rely on IPI to bring up
> secondary HARTs, since the IPI will break the WFI. However, for
> HSM-capable platforms that start secondary HARTs without IPI, there may
> be no pending interrupt to wake the WFI.
>
>  >      REG_L sp, (a1)
>  >      REG_L tp, (a2)
>  >      beqz sp, .Lwait_for_cpu_up