Re: [PATCH] arm64: Force the use of CNTVCT_EL0 in __delay()

[Marc Zyngier <maz@kernel.org>]

On Wed, 25 Feb 2026 22:36:07 +0000,
Will Deacon <will@kernel.org> wrote:
> 
> On Mon, Feb 23, 2026 at 02:31:44PM +0000, Marc Zyngier wrote:
> > Crucially, arch_counter_get_cntvct_stable() does disable preemption,
> > and we should preserve it. Something like this:
> > 
> > diff --git a/arch/arm64/lib/delay.c b/arch/arm64/lib/delay.c
> > index d02341303899e..25fb593f95b0c 100644
> > --- a/arch/arm64/lib/delay.c
> > +++ b/arch/arm64/lib/delay.c
> > @@ -32,7 +32,16 @@ static inline unsigned long xloops_to_cycles(unsigned long xloops)
> >   * Note that userspace cannot change the offset behind our back either,
> >   * as the vcpu mutex is held as long as KVM_RUN is in progress.
> >   */
> > -#define __delay_cycles()	__arch_counter_get_cntvct_stable()
> > +static cycles_t __delay_cycles(void)
> > +{
> > +	cycles_t val;
> > +
> > +	preempt_disable();
> > +	val = __arch_counter_get_cntvct_stable();
> > +	preenpt_enable();
> > +
> > +	return val;
> > +}
> 
> (nit: arch_counter_get_cntvct_stable() uses the _notrace() variants of
>  the preempt disable/enable helpers)

That's because arch_counter_get_cntvct_stable() itself is notrace.
I'm not sure we need this function to be notrace as well, but I'll
change that and we can revisit it.

> 
> >  void __delay(unsigned long cycles)
> >  {
> > 
> > The question is whether there is a material benefit in replicating the
> > arch_timer_read_counter() indirection for the virtual counter in order
> > to not pay the price of preempt_disable() when we're on a non-broken
> > system (hopefully the vast majority of implementations).
> 
> That sounds nice, especially as we can assume (for now) that CPUs
> implementing WFIT don't need the cntvct workarounds. However, I can't
> really figure out how to implement it after reminding myself of all the
> fun we had trying to use a static key for these workarounds in the past.
>
> If a CPU being onlined has a timer erratum, we wouldn't be able to
> migrate any tasks in the middle of a preempt-enabled delay loop onto
> it. :/

Why would it be any different than, say, sched_clock(), which
ultimately uses arch_timer_read_counter()? We already rely on this
indirection to do the right thing everywhere, and I don't recall we
have any issue with this.

Anyway, I'll shortly post what I have and we can discuss whether this
is the correct approach.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.