> -----Original Message----- > From: Avi Kivity [mailto:avi@xxxxxxxxxx] > Sent: Tuesday, September 11, 2012 3:54 PM > To: Hao, Xudong > Cc: kvm@xxxxxxxxxxxxxxx; Zhang, Xiantao; Joerg.Roedel@xxxxxxx > Subject: Re: [PATCH v2] kvm/fpu: Enable fully eager restore kvm FPU > > On 09/11/2012 09:43 AM, Hao, Xudong wrote: > >> -----Original Message----- > >> From: Avi Kivity [mailto:avi@xxxxxxxxxx] > >> Sent: Monday, September 10, 2012 4:07 PM > >> To: Hao, Xudong > >> Cc: kvm@xxxxxxxxxxxxxxx; Zhang, Xiantao; Joerg.Roedel@xxxxxxx > >> Subject: Re: [PATCH v2] kvm/fpu: Enable fully eager restore kvm FPU > >> > >> > > >> > Avi, I'm not sure if I fully understand of you. Do you mean enter guest with > a > >> fpu_active=0 and then fpu does not restore? > >> > >> Yes. > >> > >> > If so, I will add fpu_active=1 in the no-lazy case. > >> > > >> > - kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); > >> > + if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) && > >> > + (vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY))) { > >> > + kvm_x86_ops->fpu_activate(vcpu); > >> > + vcpu->fpu_active=1; > >> > + } > >> > + else > >> > + kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); > >> > > >> > >> It doesn't help here. > >> > >> 1 guest boot > >> 2 kvm_userspace_exit (deactivates fpu) > >> 3 XSETBV exit that sets xcr0.new_bit > >> 4 kvm_enter > >> > >> There is no call to kvm_put_guest_fpu() between 3 and 4, you need > >> something in __kvm_set_xcr() to activate the fpu. > >> > > > > Yes, it's code path when enable xsave in guest, I'll add fpu activate there and > remain v2 patch in kvm_put_guest_fpu(). > > > > @@ -554,6 +554,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, > u64 xcr) > > xcr0 = xcr; > > if (kvm_x86_ops->get_cpl(vcpu) != 0) > > return 1; > > + if (xcr0 & ~((u64)KVM_XSTATE_LAZY)) > > + kvm_x86_ops->fpu_activate(vcpu); > > if (!(xcr0 & XSTATE_FP)) > > return 1; > > if ((xcr0 & XSTATE_YMM) && !(xcr0 & XSTATE_SSE)) > > > >> Note you also need to consider writes to xcr0 and cr4 that happen in the > >> reverse order due to live migration. > >> > > > > I'm confused of this, doesn't setting cr4 firstly then xcr0? > > Do you mean current live migration has a reverse order, or it must be a > reverse order with my eager restore patch? > > I mean I want the code to work regardless of whether KVM_SET_SREGS or > KVM_SET_XCRS is called first. > Okay, I got it. fpu_active(vcpu) in __kvm_set_xcr () read guest cr0, so KVM_SET_XCRS depends on KVM_SET_SREGS in live migration case. Here only set fpu_active=1 in __kvm_set_xcr(), and clear TS bit in set_cr0 should solve this issue. --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -3028,6 +3028,8 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0) if (!vcpu->fpu_active) hw_cr0 |= X86_CR0_TS | X86_CR0_MP; + else + hw_cr0 &= ~(X86_CR0_TS | X86_CR0_MP); vmcs_writel(CR0_READ_SHADOW, cr0); vmcs_writel(GUEST_CR0, hw_cr0); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 20f2266..183cf60 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -560,6 +560,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr) return 1; if (xcr0 & ~host_xcr0) return 1; + if (xcr0 & ~((u64)KVM_XSTATE_LAZY)) + vcpu->fpu_active = 1; vcpu->arch.xcr0 = xcr0; vcpu->guest_xcr0_loaded = 0; return 0; -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
