Many moons ago, commit 4c9fc8ef5017 ("KVM: VMX: Add module option to
disable flexpriority") introduced kvm-intel.flexpriority as it was
"Useful for debugging". At that time, kvm-intel.flexpriority only
determined whether or not KVM would enable VIRTUALIZE_APIC_ACCESSES.
In short, it was intended as a way to disable virtualization of APIC
accesses for debug purposes. Nowadays, kvm-intel.flexpriority is a
haphazard param that is inconsistently honored by KVM, e.g. it still
controls VIRTUALIZE_APIC_ACCESSES but not TPR_SHADOW, CR8-exiting or
VIRTUALIZE_X2APIC_MODE, and only for non-nested guests. Disabling
the param also announces support for KVM_TPR_ACCESS_REPORTING (via
KVM_CAP_VAPIC), which may be functionally desirable, e.g. Qemu uses
KVM_TPR_ACCESS_REPORTING only to patch MMIO APIC access, but isn't
exactly accurate given its name since KVM may not intercept/report
TPR accesses via CR8 or MSR.
Remove kvm-intel.flexpriority as its usefulness for debug is dubious
given the current code base, while its existence is confusing and
can complicate code changes and/or lead to new bugs being introduced.
For example, as of commit 8d860bbeedef ("kvm: vmx: Basic APIC
virtualization controls have three settings"), KVM will disable
VIRTUALIZE_APIC_ACCESSES when a nested guest writes APIC_BASE MSR and
kvm-intel.flexpriority=0, whereas previously KVM would allow a nested
guest to enable VIRTUALIZE_APIC_ACCESSES so long as it's supported in
hardware. I.e. KVM now advertises VIRTUALIZE_APIC_ACCESSES to a guest
but doesn't (always) allow setting it when kvm-intel.flexpriority=0,
and may even initially allow the control and then clear it when the
nested guest writes APIC_BASE MSR, which is decidedly odd even if it
doesn't cause functional issues.
Fixes: 8d860bbeedef ("kvm: vmx: Basic APIC virtualization controls have three settings")
Cc: Jim Mattson <jmattson@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
---
.../admin-guide/kernel-parameters.txt | 4 ----
arch/x86/kvm/vmx.c | 19 ++++---------------
2 files changed, 4 insertions(+), 19 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 9871e649ffef..670f0b0c6806 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1983,10 +1983,6 @@
[KVM,Intel] Enable emulation of invalid guest states
Default is 0 (disabled)
- kvm-intel.flexpriority=
- [KVM,Intel] Disable FlexPriority feature (TPR shadow).
- Default is 1 (enabled)
-
kvm-intel.nested=
[KVM,Intel] Enable VMX nesting (nVMX).
Default is 0 (disabled)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 1d26f3c4985b..e1b8ea9a2bc4 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -78,9 +78,6 @@ module_param_named(vpid, enable_vpid, bool, 0444);
static bool __read_mostly enable_vnmi = 1;
module_param_named(vnmi, enable_vnmi, bool, S_IRUGO);
-static bool __read_mostly flexpriority_enabled = 1;
-module_param_named(flexpriority, flexpriority_enabled, bool, S_IRUGO);
-
static bool __read_mostly enable_ept = 1;
module_param_named(ept, enable_ept, bool, S_IRUGO);
@@ -1857,7 +1854,7 @@ static inline bool cpu_has_vmx_basic_inout(void)
static inline bool cpu_need_virtualize_apic_accesses(struct kvm_vcpu *vcpu)
{
- return flexpriority_enabled && lapic_in_kernel(vcpu);
+ return cpu_has_vmx_flexpriority() && lapic_in_kernel(vcpu);
}
static inline bool cpu_has_vmx_vpid(void)
@@ -1924,11 +1921,6 @@ static bool vmx_umip_emulated(void)
SECONDARY_EXEC_DESC;
}
-static inline bool report_flexpriority(void)
-{
- return flexpriority_enabled;
-}
-
static inline unsigned nested_cpu_vmx_misc_cr3_count(struct kvm_vcpu *vcpu)
{
return vmx_misc_cr3_count(to_vmx(vcpu)->nested.msrs.misc_low);
@@ -7895,9 +7887,6 @@ static __init int hardware_setup(void)
if (!cpu_has_vmx_unrestricted_guest() || !enable_ept)
enable_unrestricted_guest = 0;
- if (!cpu_has_vmx_flexpriority())
- flexpriority_enabled = 0;
-
if (!cpu_has_virtual_nmis())
enable_vnmi = 0;
@@ -7906,7 +7895,7 @@ static __init int hardware_setup(void)
* page upon invalidation. No need to do anything if not
* using the APIC_ACCESS_ADDR VMCS field.
*/
- if (!flexpriority_enabled)
+ if (!cpu_has_vmx_flexpriority())
kvm_x86_ops->set_apic_access_page_addr = NULL;
if (!cpu_has_vmx_tpr_shadow())
@@ -10233,7 +10222,7 @@ static void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu)
case LAPIC_MODE_DISABLED:
break;
case LAPIC_MODE_XAPIC:
- if (flexpriority_enabled) {
+ if (cpu_has_vmx_flexpriority()) {
sec_exec_control |=
SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
vmx_flush_tlb(vcpu, true);
@@ -14007,7 +13996,7 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = {
.check_processor_compatibility = vmx_check_processor_compat,
.hardware_enable = hardware_enable,
.hardware_disable = hardware_disable,
- .cpu_has_accelerated_tpr = report_flexpriority,
+ .cpu_has_accelerated_tpr = cpu_has_vmx_flexpriority,
.has_emulated_msr = vmx_has_emulated_msr,
.vm_init = vmx_vm_init,
--
2.18.0
