[PATCH v2 0/3] KVM: x86: kvm_mwait_in_guest() cleanup

[PATCH v2 0/3] KVM: x86: kvm_mwait_in_guest() cleanup

[Radim Krčmář]

This is a rebased version of an old series that simplified
kvm_mwait_in_guest: https://www.spinics.net/lists/kvm/msg149238.html

AMD errata 400 patch was dropped thanks to Boris's review;
[2/3] got an expanded commit message and I didn't include Alexander's
r-b since the context changed when we didn't drop support for ancient
CPUs.

Radim Krčmář (3):
  KVM: x86: prevent MWAIT in guest with buggy MONITOR
  KVM: x86: drop bogus MWAIT check
  KVM: x86: simplify kvm_mwait_in_guest()

 arch/x86/kvm/x86.h | 32 ++------------------------------
 1 file changed, 2 insertions(+), 30 deletions(-)

-- 
2.14.2

[PATCH v2 1/3] KVM: x86: prevent MWAIT in guest with buggy MONITOR

[Radim Krčmář]

The bug prevents MWAIT from waking up after a write to the monitored
cache line.
KVM might emulate a CPU model that shouldn't have the bug, so the guest
would not employ a workaround and possibly miss wakeups.
Better to avoid the situation.

Reviewed-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 arch/x86/kvm/x86.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index d0b95b7a90b4..81f5f50794f6 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -281,6 +281,9 @@ static inline bool kvm_mwait_in_guest(void)
 		return false;
 	}
 
+	if (boot_cpu_has_bug(X86_BUG_MONITOR))
+		return false;
+
 	/*
 	 * Intel CPUs without CPUID5_ECX_INTERRUPT_BREAK are problematic as
 	 * they would allow guest to stop the CPU completely by disabling
-- 
2.14.2

[PATCH v2 2/3] KVM: x86: drop bogus MWAIT check

[Radim Krčmář]

The check was added in some iteration while trying to fix a reported OS
X on Core 2 bug, but that bug is elsewhere.

The comment is misleading because the guest can call MWAIT with ECX = 0
even if we enforce CPUID5_ECX_INTERRUPT_BREAK;  the call would have the
exactly the same effect as if the host didn't have the feature.

A problem is that a QEMU feature exposes CPUID5_ECX_INTERRUPT_BREAK on
CPUs that do not support it.  Removing the check changes behavior on
last Pentium 4 lines (Presler, Dempsey, and Tulsa, which had VMX and
MONITOR while missing INTERRUPT_BREAK) when running a guest OS that uses
MWAIT without checking for its presence (QEMU doesn't expose MONITOR).

The only known OS that ignores the MONITOR flag is old Mac OS X and we
allowed it to bug on Core 2 (MWAIT used to throw #UD and only that OS
noticed), so we can save another 20 lines letting it bug on even older
CPUs.  Alternatively, we can return MWAIT exiting by default and let
userspace toggle it.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 arch/x86/kvm/x86.h | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 81f5f50794f6..d15859ec5e92 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -265,8 +265,6 @@ static inline u64 nsec_to_cycles(struct kvm_vcpu *vcpu, u64 nsec)
 
 static inline bool kvm_mwait_in_guest(void)
 {
-	unsigned int eax, ebx, ecx, edx;
-
 	if (!cpu_has(&boot_cpu_data, X86_FEATURE_MWAIT))
 		return false;
 
@@ -275,29 +273,10 @@ static inline bool kvm_mwait_in_guest(void)
 		/* All AMD CPUs have a working MWAIT implementation */
 		return true;
 	case X86_VENDOR_INTEL:
-		/* Handle Intel below */
-		break;
+		return !boot_cpu_has_bug(X86_BUG_MONITOR);
 	default:
 		return false;
 	}
-
-	if (boot_cpu_has_bug(X86_BUG_MONITOR))
-		return false;
-
-	/*
-	 * Intel CPUs without CPUID5_ECX_INTERRUPT_BREAK are problematic as
-	 * they would allow guest to stop the CPU completely by disabling
-	 * interrupts then invoking MWAIT.
-	 */
-	if (boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF)
-		return false;
-
-	cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &edx);
-
-	if (!(ecx & CPUID5_ECX_INTERRUPT_BREAK))
-		return false;
-
-	return true;
 }
 
 #endif
-- 
2.14.2

[PATCH v2 3/3] KVM: x86: simplify kvm_mwait_in_guest()

[Radim Krčmář]

If Intel/AMD implements MWAIT, we expect that it works well and only
reject known bugs;  no reason to do it the other way around for minor
vendors.  (Not that they are relevant ATM.)

This allows further simplification of kvm_mwait_in_guest().
And use boot_cpu_has() instead of "cpu_has(&boot_cpu_data," while at it.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 arch/x86/kvm/x86.h | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index d15859ec5e92..c69f973111cb 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -265,18 +265,8 @@ static inline u64 nsec_to_cycles(struct kvm_vcpu *vcpu, u64 nsec)
 
 static inline bool kvm_mwait_in_guest(void)
 {
-	if (!cpu_has(&boot_cpu_data, X86_FEATURE_MWAIT))
-		return false;
-
-	switch (boot_cpu_data.x86_vendor) {
-	case X86_VENDOR_AMD:
-		/* All AMD CPUs have a working MWAIT implementation */
-		return true;
-	case X86_VENDOR_INTEL:
-		return !boot_cpu_has_bug(X86_BUG_MONITOR);
-	default:
-		return false;
-	}
+	return boot_cpu_has(X86_FEATURE_MWAIT) &&
+		!boot_cpu_has_bug(X86_BUG_MONITOR);
 }
 
 #endif
-- 
2.14.2

Re: [PATCH v2 0/3] KVM: x86: kvm_mwait_in_guest() cleanup

[Borislav Petkov]

On Wed, Nov 29, 2017 at 10:23:40PM +0100, Radim Krčmář wrote:
> This is a rebased version of an old series that simplified
> kvm_mwait_in_guest: https://www.spinics.net/lists/kvm/msg149238.html
> 
> AMD errata 400 patch was dropped thanks to Boris's review;
> [2/3] got an expanded commit message and I didn't include Alexander's
> r-b since the context changed when we didn't drop support for ancient
> CPUs.
> 
> Radim Krčmář (3):
>   KVM: x86: prevent MWAIT in guest with buggy MONITOR
>   KVM: x86: drop bogus MWAIT check
>   KVM: x86: simplify kvm_mwait_in_guest()
> 
>  arch/x86/kvm/x86.h | 32 ++------------------------------
>  1 file changed, 2 insertions(+), 30 deletions(-)

Looks ok to me, AFAICT. So FWIW:

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 0/3] KVM: x86: kvm_mwait_in_guest() cleanup

[Michael S. Tsirkin]

On Wed, Nov 29, 2017 at 10:23:40PM +0100, Radim Krčmář wrote:
> This is a rebased version of an old series that simplified
> kvm_mwait_in_guest: https://www.spinics.net/lists/kvm/msg149238.html
> 
> AMD errata 400 patch was dropped thanks to Boris's review;
> [2/3] got an expanded commit message and I didn't include Alexander's
> r-b since the context changed when we didn't drop support for ancient
> CPUs.

Series

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> Radim Krčmář (3):
>   KVM: x86: prevent MWAIT in guest with buggy MONITOR
>   KVM: x86: drop bogus MWAIT check
>   KVM: x86: simplify kvm_mwait_in_guest()
> 
>  arch/x86/kvm/x86.h | 32 ++------------------------------
>  1 file changed, 2 insertions(+), 30 deletions(-)
> 
> -- 
> 2.14.2