public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] riscv: Add WFI to secondary hart spinwait loop
@ 2026-04-07  7:45 Adriano Vero
  2026-04-07  8:40 ` Nam Cao
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Adriano Vero @ 2026-04-07  7:45 UTC (permalink / raw)
  To: palmer, pjw, aou; +Cc: alex, linux-riscv, linux-kernel, Adriano Vero

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.

Signed-off-by: Adriano Vero <litaliano00.contact@gmail.com>
---
 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
 	REG_L sp, (a1)
 	REG_L tp, (a2)
 	beqz sp, .Lwait_for_cpu_up
-- 
2.53.0


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

* Re: [PATCH] riscv: Add WFI to secondary hart spinwait loop
  2026-04-07  7:45 [PATCH] riscv: Add WFI to secondary hart spinwait loop Adriano Vero
@ 2026-04-07  8:40 ` Nam Cao
  2026-04-07  8:45 ` Nam Cao
       [not found] ` <8c77810f-8198-46b5-9b6b-23095dbfee1a@lanxincomputing.com>
  2 siblings, 0 replies; 5+ messages in thread
From: Nam Cao @ 2026-04-07  8:40 UTC (permalink / raw)
  To: Adriano Vero, palmer, pjw, aou
  Cc: alex, linux-riscv, linux-kernel, Adriano Vero

Adriano Vero <litaliano00.contact@gmail.com> writes:

> 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.
>
> Signed-off-by: Adriano Vero <litaliano00.contact@gmail.com>
> ---
>  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
>  	REG_L sp, (a1)
>  	REG_L tp, (a2)
>  	beqz sp, .Lwait_for_cpu_up
> -- 
> 2.53.0
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] riscv: Add WFI to secondary hart spinwait loop
  2026-04-07  7:45 [PATCH] riscv: Add WFI to secondary hart spinwait loop Adriano Vero
  2026-04-07  8:40 ` Nam Cao
@ 2026-04-07  8:45 ` Nam Cao
       [not found]   ` <CAK-CEVO6h+v-2iRdsR6THbOvoTrxM7xnmsDyXfnP5sAgUdmhYQ@mail.gmail.com>
       [not found] ` <8c77810f-8198-46b5-9b6b-23095dbfee1a@lanxincomputing.com>
  2 siblings, 1 reply; 5+ messages in thread
From: Nam Cao @ 2026-04-07  8:45 UTC (permalink / raw)
  To: Adriano Vero; +Cc: palmer, pjw, aou, alex, linux-riscv, linux-kernel

On Tue, Apr 07, 2026 at 09:45:34AM +0200, 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.
> 
> Signed-off-by: Adriano Vero <litaliano00.contact@gmail.com>
> ---
>  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
>  	REG_L sp, (a1)
>  	REG_L tp, (a2)
>  	beqz sp, .Lwait_for_cpu_up

Which interrupt is supposed to wake these CPUs up?

Nam

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

* Re: [PATCH] riscv: Add WFI to secondary hart spinwait loop
       [not found]   ` <CAK-CEVO6h+v-2iRdsR6THbOvoTrxM7xnmsDyXfnP5sAgUdmhYQ@mail.gmail.com>
@ 2026-04-07 11:48     ` Nam Cao
  0 siblings, 0 replies; 5+ messages in thread
From: Nam Cao @ 2026-04-07 11:48 UTC (permalink / raw)
  To: litaliano00; +Cc: palmer, pjw, aou, alex, linux-riscv, linux-kernel

litaliano00 <litaliano00.contact@gmail.com> writes:
> You are correct to question this. There is no interrupt sent to the
> waiting harts in the current spinwait design — the primary hart simply
> writes to __cpu_spinwait_stack_pointer and __cpu_spinwait_task_pointer.
> A bare WFI would cause secondary harts to sleep indefinitely since no
> IPI is issued after those writes.
>
> The correct fix would require the primary hart to send an IPI to all
> waiting harts after populating the pointers, which is a more involved
> change. The current patch is insufficient as-is.

Yeah, that's also what I would do.

> I will send a v2 that reverts this change. Apologies for the noise.

There's no need for that, this patch has not been applied.

Best regards,
Nam

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

* Re: [PATCH] riscv: Add WFI to secondary hart spinwait loop
       [not found] ` <8c77810f-8198-46b5-9b6b-23095dbfee1a@lanxincomputing.com>
@ 2026-04-07 12:27   ` litaliano00
  0 siblings, 0 replies; 5+ messages in thread
From: litaliano00 @ 2026-04-07 12:27 UTC (permalink / raw)
  To: BillXiang; +Cc: alex, linux-riscv, linux-kernel

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@lanxincomputing.com> 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@gmail.com>
>  > ---
>  >   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

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

end of thread, other threads:[~2026-04-07 12:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-07  7:45 [PATCH] riscv: Add WFI to secondary hart spinwait loop Adriano Vero
2026-04-07  8:40 ` Nam Cao
2026-04-07  8:45 ` Nam Cao
     [not found]   ` <CAK-CEVO6h+v-2iRdsR6THbOvoTrxM7xnmsDyXfnP5sAgUdmhYQ@mail.gmail.com>
2026-04-07 11:48     ` Nam Cao
     [not found] ` <8c77810f-8198-46b5-9b6b-23095dbfee1a@lanxincomputing.com>
2026-04-07 12:27   ` litaliano00

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