| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aXNln7dzZzf-iGGI@google.com>
Date: Fri, 23 Jan 2026 12:12:15 +0000
From: Vincent Donnefort <vdonnefort@...gle.com>
To: Marc Zyngier <maz@...nel.org>
Cc: rostedt@...dmis.org, mhiramat@...nel.org,
mathieu.desnoyers@...icios.com, linux-trace-kernel@...r.kernel.org,
oliver.upton@...ux.dev, joey.gouly@....com, suzuki.poulose@....com,
yuzenghui@...wei.com, kvmarm@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, jstultz@...gle.com,
qperret@...gle.com, will@...nel.org, aneesh.kumar@...nel.org,
kernel-team@...roid.com, linux-kernel@...r.kernel.org,
Thomas Gleixner <tglx@...utronix.de>,
Stephen Boyd <sboyd@...nel.org>,
"Christopher S. Hall" <christopher.s.hall@...el.com>,
Richard Cochran <richardcochran@...il.com>
Subject: Re: [PATCH v9 25/30] KVM: arm64: Sync boot clock with the nVHE/pKVM
hyp
On Wed, Jan 07, 2026 at 02:23:16PM +0000, Marc Zyngier wrote:
> On Tue, 02 Dec 2025 09:36:18 +0000,
> Vincent Donnefort <vdonnefort@...gle.com> wrote:
> >
> > Configure the hypervisor tracing clock with the kernel boot clock. For
> > tracing purposes, the boot clock is interesting: it doesn't stop on
> > suspend. However, it is corrected on a regular basis, which implies the
> > need to re-evaluate it every once in a while.
> >
> > Cc: John Stultz <jstultz@...gle.com>
> > Cc: Thomas Gleixner <tglx@...utronix.de>
> > Cc: Stephen Boyd <sboyd@...nel.org>
> > Cc: Christopher S. Hall <christopher.s.hall@...el.com>
> > Cc: Richard Cochran <richardcochran@...il.com>
> > Signed-off-by: Vincent Donnefort <vdonnefort@...gle.com>
> >
> > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> > index f83650a7aad9..375607c67285 100644
> > --- a/arch/arm64/include/asm/kvm_asm.h
> > +++ b/arch/arm64/include/asm/kvm_asm.h
> > @@ -93,6 +93,7 @@ enum __kvm_host_smccc_func {
> > __KVM_HOST_SMCCC_FUNC___tracing_unload,
> > __KVM_HOST_SMCCC_FUNC___tracing_enable,
> > __KVM_HOST_SMCCC_FUNC___tracing_swap_reader,
> > + __KVM_HOST_SMCCC_FUNC___tracing_update_clock,
> > };
> >
> > #define DECLARE_KVM_VHE_SYM(sym) extern char sym[]
> > diff --git a/arch/arm64/kvm/hyp/include/nvhe/trace.h b/arch/arm64/kvm/hyp/include/nvhe/trace.h
> > index 7da8788ce527..fd641e1b1c23 100644
> > --- a/arch/arm64/kvm/hyp/include/nvhe/trace.h
> > +++ b/arch/arm64/kvm/hyp/include/nvhe/trace.h
> > @@ -11,6 +11,7 @@ int __tracing_load(unsigned long desc_va, size_t desc_size);
> > void __tracing_unload(void);
> > int __tracing_enable(bool enable);
> > int __tracing_swap_reader(unsigned int cpu);
> > +void __tracing_update_clock(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc);
> > #else
> > static inline void *tracing_reserve_entry(unsigned long length) { return NULL; }
> > static inline void tracing_commit_entry(void) { }
> > @@ -19,5 +20,6 @@ static inline int __tracing_load(unsigned long desc_va, size_t desc_size) { retu
> > static inline void __tracing_unload(void) { }
> > static inline int __tracing_enable(bool enable) { return -ENODEV; }
> > static inline int __tracing_swap_reader(unsigned int cpu) { return -ENODEV; }
> > +static inline void __tracing_update_clock(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc) { }
> > #endif
> > #endif
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > index 8b78b29c2069..45b8f70828de 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > @@ -613,6 +613,18 @@ static void handle___tracing_swap_reader(struct kvm_cpu_context *host_ctxt)
> > cpu_reg(host_ctxt, 1) = __tracing_swap_reader(cpu);
> > }
> >
> > +static void handle___tracing_update_clock(struct kvm_cpu_context *host_ctxt)
> > +{
> > + DECLARE_REG(u32, mult, host_ctxt, 1);
> > + DECLARE_REG(u32, shift, host_ctxt, 2);
> > + DECLARE_REG(u64, epoch_ns, host_ctxt, 3);
> > + DECLARE_REG(u64, epoch_cyc, host_ctxt, 4);
> > +
> > + __tracing_update_clock(mult, shift, epoch_ns, epoch_cyc);
> > +
> > + cpu_reg(host_ctxt, 1) = 0;
>
> What's the purpose of setting X1 to 0? This is a call returning void,
> so I don't immediately see the need for this.
Hum, nope nothing. And actually the same applies to __tracing_write_event() and
__tracing_unload()
>
> > +}
> > +
> > typedef void (*hcall_t)(struct kvm_cpu_context *);
> >
> > #define HANDLE_FUNC(x) [__KVM_HOST_SMCCC_FUNC_##x] = (hcall_t)handle_##x
> > @@ -658,6 +670,7 @@ static const hcall_t host_hcall[] = {
> > HANDLE_FUNC(__tracing_unload),
> > HANDLE_FUNC(__tracing_enable),
> > HANDLE_FUNC(__tracing_swap_reader),
> > + HANDLE_FUNC(__tracing_update_clock),
> > };
> >
> > static void handle_host_hcall(struct kvm_cpu_context *host_ctxt)
> > diff --git a/arch/arm64/kvm/hyp/nvhe/trace.c b/arch/arm64/kvm/hyp/nvhe/trace.c
> > index df9d66fcb3c9..97e9f6c1a52c 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/trace.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/trace.c
> > @@ -271,3 +271,19 @@ int __tracing_swap_reader(unsigned int cpu)
> >
> > return ret;
> > }
> > +
> > +void __tracing_update_clock(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc)
> > +{
> > + int cpu;
> > +
> > + /* After this loop, all CPUs are observing the new bank... */
> > + for (cpu = 0; cpu < hyp_nr_cpus; cpu++) {
> > + struct simple_rb_per_cpu *simple_rb = per_cpu_ptr(trace_buffer.simple_rbs, cpu);
> > +
> > + while (READ_ONCE(simple_rb->status) == SIMPLE_RB_WRITING)
> > + ;
> > + }
> > +
> > + /* ...we can now override the old one and swap. */
> > + trace_clock_update(mult, shift, epoch_ns, epoch_cyc);
> > +}
> > diff --git a/arch/arm64/kvm/hyp_trace.c b/arch/arm64/kvm/hyp_trace.c
> > index 2866effe28ec..1e5fc27f0e9d 100644
> > --- a/arch/arm64/kvm/hyp_trace.c
> > +++ b/arch/arm64/kvm/hyp_trace.c
> > @@ -4,15 +4,133 @@
> > * Author: Vincent Donnefort <vdonnefort@...gle.com>
> > */
> >
> > +#include <linux/cpumask.h>
> > #include <linux/trace_remote.h>
> > +#include <linux/tracefs.h>
> > #include <linux/simple_ring_buffer.h>
> >
> > +#include <asm/arch_timer.h>
> > #include <asm/kvm_host.h>
> > #include <asm/kvm_hyptrace.h>
> > #include <asm/kvm_mmu.h>
> >
> > #include "hyp_trace.h"
> >
> > +/* Same 10min used by clocksource when width is more than 32-bits */
> > +#define CLOCK_MAX_CONVERSION_S 600
> > +/*
> > + * Time to give for the clock init. Long enough to get a good mult/shift
> > + * estimation. Short enough to not delay the tracing start too much.
> > + */
> > +#define CLOCK_INIT_MS 100
> > +/*
> > + * Time between clock checks. Must be small enough to catch clock deviation when
> > + * it is still tiny.
> > + */
> > +#define CLOCK_UPDATE_MS 500
>
> If these definitions are common, can't we reuse the existing ones?
> Specially given that this isn't EL2 code.
Only the first is reused but it doesn't exist any definition for it.
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists