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

[Radim KrÄmÃÅ]

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