linux-kernel - Re: [PATCH v8 21/28] KVM: arm64: Add tracing capability for the pKVM hyp

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <877bv7fkgg.wl-maz@kernel.org>
Date: Sun, 30 Nov 2025 18:23:43 +0000
From: Marc Zyngier <maz@...nel.org>
To: Vincent Donnefort <vdonnefort@...gle.com>
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
Subject: Re: [PATCH v8 21/28] KVM: arm64: Add tracing capability for the pKVM hyp

On Thu, 20 Nov 2025 12:01:55 +0000,
Vincent Donnefort <vdonnefort@...gle.com> wrote:
> 
> On Wed, Nov 19, 2025 at 05:06:38PM +0000, Marc Zyngier wrote:
> > On Fri, 07 Nov 2025 09:38:33 +0000,
> > Vincent Donnefort <vdonnefort@...gle.com> wrote:
> > > 
> > > When running with protected mode, the host has very little knowledge
> > > about what is happening in the hypervisor. Of course this is an
> > > essential feature for security but nonetheless, that piece of code
> > > growing with more responsibilities, we need now a way to debug and
> > > profile it. Tracefs by its reliability, versatility and support for
> > > user-space is the perfect tool.
> > > 
> > > There's no way the hypervisor could log events directly into the host
> > > tracefs ring-buffers. So instead let's use our own, where the hypervisor
> > > is the writer and the host the reader.
> > > 
> > > 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 9da54d4ee49e..ad02dee140d3 100644
> > > --- a/arch/arm64/include/asm/kvm_asm.h
> > > +++ b/arch/arm64/include/asm/kvm_asm.h
> > > @@ -89,6 +89,10 @@ enum __kvm_host_smccc_func {
> > >  	__KVM_HOST_SMCCC_FUNC___pkvm_vcpu_load,
> > >  	__KVM_HOST_SMCCC_FUNC___pkvm_vcpu_put,
> > >  	__KVM_HOST_SMCCC_FUNC___pkvm_tlb_flush_vmid,
> > > +	__KVM_HOST_SMCCC_FUNC___pkvm_load_tracing,
> > > +	__KVM_HOST_SMCCC_FUNC___pkvm_unload_tracing,
> > > +	__KVM_HOST_SMCCC_FUNC___pkvm_enable_tracing,
> > > +	__KVM_HOST_SMCCC_FUNC___pkvm_swap_reader_tracing,
> > >  };
> > >  
> > >  #define DECLARE_KVM_VHE_SYM(sym)	extern char sym[]
> > > diff --git a/arch/arm64/include/asm/kvm_hyptrace.h b/arch/arm64/include/asm/kvm_hyptrace.h
> > > new file mode 100644
> > > index 000000000000..9c30a479bc36
> > > --- /dev/null
> > > +++ b/arch/arm64/include/asm/kvm_hyptrace.h
> > > @@ -0,0 +1,13 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +#ifndef __ARM64_KVM_HYPTRACE_H_
> > > +#define __ARM64_KVM_HYPTRACE_H_
> > > +
> > > +#include <linux/ring_buffer.h>
> > > +
> > > +struct hyp_trace_desc {
> > > +	unsigned long			bpages_backing_start;
> > 
> > Why is this an integer type? You keep casting it all over the place,
> > which tells me that's not the ideal type.
> 
> That's because it is a kern VA the hyp needs to convert. However it would indeed
> make my life easier to declare it as a struct simple_buffer_page * in the
> struct hyp_trace_buffer below.
> 
> > 
> > > +	size_t				bpages_backing_size;
> > > +	struct trace_buffer_desc	trace_buffer_desc;
> > > +
> > > +};
> > > +#endif
> > > diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> > > index 4f803fd1c99a..580426cdbe77 100644
> > > --- a/arch/arm64/kvm/Kconfig
> > > +++ b/arch/arm64/kvm/Kconfig
> > > @@ -83,4 +83,11 @@ config PTDUMP_STAGE2_DEBUGFS
> > >  
> > >  	  If in doubt, say N.
> > >  
> > > +config PKVM_TRACING
> > > +	bool
> > > +	depends on KVM
> > > +	depends on TRACING
> > > +	select SIMPLE_RING_BUFFER
> > > +	default y
> > 
> > I'd rather this is made to depend on NVHE_EL2_DEBUG, just like the
> > other debug options.
> 
> NVHE_EL2_DEBUG is unsafe for production because of the stage-2 relax on panic.
> While this one is. So ideally this should be usable even without NVHE_EL2_DEBUG.

I don't see what makes tracing safer, as it potentially exposes
information (timing, control flow) that the host use to infer what is
happening at EL2. Which is, after all, the whole point of tracing.

I really think this should be guarded by NVHE_EL2_DEBUG, and the
stage-2 relaxation tied to the stacktrace infrastructure, so that you
can select both independently.

> I can remove this hidden PKVM_TRACING option and use everywhere
> CONFIG_TRACING. But then I need something to select
> SIMPLE_RING_BUFFER.
> 
> Perhaps with the following?
> 
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index 2ae6bf499236..c561bf9d4754 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -38,6 +38,7 @@ menuconfig KVM
>         select SCHED_INFO
>         select GUEST_PERF_EVENTS if PERF_EVENTS
>         select KVM_GUEST_MEMFD
> +       select SIMPLE_RING_BUFFER if CONFIG_TRACING

That's better, but my ask about making this depending on DEBUG still
stand.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.