[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <42dd21d1-8300-3101-4870-bf772783588b@redhat.com>
Date: Mon, 17 Jan 2022 18:57:13 +0100
From: Paolo Bonzini <pbonzini@...hat.com>
To: Like Xu <like.xu.linux@...il.com>,
Jim Mattson <jmattson@...gle.com>
Cc: Sean Christopherson <seanjc@...gle.com>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Wanpeng Li <wanpengli@...cent.com>,
Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: x86: Making the module parameter of vPMU more common
On 1/11/22 08:38, Like Xu wrote:
> From: Like Xu <likexu@...cent.com>
>
> The new module parameter to control PMU virtualization should apply
> to Intel as well as AMD, for situations where userspace is not trusted.
> If the module parameter allows PMU virtualization, there could be a
> new KVM_CAP or guest CPUID bits whereby userspace can enable/disable
> PMU virtualization on a per-VM basis.
>
> If the module parameter does not allow PMU virtualization, there
> should be no userspace override, since we have no precedent for
> authorizing that kind of override. If it's false, other counter-based
> profiling features (such as LBR including the associated CPUID bits
> if any) will not be exposed.
>
> Change its name from "pmu" to "enable_pmu" as we have temporary
> variables with the same name in our code like "struct kvm_pmu *pmu".
>
> Fixes: b1d66dad65dc ("KVM: x86/svm: Add module param to control PMU virtualization")
> Suggested-by : Jim Mattson <jmattson@...gle.com>
> Signed-off-by: Like Xu <likexu@...cent.com>
> ---
> arch/x86/kvm/cpuid.c | 6 +++---
> arch/x86/kvm/svm/pmu.c | 2 +-
> arch/x86/kvm/svm/svm.c | 8 ++------
> arch/x86/kvm/svm/svm.h | 1 -
> arch/x86/kvm/vmx/capabilities.h | 4 ++++
> arch/x86/kvm/vmx/pmu_intel.c | 2 +-
> arch/x86/kvm/x86.c | 5 +++++
> arch/x86/kvm/x86.h | 1 +
> 8 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 0b920e12bb6d..b2ff8bfd8220 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -770,10 +770,10 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
> perf_get_x86_pmu_capability(&cap);
>
> /*
> - * Only support guest architectural pmu on a host
> - * with architectural pmu.
> + * The guest architecture pmu is only supported if the architecture
> + * pmu exists on the host and the module parameters allow it.
> */
> - if (!cap.version)
> + if (!cap.version || !enable_pmu)
> memset(&cap, 0, sizeof(cap));
>
> eax.split.version_id = min(cap.version, 2);
> diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
> index 12d8b301065a..5aa45f13b16d 100644
> --- a/arch/x86/kvm/svm/pmu.c
> +++ b/arch/x86/kvm/svm/pmu.c
> @@ -101,7 +101,7 @@ static inline struct kvm_pmc *get_gp_pmc_amd(struct kvm_pmu *pmu, u32 msr,
> {
> struct kvm_vcpu *vcpu = pmu_to_vcpu(pmu);
>
> - if (!pmu)
> + if (!enable_pmu)
> return NULL;
>
> switch (msr) {
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 6cb38044a860..549f73ce5ebc 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -192,10 +192,6 @@ module_param(vgif, int, 0444);
> static int lbrv = true;
> module_param(lbrv, int, 0444);
>
> -/* enable/disable PMU virtualization */
> -bool pmu = true;
> -module_param(pmu, bool, 0444);
> -
> static int tsc_scaling = true;
> module_param(tsc_scaling, int, 0444);
>
> @@ -4573,7 +4569,7 @@ static __init void svm_set_cpu_caps(void)
> kvm_cpu_cap_set(X86_FEATURE_VIRT_SSBD);
>
> /* AMD PMU PERFCTR_CORE CPUID */
> - if (pmu && boot_cpu_has(X86_FEATURE_PERFCTR_CORE))
> + if (enable_pmu && boot_cpu_has(X86_FEATURE_PERFCTR_CORE))
> kvm_cpu_cap_set(X86_FEATURE_PERFCTR_CORE);
>
> /* CPUID 0x8000001F (SME/SEV features) */
> @@ -4712,7 +4708,7 @@ static __init int svm_hardware_setup(void)
> pr_info("LBR virtualization supported\n");
> }
>
> - if (!pmu)
> + if (!enable_pmu)
> 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 daa8ca84afcc..47ef8f4a9358 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -32,7 +32,6 @@
> extern u32 msrpm_offsets[MSRPM_OFFSETS] __read_mostly;
> extern bool npt_enabled;
> extern bool intercept_smi;
> -extern bool pmu;
>
> /*
> * Clean bits in VMCB.
> diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
> index c8029b7845b6..959b59d13b5a 100644
> --- a/arch/x86/kvm/vmx/capabilities.h
> +++ b/arch/x86/kvm/vmx/capabilities.h
> @@ -5,6 +5,7 @@
> #include <asm/vmx.h>
>
> #include "lapic.h"
> +#include "x86.h"
>
> extern bool __read_mostly enable_vpid;
> extern bool __read_mostly flexpriority_enabled;
> @@ -389,6 +390,9 @@ static inline u64 vmx_get_perf_capabilities(void)
> {
> u64 perf_cap = 0;
>
> + if (!enable_pmu)
> + return perf_cap;
> +
> if (boot_cpu_has(X86_FEATURE_PDCM))
> rdmsrl(MSR_IA32_PERF_CAPABILITIES, perf_cap);
>
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index ffccfd9823c0..466d18fc0c5d 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -487,7 +487,7 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
> pmu->reserved_bits = 0xffffffff00200000ull;
>
> entry = kvm_find_cpuid_entry(vcpu, 0xa, 0);
> - if (!entry)
> + if (!entry || !enable_pmu)
> return;
> eax.full = entry->eax;
> edx.full = entry->edx;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index c194a8cbd25f..bff2ff8cb35f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -187,6 +187,11 @@ module_param(force_emulation_prefix, bool, S_IRUGO);
> int __read_mostly pi_inject_timer = -1;
> module_param(pi_inject_timer, bint, S_IRUGO | S_IWUSR);
>
> +/* Enable/disable PMU virtualization */
> +bool __read_mostly enable_pmu = true;
> +EXPORT_SYMBOL_GPL(enable_pmu);
> +module_param(enable_pmu, bool, 0444);
> +
> /*
> * Restoring the host value for MSRs that are only consumed when running in
> * usermode, e.g. SYSCALL MSRs and TSC_AUX, can be deferred until the CPU
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index da7031e80f23..1ebd5a7594da 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -336,6 +336,7 @@ extern u64 host_xcr0;
> extern u64 supported_xcr0;
> extern u64 host_xss;
> extern u64 supported_xss;
> +extern bool enable_pmu;
>
> static inline bool kvm_mpx_supported(void)
> {
Queued, thanks.
Paolo
Powered by blists - more mailing lists