RE: [PATCH 0/4] KVM: x86: Collect host state snapshots into a struct
From: Wang, Wei W
Date: Thu Apr 25 2024 - 07:17:26 EST
On Wednesday, April 24, 2024 6:15 AM, Sean Christopherson wrote:
> Add a global "kvm_host" structure to hold various host values, e.g. for EFER,
> XCR0, raw MAXPHYADDR etc., instead of having a bunch of one-off variables
> that inevitably need to be exported, or in the case of shadow_phys_bits, are
> buried in a random location and are awkward to use, leading to duplicate
> code.
Looks good. How about applying similar improvements to the module
parameters as well? I've changed the "enable_pmu" parameter as an example below:
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 77352a4abd87..a221ba7b546f 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -1013,7 +1013,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
union cpuid10_eax eax;
union cpuid10_edx edx;
- if (!enable_pmu || !static_cpu_has(X86_FEATURE_ARCH_PERFMON)) {
+ if (!kvm_caps.enable_pmu || !static_cpu_has(X86_FEATURE_ARCH_PERFMON)) {
entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
break;
}
@@ -1306,7 +1306,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
union cpuid_0x80000022_ebx ebx;
entry->ecx = entry->edx = 0;
- if (!enable_pmu || !kvm_cpu_cap_has(X86_FEATURE_PERFMON_V2)) {
+ if (!kvm_caps.enable_pmu || !kvm_cpu_cap_has(X86_FEATURE_PERFMON_V2)) {
entry->eax = entry->ebx;
break;
}
diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
index 4d52b0b539ba..7e359db64dbd 100644
--- a/arch/x86/kvm/pmu.h
+++ b/arch/x86/kvm/pmu.h
@@ -190,9 +190,9 @@ static inline void kvm_init_pmu_capability(const struct kvm_pmu_ops *pmu_ops)
* for hybrid PMUs until KVM gains a way to let userspace opt-in.
*/
if (cpu_feature_enabled(X86_FEATURE_HYBRID_CPU))
- enable_pmu = false;
+ kvm_caps.enable_pmu = false;
- if (enable_pmu) {
+ if (kvm_caps.enable_pmu) {
perf_get_x86_pmu_capability(&kvm_pmu_cap);
/*
@@ -203,12 +203,12 @@ static inline void kvm_init_pmu_capability(const struct kvm_pmu_ops *pmu_ops)
*/
if (!kvm_pmu_cap.num_counters_gp ||
WARN_ON_ONCE(kvm_pmu_cap.num_counters_gp < min_nr_gp_ctrs))
- enable_pmu = false;
+ kvm_caps.enable_pmu = false;
else if (is_intel && !kvm_pmu_cap.version)
- enable_pmu = false;
+ kvm_caps.enable_pmu = false;
}
- if (!enable_pmu) {
+ if (!kvm_caps.enable_pmu) {
memset(&kvm_pmu_cap, 0, sizeof(kvm_pmu_cap));
return;
}
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 7ea0e7f13da4..4ed8c73f88e4 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7869,7 +7869,7 @@ static __init u64 vmx_get_perf_capabilities(void)
u64 perf_cap = PMU_CAP_FW_WRITES;
u64 host_perf_cap = 0;
- if (!enable_pmu)
+ if (!kvm_caps.enable_pmu)
return 0;
if (boot_cpu_has(X86_FEATURE_PDCM))
@@ -7938,7 +7938,7 @@ static __init void vmx_set_cpu_caps(void)
kvm_cpu_cap_check_and_set(X86_FEATURE_DTES64);
}
- if (!enable_pmu)
+ if (!kvm_caps.enable_pmu)
kvm_cpu_cap_clear(X86_FEATURE_PDCM);
kvm_caps.supported_perf_cap = vmx_get_perf_capabilities();
@@ -8683,7 +8683,7 @@ static __init int hardware_setup(void)
if (pt_mode != PT_MODE_SYSTEM && pt_mode != PT_MODE_HOST_GUEST)
return -EINVAL;
- if (!enable_ept || !enable_pmu || !cpu_has_vmx_intel_pt())
+ if (!enable_ept || !kvm_caps.enable_pmu || !cpu_has_vmx_intel_pt())
pt_mode = PT_MODE_SYSTEM;
if (pt_mode == PT_MODE_HOST_GUEST)
vmx_init_ops.handle_intel_pt_intr = vmx_handle_intel_pt_intr;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cdcda1bbf5a3..36d471a7af87 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -94,6 +94,7 @@
struct kvm_caps kvm_caps __read_mostly = {
.supported_mce_cap = MCG_CTL_P | MCG_SER_P,
+ .enable_pmu = true,
};
EXPORT_SYMBOL_GPL(kvm_caps);
@@ -192,9 +193,7 @@ int __read_mostly pi_inject_timer = -1;
module_param(pi_inject_timer, bint, 0644);
/* Enable/disable PMU virtualization */
-bool __read_mostly enable_pmu = true;
-EXPORT_SYMBOL_GPL(enable_pmu);
-module_param(enable_pmu, bool, 0444);
+module_param_named(enable_pmu, kvm_caps.enable_pmu, bool, 0444);
bool __read_mostly eager_page_split = true;
module_param(eager_page_split, bool, 0644);
@@ -4815,7 +4814,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
break;
}
case KVM_CAP_PMU_CAPABILITY:
- r = enable_pmu ? KVM_CAP_PMU_VALID_MASK : 0;
+ r = kvm_caps.enable_pmu ? KVM_CAP_PMU_VALID_MASK : 0;
break;
case KVM_CAP_DISABLE_QUIRKS2:
r = KVM_X86_VALID_QUIRKS;
@@ -6652,7 +6651,7 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
break;
case KVM_CAP_PMU_CAPABILITY:
r = -EINVAL;
- if (!enable_pmu || (cap->args[0] & ~KVM_CAP_PMU_VALID_MASK))
+ if (!kvm_caps.enable_pmu || (cap->args[0] & ~KVM_CAP_PMU_VALID_MASK))
break;
mutex_lock(&kvm->lock);
@@ -7438,7 +7437,7 @@ static void kvm_init_msr_lists(void)
for (i = 0; i < ARRAY_SIZE(msrs_to_save_base); i++)
kvm_probe_msr_to_save(msrs_to_save_base[i]);
- if (enable_pmu) {
+ if (kvm_caps.enable_pmu) {
for (i = 0; i < ARRAY_SIZE(msrs_to_save_pmu); i++)
kvm_probe_msr_to_save(msrs_to_save_pmu[i]);
}
@@ -12555,7 +12554,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
kvm->arch.default_tsc_khz = max_tsc_khz ? : tsc_khz;
kvm->arch.guest_can_read_msr_platform_info = true;
- kvm->arch.enable_pmu = enable_pmu;
+ kvm->arch.enable_pmu = kvm_caps.enable_pmu;
#if IS_ENABLED(CONFIG_HYPERV)
spin_lock_init(&kvm->arch.hv_root_tdp_lock);
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 102754dc85bc..c4d99338aaa1 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -29,6 +29,9 @@ struct kvm_caps {
u64 supported_xcr0;
u64 supported_xss;
u64 supported_perf_cap;
+
+ /* KVM module parameters */
+ bool enable_pmu;
};
struct kvm_host_values {
@@ -340,8 +343,6 @@ fastpath_t handle_fastpath_set_msr_irqoff(struct kvm_vcpu *vcpu);
extern struct kvm_caps kvm_caps;
extern struct kvm_host_values kvm_host;
-extern bool enable_pmu;