diff mbox series

[15/15] KVM: x86: Hide RDTSCP and RDPID if MSR_TSC_AUX probing failed

Message ID 20210504171734.1434054-16-seanjc@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: x86: RDPID/RDTSCP fixes and uret MSR cleanups | expand

Commit Message

Sean Christopherson May 4, 2021, 5:17 p.m. UTC
If probing MSR_TSC_AUX failed, hide RDTSCP and RDPID, and WARN if either
feature was reported as supported.  In theory, such a scenario should
never happen as both Intel and AMD state that MSR_TSC_AUX is available if
RDTSCP or RDPID is supported.  But, KVM injects #GP on MSR_TSC_AUX
accesses if probing failed, faults on WRMSR(MSR_TSC_AUX) may be fatal to
the guest (because they happen during early CPU bringup), and KVM itself
has effectively misreported RDPID support in the past.

Note, this also has the happy side effect of omitting MSR_TSC_AUX from
the list of MSRs that are exposed to userspace if probing the MSR fails.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/cpuid.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Maxim Levitsky May 10, 2021, 8:29 a.m. UTC | #1
On Tue, 2021-05-04 at 10:17 -0700, Sean Christopherson wrote:
> If probing MSR_TSC_AUX failed, hide RDTSCP and RDPID, and WARN if either
> feature was reported as supported.  In theory, such a scenario should
> never happen as both Intel and AMD state that MSR_TSC_AUX is available if
> RDTSCP or RDPID is supported.  But, KVM injects #GP on MSR_TSC_AUX
> accesses if probing failed, faults on WRMSR(MSR_TSC_AUX) may be fatal to
> the guest (because they happen during early CPU bringup), and KVM itself
> has effectively misreported RDPID support in the past.
> 
> Note, this also has the happy side effect of omitting MSR_TSC_AUX from
> the list of MSRs that are exposed to userspace if probing the MSR fails.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/cpuid.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index c96f79c9fff2..bf0f74ce4974 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -567,6 +567,21 @@ void kvm_set_cpu_caps(void)
>  		F(ACE2) | F(ACE2_EN) | F(PHE) | F(PHE_EN) |
>  		F(PMM) | F(PMM_EN)
>  	);
> +
> +	/*
> +	 * Hide RDTSCP and RDPID if either feature is reported as supported but
> +	 * probing MSR_TSC_AUX failed.  This is purely a sanity check and
> +	 * should never happen, but the guest will likely crash if RDTSCP or
> +	 * RDPID is misreported, and KVM has botched MSR_TSC_AUX emulation in
> +	 * the past, e.g. the sanity check may fire if this instance of KVM is
> +	 * running as L1 on top of an older, broken KVM.
> +	 */
> +	if (WARN_ON((kvm_cpu_cap_has(X86_FEATURE_RDTSCP) ||
> +		     kvm_cpu_cap_has(X86_FEATURE_RDPID)) &&
> +		     !kvm_is_supported_user_return_msr(MSR_TSC_AUX))) {
> +		kvm_cpu_cap_clear(X86_FEATURE_RDTSCP);
> +		kvm_cpu_cap_clear(X86_FEATURE_RDPID);
> +	}
>  }
>  EXPORT_SYMBOL_GPL(kvm_set_cpu_caps);
>  
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Best regards,
	Maxim Levitsky
diff mbox series

Patch

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index c96f79c9fff2..bf0f74ce4974 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -567,6 +567,21 @@  void kvm_set_cpu_caps(void)
 		F(ACE2) | F(ACE2_EN) | F(PHE) | F(PHE_EN) |
 		F(PMM) | F(PMM_EN)
 	);
+
+	/*
+	 * Hide RDTSCP and RDPID if either feature is reported as supported but
+	 * probing MSR_TSC_AUX failed.  This is purely a sanity check and
+	 * should never happen, but the guest will likely crash if RDTSCP or
+	 * RDPID is misreported, and KVM has botched MSR_TSC_AUX emulation in
+	 * the past, e.g. the sanity check may fire if this instance of KVM is
+	 * running as L1 on top of an older, broken KVM.
+	 */
+	if (WARN_ON((kvm_cpu_cap_has(X86_FEATURE_RDTSCP) ||
+		     kvm_cpu_cap_has(X86_FEATURE_RDPID)) &&
+		     !kvm_is_supported_user_return_msr(MSR_TSC_AUX))) {
+		kvm_cpu_cap_clear(X86_FEATURE_RDTSCP);
+		kvm_cpu_cap_clear(X86_FEATURE_RDPID);
+	}
 }
 EXPORT_SYMBOL_GPL(kvm_set_cpu_caps);