All of lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: Yureka Lilian <yureka@cyberchaos.dev>
Cc: Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2] arch: arm64: add early_param idle=<wfi|yield|nop>
Date: Fri, 31 Jul 2026 17:16:38 +0100	[thread overview]
Message-ID: <amzKZgqrqqM7E_9Q@google.com> (raw)
In-Reply-To: <a6602971-87f8-4365-9ec2-dfe3a1ccfeb6@cyberchaos.dev>

Hi Yureka,

On Mon, Jul 27, 2026 at 10:49:57AM +0200, Yureka Lilian wrote:
> On 7/22/26 23:43, Will Deacon wrote:
> > On Sat, Jul 11, 2026 at 09:35:25AM +0200, Yureka Lilian wrote:
> > > diff --git a/arch/arm64/lib/delay.c b/arch/arm64/lib/delay.c
> > > index e278e060e78a..2452990ed37a 100644
> > > --- a/arch/arm64/lib/delay.c
> > > +++ b/arch/arm64/lib/delay.c
> > > @@ -15,6 +15,8 @@
> > >   #include <clocksource/arm_arch_timer.h>
> > > +#include "../kernel/idle.h"
> > > +
> > >   #define USECS_TO_CYCLES(time_usecs)			\
> > >   	xloops_to_cycles((time_usecs) * 0x10C7UL)
> > > @@ -49,7 +51,8 @@ void __delay(unsigned long cycles)
> > >   		 * Start with WFIT. If an interrupt makes us resume
> > >   		 * early, use a WFET loop to complete the delay.
> > >   		 */
> > > -		wfit(end);
> > > +		if (likely(idle == ARM64_IDLE_WFI))
> > > +			wfit(end);
> > Rather than scatter the idle implementation check across all users of
> > WFI*, why not move this into the macro itself? That way, the callers can
> > all stay like they are but the macro behaves as specified.
> 
> In practise, I could only find the following uses of WFI / WFIT / wfi() /
> wfit() in arm64 code: 1) the default idle loop and delay() function;
> arguably the only "real" users 2) parking cores after different kinds of
> unexpected situations / crashes.
> 
> Just to confirm, you are suggesting to add the conditionals to
> arch/arm64/include/asm/barrier.h, and have wfi() and wfit() macros not
> actually do WFI / WFIT depending on the value of the idle param?
> 
> I'm torn about this: We should maybe first discuss what effect idle= should
> have: Should it prevent WFI running anywhere in the kernel when idle=nop, OR
> is its intended use case to change the default arm64 implementations for
> idle and delay, while still allowing other parts of the kernel to use WFI
> (for example, for custom cpuidle implementation)? I think the existing nohlt
> parameter, which disables all idle states, is more fitting for the first
> goal, even though it currently does not prevent the wfit in the delay
> function.

I think nohlt will break the PSCI cpuidle proposal here:

https://lore.kernel.org/all/20260708-efi-psci-v1-0-9efb3abf0e4c@kernel.org/

> For preventing WFI anywhere, I think alternatives patching based on the
> earlyparam would be the most reliable way to achieve that, and second most
> reliable way is putting the conditional in the wfi() / wfit() macros.
> 
> But my assumption is that what we want to achieve is actually closer to
> changing only the default idle implementation, while allowing an idle state
> registered at later point to still do its thing (including using WFI for
> this purpose). And for this (and to be more flexible with other idle modes
> e.g. yield), what I proposed in this patch makes more sense, and I'm
> relatively confident it will fulfill the Apple Silicon use case without
> adding many more scattered checks other than these two.
> Does this make sense?

I keep changing my mind about this patch :/

What do you think about a funny sort of hybrid approach where:

  * We have idle=, but it really only affects the idle loop
  * We have a cpu_errata entry to detect (based on the MIDR) this CPU
    and then avoid patching in the WFIT instructions (i.e. don't detect
    the ARM64_HAS_WFXT capability).

I think that would solve the case for you, as well as giving others finer
grained control over the idle implementation and adding the infrastructure
we need to handle a CPU with broken wfit.

Will

  reply	other threads:[~2026-07-31 16:16 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-11  7:35 [PATCH v2] arch: arm64: add early_param idle=<wfi|yield|nop> Yureka Lilian
2026-07-13  9:57 ` Sudeep Holla
2026-07-13 11:59   ` Will Deacon
2026-07-13 15:26     ` Sudeep Holla
2026-07-13 12:24   ` Anshuman Khandual
2026-07-13 15:36     ` Sudeep Holla
2026-07-15 13:24   ` Yureka Lilian
2026-07-16 10:58     ` Sudeep Holla
2026-07-22 21:43 ` Will Deacon
2026-07-27  8:49   ` Yureka Lilian
2026-07-31 16:16     ` Will Deacon [this message]
2026-07-31 19:32       ` Yureka Lilian

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amzKZgqrqqM7E_9Q@google.com \
    --to=will@kernel.org \
    --cc=anshuman.khandual@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=corbet@lwn.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=yureka@cyberchaos.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.