Re: [PATCH v8 22/28] KVM: arm64: Add trace remote for the pKVM hyp

[Marc Zyngier <maz@kernel.org>]

On Fri, 07 Nov 2025 09:38:34 +0000,
Vincent Donnefort <vdonnefort@google.com> wrote:
> 
> When running with KVM protected mode, the hypervisor is able to generate
> events into tracefs compatible ring-buffers. Create a trace remote so
> the kernel can read those buffers.
> 
> This currently doesn't provide any event support which will come later.
> 
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> 
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index 580426cdbe77..64db254f0448 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -87,6 +87,7 @@ config PKVM_TRACING
>  	bool
>  	depends on KVM
>  	depends on TRACING
> +	select TRACE_REMOTE
>  	select SIMPLE_RING_BUFFER
>  	default y
>  
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index 3ebc0570345c..2c184e3abd8e 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -30,6 +30,8 @@ kvm-$(CONFIG_HW_PERF_EVENTS)  += pmu-emul.o pmu.o
>  kvm-$(CONFIG_ARM64_PTR_AUTH)  += pauth.o
>  kvm-$(CONFIG_PTDUMP_STAGE2_DEBUGFS) += ptdump.o
>  
> +kvm-$(CONFIG_PKVM_TRACING) += hyp_trace.o
> +
>  always-y := hyp_constants.h hyp-constants.s
>  
>  define rule_gen_hyp_constants
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 870953b4a8a7..c485e54417e2 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -24,6 +24,7 @@
>  
>  #define CREATE_TRACE_POINTS
>  #include "trace_arm.h"
> +#include "hyp_trace.h"
>  
>  #include <linux/uaccess.h>
>  #include <asm/ptrace.h>
> @@ -2345,6 +2346,9 @@ static int __init init_subsystems(void)
>  
>  	kvm_register_perf_callbacks(NULL);
>  
> +	err = hyp_trace_init();
> +	if (err)
> +		kvm_err("Failed to initialize Hyp tracing\n");
>  out:
>  	if (err)
>  		hyp_cpu_pm_exit();
> diff --git a/arch/arm64/kvm/hyp_trace.c b/arch/arm64/kvm/hyp_trace.c
> new file mode 100644
> index 000000000000..98051c3fb0c2
> --- /dev/null
> +++ b/arch/arm64/kvm/hyp_trace.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2025 Google LLC
> + * Author: Vincent Donnefort <vdonnefort@google.com>
> + */
> +
> +#include <linux/trace_remote.h>
> +#include <linux/simple_ring_buffer.h>
> +
> +#include <asm/kvm_host.h>
> +#include <asm/kvm_hyptrace.h>
> +
> +#include "hyp_trace.h"
> +
> +/* Access to this struct within the trace_remote_callbacks are protected by the trace_remote lock */
> +static struct hyp_trace_buffer {
> +	struct hyp_trace_desc	*desc;
> +	size_t			desc_size;
> +} trace_buffer;
> +
> +static int hyp_trace_buffer_alloc_bpages_backing(struct hyp_trace_buffer *trace_buffer, size_t size)
> +{
> +	int nr_bpages = (PAGE_ALIGN(size) / PAGE_SIZE) + 1;
> +	size_t backing_size;
> +	void *start;
> +
> +	backing_size = PAGE_ALIGN(sizeof(struct simple_buffer_page) * nr_bpages *
> +				  num_possible_cpus());
> +
> +	start = alloc_pages_exact(backing_size, GFP_KERNEL_ACCOUNT);
> +	if (!start)
> +		return -ENOMEM;
> +
> +	trace_buffer->desc->bpages_backing_start = (unsigned long)start;
> +	trace_buffer->desc->bpages_backing_size = backing_size;
> +
> +	return 0;
> +}
> +
> +static void hyp_trace_buffer_free_bpages_backing(struct hyp_trace_buffer *trace_buffer)
> +{
> +	free_pages_exact((void *)trace_buffer->desc->bpages_backing_start,
> +			 trace_buffer->desc->bpages_backing_size);
> +}
> +
> +static int __load_page(unsigned long va)
> +{
> +	return kvm_call_hyp_nvhe(__pkvm_host_share_hyp, virt_to_pfn((void *)va), 1);
> +}

I struggle a bit with the nomenclature here. Why is that called
"load"? Surely this is a "map" operation, right? Is that because this
is called at "vcpu load" time? Something else?

Also, how is this working without pKVM, in a normal nVHE environment?
Being able to trace in nVHE is a basic requirement, and I don't see
how this works here.

Thanks,

	M.

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