kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KVM: x86: Making the module parameter of vPMU more common
@ 2022-01-11  7:38 Like Xu
  2022-01-11 17:04 ` Sean Christopherson
  2022-01-17 17:57 ` Paolo Bonzini
  0 siblings, 2 replies; 3+ messages in thread
From: Like Xu @ 2022-01-11  7:38 UTC (permalink / raw)
  To: Paolo Bonzini, Jim Mattson
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel,
	kvm, linux-kernel

From: Like Xu <likexu@tencent.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@google.com>
Signed-off-by: Like Xu <likexu@tencent.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)
 {
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] KVM: x86: Making the module parameter of vPMU more common
  2022-01-11  7:38 [PATCH] KVM: x86: Making the module parameter of vPMU more common Like Xu
@ 2022-01-11 17:04 ` Sean Christopherson
  2022-01-17 17:57 ` Paolo Bonzini
  1 sibling, 0 replies; 3+ messages in thread
From: Sean Christopherson @ 2022-01-11 17:04 UTC (permalink / raw)
  To: Like Xu
  Cc: Paolo Bonzini, Jim Mattson, Vitaly Kuznetsov, Wanpeng Li,
	Joerg Roedel, kvm, linux-kernel

Nit, s/Making/Make.

On Tue, Jan 11, 2022, Like Xu wrote:
> 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);

I'm definitely in favor of enable_pmu instead of pmu for the KVM, variable, but
I'd prefer we keep "pmu" for the module param, e.g. same as ept, vpid, etc...

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] KVM: x86: Making the module parameter of vPMU more common
  2022-01-11  7:38 [PATCH] KVM: x86: Making the module parameter of vPMU more common Like Xu
  2022-01-11 17:04 ` Sean Christopherson
@ 2022-01-17 17:57 ` Paolo Bonzini
  1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2022-01-17 17:57 UTC (permalink / raw)
  To: Like Xu, Jim Mattson
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel,
	kvm, linux-kernel

On 1/11/22 08:38, Like Xu wrote:
> From: Like Xu <likexu@tencent.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@google.com>
> Signed-off-by: Like Xu <likexu@tencent.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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-01-17 17:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-11  7:38 [PATCH] KVM: x86: Making the module parameter of vPMU more common Like Xu
2022-01-11 17:04 ` Sean Christopherson
2022-01-17 17:57 ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).