* [PATCH] KVM: arm64: Avoid naming collision in tracing
@ 2026-07-12 15:38 Mostafa Saleh
2026-07-12 16:19 ` Fuad Tabba
2026-07-13 8:02 ` Vincent Donnefort
0 siblings, 2 replies; 12+ messages in thread
From: Mostafa Saleh @ 2026-07-12 15:38 UTC (permalink / raw)
To: linux-kernel, kvmarm, linux-arm-kernel
Cc: maz, oupton, seiden, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, will, vdonnefort, tabba, Mostafa Saleh
When the hypervisor tracing (CONFIG_NVHE_EL2_TRACING) is disabled, it
defines a static inline stub for trace_clock().
However, trace_clock() is already declared as an extern function in
linux/trace_clock.h which is pulled in EL2 compilation.
If the file <nvhe/clock.h> is included when CONFIG_NVHE_EL2_TRACING
is disabled (by including it manually in setup.c) it will cause:
In file included from arch/arm64/kvm/hyp/nvhe/setup.c:22:
./arch/arm64/kvm/hyp/include/nvhe/clock.h:14:19: error: static declaration of ‘trace_clock’ follows non-static declaration
14 | static inline u64 trace_clock(void) { return 0; }
| ^~~~~~~~~~~
on GCC and a linker error on LLVM (it seems to change the linkage to
global)
Although that is not a problem at the moment, as no other files
include <nvhe/clock.h>. That does not seem to be the intent of
this code and that will cause issues with more users as the SMMUv3
driver.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
--
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);
#else
static inline void
-trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc) { }
-static inline u64 trace_clock(void) { return 0; }
+hyp_trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc) { }
+static inline u64 hyp_trace_clock(void) { return 0; }
#endif
#endif
diff --git a/arch/arm64/kvm/hyp/nvhe/clock.c b/arch/arm64/kvm/hyp/nvhe/clock.c
index a7fc61976fd0..8adefb0b696c 100644
--- a/arch/arm64/kvm/hyp/nvhe/clock.c
+++ b/arch/arm64/kvm/hyp/nvhe/clock.c
@@ -30,7 +30,7 @@ static u64 __clock_mult_uint128(u64 cyc, u32 mult, u32 shift)
}
/* Does not guarantee no reader on the modified bank. */
-void trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc)
+void hyp_trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc)
{
struct clock_data *clock = &trace_clock_data;
u64 bank = clock->cur ^ 1;
@@ -48,7 +48,7 @@ void trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc)
}
/* Use untrusted host data */
-u64 trace_clock(void)
+u64 hyp_trace_clock(void)
{
struct clock_data *clock = &trace_clock_data;
u64 bank = smp_load_acquire(&clock->cur);
diff --git a/arch/arm64/kvm/hyp/nvhe/trace.c b/arch/arm64/kvm/hyp/nvhe/trace.c
index e7e150ab265f..f2a32463c6c2 100644
--- a/arch/arm64/kvm/hyp/nvhe/trace.c
+++ b/arch/arm64/kvm/hyp/nvhe/trace.c
@@ -35,7 +35,7 @@ static bool hyp_trace_buffer_loaded(struct hyp_trace_buffer *trace_buffer)
void *tracing_reserve_entry(unsigned long length)
{
return simple_ring_buffer_reserve(this_cpu_ptr(trace_buffer.simple_rbs), length,
- trace_clock());
+ hyp_trace_clock());
}
void tracing_commit_entry(void)
@@ -290,7 +290,7 @@ void __tracing_update_clock(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc)
}
/* ...we can now override the old one and swap. */
- trace_clock_update(mult, shift, epoch_ns, epoch_cyc);
+ hyp_trace_clock_update(mult, shift, epoch_ns, epoch_cyc);
}
int __tracing_reset(unsigned int cpu)
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] KVM: arm64: Avoid naming collision in tracing
2026-07-12 15:38 [PATCH] KVM: arm64: Avoid naming collision in tracing Mostafa Saleh
@ 2026-07-12 16:19 ` Fuad Tabba
2026-07-12 19:19 ` Mostafa Saleh
2026-07-13 8:02 ` Vincent Donnefort
1 sibling, 1 reply; 12+ messages in thread
From: Fuad Tabba @ 2026-07-12 16:19 UTC (permalink / raw)
To: Mostafa Saleh
Cc: linux-kernel, kvmarm, linux-arm-kernel, maz, oupton, seiden,
joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas, will,
vdonnefort
Hi Mostafa,
On Sun, 12 Jul 2026 at 16:38, Mostafa Saleh <smostafa@google.com> wrote:
>
> When the hypervisor tracing (CONFIG_NVHE_EL2_TRACING) is disabled, it
> defines a static inline stub for trace_clock().
>
> However, trace_clock() is already declared as an extern function in
> linux/trace_clock.h which is pulled in EL2 compilation.
The rename itself looks correct and complete: all three sites (the
declarations and stubs in nvhe/clock.h, the definitions in clock.c,
and the two callers in trace.c) are updated, and no other reference to
the old names remains under arch/arm64/kvm.
Two small things...
>
> If the file <nvhe/clock.h> is included when CONFIG_NVHE_EL2_TRACING
> is disabled (by including it manually in setup.c) it will cause:
> In file included from arch/arm64/kvm/hyp/nvhe/setup.c:22:
>
> ./arch/arm64/kvm/hyp/include/nvhe/clock.h:14:19: error: static declaration of ‘trace_clock’ follows non-static declaration
>
> 14 | static inline u64 trace_clock(void) { return 0; }
>
> | ^~~~~~~~~~~
>
> on GCC and a linker error on LLVM (it seems to change the linkage to
> global)
>
> Although that is not a problem at the moment, as no other files
> include <nvhe/clock.h>. That does not seem to be the intent of
> this code and that will cause issues with more users as the SMMUv3
> driver.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
>
> --
git am only treats a three-dash "---" line as the scissors; the "--"
here is two dashes, so mailinfo keeps everything above the diffstat.
Worth a "---" on the repost so the note stays out of the log.
> 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 :)
Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
Tested-by: Fuad Tabba < fuad.tabba@linux.dev>
Test: builds fine with the different config enables.
Cheers,
/fuad
> #else
> static inline void
> -trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc) { }
> -static inline u64 trace_clock(void) { return 0; }
> +hyp_trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc) { }
> +static inline u64 hyp_trace_clock(void) { return 0; }
> #endif
> #endif
> diff --git a/arch/arm64/kvm/hyp/nvhe/clock.c b/arch/arm64/kvm/hyp/nvhe/clock.c
> index a7fc61976fd0..8adefb0b696c 100644
> --- a/arch/arm64/kvm/hyp/nvhe/clock.c
> +++ b/arch/arm64/kvm/hyp/nvhe/clock.c
> @@ -30,7 +30,7 @@ static u64 __clock_mult_uint128(u64 cyc, u32 mult, u32 shift)
> }
>
> /* Does not guarantee no reader on the modified bank. */
> -void trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc)
> +void hyp_trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc)
> {
> struct clock_data *clock = &trace_clock_data;
> u64 bank = clock->cur ^ 1;
> @@ -48,7 +48,7 @@ void trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc)
> }
>
> /* Use untrusted host data */
> -u64 trace_clock(void)
> +u64 hyp_trace_clock(void)
> {
> struct clock_data *clock = &trace_clock_data;
> u64 bank = smp_load_acquire(&clock->cur);
> diff --git a/arch/arm64/kvm/hyp/nvhe/trace.c b/arch/arm64/kvm/hyp/nvhe/trace.c
> index e7e150ab265f..f2a32463c6c2 100644
> --- a/arch/arm64/kvm/hyp/nvhe/trace.c
> +++ b/arch/arm64/kvm/hyp/nvhe/trace.c
> @@ -35,7 +35,7 @@ static bool hyp_trace_buffer_loaded(struct hyp_trace_buffer *trace_buffer)
> void *tracing_reserve_entry(unsigned long length)
> {
> return simple_ring_buffer_reserve(this_cpu_ptr(trace_buffer.simple_rbs), length,
> - trace_clock());
> + hyp_trace_clock());
> }
>
> void tracing_commit_entry(void)
> @@ -290,7 +290,7 @@ void __tracing_update_clock(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc)
> }
>
> /* ...we can now override the old one and swap. */
> - trace_clock_update(mult, shift, epoch_ns, epoch_cyc);
> + hyp_trace_clock_update(mult, shift, epoch_ns, epoch_cyc);
> }
>
> int __tracing_reset(unsigned int cpu)
> --
> 2.55.0.795.g602f6c329a-goog
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] KVM: arm64: Avoid naming collision in tracing
2026-07-12 16:19 ` Fuad Tabba
@ 2026-07-12 19:19 ` Mostafa Saleh
2026-07-12 19:30 ` Fuad Tabba
0 siblings, 1 reply; 12+ messages in thread
From: Mostafa Saleh @ 2026-07-12 19:19 UTC (permalink / raw)
To: Fuad Tabba
Cc: linux-kernel, kvmarm, linux-arm-kernel, maz, oupton, seiden,
joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas, will,
vdonnefort
Hi Fuad,
On Sun, Jul 12, 2026 at 05:19:22PM +0100, Fuad Tabba wrote:
> Hi Mostafa,
>
> On Sun, 12 Jul 2026 at 16:38, Mostafa Saleh <smostafa@google.com> wrote:
> >
> > When the hypervisor tracing (CONFIG_NVHE_EL2_TRACING) is disabled, it
> > defines a static inline stub for trace_clock().
> >
> > However, trace_clock() is already declared as an extern function in
> > linux/trace_clock.h which is pulled in EL2 compilation.
>
> The rename itself looks correct and complete: all three sites (the
> declarations and stubs in nvhe/clock.h, the definitions in clock.c,
> and the two callers in trace.c) are updated, and no other reference to
> the old names remains under arch/arm64/kvm.
>
> Two small things...
>
> >
> > If the file <nvhe/clock.h> is included when CONFIG_NVHE_EL2_TRACING
> > is disabled (by including it manually in setup.c) it will cause:
> > In file included from arch/arm64/kvm/hyp/nvhe/setup.c:22:
> >
> > ./arch/arm64/kvm/hyp/include/nvhe/clock.h:14:19: error: static declaration of ‘trace_clock’ follows non-static declaration
> >
> > 14 | static inline u64 trace_clock(void) { return 0; }
> >
> > | ^~~~~~~~~~~
> >
> > on GCC and a linker error on LLVM (it seems to change the linkage to
> > global)
> >
> > Although that is not a problem at the moment, as no other files
> > include <nvhe/clock.h>. That does not seem to be the intent of
> > this code and that will cause issues with more users as the SMMUv3
> > driver.
> >
> > Signed-off-by: Mostafa Saleh <smostafa@google.com>
> >
> > --
>
> git am only treats a three-dash "---" line as the scissors; the "--"
> here is two dashes, so mailinfo keeps everything above the diffstat.
> Worth a "---" on the repost so the note stays out of the log.
>
ops, I will fix that.
> > 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 :)
Yes, that shouldn't be a problem, I just added hyp_ prefix, but I
am ok with any suggestions!
Thanks,
Mostafa
>
> Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
> Tested-by: Fuad Tabba < fuad.tabba@linux.dev>
>
> Test: builds fine with the different config enables.
>
> Cheers,
> /fuad
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] KVM: arm64: Avoid naming collision in tracing
2026-07-12 19:19 ` Mostafa Saleh
@ 2026-07-12 19:30 ` Fuad Tabba
2026-07-13 8:11 ` Vincent Donnefort
0 siblings, 1 reply; 12+ messages in thread
From: Fuad Tabba @ 2026-07-12 19:30 UTC (permalink / raw)
To: Mostafa Saleh
Cc: linux-kernel, kvmarm, linux-arm-kernel, maz, oupton, seiden,
joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas, will,
vdonnefort
On Sun, 12 Jul 2026 at 20:19, Mostafa Saleh <smostafa@google.com> wrote:
>
> Hi Fuad,
>
> On Sun, Jul 12, 2026 at 05:19:22PM +0100, Fuad Tabba wrote:
> > Hi Mostafa,
> >
> > On Sun, 12 Jul 2026 at 16:38, Mostafa Saleh <smostafa@google.com> wrote:
> > >
> > > When the hypervisor tracing (CONFIG_NVHE_EL2_TRACING) is disabled, it
> > > defines a static inline stub for trace_clock().
> > >
> > > However, trace_clock() is already declared as an extern function in
> > > linux/trace_clock.h which is pulled in EL2 compilation.
> >
> > The rename itself looks correct and complete: all three sites (the
> > declarations and stubs in nvhe/clock.h, the definitions in clock.c,
> > and the two callers in trace.c) are updated, and no other reference to
> > the old names remains under arch/arm64/kvm.
> >
> > Two small things...
> >
> > >
> > > If the file <nvhe/clock.h> is included when CONFIG_NVHE_EL2_TRACING
> > > is disabled (by including it manually in setup.c) it will cause:
> > > In file included from arch/arm64/kvm/hyp/nvhe/setup.c:22:
> > >
> > > ./arch/arm64/kvm/hyp/include/nvhe/clock.h:14:19: error: static declaration of ‘trace_clock’ follows non-static declaration
> > >
> > > 14 | static inline u64 trace_clock(void) { return 0; }
> > >
> > > | ^~~~~~~~~~~
> > >
> > > on GCC and a linker error on LLVM (it seems to change the linkage to
> > > global)
> > >
> > > Although that is not a problem at the moment, as no other files
> > > include <nvhe/clock.h>. That does not seem to be the intent of
> > > this code and that will cause issues with more users as the SMMUv3
> > > driver.
> > >
> > > Signed-off-by: Mostafa Saleh <smostafa@google.com>
> > >
> > > --
> >
> > git am only treats a three-dash "---" line as the scissors; the "--"
> > here is two dashes, so mailinfo keeps everything above the diffstat.
> > Worth a "---" on the repost so the note stays out of the log.
> >
>
> ops, I will fix that.
>
>
> > > 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 :)
>
> Yes, that shouldn't be a problem, I just added hyp_ prefix, but I
> am ok with any suggestions!
Didn't I say naming is hard? :) How about, el2_? Not really happy with
that either tbh... but can't think of a better one...
/fuad
>
> Thanks,
> Mostafa
>
> >
> > Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
> > Tested-by: Fuad Tabba < fuad.tabba@linux.dev>
> >
> > Test: builds fine with the different config enables.
> >
> > Cheers,
> > /fuad
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] KVM: arm64: Avoid naming collision in tracing
2026-07-12 19:30 ` Fuad Tabba
@ 2026-07-13 8:11 ` Vincent Donnefort
2026-07-13 8:26 ` Mostafa Saleh
0 siblings, 1 reply; 12+ messages in thread
From: Vincent Donnefort @ 2026-07-13 8:11 UTC (permalink / raw)
To: Fuad Tabba
Cc: Mostafa Saleh, linux-kernel, kvmarm, linux-arm-kernel, maz,
oupton, seiden, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, will
[...]
> >
> > > > 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?
> >
> > Yes, that shouldn't be a problem, I just added hyp_ prefix, but I
> > am ok with any suggestions!
>
> Didn't I say naming is hard? :) How about, el2_? Not really happy with
> that either tbh... but can't think of a better one...
>
> /fuad
>
> >
> > Thanks,
> > Mostafa
> >
> > >
> > > Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
> > > Tested-by: Fuad Tabba < fuad.tabba@linux.dev>
> > >
> > > Test: builds fine with the different config enables.
> > >
> > > Cheers,
> > > /fuad
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] KVM: arm64: Avoid naming collision in tracing
2026-07-13 8:11 ` Vincent Donnefort
@ 2026-07-13 8:26 ` Mostafa Saleh
2026-07-13 8:34 ` Vincent Donnefort
0 siblings, 1 reply; 12+ messages in thread
From: Mostafa Saleh @ 2026-07-13 8:26 UTC (permalink / raw)
To: Vincent Donnefort
Cc: Fuad Tabba, linux-kernel, kvmarm, linux-arm-kernel, maz, oupton,
seiden, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
will
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.
Thanks,
Mostafa
>
> > >
> > > Yes, that shouldn't be a problem, I just added hyp_ prefix, but I
> > > am ok with any suggestions!
> >
> > Didn't I say naming is hard? :) How about, el2_? Not really happy with
> > that either tbh... but can't think of a better one...
> >
> > /fuad
> >
> > >
> > > Thanks,
> > > Mostafa
> > >
> > > >
> > > > Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
> > > > Tested-by: Fuad Tabba < fuad.tabba@linux.dev>
> > > >
> > > > Test: builds fine with the different config enables.
> > > >
> > > > Cheers,
> > > > /fuad
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] KVM: arm64: Avoid naming collision in tracing
2026-07-13 8:26 ` Mostafa Saleh
@ 2026-07-13 8:34 ` Vincent Donnefort
2026-07-13 8:40 ` Mostafa Saleh
0 siblings, 1 reply; 12+ messages in thread
From: Vincent Donnefort @ 2026-07-13 8:34 UTC (permalink / raw)
To: Mostafa Saleh
Cc: Fuad Tabba, linux-kernel, kvmarm, linux-arm-kernel, maz, oupton,
seiden, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
will
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() ?
>
> Thanks,
> Mostafa
>
> >
> > > >
> > > > Yes, that shouldn't be a problem, I just added hyp_ prefix, but I
> > > > am ok with any suggestions!
> > >
> > > Didn't I say naming is hard? :) How about, el2_? Not really happy with
> > > that either tbh... but can't think of a better one...
> > >
> > > /fuad
> > >
> > > >
> > > > Thanks,
> > > > Mostafa
> > > >
> > > > >
> > > > > Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
> > > > > Tested-by: Fuad Tabba < fuad.tabba@linux.dev>
> > > > >
> > > > > Test: builds fine with the different config enables.
> > > > >
> > > > > Cheers,
> > > > > /fuad
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] KVM: arm64: Avoid naming collision in tracing
2026-07-13 8:34 ` Vincent Donnefort
@ 2026-07-13 8:40 ` Mostafa Saleh
2026-07-13 8:55 ` Vincent Donnefort
0 siblings, 1 reply; 12+ messages in thread
From: Mostafa Saleh @ 2026-07-13 8:40 UTC (permalink / raw)
To: Vincent Donnefort
Cc: Fuad Tabba, linux-kernel, kvmarm, linux-arm-kernel, maz, oupton,
seiden, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
will
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,
Mostafa
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] KVM: arm64: Avoid naming collision in tracing
2026-07-13 8:40 ` Mostafa Saleh
@ 2026-07-13 8:55 ` Vincent Donnefort
0 siblings, 0 replies; 12+ messages in thread
From: Vincent Donnefort @ 2026-07-13 8:55 UTC (permalink / raw)
To: Mostafa Saleh
Cc: Fuad Tabba, linux-kernel, kvmarm, linux-arm-kernel, maz, oupton,
seiden, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
will
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] KVM: arm64: Avoid naming collision in tracing
2026-07-12 15:38 [PATCH] KVM: arm64: Avoid naming collision in tracing Mostafa Saleh
2026-07-12 16:19 ` Fuad Tabba
@ 2026-07-13 8:02 ` Vincent Donnefort
2026-07-13 8:05 ` Vincent Donnefort
1 sibling, 1 reply; 12+ messages in thread
From: Vincent Donnefort @ 2026-07-13 8:02 UTC (permalink / raw)
To: Mostafa Saleh
Cc: linux-kernel, kvmarm, linux-arm-kernel, maz, oupton, seiden,
joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas, will,
tabba
On Sun, Jul 12, 2026 at 03:38:35PM +0000, Mostafa Saleh wrote:
> When the hypervisor tracing (CONFIG_NVHE_EL2_TRACING) is disabled, it
> defines a static inline stub for trace_clock().
>
> However, trace_clock() is already declared as an extern function in
> linux/trace_clock.h which is pulled in EL2 compilation.
I am completely unable to reproduce that. CONFIG_NVHE_EL2_TRACING, isn't a
selectable option, it depends on NVHE_EL2_DEBUG.
How does your .config look like?
>
> If the file <nvhe/clock.h> is included when CONFIG_NVHE_EL2_TRACING
> is disabled (by including it manually in setup.c) it will cause:
> In file included from arch/arm64/kvm/hyp/nvhe/setup.c:22:
nvhe/clock.h seems already included in setup.c
>
> ./arch/arm64/kvm/hyp/include/nvhe/clock.h:14:19: error: static declaration of ‘trace_clock’ follows non-static declaration
>
> 14 | static inline u64 trace_clock(void) { return 0; }
>
> | ^~~~~~~~~~~
>
> on GCC and a linker error on LLVM (it seems to change the linkage to
> global)
>
> Although that is not a problem at the moment, as no other files
> include <nvhe/clock.h>. That does not seem to be the intent of
> this code and that will cause issues with more users as the SMMUv3
> driver.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
>
> --
> 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);
> #else
> static inline void
> -trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc) { }
> -static inline u64 trace_clock(void) { return 0; }
> +hyp_trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc) { }
> +static inline u64 hyp_trace_clock(void) { return 0; }
> #endif
> #endif
> diff --git a/arch/arm64/kvm/hyp/nvhe/clock.c b/arch/arm64/kvm/hyp/nvhe/clock.c
> index a7fc61976fd0..8adefb0b696c 100644
> --- a/arch/arm64/kvm/hyp/nvhe/clock.c
> +++ b/arch/arm64/kvm/hyp/nvhe/clock.c
> @@ -30,7 +30,7 @@ static u64 __clock_mult_uint128(u64 cyc, u32 mult, u32 shift)
> }
>
> /* Does not guarantee no reader on the modified bank. */
> -void trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc)
> +void hyp_trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc)
> {
> struct clock_data *clock = &trace_clock_data;
> u64 bank = clock->cur ^ 1;
> @@ -48,7 +48,7 @@ void trace_clock_update(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc)
> }
>
> /* Use untrusted host data */
> -u64 trace_clock(void)
> +u64 hyp_trace_clock(void)
> {
> struct clock_data *clock = &trace_clock_data;
> u64 bank = smp_load_acquire(&clock->cur);
> diff --git a/arch/arm64/kvm/hyp/nvhe/trace.c b/arch/arm64/kvm/hyp/nvhe/trace.c
> index e7e150ab265f..f2a32463c6c2 100644
> --- a/arch/arm64/kvm/hyp/nvhe/trace.c
> +++ b/arch/arm64/kvm/hyp/nvhe/trace.c
> @@ -35,7 +35,7 @@ static bool hyp_trace_buffer_loaded(struct hyp_trace_buffer *trace_buffer)
> void *tracing_reserve_entry(unsigned long length)
> {
> return simple_ring_buffer_reserve(this_cpu_ptr(trace_buffer.simple_rbs), length,
> - trace_clock());
> + hyp_trace_clock());
> }
>
> void tracing_commit_entry(void)
> @@ -290,7 +290,7 @@ void __tracing_update_clock(u32 mult, u32 shift, u64 epoch_ns, u64 epoch_cyc)
> }
>
> /* ...we can now override the old one and swap. */
> - trace_clock_update(mult, shift, epoch_ns, epoch_cyc);
> + hyp_trace_clock_update(mult, shift, epoch_ns, epoch_cyc);
> }
>
> int __tracing_reset(unsigned int cpu)
> --
> 2.55.0.795.g602f6c329a-goog
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] KVM: arm64: Avoid naming collision in tracing
2026-07-13 8:02 ` Vincent Donnefort
@ 2026-07-13 8:05 ` Vincent Donnefort
2026-07-13 8:19 ` Mostafa Saleh
0 siblings, 1 reply; 12+ messages in thread
From: Vincent Donnefort @ 2026-07-13 8:05 UTC (permalink / raw)
To: Mostafa Saleh
Cc: linux-kernel, kvmarm, linux-arm-kernel, maz, oupton, seiden,
joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas, will,
tabba
On Mon, Jul 13, 2026 at 09:02:48AM +0100, Vincent Donnefort wrote:
> On Sun, Jul 12, 2026 at 03:38:35PM +0000, Mostafa Saleh wrote:
> > When the hypervisor tracing (CONFIG_NVHE_EL2_TRACING) is disabled, it
> > defines a static inline stub for trace_clock().
> >
> > However, trace_clock() is already declared as an extern function in
> > linux/trace_clock.h which is pulled in EL2 compilation.
>
> I am completely unable to reproduce that. CONFIG_NVHE_EL2_TRACING, isn't a
> selectable option, it depends on NVHE_EL2_DEBUG.
>
> How does your .config look like?
>
> >
> > If the file <nvhe/clock.h> is included when CONFIG_NVHE_EL2_TRACING
> > is disabled (by including it manually in setup.c) it will cause:
> > In file included from arch/arm64/kvm/hyp/nvhe/setup.c:22:
>
> nvhe/clock.h seems already included in setup.c
Ha no appologies, I was looking at the wrong branch.
However I am still enable to reproduce this issue. So I am still interested in
knowing your .defconfig
>
> >
> > ./arch/arm64/kvm/hyp/include/nvhe/clock.h:14:19: error: static declaration of ‘trace_clock’ follows non-static declaration
> >
> > 14 | static inline u64 trace_clock(void) { return 0; }
> >
> > | ^~~~~~~~~~~
> >
> > on GCC and a linker error on LLVM (it seems to change the linkage to
> > global)
> >
> > Although that is not a problem at the moment, as no other files
> > include <nvhe/clock.h>. That does not seem to be the intent of
> > this code and that will cause issues with more users as the SMMUv3
> > driver.
> >
> > Signed-off-by: Mostafa Saleh <smostafa@google.com>
> >
> > --
[...]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] KVM: arm64: Avoid naming collision in tracing
2026-07-13 8:05 ` Vincent Donnefort
@ 2026-07-13 8:19 ` Mostafa Saleh
0 siblings, 0 replies; 12+ messages in thread
From: Mostafa Saleh @ 2026-07-13 8:19 UTC (permalink / raw)
To: Vincent Donnefort
Cc: linux-kernel, kvmarm, linux-arm-kernel, maz, oupton, seiden,
joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas, will,
tabba
On Mon, Jul 13, 2026 at 09:05:55AM +0100, Vincent Donnefort wrote:
> On Mon, Jul 13, 2026 at 09:02:48AM +0100, Vincent Donnefort wrote:
> > On Sun, Jul 12, 2026 at 03:38:35PM +0000, Mostafa Saleh wrote:
> > > When the hypervisor tracing (CONFIG_NVHE_EL2_TRACING) is disabled, it
> > > defines a static inline stub for trace_clock().
> > >
> > > However, trace_clock() is already declared as an extern function in
> > > linux/trace_clock.h which is pulled in EL2 compilation.
> >
> > I am completely unable to reproduce that. CONFIG_NVHE_EL2_TRACING, isn't a
> > selectable option, it depends on NVHE_EL2_DEBUG.
> >
> > How does your .config look like?
> >
> > >
> > > If the file <nvhe/clock.h> is included when CONFIG_NVHE_EL2_TRACING
> > > is disabled (by including it manually in setup.c) it will cause:
> > > In file included from arch/arm64/kvm/hyp/nvhe/setup.c:22:
> >
> > nvhe/clock.h seems already included in setup.c
>
> Ha no appologies, I was looking at the wrong branch.
>
> However I am still enable to reproduce this issue. So I am still interested in
> knowing your .defconfig
It is just defconfig (CONFIG_NVHE_EL2_TRACING is already disabled).
If you attempt to include <nvhe/clock.h> in another file:
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index d3c69de698f4..ca91e453fb10 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -15,6 +15,7 @@
#include <asm/kvm_hypevents.h>
#include <asm/kvm_mmu.h>
+#include <nvhe/clock.h>
#include <nvhe/ffa.h>
#include <nvhe/mem_protect.h>
#include <nvhe/mm.h>
With GCC: make ARCH=arm64 CROSS_COMPILE="aarch64-linux-gnu-" -j`nproc`
It will show the above error.
With LLVM, you have to include it in more than one file, which results
in:
>>> defined at clock.h:14 (./arch/arm64/kvm/hyp/include/nvhe/clock.h:14)
>>> arch/arm64/kvm/hyp/nvhe/hyp-main.nvhe.o:(trace_clock)
>>> defined at clock.h:14 (./arch/arm64/kvm/hyp/include/nvhe/clock.h:14)
>>> arch/arm64/kvm/hyp/nvhe/mem_protect.nvhe.o:(.text+0x0)
>>>
As the compiler would convert the function global.
Thanks,
Mostafa
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-07-13 8:55 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-12 15:38 [PATCH] KVM: arm64: Avoid naming collision in tracing Mostafa Saleh
2026-07-12 16:19 ` Fuad Tabba
2026-07-12 19:19 ` Mostafa Saleh
2026-07-12 19:30 ` Fuad Tabba
2026-07-13 8:11 ` Vincent Donnefort
2026-07-13 8:26 ` Mostafa Saleh
2026-07-13 8:34 ` Vincent Donnefort
2026-07-13 8:40 ` Mostafa Saleh
2026-07-13 8:55 ` Vincent Donnefort
2026-07-13 8:02 ` Vincent Donnefort
2026-07-13 8:05 ` Vincent Donnefort
2026-07-13 8:19 ` Mostafa Saleh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox