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

[Avi Kivity <avi@redhat.com>]

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.


-- 
error compiling committee.c: too many arguments to function