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

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>

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 472544cf1de2eff49f8f9a574a6a6a7f0635ca0b..9b59e63567bbe1a621abef25ce784673c30cf0de 100644 (file)
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2522,7 +2522,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 c6e96e2ef4d6b0bd0b5c96e7c963aca95a4357af..0d526b32f041f1964407efd4d0d0569598fec17d 100644 (file)
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2062,7 +2062,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 c432a445cbbe5cc9ed9104d88a23655390985492..7f32169e8449f88386a9b5a961b727f23245d583 100644 (file)
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10693,28 +10693,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
+        */
+
+       u64 saved_value;
+       unsigned long flags;
+       int ret = 0;
 
-       /* 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);
+       local_irq_save(flags);
 
-       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;
+       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);
 
-       return bits;
+       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 224670d7c24547199342560878585f2b5acf5f5f..3308c3ccc0fd8ba2be3cf8332f997656fb426361 100644 (file)
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -368,7 +368,7 @@ static inline bool kvm_dr6_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);
 int kvm_valid_cr4(struct kvm_vcpu *vcpu, unsigned long cr4);
 bool kvm_vcpu_exit_request(struct kvm_vcpu *vcpu);