kvm: x86: replace kvm_spec_ctrl_test_value with runtime test on the host

commit 841c2be09fe4f495fe5224952a419bd8c7e5b455 upstream.

To avoid complex and in some cases incorrect logic in
kvm_spec_ctrl_test_value, just try the guest's given value on the host
processor instead, and if it doesn't #GP, allow the guest to set it.

One such case is when host CPU supports STIBP mitigation
but doesn't support IBRS (as is the case with some Zen2 AMD cpus),
and in this case we were giving guest #GP when it tried to use STIBP

The reason why can can do the host test is that IA32_SPEC_CTRL msr is
passed to the guest, after the guest sets it to a non zero value
for the first time (due to performance reasons),
and as as result of this, it is pointless to emulate #GP condition on
this first access, in a different way than what the host CPU does.

This is based on a patch from Sean Christopherson, who suggested this idea.

Fixes: 6441fa6178f5 ("KVM: x86: avoid incorrect writes to host MSR_IA32_SPEC_CTRL")
Cc: stable@vger.kernel.org
Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Message-Id: <20200708115731.180097-1-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 7dbfc0bc738c1579e34718f20ce446f11404feaa..27c0cc61fb081617f722afbfcbc1c84195c7f367 100644 (file)
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2509,7 +2509,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
                    !guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD))
                        return 1;
 
-               if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
+               if (kvm_spec_ctrl_test_value(data))
                        return 1;
 
                svm->spec_ctrl = data;

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 8fafcb2cd103104dd72e44b0408495de0a490ebc..9938a7e698db51e6cfc220adc58c555b50eb596c 100644 (file)
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2015,7 +2015,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
                    !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
                        return 1;
 
-               if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
+               if (kvm_spec_ctrl_test_value(data))
                        return 1;
 
                vmx->spec_ctrl = data;

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 738a558c915c389038f99a833a79c4678ad0a07a..51ccb4dfaad26b132463ad2e93e3c60ba06eff3d 100644 (file)
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10573,28 +10573,32 @@ bool kvm_arch_no_poll(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_GPL(kvm_arch_no_poll);
 
-u64 kvm_spec_ctrl_valid_bits(struct kvm_vcpu *vcpu)
+
+int kvm_spec_ctrl_test_value(u64 value)
 {
-       uint64_t bits = SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | SPEC_CTRL_SSBD;
+       /*
+        * test that setting IA32_SPEC_CTRL to given value
+        * is allowed by the host processor
+        */
 
-       /* The STIBP bit doesn't fault even if it's not advertised */
-       if (!guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
-           !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS))
-               bits &= ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP);
-       if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL) &&
-           !boot_cpu_has(X86_FEATURE_AMD_IBRS))
-               bits &= ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP);
+       u64 saved_value;
+       unsigned long flags;
+       int ret = 0;
 
-       if (!guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL_SSBD) &&
-           !guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD))
-               bits &= ~SPEC_CTRL_SSBD;
-       if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) &&
-           !boot_cpu_has(X86_FEATURE_AMD_SSBD))
-               bits &= ~SPEC_CTRL_SSBD;
+       local_irq_save(flags);
 
-       return bits;
+       if (rdmsrl_safe(MSR_IA32_SPEC_CTRL, &saved_value))
+               ret = 1;
+       else if (wrmsrl_safe(MSR_IA32_SPEC_CTRL, value))
+               ret = 1;
+       else
+               wrmsrl(MSR_IA32_SPEC_CTRL, saved_value);
+
+       local_irq_restore(flags);
+
+       return ret;
 }
-EXPORT_SYMBOL_GPL(kvm_spec_ctrl_valid_bits);
+EXPORT_SYMBOL_GPL(kvm_spec_ctrl_test_value);
 
 EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_exit);
 EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_fast_mmio);

diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index b968acc0516fe41876b104a2d2ea4815b1c71691..73c62b5d27654592c67af85a4f6436a850d54d1a 100644 (file)
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -357,6 +357,6 @@ static inline bool kvm_dr7_valid(u64 data)
 
 void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu);
 void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu);
-u64 kvm_spec_ctrl_valid_bits(struct kvm_vcpu *vcpu);
+int kvm_spec_ctrl_test_value(u64 value);
 
 #endif