diff mbox

kvm: better MWAIT emulation for guests

Message ID 20170313214355.GC4547@potion (mailing list archive)
State New, archived
Headers show

Commit Message

Radim Krčmář March 13, 2017, 9:43 p.m. UTC
2017-03-13 22:03+0200, Michael S. Tsirkin:
> On Mon, Mar 13, 2017 at 08:39:11PM +0100, Radim Krčmář wrote:
> > 2017-03-13 18:08+0200, Michael S. Tsirkin:
> > > On Mon, Mar 13, 2017 at 04:46:20PM +0100, Radim Krčmář wrote:
>> >> What about keeping just the last hunk to improve OS X, for now?
>> > 
>> > IMHO if we have a new functionality we are better of creating
>> > some way for guests to discover it is there.
>> > 
>> > Do we really have to argue about a single bit in HV leaf?
>> > What harm does it do?
>> 
>> It adds code to both guest and hosts and needs documentation ...
>> The bit is acceptable.  I just see no point in having it when there
>> already is a detection mechanism for mwait.
> 
> We don't want to use that standard detection mechanism IMHO at least
> not in all cases.

Enabling mwait by default would make sense if the guest OS monitored its
steal time and disabled mwait when it detects that it is not the main
user of the CPU, because mwait then hurts the host as well as the guest.

This would warrant some kind of paravirt as we still wouldn't want to
have standard mwait by default.  My problem is that the paravirt flag
alone is not enough for a normal mwait use on Intel.

>> In any case, this patch should also remove VM exits under SVM
> 
> AMD does not have MWAIT AFAIK. In any case, I don't see
> why can't SVM be a separate patch.

AMD just doesn't have MWAIT hints. (AMD has even MWAIT in userspace and
MWAITX, but they are not supported by KVM.)

The separate patch would have to be part of the same series as we don't
want to have vendor-specific detection, so I'd just remove these two in
the same patch to simplify handling:


Thanks.

Comments

Gabriel L. Somlo March 15, 2017, 6:14 p.m. UTC | #1
Michael,

I tested this on OS X 10.7 (Lion), the last version that doesn't check
CPUID for MWAIT support.

I used the latest kvm from git://git.kernel.org/pub/scm/virt/kvm/kvm.git
first as-is, then with your v2 MWAIT patch applied.

Single-(V)CPU guest works as expected (but then again, single-vcpu
guests worked even back when I tried emulating MWAIT the same as HLT).

When I try starting a SMP guest (with "-smp 4,cores=2"), the guest OS
hangs after generating some output in text/verbose boot mode -- I gave
up waiting for it after about 5 minutes. Works fine before your patch,
which leads me to suspect that, as I feared, MWAIT doesn't wake
immediately upon another VCPU writing to the MONITOR-ed memory location.

Tangentially, I remember back in the days of OS X 10.7, the
alternative to exiting guest mode and emulating MWAIT and MONITOR as
NOPs was to allow them both to run in guest mode.

While poorly documented by Intel at the time, MWAIT at L>0 effectively
behaves as a NOP (i.e., doesn't actually put the physical core into
low-power mode, because doing that would allow a guest to effectively
DOS the host hardware).

Given how unusual it is for a guest to use MONITOR/MWAIT in the first
place, what's wrong with leaving it all as is (i.e., emulated as NOP)?

Thanks,
--Gabriel

On Mon, Mar 13, 2017 at 10:43:55PM +0100, Radim Krčmář wrote:
> 2017-03-13 22:03+0200, Michael S. Tsirkin:
> > On Mon, Mar 13, 2017 at 08:39:11PM +0100, Radim Krčmář wrote:
> > > 2017-03-13 18:08+0200, Michael S. Tsirkin:
> > > > On Mon, Mar 13, 2017 at 04:46:20PM +0100, Radim Krčmář wrote:
> >> >> What about keeping just the last hunk to improve OS X, for now?
> >> > 
> >> > IMHO if we have a new functionality we are better of creating
> >> > some way for guests to discover it is there.
> >> > 
> >> > Do we really have to argue about a single bit in HV leaf?
> >> > What harm does it do?
> >> 
> >> It adds code to both guest and hosts and needs documentation ...
> >> The bit is acceptable.  I just see no point in having it when there
> >> already is a detection mechanism for mwait.
> > 
> > We don't want to use that standard detection mechanism IMHO at least
> > not in all cases.
> 
> Enabling mwait by default would make sense if the guest OS monitored its
> steal time and disabled mwait when it detects that it is not the main
> user of the CPU, because mwait then hurts the host as well as the guest.
> 
> This would warrant some kind of paravirt as we still wouldn't want to
> have standard mwait by default.  My problem is that the paravirt flag
> alone is not enough for a normal mwait use on Intel.
> 
> >> In any case, this patch should also remove VM exits under SVM
> > 
> > AMD does not have MWAIT AFAIK. In any case, I don't see
> > why can't SVM be a separate patch.
> 
> AMD just doesn't have MWAIT hints. (AMD has even MWAIT in userspace and
> MWAITX, but they are not supported by KVM.)
> 
> The separate patch would have to be part of the same series as we don't
> want to have vendor-specific detection, so I'd just remove these two in
> the same patch to simplify handling:
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index d1efe2c62b3f..18e53bc185d6 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1198,8 +1198,6 @@ static void init_vmcb(struct vcpu_svm *svm)
>  	set_intercept(svm, INTERCEPT_CLGI);
>  	set_intercept(svm, INTERCEPT_SKINIT);
>  	set_intercept(svm, INTERCEPT_WBINVD);
> -	set_intercept(svm, INTERCEPT_MONITOR);
> -	set_intercept(svm, INTERCEPT_MWAIT);
>  	set_intercept(svm, INTERCEPT_XSETBV);
>  
>  	control->iopm_base_pa = iopm_base;
> 
> Thanks.
Michael S. Tsirkin March 15, 2017, 6:29 p.m. UTC | #2
On Wed, Mar 15, 2017 at 02:14:26PM -0400, Gabriel L. Somlo wrote:
> Michael,
> 
> I tested this on OS X 10.7 (Lion), the last version that doesn't check
> CPUID for MWAIT support.
> 
> I used the latest kvm from git://git.kernel.org/pub/scm/virt/kvm/kvm.git
> first as-is, then with your v2 MWAIT patch applied.
> 
> Single-(V)CPU guest works as expected (but then again, single-vcpu
> guests worked even back when I tried emulating MWAIT the same as HLT).
> 
> When I try starting a SMP guest (with "-smp 4,cores=2"), the guest OS
> hangs after generating some output in text/verbose boot mode -- I gave
> up waiting for it after about 5 minutes. Works fine before your patch,
> which leads me to suspect that, as I feared, MWAIT doesn't wake
> immediately upon another VCPU writing to the MONITOR-ed memory location.
> 
> Tangentially, I remember back in the days of OS X 10.7, the
> alternative to exiting guest mode and emulating MWAIT and MONITOR as
> NOPs was to allow them both to run in guest mode.
> 
> While poorly documented by Intel at the time, MWAIT at L>0 effectively
> behaves as a NOP (i.e., doesn't actually put the physical core into
> low-power mode, because doing that would allow a guest to effectively
> DOS the host hardware).

Thanks for the testing, interesting.
Testing with Linux guest seems to show it works.
This could be an interrupt thing not a monitor thing.
Question: does your host CPU have this in its MWAIT leaf?
	Bit 01: Supports treating interrupts as break-event for MWAIT, even when interrupts disabled

We really should check that before enabling,
I'll add that.

> 
> Given how unusual it is for a guest to use MONITOR/MWAIT in the first
> place, what's wrong with leaving it all as is (i.e., emulated as NOP)?
> 
> Thanks,
> --Gabriel

I'm really looking into ways to use mwait within Linux guests,
this is just a building block that should help Mac OSX
as a side effect (and we do not want it broken if at all possible).
Gabriel L. Somlo March 15, 2017, 7:01 p.m. UTC | #3
On Wed, Mar 15, 2017 at 08:29:23PM +0200, Michael S. Tsirkin wrote:
> On Wed, Mar 15, 2017 at 02:14:26PM -0400, Gabriel L. Somlo wrote:
> > Michael,
> > 
> > I tested this on OS X 10.7 (Lion), the last version that doesn't check
> > CPUID for MWAIT support.
> > 
> > I used the latest kvm from git://git.kernel.org/pub/scm/virt/kvm/kvm.git
> > first as-is, then with your v2 MWAIT patch applied.
> > 
> > Single-(V)CPU guest works as expected (but then again, single-vcpu
> > guests worked even back when I tried emulating MWAIT the same as HLT).
> > 
> > When I try starting a SMP guest (with "-smp 4,cores=2"), the guest OS
> > hangs after generating some output in text/verbose boot mode -- I gave
> > up waiting for it after about 5 minutes. Works fine before your patch,
> > which leads me to suspect that, as I feared, MWAIT doesn't wake
> > immediately upon another VCPU writing to the MONITOR-ed memory location.
> > 
> > Tangentially, I remember back in the days of OS X 10.7, the
> > alternative to exiting guest mode and emulating MWAIT and MONITOR as
> > NOPs was to allow them both to run in guest mode.
> > 
> > While poorly documented by Intel at the time, MWAIT at L>0 effectively
> > behaves as a NOP (i.e., doesn't actually put the physical core into
> > low-power mode, because doing that would allow a guest to effectively
> > DOS the host hardware).
> 
> Thanks for the testing, interesting.
> Testing with Linux guest seems to show it works.
> This could be an interrupt thing not a monitor thing.
> Question: does your host CPU have this in its MWAIT leaf?
> 	Bit 01: Supports treating interrupts as break-event for MWAIT, even when interrupts disabled

How would I check for this (I'm sorry, haven't hacked on any KVM
related thing in a while, so I don't have it "cached") :)

> 
> We really should check that before enabling,
> I'll add that.
> 
> > 
> > Given how unusual it is for a guest to use MONITOR/MWAIT in the first
> > place, what's wrong with leaving it all as is (i.e., emulated as NOP)?
> > 
> 
> I'm really looking into ways to use mwait within Linux guests,
> this is just a building block that should help Mac OSX
> as a side effect (and we do not want it broken if at all possible).

A few years ago I tried really emulating MONITOR and MWAIT for a
project -- while not a total abject failure, the resulting patch
worked only intermittently (on OS X 10.7, which was the hot new thing
at the time, and hadn't started checking CPUID yet).

My collected wisdom on the topic from back then is here:

   http://www.contrib.andrew.cmu.edu/~somlo/OSXKVM/mwait.html

The problem is that MWAIT is required to wake synchronously with
any other "thing" (either another (v)CPU, or DMA, or whatever) writing
to the memory location "marked" by the last preceding MONITOR. While
interrupts of any kind may also wake an MWAIT, it is strictly not allowed
to "miss" a write to the MONITOR-ed memory location. So unless we implement
some sort of condition queue that guarantees re-enabling the "parked" vcpu
on an intercepted write to a specific memory location by another vcpu,
we can't guarantee architecturally correct behavior.

If linux uses it in a very specific way that can be "faked" even
without ISA compliance, that's OK with me -- but other guest OSs might
take the x86 ISA more literally :)

Let me know if there's anything else you'd like me to test, now that I
have set up a 4.11.0-rc2+ (a.k.a. kvm git master) testing rig...

Regards,
--Gabe
Michael S. Tsirkin March 15, 2017, 7:05 p.m. UTC | #4
On Wed, Mar 15, 2017 at 03:01:12PM -0400, Gabriel L. Somlo wrote:
> On Wed, Mar 15, 2017 at 08:29:23PM +0200, Michael S. Tsirkin wrote:
> > On Wed, Mar 15, 2017 at 02:14:26PM -0400, Gabriel L. Somlo wrote:
> > > Michael,
> > > 
> > > I tested this on OS X 10.7 (Lion), the last version that doesn't check
> > > CPUID for MWAIT support.
> > > 
> > > I used the latest kvm from git://git.kernel.org/pub/scm/virt/kvm/kvm.git
> > > first as-is, then with your v2 MWAIT patch applied.
> > > 
> > > Single-(V)CPU guest works as expected (but then again, single-vcpu
> > > guests worked even back when I tried emulating MWAIT the same as HLT).
> > > 
> > > When I try starting a SMP guest (with "-smp 4,cores=2"), the guest OS
> > > hangs after generating some output in text/verbose boot mode -- I gave
> > > up waiting for it after about 5 minutes. Works fine before your patch,
> > > which leads me to suspect that, as I feared, MWAIT doesn't wake
> > > immediately upon another VCPU writing to the MONITOR-ed memory location.
> > > 
> > > Tangentially, I remember back in the days of OS X 10.7, the
> > > alternative to exiting guest mode and emulating MWAIT and MONITOR as
> > > NOPs was to allow them both to run in guest mode.
> > > 
> > > While poorly documented by Intel at the time, MWAIT at L>0 effectively
> > > behaves as a NOP (i.e., doesn't actually put the physical core into
> > > low-power mode, because doing that would allow a guest to effectively
> > > DOS the host hardware).
> > 
> > Thanks for the testing, interesting.
> > Testing with Linux guest seems to show it works.
> > This could be an interrupt thing not a monitor thing.
> > Question: does your host CPU have this in its MWAIT leaf?
> > 	Bit 01: Supports treating interrupts as break-event for MWAIT, even when interrupts disabled
> 
> How would I check for this (I'm sorry, haven't hacked on any KVM
> related thing in a while, so I don't have it "cached") :)
> 
> > 
> > We really should check that before enabling,
> > I'll add that.
> > 
> > > 
> > > Given how unusual it is for a guest to use MONITOR/MWAIT in the first
> > > place, what's wrong with leaving it all as is (i.e., emulated as NOP)?
> > > 
> > 
> > I'm really looking into ways to use mwait within Linux guests,
> > this is just a building block that should help Mac OSX
> > as a side effect (and we do not want it broken if at all possible).
> 
> A few years ago I tried really emulating MONITOR and MWAIT for a
> project -- while not a total abject failure, the resulting patch
> worked only intermittently (on OS X 10.7, which was the hot new thing
> at the time, and hadn't started checking CPUID yet).
> 
> My collected wisdom on the topic from back then is here:
> 
>    http://www.contrib.andrew.cmu.edu/~somlo/OSXKVM/mwait.html
> 
> The problem is that MWAIT is required to wake synchronously with
> any other "thing" (either another (v)CPU, or DMA, or whatever) writing
> to the memory location "marked" by the last preceding MONITOR. While
> interrupts of any kind may also wake an MWAIT, it is strictly not allowed
> to "miss" a write to the MONITOR-ed memory location. So unless we implement
> some sort of condition queue that guarantees re-enabling the "parked" vcpu
> on an intercepted write to a specific memory location by another vcpu,
> we can't guarantee architecturally correct behavior.
> 
> If linux uses it in a very specific way that can be "faked" even
> without ISA compliance, that's OK with me -- but other guest OSs might
> take the x86 ISA more literally :)
> 
> Let me know if there's anything else you'd like me to test, now that I
> have set up a 4.11.0-rc2+ (a.k.a. kvm git master) testing rig...
> 
> Regards,
> --Gabe

I'm going to post a patch in a couple of minutes.
Michael S. Tsirkin March 15, 2017, 7:29 p.m. UTC | #5
On Wed, Mar 15, 2017 at 03:01:12PM -0400, Gabriel L. Somlo wrote:
> On Wed, Mar 15, 2017 at 08:29:23PM +0200, Michael S. Tsirkin wrote:
> > On Wed, Mar 15, 2017 at 02:14:26PM -0400, Gabriel L. Somlo wrote:
> > > Michael,
> > > 
> > > I tested this on OS X 10.7 (Lion), the last version that doesn't check
> > > CPUID for MWAIT support.
> > > 
> > > I used the latest kvm from git://git.kernel.org/pub/scm/virt/kvm/kvm.git
> > > first as-is, then with your v2 MWAIT patch applied.
> > > 
> > > Single-(V)CPU guest works as expected (but then again, single-vcpu
> > > guests worked even back when I tried emulating MWAIT the same as HLT).
> > > 
> > > When I try starting a SMP guest (with "-smp 4,cores=2"), the guest OS
> > > hangs after generating some output in text/verbose boot mode -- I gave
> > > up waiting for it after about 5 minutes. Works fine before your patch,
> > > which leads me to suspect that, as I feared, MWAIT doesn't wake
> > > immediately upon another VCPU writing to the MONITOR-ed memory location.
> > > 
> > > Tangentially, I remember back in the days of OS X 10.7, the
> > > alternative to exiting guest mode and emulating MWAIT and MONITOR as
> > > NOPs was to allow them both to run in guest mode.
> > > 
> > > While poorly documented by Intel at the time, MWAIT at L>0 effectively
> > > behaves as a NOP (i.e., doesn't actually put the physical core into
> > > low-power mode, because doing that would allow a guest to effectively
> > > DOS the host hardware).
> > 
> > Thanks for the testing, interesting.
> > Testing with Linux guest seems to show it works.
> > This could be an interrupt thing not a monitor thing.
> > Question: does your host CPU have this in its MWAIT leaf?
> > 	Bit 01: Supports treating interrupts as break-event for MWAIT, even when interrupts disabled
> 
> How would I check for this (I'm sorry, haven't hacked on any KVM
> related thing in a while, so I don't have it "cached") :)
> 
> > 
> > We really should check that before enabling,
> > I'll add that.
> > 
> > > 
> > > Given how unusual it is for a guest to use MONITOR/MWAIT in the first
> > > place, what's wrong with leaving it all as is (i.e., emulated as NOP)?
> > > 
> > 
> > I'm really looking into ways to use mwait within Linux guests,
> > this is just a building block that should help Mac OSX
> > as a side effect (and we do not want it broken if at all possible).
> 
> A few years ago I tried really emulating MONITOR and MWAIT for a
> project -- while not a total abject failure, the resulting patch
> worked only intermittently (on OS X 10.7, which was the hot new thing
> at the time, and hadn't started checking CPUID yet).
> 
> My collected wisdom on the topic from back then is here:
> 
>    http://www.contrib.andrew.cmu.edu/~somlo/OSXKVM/mwait.html
> 
> The problem is that MWAIT is required to wake synchronously with
> any other "thing" (either another (v)CPU, or DMA, or whatever) writing
> to the memory location "marked" by the last preceding MONITOR. While
> interrupts of any kind may also wake an MWAIT, it is strictly not allowed
> to "miss" a write to the MONITOR-ed memory location. So unless we implement
> some sort of condition queue that guarantees re-enabling the "parked" vcpu
> on an intercepted write to a specific memory location by another vcpu,
> we can't guarantee architecturally correct behavior.
> 
> If linux uses it in a very specific way that can be "faked" even
> without ISA compliance, that's OK with me -- but other guest OSs might
> take the x86 ISA more literally :)
> 
> Let me know if there's anything else you'd like me to test, now that I
> have set up a 4.11.0-rc2+ (a.k.a. kvm git master) testing rig...
> 
> Regards,
> --Gabe

Doing that corrently in software would be very hard.
I suspect your host CPU has an issue, sent a patch to
detect that. Let's see what happens.
Gabriel L. Somlo March 15, 2017, 7:43 p.m. UTC | #6
Applies cleanly over git://git.kernel.org/pub/scm/virt/kvm/kvm.git,
but then I get:

  CC [M]  arch/x86/kvm/x86.o
In file included from arch/x86/kvm/x86.c:28:0:
arch/x86/kvm/x86.h: In function ‘kvm_mwait_in_guest’:
arch/x86/kvm/x86.h:231:34: error: ‘CPUID_MWAIT_LEAF’ undeclared (first use in this function)
  if (boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF)
                                  ^
arch/x86/kvm/x86.h:231:34: note: each undeclared identifier is reported only once for each function it appears in
arch/x86/kvm/x86.h:234:45: error: ‘mwait_substates’ undeclared (first use in this function)
  cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &mwait_substates);
                                             ^
arch/x86/kvm/x86.h:236:14: error: ‘CPUID5_ECX_INTERRUPT_BREAK’ undeclared (first use in this function)
  if (!(ecx & CPUID5_ECX_INTERRUPT_BREAK))
              ^
arch/x86/kvm/x86.h:238:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
scripts/Makefile.build:294: recipe for target 'arch/x86/kvm/x86.o' failed
make[2]: *** [arch/x86/kvm/x86.o] Error 1
scripts/Makefile.build:553: recipe for target 'arch/x86/kvm' failed
make[1]: *** [arch/x86/kvm] Error 2
Makefile:1002: recipe for target 'arch/x86' failed
make: *** [arch/x86] Error 2


Did you accidentally leave out something that went into a .h file
somewhere ?

Thx,
--G

On Wed, Mar 15, 2017 at 09:29:57PM +0200, Michael S. Tsirkin wrote:
> On Wed, Mar 15, 2017 at 03:01:12PM -0400, Gabriel L. Somlo wrote:
> > On Wed, Mar 15, 2017 at 08:29:23PM +0200, Michael S. Tsirkin wrote:
> > > On Wed, Mar 15, 2017 at 02:14:26PM -0400, Gabriel L. Somlo wrote:
> > > > Michael,
> > > > 
> > > > I tested this on OS X 10.7 (Lion), the last version that doesn't check
> > > > CPUID for MWAIT support.
> > > > 
> > > > I used the latest kvm from git://git.kernel.org/pub/scm/virt/kvm/kvm.git
> > > > first as-is, then with your v2 MWAIT patch applied.
> > > > 
> > > > Single-(V)CPU guest works as expected (but then again, single-vcpu
> > > > guests worked even back when I tried emulating MWAIT the same as HLT).
> > > > 
> > > > When I try starting a SMP guest (with "-smp 4,cores=2"), the guest OS
> > > > hangs after generating some output in text/verbose boot mode -- I gave
> > > > up waiting for it after about 5 minutes. Works fine before your patch,
> > > > which leads me to suspect that, as I feared, MWAIT doesn't wake
> > > > immediately upon another VCPU writing to the MONITOR-ed memory location.
> > > > 
> > > > Tangentially, I remember back in the days of OS X 10.7, the
> > > > alternative to exiting guest mode and emulating MWAIT and MONITOR as
> > > > NOPs was to allow them both to run in guest mode.
> > > > 
> > > > While poorly documented by Intel at the time, MWAIT at L>0 effectively
> > > > behaves as a NOP (i.e., doesn't actually put the physical core into
> > > > low-power mode, because doing that would allow a guest to effectively
> > > > DOS the host hardware).
> > > 
> > > Thanks for the testing, interesting.
> > > Testing with Linux guest seems to show it works.
> > > This could be an interrupt thing not a monitor thing.
> > > Question: does your host CPU have this in its MWAIT leaf?
> > > 	Bit 01: Supports treating interrupts as break-event for MWAIT, even when interrupts disabled
> > 
> > How would I check for this (I'm sorry, haven't hacked on any KVM
> > related thing in a while, so I don't have it "cached") :)
> > 
> > > 
> > > We really should check that before enabling,
> > > I'll add that.
> > > 
> > > > 
> > > > Given how unusual it is for a guest to use MONITOR/MWAIT in the first
> > > > place, what's wrong with leaving it all as is (i.e., emulated as NOP)?
> > > > 
> > > 
> > > I'm really looking into ways to use mwait within Linux guests,
> > > this is just a building block that should help Mac OSX
> > > as a side effect (and we do not want it broken if at all possible).
> > 
> > A few years ago I tried really emulating MONITOR and MWAIT for a
> > project -- while not a total abject failure, the resulting patch
> > worked only intermittently (on OS X 10.7, which was the hot new thing
> > at the time, and hadn't started checking CPUID yet).
> > 
> > My collected wisdom on the topic from back then is here:
> > 
> >    http://www.contrib.andrew.cmu.edu/~somlo/OSXKVM/mwait.html
> > 
> > The problem is that MWAIT is required to wake synchronously with
> > any other "thing" (either another (v)CPU, or DMA, or whatever) writing
> > to the memory location "marked" by the last preceding MONITOR. While
> > interrupts of any kind may also wake an MWAIT, it is strictly not allowed
> > to "miss" a write to the MONITOR-ed memory location. So unless we implement
> > some sort of condition queue that guarantees re-enabling the "parked" vcpu
> > on an intercepted write to a specific memory location by another vcpu,
> > we can't guarantee architecturally correct behavior.
> > 
> > If linux uses it in a very specific way that can be "faked" even
> > without ISA compliance, that's OK with me -- but other guest OSs might
> > take the x86 ISA more literally :)
> > 
> > Let me know if there's anything else you'd like me to test, now that I
> > have set up a 4.11.0-rc2+ (a.k.a. kvm git master) testing rig...
> > 
> > Regards,
> > --Gabe
> 
> Doing that corrently in software would be very hard.
> I suspect your host CPU has an issue, sent a patch to
> detect that. Let's see what happens.
> 
> -- 
> MST
Michael S. Tsirkin March 15, 2017, 8:13 p.m. UTC | #7
On Wed, Mar 15, 2017 at 03:43:03PM -0400, Gabriel L. Somlo wrote:
> Applies cleanly over git://git.kernel.org/pub/scm/virt/kvm/kvm.git,
> but then I get:
> 
>   CC [M]  arch/x86/kvm/x86.o
> In file included from arch/x86/kvm/x86.c:28:0:
> arch/x86/kvm/x86.h: In function ‘kvm_mwait_in_guest’:
> arch/x86/kvm/x86.h:231:34: error: ‘CPUID_MWAIT_LEAF’ undeclared (first use in this function)
>   if (boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF)
>                                   ^
> arch/x86/kvm/x86.h:231:34: note: each undeclared identifier is reported only once for each function it appears in
> arch/x86/kvm/x86.h:234:45: error: ‘mwait_substates’ undeclared (first use in this function)
>   cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &mwait_substates);
>                                              ^
> arch/x86/kvm/x86.h:236:14: error: ‘CPUID5_ECX_INTERRUPT_BREAK’ undeclared (first use in this function)
>   if (!(ecx & CPUID5_ECX_INTERRUPT_BREAK))
>               ^
> arch/x86/kvm/x86.h:238:1: warning: control reaches end of non-void function [-Wreturn-type]
>  }
>  ^
> scripts/Makefile.build:294: recipe for target 'arch/x86/kvm/x86.o' failed
> make[2]: *** [arch/x86/kvm/x86.o] Error 1
> scripts/Makefile.build:553: recipe for target 'arch/x86/kvm' failed
> make[1]: *** [arch/x86/kvm] Error 2
> Makefile:1002: recipe for target 'arch/x86' failed
> make: *** [arch/x86] Error 2

forgot to commit :(
Will resend, sorry.

> 
> Did you accidentally leave out something that went into a .h file
> somewhere ?
> 
> Thx,
> --G
> 
> On Wed, Mar 15, 2017 at 09:29:57PM +0200, Michael S. Tsirkin wrote:
> > On Wed, Mar 15, 2017 at 03:01:12PM -0400, Gabriel L. Somlo wrote:
> > > On Wed, Mar 15, 2017 at 08:29:23PM +0200, Michael S. Tsirkin wrote:
> > > > On Wed, Mar 15, 2017 at 02:14:26PM -0400, Gabriel L. Somlo wrote:
> > > > > Michael,
> > > > > 
> > > > > I tested this on OS X 10.7 (Lion), the last version that doesn't check
> > > > > CPUID for MWAIT support.
> > > > > 
> > > > > I used the latest kvm from git://git.kernel.org/pub/scm/virt/kvm/kvm.git
> > > > > first as-is, then with your v2 MWAIT patch applied.
> > > > > 
> > > > > Single-(V)CPU guest works as expected (but then again, single-vcpu
> > > > > guests worked even back when I tried emulating MWAIT the same as HLT).
> > > > > 
> > > > > When I try starting a SMP guest (with "-smp 4,cores=2"), the guest OS
> > > > > hangs after generating some output in text/verbose boot mode -- I gave
> > > > > up waiting for it after about 5 minutes. Works fine before your patch,
> > > > > which leads me to suspect that, as I feared, MWAIT doesn't wake
> > > > > immediately upon another VCPU writing to the MONITOR-ed memory location.
> > > > > 
> > > > > Tangentially, I remember back in the days of OS X 10.7, the
> > > > > alternative to exiting guest mode and emulating MWAIT and MONITOR as
> > > > > NOPs was to allow them both to run in guest mode.
> > > > > 
> > > > > While poorly documented by Intel at the time, MWAIT at L>0 effectively
> > > > > behaves as a NOP (i.e., doesn't actually put the physical core into
> > > > > low-power mode, because doing that would allow a guest to effectively
> > > > > DOS the host hardware).
> > > > 
> > > > Thanks for the testing, interesting.
> > > > Testing with Linux guest seems to show it works.
> > > > This could be an interrupt thing not a monitor thing.
> > > > Question: does your host CPU have this in its MWAIT leaf?
> > > > 	Bit 01: Supports treating interrupts as break-event for MWAIT, even when interrupts disabled
> > > 
> > > How would I check for this (I'm sorry, haven't hacked on any KVM
> > > related thing in a while, so I don't have it "cached") :)
> > > 
> > > > 
> > > > We really should check that before enabling,
> > > > I'll add that.
> > > > 
> > > > > 
> > > > > Given how unusual it is for a guest to use MONITOR/MWAIT in the first
> > > > > place, what's wrong with leaving it all as is (i.e., emulated as NOP)?
> > > > > 
> > > > 
> > > > I'm really looking into ways to use mwait within Linux guests,
> > > > this is just a building block that should help Mac OSX
> > > > as a side effect (and we do not want it broken if at all possible).
> > > 
> > > A few years ago I tried really emulating MONITOR and MWAIT for a
> > > project -- while not a total abject failure, the resulting patch
> > > worked only intermittently (on OS X 10.7, which was the hot new thing
> > > at the time, and hadn't started checking CPUID yet).
> > > 
> > > My collected wisdom on the topic from back then is here:
> > > 
> > >    http://www.contrib.andrew.cmu.edu/~somlo/OSXKVM/mwait.html
> > > 
> > > The problem is that MWAIT is required to wake synchronously with
> > > any other "thing" (either another (v)CPU, or DMA, or whatever) writing
> > > to the memory location "marked" by the last preceding MONITOR. While
> > > interrupts of any kind may also wake an MWAIT, it is strictly not allowed
> > > to "miss" a write to the MONITOR-ed memory location. So unless we implement
> > > some sort of condition queue that guarantees re-enabling the "parked" vcpu
> > > on an intercepted write to a specific memory location by another vcpu,
> > > we can't guarantee architecturally correct behavior.
> > > 
> > > If linux uses it in a very specific way that can be "faked" even
> > > without ISA compliance, that's OK with me -- but other guest OSs might
> > > take the x86 ISA more literally :)
> > > 
> > > Let me know if there's anything else you'd like me to test, now that I
> > > have set up a 4.11.0-rc2+ (a.k.a. kvm git master) testing rig...
> > > 
> > > Regards,
> > > --Gabe
> > 
> > Doing that corrently in software would be very hard.
> > I suspect your host CPU has an issue, sent a patch to
> > detect that. Let's see what happens.
> > 
> > -- 
> > MST
diff mbox

Patch

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index d1efe2c62b3f..18e53bc185d6 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1198,8 +1198,6 @@  static void init_vmcb(struct vcpu_svm *svm)
 	set_intercept(svm, INTERCEPT_CLGI);
 	set_intercept(svm, INTERCEPT_SKINIT);
 	set_intercept(svm, INTERCEPT_WBINVD);
-	set_intercept(svm, INTERCEPT_MONITOR);
-	set_intercept(svm, INTERCEPT_MWAIT);
 	set_intercept(svm, INTERCEPT_XSETBV);
 
 	control->iopm_base_pa = iopm_base;