Re: [PATCH] KVM: arm64: Avoid naming collision in tracing

[Vincent Donnefort]

On Mon, Jul 13, 2026 at 08:40:58AM +0000, Mostafa Saleh wrote:
> On Mon, Jul 13, 2026 at 09:34:32AM +0100, Vincent Donnefort wrote:
> > On Mon, Jul 13, 2026 at 08:26:35AM +0000, Mostafa Saleh wrote:
> > > On Mon, Jul 13, 2026 at 09:11:27AM +0100, Vincent Donnefort wrote:
> > > > [...]
> > > > 
> > > > > >
> > > > > > > > I did not add Fixes tag as this is currently dormant and not breaking
> > > > > > > > anything.
> > > > > > > > ---
> > > > > > > >  arch/arm64/kvm/hyp/include/nvhe/clock.h | 8 ++++----
> > > > > > > >  arch/arm64/kvm/hyp/nvhe/clock.c         | 4 ++--
> > > > > > > >  arch/arm64/kvm/hyp/nvhe/trace.c         | 4 ++--
> > > > > > > >  3 files changed, 8 insertions(+), 8 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/arch/arm64/kvm/hyp/include/nvhe/clock.h b/arch/arm64/kvm/hyp/include/nvhe/clock.h
> > > > > > > > index 9f429f5c0664..c2ccd0e8bf22 100644
> > > > > > > > --- a/arch/arm64/kvm/hyp/include/nvhe/clock.h
> > > > > > > > +++ b/arch/arm64/kvm/hyp/include/nvhe/clock.h
> > > > > > > > @@ -6,11 +6,11 @@
> > > > > > > >  #include <asm/kvm_hyp.h>
> > > > > > > >
> > > > > > > >  #ifdef CONFIG_NVHE_EL2_TRACING
> > > > > > > > -void trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc);
> > > > > > > > -u64 trace_clock(void);
> > > > > > > > +void hyp_trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc);
> > > > > > > > +u64 hyp_trace_clock(void);
> > > > > > >
> > > > > > > hyp_trace_clock overlaps the host side: arch/arm64/kvm/hyp_trace.c
> > > > > > > already has a struct hyp_trace_clock and static helpers
> > > > > > > hyp_trace_clock_enable() / hyp_trace_clock_show() for the debugfs view
> > > > > > > of the same clock. No actual collision, so this is only a readability
> > > > > > > point, but a reader grepping hyp_trace_clock now gets two unrelated
> > > > > > > things. Maybe you'd want to consider a different name, but naming is
> > > > > > > hard :)
> > > > 
> > > > If we were to rename: the clock is a "hyp_clock" so probably trace_hyp_clock()
> > > > is the right thing here.
> > > > 
> > > > while the "hyp_trace_" is the prefix for hyp_trace.c file content.
> > > > 
> > > > Although I would like to see why we pull trace_clock() from the kernel into EL2.
> > > > That bit sounds wrong and if we have a way around perhaps that's better?
> > > 
> > > Most include path look like this:
> > > In file included from ./include/linux/ftrace.h:11,
> > >                  from ./include/linux/kprobes.h:28,
> > >                  from ./include/linux/kgdb.h:17,
> > >                  from ./arch/arm64/include/asm/cacheflush.h:11,
> > >                  from ./include/linux/cacheflush.h:5,
> > >                  from ./include/linux/highmem.h:8,
> > >                  from ./include/linux/bvec.h:10,
> > >                  from ./include/linux/blk_types.h:10,
> > >                  from ./include/linux/writeback.h:13,
> > >                  from ./include/linux/memcontrol.h:23,
> > >                  from ./include/linux/resume_user_mode.h:8,
> > >                  from ./include/linux/entry-virt.h:6,
> > >                  from ./include/linux/kvm_host.h:5,
> > > 
> > > Although, some files have ./arch/arm64/include/asm/cacheflush.h
> > > directly.
> > 
> > Ha sad, no way around that... 
> > 
> > So about trace_hyp_clock() / trace_hyp_clock_update() ?
> > 
> 
> Makes sense, I will respin with that.

Thanks. And small nit for the patch title: use "hyp tracing" to distinguish from
the kernel tracing.

> 
> Thanks,
> Mostafa
>