Re: [PATCH] KVM: x86/svm: Add module param to control PMU virtualization

From: Like Xu
Date: Sun Jan 16 2022 - 21:34:27 EST


On 15/1/2022 9:26 am, Jim Mattson wrote:
On Thu, Nov 18, 2021 at 5:25 AM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:

On 11/17/21 09:03, Like Xu wrote:
From: Like Xu <likexu@xxxxxxxxxxx>

For Intel, the guest PMU can be disabled via clearing the PMU CPUID.
For AMD, all hw implementations support the base set of four
performance counters, with current mainstream hardware indicating
the presence of two additional counters via X86_FEATURE_PERFCTR_CORE.

In the virtualized world, the AMD guest driver may detect
the presence of at least one counter MSR. Most hypervisor
vendors would introduce a module param (like lbrv for svm)
to disable PMU for all guests.

Another control proposal per-VM is to pass PMU disable information
via MSR_IA32_PERF_CAPABILITIES or one bit in CPUID Fn4000_00[FF:00].
Both of methods require some guest-side changes, so a module
parameter may not be sufficiently granular, but practical enough.

Signed-off-by: Like Xu <likexu@xxxxxxxxxxx>
---
arch/x86/kvm/cpuid.c | 2 +-
arch/x86/kvm/svm/pmu.c | 4 ++++
arch/x86/kvm/svm/svm.c | 11 +++++++++++
arch/x86/kvm/svm/svm.h | 1 +
4 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 2d70edb0f323..647af2a184ad 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -487,7 +487,7 @@ void kvm_set_cpu_caps(void)
F(CR8_LEGACY) | F(ABM) | F(SSE4A) | F(MISALIGNSSE) |
F(3DNOWPREFETCH) | F(OSVW) | 0 /* IBS */ | F(XOP) |
0 /* SKINIT, WDT, LWP */ | F(FMA4) | F(TBM) |
- F(TOPOEXT) | F(PERFCTR_CORE)
+ F(TOPOEXT) | 0 /* PERFCTR_CORE */
);

kvm_cpu_cap_mask(CPUID_8000_0001_EDX,
diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
index fdf587f19c5f..a0bcf0144664 100644
--- a/arch/x86/kvm/svm/pmu.c
+++ b/arch/x86/kvm/svm/pmu.c
@@ -16,6 +16,7 @@
#include "cpuid.h"
#include "lapic.h"
#include "pmu.h"
+#include "svm.h"

enum pmu_type {
PMU_TYPE_COUNTER = 0,
@@ -100,6 +101,9 @@ static inline struct kvm_pmc *get_gp_pmc_amd(struct kvm_pmu *pmu, u32 msr,
{
struct kvm_vcpu *vcpu = pmu_to_vcpu(pmu);

+ if (!pmuv)
+ return NULL;
+
switch (msr) {
case MSR_F15H_PERF_CTL0:
case MSR_F15H_PERF_CTL1:
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 21bb81710e0f..062e48c191ee 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -190,6 +190,10 @@ module_param(vgif, int, 0444);
static int lbrv = true;
module_param(lbrv, int, 0444);

+/* enable/disable PMU virtualization */
+bool pmuv = true;
+module_param(pmuv, bool, 0444);
+
static int tsc_scaling = true;
module_param(tsc_scaling, int, 0444);

@@ -952,6 +956,10 @@ static __init void svm_set_cpu_caps(void)
boot_cpu_has(X86_FEATURE_AMD_SSBD))
kvm_cpu_cap_set(X86_FEATURE_VIRT_SSBD);

+ /* AMD PMU PERFCTR_CORE CPUID */
+ if (pmuv && boot_cpu_has(X86_FEATURE_PERFCTR_CORE))
+ kvm_cpu_cap_set(X86_FEATURE_PERFCTR_CORE);
+
/* CPUID 0x8000001F (SME/SEV features) */
sev_set_cpu_caps();
}
@@ -1085,6 +1093,9 @@ static __init int svm_hardware_setup(void)
pr_info("LBR virtualization supported\n");
}

+ if (!pmuv)
+ pr_info("PMU virtualization is disabled\n");
+
svm_set_cpu_caps();

/*
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 0d7bbe548ac3..08e1c19ffbdf 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -32,6 +32,7 @@
extern u32 msrpm_offsets[MSRPM_OFFSETS] __read_mostly;
extern bool npt_enabled;
extern bool intercept_smi;
+extern bool pmuv;

/*
* Clean bits in VMCB.


Queued, thanks - just changed the parameter name to "pmu".

Whoops! The global 'pmu' is hidden by a local 'pmu' in get_gp_pmc_amd().

Indeed, I wonder if Poalo would like to take a look at this fix:
https://lore.kernel.org/kvm/20220113035324.59572-1-likexu@xxxxxxxxxxx/