Re: [PATCH 2/2] KVM: stats: Add counters for SVM exit reasons

[Sean Christopherson <seanjc@google.com>]

On Fri, Aug 27, 2021, Jing Zhang wrote:
> Three different exit code ranges are named as low, high and vmgexit,
> which start from 0x0, 0x400 and 0x80000000.
> 
> Original-by: Jim Mattson <jmattson@google.com>
> Signed-off-by: Jing Zhang <jingzhangos@google.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  7 +++++++
>  arch/x86/include/uapi/asm/svm.h |  7 +++++++
>  arch/x86/kvm/svm/svm.c          | 21 +++++++++++++++++++++
>  arch/x86/kvm/x86.c              |  9 +++++++++
>  4 files changed, 44 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index dd2380c9ea96..6e3c11a29afe 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -35,6 +35,7 @@
>  #include <asm/kvm_vcpu_regs.h>
>  #include <asm/hyperv-tlfs.h>
>  #include <asm/vmx.h>
> +#include <asm/svm.h>
>  
>  #define __KVM_HAVE_ARCH_VCPU_DEBUGFS
>  
> @@ -1261,6 +1262,12 @@ struct kvm_vcpu_stat {
>  	u64 vmx_all_exits[EXIT_REASON_NUM];
>  	u64 vmx_l2_exits[EXIT_REASON_NUM];
>  	u64 vmx_nested_exits[EXIT_REASON_NUM];
> +	u64 svm_exits_low[SVM_EXIT_LOW_END - SVM_EXIT_LOW_START];
> +	u64 svm_exits_high[SVM_EXIT_HIGH_END - SVM_EXIT_HIGH_START];
> +	u64 svm_vmgexits[SVM_VMGEXIT_END - SVM_VMGEXIT_START];

This is, for lack of a better word, a very lazy approach.  With a bit more (ok,
probably a lot more) effort and abstraction, we can have parity between VMX and
SVM, and eliminate a bunch of dead weight.  Having rough parity would likely be
quite helpful for the end user, e.g. reduces/eliminates vendor specific userspace
code.

E.g. this more or less doubles the memory footprint due to tracking VMX and SVM
separately, SVM has finer granularity than VMX (often too fine),  VMX tracks nested
exits but SVM does not, etc...

If we use KVM-defined exit reasons, then we can omit the exits that should never
happen (SVM has quite a few), and consolidate the ones no one should ever care
about, e.g. DR0..DR4 and DR6 can be collapsed (if I'm remembering my DR aliases
correctly).  And on the VMX side we can provide better granularity than the raw
exit reason, e.g. VMX's bundling of NMIs and exceptions is downright gross.

	#define KVM_GUEST_EXIT_READ_CR0
	#define KVM_GUEST_EXIT_READ_CR3
	#define KVM_GUEST_EXIT_READ_CR4
	#define KVM_GUEST_EXIT_READ_CR8
	#define KVM_GUEST_EXIT_DR_READ
	#define KVM_GUEST_EXIT_DR_WRITE
	#define KVM_GUEST_EXIT_DB_EXCEPTION
	#define KVM_GUEST_EXIT_BP_EXCEPTION
	#define KVM_GUEST_EXIT_UD_EXCEPTION

	...
	#define KVM_NR_GUEST_EXIT_REASONS

	u64 l1_exits[KVM_NR_GUEST_EXIT_REASONS];
	u64 l2_exits[KVM_NR_GUEST_EXIT_REASONS];
	u64 nested_exits[KVM_NR_GUEST_EXIT_REASONS];


The downside is that it will require KVM-defined exit reasons and will have a
slightly higher maintenance cost because of it, but I think overall it would be
a net positive.  Paolo can probably even concoct some clever arithmetic to avoid
conditionals when massaging SVM exits reasons.  VMX will require a bit more
manual effort, especially to play nice with nested, but in the end I think we'll
be happier.

> +	u64 svm_vmgexit_unsupported_event;
> +	u64 svm_exit_sw;
> +	u64 svm_exit_err;
>  };