Date: Wed, 15 Mar 2017 15:43:03 -0400
From: "Gabriel L. Somlo" <gsomlo@gmail.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>,
        linux-kernel@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
        Jonathan Corbet <corbet@lwn.net>, Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
        x86@kernel.org, kvm@vger.kernel.org, linux-doc@vger.kernel.org
Subject: Re: [PATCH] kvm: better MWAIT emulation for guests
Message-ID: <20170315194302.GC2239@HEDWIG.INI.CMU.EDU>
References: <1489098555-23856-1-git-send-email-mst@redhat.com>
 <20170313154618.GA4547@potion>
 <20170313180046-mutt-send-email-mst@kernel.org>
 <20170313193910.GB4547@potion>
 <20170313215016-mutt-send-email-mst@kernel.org>
 <20170313214355.GC4547@potion>
 <20170315181425.GA2239@HEDWIG.INI.CMU.EDU>
 <20170315202049-mutt-send-email-mst@kernel.org>
 <20170315190112.GB2239@HEDWIG.INI.CMU.EDU>
 <20170315212906-mutt-send-email-mst@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20170315212906-mutt-send-email-mst@kernel.org>
User-Agent: Mutt/1.7.1 (2016-10-04)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 5472
Lines: 119

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