Re: [PATCH bpf-next v13 6/6] bpf, arm64: Add JIT support for cpu time counter kfuncs

[Marc Zyngier <maz@kernel.org>]

On Mon, 20 Apr 2026 11:16:34 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> [+ Marc]
> 
> On Sat, Apr 18, 2026 at 06:16:04AM -0700, Puranjay Mohan wrote:
> > Add ARM64 JIT inlining for bpf_get_cpu_time_counter() and
> > bpf_cpu_time_counter_to_ns() kfuncs.
> > 
> > bpf_get_cpu_time_counter() is JIT-inlined as:
> > 
> >   ISB                       // serialize instruction stream
> >   MRS Xn, CNTVCT_EL0        // read architected timer counter
> > 
> > The ISB before the MRS is required for ordering, matching the kernel's
> > arch_timer_read_cntvct_el0() implementation.
> 
> Careful here: this will _not_ order counter accesses against normal
> memory barriers (e.g. smp_mb(), acquire/release). If you need that, then
> you need something like arch_counter_enforce_ordering(), which we do
> have in __arch_counter_get_cntvct().
> 
> Furthermore, using the virtual counter may expose you to situations
> where a guest value for CNTVOFF is installed (e.g. during the VCPU run
> loop). Given all the contexts in which BPF can run, this worries me a
> little as you might end up seeing a non-monotonic view of time between
> BPF programs.

Yup, and this will catch you out in subtle ways depending on whether
the host runs at EL2 or not.

The kernel (mostly) avoids this pitfall by picking the physical
counter for sched_clock() when running at EL2, so that it isn't
directly affected by CNTVOFF_EL2.

I suspect that you'll need something similar if you insist on a
globally monotonic view of the counter.

>
> > On newer CPUs it will be JITed to:
> > 
> >   MRS Xn, CNTVCTSS_EL0      // self-synchronized (ISB not needed)
> > 
> > bpf_cpu_time_counter_to_ns() is JIT-inlined using mult/shift constants
> > computed at JIT time from the architected timer frequency (CNTFRQ_EL0):

And what happens when CNTFRQ_EL0 is not populated, as it is the case
on a lot of stupidly integrated HW? The kernel has a fallback, but you
don't seem to make use of it here.

> >
> >   MOV Xtmp, #mult           // load conversion multiplier
> >   MUL Xn, Xarg, Xtmp        // delta_ticks * mult
> >   LSR Xn, Xn, #shift        // >> shift = nanoseconds
> > 
> > On systems with a 1GHz counter (e.g., Neoverse-V2), mult=1 and shift=0,
> > so the conversion collapses to a single MOV (identity).
> 
> Do you have any performance numbers to show that this is worthwhile
> compared to calling a helper wrapping __arch_counter_get_cntvct().
> 
> > Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
> > ---
> >  arch/arm64/include/asm/insn.h                 |  2 +
> >  arch/arm64/net/bpf_jit.h                      |  4 ++
> >  arch/arm64/net/bpf_jit_comp.c                 | 54 +++++++++++++++++++
> >  .../selftests/bpf/progs/verifier_cpu_cycles.c | 50 ++++++++++++++++-
> >  4 files changed, 109 insertions(+), 1 deletion(-)
> 
> Aren't you conveniently ignoring CPU errata here? I suspect this needs
> to be predicated on the absence of those, in a similar way to how the
> vDSO deals with the 'vsdo_clockmode'.

And I'm afraid this still is the common case...

	M.

-- 
Without deviation from the norm, progress is not possible.