Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1635234 > unrolled thread

[PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes Radim Krčmář <rkrcmar@redhat.com> - 2017-05-03 21:40 +0200
    [PATCH 3/4] KVM: x86: drop bogus MWAIT check Radim Krčmář <rkrcmar@redhat.com> - 2017-05-03 21:40 +0200
      Re: [PATCH 3/4] KVM: x86: drop bogus MWAIT check Paolo Bonzini <pbonzini@redhat.com> - 2017-05-04 13:00 +0200
        Re: [PATCH 3/4] KVM: x86: drop bogus MWAIT check Radim Krčmář <rkrcmar@redhat.com> - 2017-05-04 16:40 +0200
          Re: [PATCH 3/4] KVM: x86: drop bogus MWAIT check "Michael S. Tsirkin" <mst@redhat.com> - 2017-05-04 20:40 +0200
            Re: [PATCH 3/4] KVM: x86: drop bogus MWAIT check Radim Krčmář <rkrcmar@redhat.com> - 2017-05-04 22:10 +0200
        Re: [PATCH 3/4] KVM: x86: drop bogus MWAIT check "Michael S. Tsirkin" <mst@redhat.com> - 2017-05-04 20:30 +0200
    [PATCH 2/4] KVM: x86: prevent MWAIT in guest with buggy MONITOR Radim Krčmář <rkrcmar@redhat.com> - 2017-05-03 21:40 +0200
    Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes Alexander Graf <agraf@suse.de> - 2017-05-03 21:50 +0200
    Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes "Gabriel L. Somlo" <gsomlo@gmail.com> - 2017-05-04 20:00 +0200
      Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes Radim Krčmář <rkrcmar@redhat.com> - 2017-05-04 20:10 +0200
        Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes "Gabriel L. Somlo" <gsomlo@gmail.com> - 2017-05-05 15:10 +0200
        Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes "Gabriel L. Somlo" <gsomlo@gmail.com> - 2017-05-06 18:50 +0200
          Re: [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes Paolo Bonzini <pbonzini@redhat.com> - 2017-05-11 07:10 +0200

#1635234 — [PATCH 0/4] KVM: x86: kvm_mwait_in_guest() cleanup and fixes

kvm_mwait_in_guest() was overcomplicated and also missed one AMD bug
that should prevent MWAIT pass through.

This series ignores errata that don't have any Linux bug defined;
I know of two minor (not affecting the host) Core 2 errata:
  AG36.  Split Locked Stores May not Trigger the Monitoring Hardware
  AG106.  A REP STOS/MOVS to a MONITOR/MWAIT Address Range May Prevent
          Triggering of the Monitoring Hardware

None of them are really worthy of a new condition if Linux never hit
them ... we still have the OS X bug that Gabriel is hitting, but I'm ok
with the original approach that sacrificed it for "greater good".


Radim Krčmář (4):
  KVM: svm: prevent MWAIT in guest with erratum 400
  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 | 33 +++------------------------------
 1 file changed, 3 insertions(+), 30 deletions(-)

-- 
2.12.2

[toc] | [next] | [standalone]

#1635236 — [PATCH 3/4] KVM: x86: drop bogus MWAIT check

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 CPUID5_ECX_INTERRUPT_BREAK.

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

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 63d5fb65ea30..8ea4e80c24d1 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -216,8 +216,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;
 
@@ -225,29 +223,10 @@ static inline bool kvm_mwait_in_guest(void)
 	case X86_VENDOR_AMD:
 		return !boot_cpu_has_bug(X86_BUG_AMD_E400);
 	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.12.2

[toc] | [prev] | [next] | [standalone]

#1635633 — Re: [PATCH 3/4] KVM: x86: drop bogus MWAIT check

On 03/05/2017 21:37, Radim Krčmář wrote:
> 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 CPUID5_ECX_INTERRUPT_BREAK.
> 
> The check was added in some iteration while trying to fix a reported
> OS X on Core 2 bug, but the CPU had CPUID5_ECX_INTERRUPT_BREAK and the
> bug is elsewhere.

The reason for this, as I understood it, is that we have historically
not published leaf 5 information via KVM_GET_SUPPORTED_CPUID.  For this
reason, QEMU is publishing CPUID5_ECX_INTERRUPT_BREAK.  Then if:

- the host doesn't have ECX[0]=1 support

- the guest sets ECX[0]

you get a #GP in the guest.  So wrong comment but right thing to do.

Paolo

> 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 63d5fb65ea30..8ea4e80c24d1 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -216,8 +216,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;
>  
> @@ -225,29 +223,10 @@ static inline bool kvm_mwait_in_guest(void)
>  	case X86_VENDOR_AMD:
>  		return !boot_cpu_has_bug(X86_BUG_AMD_E400);
>  	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
> 

[toc] | [prev] | [next] | [standalone]

#1635803 — Re: [PATCH 3/4] KVM: x86: drop bogus MWAIT check

2017-05-04 12:58+0200, Paolo Bonzini:
> On 03/05/2017 21:37, Radim Krčmář wrote:
>> 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 CPUID5_ECX_INTERRUPT_BREAK.
>> 
>> The check was added in some iteration while trying to fix a reported
>> OS X on Core 2 bug, but the CPU had CPUID5_ECX_INTERRUPT_BREAK and the
>> bug is elsewhere.
> 
> The reason for this, as I understood it, is that we have historically
> not published leaf 5 information via KVM_GET_SUPPORTED_CPUID.  For this
> reason, QEMU is publishing CPUID5_ECX_INTERRUPT_BREAK.  Then if:

I see, it was added to QEMU in e737b32a3688 ("Core 2 Duo specification
(Alexander Graf)").

> - the host doesn't have ECX[0]=1 support
> 
> - the guest sets ECX[0]
> 
> you get a #GP in the guest.  So wrong comment but right thing to do.

That userspace didn't set CPUID.01H:ECX.MONITOR[bit 3], so a guest
should get #UD instead, but MWAIT couldn't be expected to work.

I think that the guest bug is very unlikely, so I'd get rid of the
condition anyway ... we have also recently killed support for pre-Core 2
hosts and AFAIK, all newer Intels have it.

(Not so sure about AMDs, which share the same problem, so we do need to
 do more than just comment it better in any case.)

[toc] | [prev] | [next] | [standalone]

#1635963 — Re: [PATCH 3/4] KVM: x86: drop bogus MWAIT check

On Thu, May 04, 2017 at 04:33:28PM +0200, Radim Krčmář wrote:
> 2017-05-04 12:58+0200, Paolo Bonzini:
> > On 03/05/2017 21:37, Radim Krčmář wrote:
> >> 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 CPUID5_ECX_INTERRUPT_BREAK.
> >> 
> >> The check was added in some iteration while trying to fix a reported
> >> OS X on Core 2 bug, but the CPU had CPUID5_ECX_INTERRUPT_BREAK and the
> >> bug is elsewhere.
> > 
> > The reason for this, as I understood it, is that we have historically
> > not published leaf 5 information via KVM_GET_SUPPORTED_CPUID.  For this
> > reason, QEMU is publishing CPUID5_ECX_INTERRUPT_BREAK.  Then if:
> 
> I see, it was added to QEMU in e737b32a3688 ("Core 2 Duo specification
> (Alexander Graf)").
> 
> > - the host doesn't have ECX[0]=1 support
> > 
> > - the guest sets ECX[0]
> > 
> > you get a #GP in the guest.  So wrong comment but right thing to do.
> 
> That userspace didn't set CPUID.01H:ECX.MONITOR[bit 3], so a guest
> should get #UD instead, but MWAIT couldn't be expected to work.
> 
> I think that the guest bug is very unlikely, so I'd get rid of the
> condition anyway ... we have also recently killed support for pre-Core 2
> hosts and AFAIK, all newer Intels have it.

That's a strange approach.  If other software followed the same logic,
it would say all newer intels have MWAIT support without
checking the MWAIT leaf :)

> (Not so sure about AMDs, which share the same problem, so we do need to
>  do more than just comment it better in any case.)
-- 
MST

[toc] | [prev] | [next] | [standalone]

#1636005 — Re: [PATCH 3/4] KVM: x86: drop bogus MWAIT check

2017-05-04 21:29+0300, Michael S. Tsirkin:
> On Thu, May 04, 2017 at 04:33:28PM +0200, Radim Krčmář wrote:
>> 2017-05-04 12:58+0200, Paolo Bonzini:
>> > On 03/05/2017 21:37, Radim Krčmář wrote:
>> >> 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 CPUID5_ECX_INTERRUPT_BREAK.
>> >> 
>> >> The check was added in some iteration while trying to fix a reported
>> >> OS X on Core 2 bug, but the CPU had CPUID5_ECX_INTERRUPT_BREAK and the
>> >> bug is elsewhere.
>> > 
>> > The reason for this, as I understood it, is that we have historically
>> > not published leaf 5 information via KVM_GET_SUPPORTED_CPUID.  For this
>> > reason, QEMU is publishing CPUID5_ECX_INTERRUPT_BREAK.  Then if:
>> 
>> I see, it was added to QEMU in e737b32a3688 ("Core 2 Duo specification
>> (Alexander Graf)").
>> 
>> > - the host doesn't have ECX[0]=1 support
>> > 
>> > - the guest sets ECX[0]
>> > 
>> > you get a #GP in the guest.  So wrong comment but right thing to do.
>> 
>> That userspace didn't set CPUID.01H:ECX.MONITOR[bit 3], so a guest
>> should get #UD instead, but MWAIT couldn't be expected to work.
>> 
>> I think that the guest bug is very unlikely, so I'd get rid of the
>> condition anyway ... we have also recently killed support for pre-Core 2
>> hosts and AFAIK, all newer Intels have it.
> 
> That's a strange approach.  If other software followed the same logic,
> it would say all newer intels have MWAIT support without
> checking the MWAIT leaf :)

I'd make an analogy for the condition with CPU that cannot disable a
feature because software is not checking for its presence correctly,
but I wanted to convey something different. :)

The condition is catching a combination of a questionable QEMU behavior
and a very unlikely guest bug (only old OS X is known to use MWAIT when
it should #UD).  I think that handling it in KVM doesn't make sense,
like with other obvious guest/QEMU bugs -- if we started from scratch,
there would be no reason to have this condition.

Still, we fear regressions, which is where Intel's support of that
feature comes in.  The KVM code can be simpler/better at no real cost.

(If we keep the condition, I'd also fix Gabriel's real bug as it is far
 more important.)

[toc] | [prev] | [next] | [standalone]

#1635961 — Re: [PATCH 3/4] KVM: x86: drop bogus MWAIT check

On Thu, May 04, 2017 at 12:58:05PM +0200, Paolo Bonzini wrote:
> 
> 
> On 03/05/2017 21:37, Radim Krčmář wrote:
> > 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 CPUID5_ECX_INTERRUPT_BREAK.
> > 
> > The check was added in some iteration while trying to fix a reported
> > OS X on Core 2 bug, but the CPU had CPUID5_ECX_INTERRUPT_BREAK and the
> > bug is elsewhere.
> 
> The reason for this, as I understood it, is that we have historically
> not published leaf 5 information via KVM_GET_SUPPORTED_CPUID.  For this
> reason, QEMU is publishing CPUID5_ECX_INTERRUPT_BREAK.  Then if:
> 
> - the host doesn't have ECX[0]=1 support
> 
> - the guest sets ECX[0]
> 
> you get a #GP in the guest.  So wrong comment but right thing to do.
> 
> Paolo

Exactly. And I agree the comment isn't a good one.



> > 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 63d5fb65ea30..8ea4e80c24d1 100644
> > --- a/arch/x86/kvm/x86.h
> > +++ b/arch/x86/kvm/x86.h
> > @@ -216,8 +216,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;
> >  
> > @@ -225,29 +223,10 @@ static inline bool kvm_mwait_in_guest(void)
> >  	case X86_VENDOR_AMD:
> >  		return !boot_cpu_has_bug(X86_BUG_AMD_E400);
> >  	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
> > 

[toc] | [prev] | [next] | [standalone]

#1635237 — [PATCH 2/4] KVM: x86: prevent MWAIT in guest with buggy MONITOR

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.

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 3ed7dd8737ab..63d5fb65ea30 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -231,6 +231,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.12.2

[toc] | [prev] | [next] | [standalone]

#1635238

On 03.05.17 21:37, Radim Krčmář wrote:
> kvm_mwait_in_guest() was overcomplicated and also missed one AMD bug
> that should prevent MWAIT pass through.
>
> This series ignores errata that don't have any Linux bug defined;
> I know of two minor (not affecting the host) Core 2 errata:
>   AG36.  Split Locked Stores May not Trigger the Monitoring Hardware
>   AG106.  A REP STOS/MOVS to a MONITOR/MWAIT Address Range May Prevent
>           Triggering of the Monitoring Hardware
>
> None of them are really worthy of a new condition if Linux never hit
> them ... we still have the OS X bug that Gabriel is hitting, but I'm ok
> with the original approach that sacrificed it for "greater good".

I like the series :)

Reviewed-by: Alexander Graf <agraf@suse.de>


Alex

[toc] | [prev] | [next] | [standalone]

#1635945

Hi Radim,

On Wed, May 03, 2017 at 09:37:29PM +0200, Radim Krčmář wrote:
> kvm_mwait_in_guest() was overcomplicated and also missed one AMD bug
> that should prevent MWAIT pass through.
> 
> This series ignores errata that don't have any Linux bug defined;
> I know of two minor (not affecting the host) Core 2 errata:
>   AG36.  Split Locked Stores May not Trigger the Monitoring Hardware
>   AG106.  A REP STOS/MOVS to a MONITOR/MWAIT Address Range May Prevent
>           Triggering of the Monitoring Hardware
> 
> None of them are really worthy of a new condition if Linux never hit
> them ... we still have the OS X bug that Gabriel is hitting, but I'm ok
> with the original approach that sacrificed it for "greater good".

If I wanted to test this (e.g. with OS X 10.8 guests on several of my older
Mac boxes running Fedora), which git repo would you have me use? (The series
won't apply directly on top of git://git.kernel.org/pub/scm/virt/kvm/kvm.git).

Thanks much,
--Gabriel

> Radim Krčmář (4):
>   KVM: svm: prevent MWAIT in guest with erratum 400
>   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 | 33 +++------------------------------
>  1 file changed, 3 insertions(+), 30 deletions(-)
> 
> -- 
> 2.12.2
> 

[toc] | [prev] | [next] | [standalone]

#1635951

2017-05-04 13:56-0400, Gabriel L. Somlo:
> If I wanted to test this (e.g. with OS X 10.8 guests on several of my older
> Mac boxes running Fedora), which git repo would you have me use? (The series
> won't apply directly on top of git://git.kernel.org/pub/scm/virt/kvm/kvm.git).

The queue branch of that repo.  This series depends on a patch that is
applied there:

  668fffa3f838 kvm: better MWAIT emulation for guests

I forgot to mention that, sorry.

[toc] | [prev] | [next] | [standalone]

#1636365

On Thu, May 04, 2017 at 08:07:15PM +0200, Radim Krčmář wrote:
> 2017-05-04 13:56-0400, Gabriel L. Somlo:
> > If I wanted to test this (e.g. with OS X 10.8 guests on several of my older
> > Mac boxes running Fedora), which git repo would you have me use? (The series
> > won't apply directly on top of git://git.kernel.org/pub/scm/virt/kvm/kvm.git).
> 
> The queue branch of that repo.  This series depends on a patch that is
> applied there:
> 
>   668fffa3f838 kvm: better MWAIT emulation for guests
> 
> I forgot to mention that, sorry.

Thanks; right now, I get this:

# modprobe -v kvm-intel
insmod /lib/modules/4.11.0-rc3+/kernel/virt/lib/irqbypass.ko 
insmod /lib/modules/4.11.0-rc3+/kernel/arch/x86/kvm/kvm.ko 
insmod /lib/modules/4.11.0-rc3+/kernel/arch/x86/kvm/kvm-intel.ko 
modprobe: ERROR: could not insert 'kvm_intel': Input/output error

but that appears to have nothing to do with the MWAIT patches. I'm
bisecting to find the root cause, but it's really slow...

I'll follow up when I know more... Thanks,
--Gabriel

[toc] | [prev] | [next] | [standalone]

#1636951

On Thu, May 04, 2017 at 08:07:15PM +0200, Radim Krčmář wrote:
> 2017-05-04 13:56-0400, Gabriel L. Somlo:
> > If I wanted to test this (e.g. with OS X 10.8 guests on several of my older
> > Mac boxes running Fedora), which git repo would you have me use? (The series
> > won't apply directly on top of git://git.kernel.org/pub/scm/virt/kvm/kvm.git).
> 
> The queue branch of that repo.  This series depends on a patch that is
> applied there:
> 
>   668fffa3f838 kvm: better MWAIT emulation for guests
> 
> I forgot to mention that, sorry.

OK, here's where I'm at right now:

With this series applied on top of 'queue', my MacbookAir4,2 running
F25 (with the kvm/queue kernel) works fine, i.e. loads the kvm-intel
module successfully, and mwaits in L1 guest mode, reporting 400% cpu
but staying cool (guest started with -smp 4).

So far, so good.

On the MacPro1,1, I first had to revert 2c82878b0cb38fd
("KVM: VMX: require virtual NMI support") to get around this error:

# modprobe -v kvm-intel
insmod /lib/modules/4.11.0-rc3+/kernel/virt/lib/irqbypass.ko
insmod /lib/modules/4.11.0-rc3+/kernel/arch/x86/kvm/kvm.ko
insmod /lib/modules/4.11.0-rc3+/kernel/arch/x86/kvm/kvm-intel.ko
modprobe: ERROR: could not insert 'kvm_intel': Input/output error

Next, it turns out that on the MacPro1,1 kvm_mwait_in_guest() returns
TRUE, which causes OS X 10.7 (the one that mwaits without checking
CPUID) to misbehave. Forcing the function to return 0 (FALSE) solves
the problem:

diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index b49add7..249362c 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -216,9 +216,12 @@ static inline u64 nsec_to_cycles(struct kvm_vcpu *vcpu, u64 nsec)
 
 static inline bool kvm_mwait_in_guest(void)
 {
-       return boot_cpu_has(X86_FEATURE_MWAIT) &&
+       bool ret;
+       ret =  boot_cpu_has(X86_FEATURE_MWAIT) &&
                !boot_cpu_has_bug(X86_BUG_AMD_E400) &&
                !boot_cpu_has_bug(X86_BUG_MONITOR);
+       printk(KERN_INFO "kvm_mwait_in_guest: %d\n", ret);
+       return 0;
 }
 
 #endif

After this change, I get:

[ 1201.529002] kvm_mwait_in_guest: 1
[ 1201.529024] kvm_mwait_in_guest: 1
[ 1201.529029] kvm_mwait_in_guest: 1
[ 1201.529038] kvm_mwait_in_guest: 1
[ 1201.529047] kvm_mwait_in_guest: 1
[ 1225.150235] kvm: MONITOR instruction emulated as NOP!
[ 1225.150240] kvm: MWAIT instruction emulated as NOP!

indicating that it *would* have returned TRUE if I let it :)

This is a 2x dual-core Xeon, cca 2006 vintage, and the last (4th) CPU
in /proc/cpuinfo returns:

processor       : 3
vendor_id       : GenuineIntel
cpu family      : 6
model           : 15
model name      : Intel(R) Xeon(R) CPU            5150  @ 2.66GHz
stepping        : 6
microcode       : 0xd2
cpu MHz         : 2659.977
cache size      : 4096 KB
physical id     : 3
siblings        : 2
core id         : 0
cpu cores       : 2
apicid          : 6
initial apicid  : 6
fpu             : yes
fpu_exception   : yes
cpuid level     : 10
wp              : yes
flags           : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx lm constant_tsc arch_perfmon pebs bts rep_good nopl cpuid aperfmperf pni dtes64 monitor ds_cpl vmx est tm2 ssse3 cx16 xtpr pdcm dca lahf_lm tpr_shadow dtherm
bugs            :
bogomips        : 5320.03
clflush size    : 64
cache_alignment : 64
address sizes   : 36 bits physical, 48 bits virtual
power management:

So, in conclusion; it's not important to *me* that this old machine
keeps working, I'm just volunteering test data points. So please don't
feel obligated in any way to go out of your way on my account. OTOH,
I'm happy to provide feedback as long as you would like me to.

Along the same lines: Paolo, as the author of commit 2c82878b0cb38fd,
is the Xeon chip listed above one of the "obsolete for virtualization"
models ? In that case, it makes no sense for me to keep using it for
tests, and the fact that it misbehaves with L1 MWAIT should also not
matter at all.

Let me know what you all think.

Thanks,
--Gabriel

[toc] | [prev] | [next] | [standalone]

#1639196

On 06/05/2017 18:48, Gabriel L. Somlo wrote:
> So, in conclusion; it's not important to *me* that this old machine
> keeps working, I'm just volunteering test data points. So please don't
> feel obligated in any way to go out of your way on my account. OTOH,
> I'm happy to provide feedback as long as you would like me to.
> 
> Along the same lines: Paolo, as the author of commit 2c82878b0cb38fd,
> is the Xeon chip listed above one of the "obsolete for virtualization"
> models ?

Yes - I hadn't tested this model in particular, and this one is a little
less obsolete compared to the ones I found without NMI support (a 64-bit
Prescott and a 32-bit Yonah), but I still believe it's saner to treat
them as obsolete.

Can you please run vmxcap (from QEMU's git repository) on that processor
and include the output?

Paolo

> In that case, it makes no sense for me to keep using it for
> tests, and the fact that it misbehaves with L1 MWAIT should also not
> matter at all.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web