Received-SPF: pass (google.com: domain of linux-kernel+bounces-65467-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Wed, 14 Feb 2024 15:55:33 +0000
Message-ID: <864jeb59ca.wl-maz@kernel.org>
From: Marc Zyngier <maz@kernel.org>
To: Oliver Upton <oliver.upton@linux.dev>
Cc: kvmarm@lists.linux.dev,
	kvm@vger.kernel.org,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 01/23] KVM: arm64: Add tracepoints + stats for LPI cache effectiveness
In-Reply-To: <20240213093250.3960069-2-oliver.upton@linux.dev>
References: <20240213093250.3960069-1-oliver.upton@linux.dev>
	<20240213093250.3960069-2-oliver.upton@linux.dev>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/29.1
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
Precedence: bulk
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII

On Tue, 13 Feb 2024 09:32:38 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> LPI translation and injection has been shown to have a significant
> impact on the performance of VM workloads, so it probably makes sense to
> add some signals in this area.
> 
> Introduce the concept of a KVM tracepoint that associates with a VM
> stat and use it for the LPI translation cache tracepoints. It isn't too
> uncommon for a kernel hacker to attach to tracepoints, while at the same
> time userspace may open a 'binary stats' FD to peek at the corresponding
> VM stats.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/include/asm/kvm_host.h |  3 ++
>  arch/arm64/kvm/guest.c            |  5 ++-
>  arch/arm64/kvm/vgic/trace.h       | 66 +++++++++++++++++++++++++++++++
>  arch/arm64/kvm/vgic/vgic-its.c    | 14 ++++++-
>  include/linux/kvm_host.h          |  4 ++
>  5 files changed, 89 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 21c57b812569..6f88b76373a5 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -966,6 +966,9 @@ static inline bool __vcpu_write_sys_reg_to_cpu(u64 val, int reg)
>  
>  struct kvm_vm_stat {
>  	struct kvm_vm_stat_generic generic;
> +	u64 vgic_its_trans_cache_hit;
> +	u64 vgic_its_trans_cache_miss;
> +	u64 vgic_its_trans_cache_victim;
>  };
>  
>  struct kvm_vcpu_stat {
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index aaf1d4939739..354d67251fc2 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -30,7 +30,10 @@
>  #include "trace.h"
>  
>  const struct _kvm_stats_desc kvm_vm_stats_desc[] = {
> -	KVM_GENERIC_VM_STATS()
> +	KVM_GENERIC_VM_STATS(),
> +	STATS_DESC_COUNTER(VM, vgic_its_trans_cache_hit),
> +	STATS_DESC_COUNTER(VM, vgic_its_trans_cache_miss),
> +	STATS_DESC_COUNTER(VM, vgic_its_trans_cache_victim)
>  };

We've talked about this offline, but I thought I'd make my position
public on these.

I've very concerned that exposing these statistic, however useful they
may be at a given point in time, will eventually become all wrong
and/or misleading.

Case in point, our discussion about this very series, where we landed
on a potential reimplementation of the translation cache as a per-ITS
xarray. If this comes to fruition, these stats will probably be
totally useless (hit monotonically increasing at the rate of 1 per
interrupt, miss being 0 or 1, victim being stuck to 0).

I'm *not* saying that such stats are totally useless. Just that they
are, by definition, tied to a kernel-side implementation choice, which
will evolve. The trouble is that they create an ABI that we need to
support forever.

I wish we could work on something that would allow such statistics to
be *extracted* (as opposed to published). Something like this as a
discussion subject for a future KVM Forum or LPC uConf would get my
full backing (and I'm pretty sure the networking folks have solved
that problem a long time ago).

Thanks,

	M.

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