[PATCH v8 26/28] KVM: arm64: Add hyp_enter/hyp_exit events to pKVM hyp

Vincent Donnefort posted 28 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v8 26/28] KVM: arm64: Add hyp_enter/hyp_exit events to pKVM hyp
Posted by Vincent Donnefort 1 month, 1 week ago
The hyp_enter and hyp_exit events are logged by the hypervisor any time
it is entered and exited.

Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/arch/arm64/include/asm/kvm_hypevents.h b/arch/arm64/include/asm/kvm_hypevents.h
index d6e033c96c52..ce3953bc884a 100644
--- a/arch/arm64/include/asm/kvm_hypevents.h
+++ b/arch/arm64/include/asm/kvm_hypevents.h
@@ -7,4 +7,21 @@
 #include <nvhe/trace.h>
 #endif
 
+HYP_EVENT(hyp_enter,
+	HE_PROTO(void),
+	HE_STRUCT(
+	),
+	HE_ASSIGN(
+	),
+	HE_PRINTK()
+);
+
+HYP_EVENT(hyp_exit,
+	HE_PROTO(void),
+	HE_STRUCT(
+	),
+	HE_ASSIGN(
+	),
+	HE_PRINTK()
+);
 #endif
diff --git a/arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h b/arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h
new file mode 100644
index 000000000000..4b69d33e4f2d
--- /dev/null
+++ b/arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <asm/kvm_hypevents.h>
+
+#include <linux/arm-smccc.h>
+
+#undef arm_smccc_1_1_smc
+#define arm_smccc_1_1_smc(...)					\
+	do {							\
+		trace_hyp_exit();				\
+		__arm_smccc_1_1(SMCCC_SMC_INST, __VA_ARGS__);	\
+		trace_hyp_enter();				\
+	} while (0)
diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index 4e16f9b96f63..aee829acd3dd 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -26,10 +26,10 @@
  * the duration and are therefore serialised.
  */
 
-#include <linux/arm-smccc.h>
 #include <linux/arm_ffa.h>
 #include <asm/kvm_pkvm.h>
 
+#include <nvhe/arm-smccc.h>
 #include <nvhe/ffa.h>
 #include <nvhe/mem_protect.h>
 #include <nvhe/memory.h>
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index 5e4b519e5204..bb81906878c9 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -12,6 +12,7 @@
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_host.h>
 #include <asm/kvm_hyp.h>
+#include <asm/kvm_hypevents.h>
 #include <asm/kvm_mmu.h>
 
 #include <nvhe/ffa.h>
@@ -730,7 +731,9 @@ static void handle_host_hcall(struct kvm_cpu_context *host_ctxt)
 
 static void default_host_smc_handler(struct kvm_cpu_context *host_ctxt)
 {
+	trace_hyp_exit();
 	__kvm_hyp_host_forward_smc(host_ctxt);
+	trace_hyp_enter();
 }
 
 static void handle_host_smc(struct kvm_cpu_context *host_ctxt)
@@ -754,6 +757,8 @@ void handle_trap(struct kvm_cpu_context *host_ctxt)
 {
 	u64 esr = read_sysreg_el2(SYS_ESR);
 
+	trace_hyp_enter();
+
 	switch (ESR_ELx_EC(esr)) {
 	case ESR_ELx_EC_HVC64:
 		handle_host_hcall(host_ctxt);
@@ -768,4 +773,6 @@ void handle_trap(struct kvm_cpu_context *host_ctxt)
 	default:
 		BUG();
 	}
+
+	trace_hyp_exit();
 }
diff --git a/arch/arm64/kvm/hyp/nvhe/psci-relay.c b/arch/arm64/kvm/hyp/nvhe/psci-relay.c
index c3e196fb8b18..64d1d418df1d 100644
--- a/arch/arm64/kvm/hyp/nvhe/psci-relay.c
+++ b/arch/arm64/kvm/hyp/nvhe/psci-relay.c
@@ -6,11 +6,12 @@
 
 #include <asm/kvm_asm.h>
 #include <asm/kvm_hyp.h>
+#include <asm/kvm_hypevents.h>
 #include <asm/kvm_mmu.h>
-#include <linux/arm-smccc.h>
 #include <linux/kvm_host.h>
 #include <uapi/linux/psci.h>
 
+#include <nvhe/arm-smccc.h>
 #include <nvhe/memory.h>
 #include <nvhe/trap_handler.h>
 
@@ -205,6 +206,7 @@ asmlinkage void __noreturn __kvm_host_psci_cpu_entry(bool is_cpu_on)
 	struct psci_boot_args *boot_args;
 	struct kvm_cpu_context *host_ctxt;
 
+	trace_hyp_enter();
 	host_ctxt = host_data_ptr(host_ctxt);
 
 	if (is_cpu_on)
@@ -221,6 +223,7 @@ asmlinkage void __noreturn __kvm_host_psci_cpu_entry(bool is_cpu_on)
 	write_sysreg_el1(INIT_SCTLR_EL1_MMU_OFF, SYS_SCTLR);
 	write_sysreg(INIT_PSTATE_EL1, SPSR_EL2);
 
+	trace_hyp_exit();
 	__host_enter(host_ctxt);
 }
 
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index d3b9ec8a7c28..66ed30135815 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -7,7 +7,6 @@
 #include <hyp/switch.h>
 #include <hyp/sysreg-sr.h>
 
-#include <linux/arm-smccc.h>
 #include <linux/kvm_host.h>
 #include <linux/types.h>
 #include <linux/jump_label.h>
@@ -21,6 +20,7 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_hyp.h>
+#include <asm/kvm_hypevents.h>
 #include <asm/kvm_mmu.h>
 #include <asm/fpsimd.h>
 #include <asm/debug-monitors.h>
@@ -308,10 +308,13 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 	__debug_switch_to_guest(vcpu);
 
 	do {
+		trace_hyp_exit();
+
 		/* Jump in the fire! */
 		exit_code = __guest_enter(vcpu);
 
 		/* And we're baaack! */
+		trace_hyp_enter();
 	} while (fixup_guest_exit(vcpu, &exit_code));
 
 	__sysreg_save_state_nvhe(guest_ctxt);
-- 
2.51.2.1041.gc1ab5b90ca-goog
Re: [PATCH v8 26/28] KVM: arm64: Add hyp_enter/hyp_exit events to pKVM hyp
Posted by Marc Zyngier 2 weeks, 4 days ago
On Fri, 07 Nov 2025 09:38:38 +0000,
Vincent Donnefort <vdonnefort@google.com> wrote:
> 
> The hyp_enter and hyp_exit events are logged by the hypervisor any time
> it is entered and exited.
> 
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> 
> diff --git a/arch/arm64/include/asm/kvm_hypevents.h b/arch/arm64/include/asm/kvm_hypevents.h
> index d6e033c96c52..ce3953bc884a 100644
> --- a/arch/arm64/include/asm/kvm_hypevents.h
> +++ b/arch/arm64/include/asm/kvm_hypevents.h
> @@ -7,4 +7,21 @@
>  #include <nvhe/trace.h>
>  #endif
>  
> +HYP_EVENT(hyp_enter,
> +	HE_PROTO(void),
> +	HE_STRUCT(
> +	),
> +	HE_ASSIGN(
> +	),
> +	HE_PRINTK()
> +);
> +
> +HYP_EVENT(hyp_exit,
> +	HE_PROTO(void),
> +	HE_STRUCT(
> +	),
> +	HE_ASSIGN(
> +	),
> +	HE_PRINTK()
> +);
>  #endif
> diff --git a/arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h b/arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h
> new file mode 100644
> index 000000000000..4b69d33e4f2d
> --- /dev/null
> +++ b/arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <asm/kvm_hypevents.h>
> +
> +#include <linux/arm-smccc.h>
> +
> +#undef arm_smccc_1_1_smc
> +#define arm_smccc_1_1_smc(...)					\
> +	do {							\
> +		trace_hyp_exit();				\
> +		__arm_smccc_1_1(SMCCC_SMC_INST, __VA_ARGS__);	\
> +		trace_hyp_enter();				\
> +	} while (0)

Huh. No, please. Don't override existing kernel constructs behind
everybody's back. I know that tracing folks love to play that sort of
game, but I really don't want any of that here.

Just define a wrapper that is KVM-specific, and use that.

Then from a semantic perspective, an SMC call, an ERET back to EL1
host, and an ERET back to EL1 guest are all very different events that
you lump under the "exit" category. That's not right. Same thing for
"enter".

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.
Re: [PATCH v8 26/28] KVM: arm64: Add hyp_enter/hyp_exit events to pKVM hyp
Posted by Vincent Donnefort 2 weeks, 4 days ago
On Sun, Nov 30, 2025 at 07:00:52PM +0000, Marc Zyngier wrote:
> On Fri, 07 Nov 2025 09:38:38 +0000,
> Vincent Donnefort <vdonnefort@google.com> wrote:
> > 
> > The hyp_enter and hyp_exit events are logged by the hypervisor any time
> > it is entered and exited.
> > 
> > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> > 
> > diff --git a/arch/arm64/include/asm/kvm_hypevents.h b/arch/arm64/include/asm/kvm_hypevents.h
> > index d6e033c96c52..ce3953bc884a 100644
> > --- a/arch/arm64/include/asm/kvm_hypevents.h
> > +++ b/arch/arm64/include/asm/kvm_hypevents.h
> > @@ -7,4 +7,21 @@
> >  #include <nvhe/trace.h>
> >  #endif
> >  
> > +HYP_EVENT(hyp_enter,
> > +	HE_PROTO(void),
> > +	HE_STRUCT(
> > +	),
> > +	HE_ASSIGN(
> > +	),
> > +	HE_PRINTK()
> > +);
> > +
> > +HYP_EVENT(hyp_exit,
> > +	HE_PROTO(void),
> > +	HE_STRUCT(
> > +	),
> > +	HE_ASSIGN(
> > +	),
> > +	HE_PRINTK()
> > +);
> >  #endif
> > diff --git a/arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h b/arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h
> > new file mode 100644
> > index 000000000000..4b69d33e4f2d
> > --- /dev/null
> > +++ b/arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#include <asm/kvm_hypevents.h>
> > +
> > +#include <linux/arm-smccc.h>
> > +
> > +#undef arm_smccc_1_1_smc
> > +#define arm_smccc_1_1_smc(...)					\
> > +	do {							\
> > +		trace_hyp_exit();				\
> > +		__arm_smccc_1_1(SMCCC_SMC_INST, __VA_ARGS__);	\
> > +		trace_hyp_enter();				\
> > +	} while (0)
> 
> Huh. No, please. Don't override existing kernel constructs behind
> everybody's back. I know that tracing folks love to play that sort of
> game, but I really don't want any of that here.

And you haven't seen what I had for arm_smccc_1_2_smc()

> 
> Just define a wrapper that is KVM-specific, and use that.
> 
> Then from a semantic perspective, an SMC call, an ERET back to EL1
> host, and an ERET back to EL1 guest are all very different events that
> you lump under the "exit" category. That's not right. Same thing for
> "enter".

The idea of those events is just to know when we enter and exit the hypervisor,
purely from a timing point of view. In ACK, we already have other separated
events that can give the "cause" for entering or exiting the hypervisor. I
haven't added them to that series because I thought it was big enough.

So happy to add now or do a follow-up, once we're done with that one?

> 
> Thanks,
> 
> 	M.
> 
> -- 
> Jazz isn't dead. It just smells funny.
Re: [PATCH v8 26/28] KVM: arm64: Add hyp_enter/hyp_exit events to pKVM hyp
Posted by Marc Zyngier 2 weeks, 4 days ago
On Mon, 01 Dec 2025 16:04:07 +0000,
Vincent Donnefort <vdonnefort@google.com> wrote:
> 
> On Sun, Nov 30, 2025 at 07:00:52PM +0000, Marc Zyngier wrote:
> > On Fri, 07 Nov 2025 09:38:38 +0000,
> > Vincent Donnefort <vdonnefort@google.com> wrote:
> > > 
> > > The hyp_enter and hyp_exit events are logged by the hypervisor any time
> > > it is entered and exited.
> > > 
> > > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> > > 
> > > diff --git a/arch/arm64/include/asm/kvm_hypevents.h b/arch/arm64/include/asm/kvm_hypevents.h
> > > index d6e033c96c52..ce3953bc884a 100644
> > > --- a/arch/arm64/include/asm/kvm_hypevents.h
> > > +++ b/arch/arm64/include/asm/kvm_hypevents.h
> > > @@ -7,4 +7,21 @@
> > >  #include <nvhe/trace.h>
> > >  #endif
> > >  
> > > +HYP_EVENT(hyp_enter,
> > > +	HE_PROTO(void),
> > > +	HE_STRUCT(
> > > +	),
> > > +	HE_ASSIGN(
> > > +	),
> > > +	HE_PRINTK()
> > > +);
> > > +
> > > +HYP_EVENT(hyp_exit,
> > > +	HE_PROTO(void),
> > > +	HE_STRUCT(
> > > +	),
> > > +	HE_ASSIGN(
> > > +	),
> > > +	HE_PRINTK()
> > > +);
> > >  #endif
> > > diff --git a/arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h b/arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h
> > > new file mode 100644
> > > index 000000000000..4b69d33e4f2d
> > > --- /dev/null
> > > +++ b/arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h
> > > @@ -0,0 +1,13 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +
> > > +#include <asm/kvm_hypevents.h>
> > > +
> > > +#include <linux/arm-smccc.h>
> > > +
> > > +#undef arm_smccc_1_1_smc
> > > +#define arm_smccc_1_1_smc(...)					\
> > > +	do {							\
> > > +		trace_hyp_exit();				\
> > > +		__arm_smccc_1_1(SMCCC_SMC_INST, __VA_ARGS__);	\
> > > +		trace_hyp_enter();				\
> > > +	} while (0)
> > 
> > Huh. No, please. Don't override existing kernel constructs behind
> > everybody's back. I know that tracing folks love to play that sort of
> > game, but I really don't want any of that here.
> 
> And you haven't seen what I had for arm_smccc_1_2_smc()

I have the feeling that I don't want to see it at all!

> > Just define a wrapper that is KVM-specific, and use that.
> > 
> > Then from a semantic perspective, an SMC call, an ERET back to EL1
> > host, and an ERET back to EL1 guest are all very different events that
> > you lump under the "exit" category. That's not right. Same thing for
> > "enter".
> 
> The idea of those events is just to know when we enter and exit the hypervisor,
> purely from a timing point of view. In ACK, we already have other separated
> events that can give the "cause" for entering or exiting the hypervisor. I
> haven't added them to that series because I thought it was big enough.
> 
> So happy to add now or do a follow-up, once we're done with that one?

Fixed trace points are ABI AFAIC, so I'd rather we make them useful
from the very start. What you have here is not very interesting,
because it makes all entry and exit points equal -- they very much
aren't.

So get rid of these and implement the real thing.

Thanks,

	M.

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