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. 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. >> >> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> >> --- >> 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. Paolo > >> +#define XSTATE_FP 1 >> +#define XSTATE_SSE 2 >> +#define XSTATE_YMM 4 >> + >> /* CPUID feature words */ >> typedef enum FeatureWord { >> FEAT_1_EDX, /* CPUID[1].EDX */ >> -- >> 1.8.3.1 >> > > -- > Gleb. > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >