diff mbox

[v2] kvm/fpu: Enable fully eager restore kvm FPU

Message ID 403610A45A2B5242BD291EDAE8B37D300FEBF79C@SHSMSX102.ccr.corp.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hao, Xudong Sept. 11, 2012, 9:20 a.m. UTC 
> -----Original Message-----
> From: Avi Kivity [mailto:avi@redhat.com]
> Sent: Tuesday, September 11, 2012 3:54 PM
> To: Hao, Xudong
> Cc: kvm@vger.kernel.org; Zhang, Xiantao; Joerg.Roedel@amd.com
> 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@redhat.com]
> >> Sent: Monday, September 10, 2012 4:07 PM
> >> To: Hao, Xudong
> >> Cc: kvm@vger.kernel.org; Zhang, Xiantao; Joerg.Roedel@amd.com
> >> 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.


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- 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;