From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35663) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VIxNV-0004ls-Oe for qemu-devel@nongnu.org; Mon, 09 Sep 2013 05:03:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VIxNP-0004MH-Lm for qemu-devel@nongnu.org; Mon, 09 Sep 2013 05:03:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:20193) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VIxNP-0004M7-ER for qemu-devel@nongnu.org; Mon, 09 Sep 2013 05:03:19 -0400 Date: Mon, 9 Sep 2013 12:03:15 +0300 From: Gleb Natapov Message-ID: <20130909090315.GM17294@redhat.com> References: <1378386382-415-1-git-send-email-pbonzini@redhat.com> <1378386382-415-2-git-send-email-pbonzini@redhat.com> <20130908114044.GF17294@redhat.com> <522D8753.7010009@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <522D8753.7010009@redhat.com> Subject: Re: [Qemu-devel] [PATCH uq/master 1/2] x86: fix migration from pre-version 12 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org, ehabkost@redhat.com On Mon, Sep 09, 2013 at 10:31:15AM +0200, Paolo Bonzini wrote: > Il 08/09/2013 13:40, Gleb Natapov ha scritto: > > On Thu, Sep 05, 2013 at 03:06:21PM +0200, Paolo Bonzini wrote: > >> On KVM, the KVM_SET_XSAVE would be executed with a 0 xstate_bv, > >> and not restore anything. > >> > > XRSTOR restores FP/SSE state to reset state if no bits are set in > > xstate_bv. This is what should happen on reset, no? > > Yes. The problem happens on the migration destination when XSAVE data is > not transmitted. FP/SSE data is transmitted and must be restored, but > xstate_bv is zero and KVM_SET_XSAVE restores FP/SSE state to reset > state. The vcpu then loses the values that were set in the migration data. > > >> Since FP and SSE data are always valid, set them in xstate_bv at reset > >> time. In fact, that value is the same that KVM_GET_XSAVE returns on > >> pre-XSAVE hosts. > > It is needed for migration between non xsave host to xsave host. > > Yes, and this patch does the same for migration between non-XSAVE QEMU > and XSAVE QEMU. > Can such migration happen? The commit that added xsave support (f1665b21f16c5dc0ac37de60233a4975aff31193) changed vmstate version id. > 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. > >> > >> Signed-off-by: Paolo Bonzini > >> --- > >> target-i386/cpu.c | 1 + > >> target-i386/cpu.h | 5 +++++ > >> 2 files changed, 6 insertions(+) > >> > >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c > >> index c36345e..ac83106 100644 > >> --- a/target-i386/cpu.c > >> +++ b/target-i386/cpu.c > >> @@ -2386,6 +2386,7 @@ static void x86_cpu_reset(CPUState *s) > >> env->fpuc = 0x37f; > >> > >> env->mxcsr = 0x1f80; > >> + env->xstate_bv = XSTATE_FP | XSTATE_SSE; > >> > >> env->pat = 0x0007040600070406ULL; > >> env->msr_ia32_misc_enable = MSR_IA32_MISC_ENABLE_DEFAULT; > >> diff --git a/target-i386/cpu.h b/target-i386/cpu.h > >> index 5723eff..a153078 100644 > >> --- a/target-i386/cpu.h > >> +++ b/target-i386/cpu.h > >> @@ -380,6 +380,11 @@ > >> > >> #define MSR_VM_HSAVE_PA 0xc0010117 > >> > >> +#define XSTATE_SUPPORTED (XSTATE_FP|XSTATE_SSE|XSTATE_YMM) > > Supported by whom? By QEMU? We should filer unsupported bits from CPUID.0D then too. > > 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. -- Gleb.