Re: [PATCH uq/master 1/2] x86: fix migration from pre-version 12

[Gleb Natapov <gleb@redhat.com>]

On Mon, Sep 09, 2013 at 01:07:37PM +0200, Paolo Bonzini wrote:
> >>>> In fact, another bug is that kvm_vcpu_ioctl_x86_set_xsave ignores
> >>>> xstate_bv when XSAVE is not available.  Instead, it should reset the
> >>>> FXSAVE data to processor-reset values (except for MXCSR which always
> >>>> comes from XRSTOR data), i.e. to all-zeros except for the x87 control
> >>>> and tag words.  It should also check reserved bits of MXCSR.
> >>>
> >>> I do not see why.
> >>
> >> Because otherwise it behaves in a subtly different manner for XSAVE and
> >> non-XSAVE hosts.
> > 
> > I do not see how. Can you elaborate?
> 
> Suppose userspace calls KVM_SET_XSAVE with XSTATE_BV=0.
> 
> On an XSAVE host, when the guest FPU state is loaded KVM will do an
> XRSTOR.  The XRSTOR will restore the FPU state to default values.
> 
> On a non-XSAVE host, when the guest FPU state is loaded KVM will do an
> FXRSTR.  The FXRSTR will load the FPU state from the first 512 bytes of
> the block that was passed to KVM_SET_XSAVE.
> 
> This is not a problem because userspace will usually pass to
> KVM_SET_XSAVE only something that it got from KVM_GET_XSAVE, and
> KVM_GET_XSAVE will never set XSTATE_BV=0.  However, KVM_SET_XSAVE is
> supposed to emulate XSAVE/XRSTOR if it is not available, and it is
> failing to emulate this detail.
> 
You are trying to be bug to bug compatible :) XSTATE_BV can be zero only
if FPU state is reset one, otherwise the guest will not survive. KVM_SET_XSAVE
is not suppose to emulate XSAVE/XRSTOR, it is not emulator function. It
is better to outlaw zero value for XSTATE_BV at all, but we cannot do it
because current QEMU uses it.

> >>>> Yes.  QEMU unmarshals information from the XSAVE region and back, so it
> >>>> cannot support MPX or AVX-512 yet (even if KVM were).  Separate bug, though.
> >>>>
> >>> IMO this is the main issue here, not separate bug. If we gonna let guest
> >>> use CPU state QEMU does not support we gonna have a bad time.
> >>
> >> We cannot force the guest not to use a feature; all we can do is hide
> > 
> > Of course we can't, this is correct for other features too, but this is
> > guest's problem.
> 
> Ok, then we agree that QEMU doesn't have a problem?  The XSAVE data will
Which problem exactly. The problems I see is that 1. We do not support
MPX and AVX-512 (but this is probably not the problem you meant :)) 2. 0D
data is not consistent with features. Guest may not expect it and do stupid
things.

> always be "fresh" as long as the guest obeys CPUID bits it receives, and
> the CPUID bits that QEMU passes will never enable XSAVE data that QEMU
> does not support.
> 

--
			Gleb.