Re: [Xen-devel] [PATCH] X86: Fix vcpu xsave bug

["Liu, Jinsong" <jinsong.liu@xxxxxxxxx>]

Jan Beulich wrote:
>>>> On 18.11.13 at 13:24, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
>> Jan Beulich wrote:
>>>>>> On 18.11.13 at 11:35, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx>
>>>>>> wrote: 
>>>> Jan Beulich wrote:
>>>>>>>> On 15.11.13 at 17:55, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx>
>>>>>>>> wrote:
>>>>>> @@ -257,22 +257,29 @@ void vcpu_restore_fpu_lazy(struct vcpu *v)
>>>>>>  */ void vcpu_save_fpu(struct vcpu *v)
>>>>>>  {
>>>>>> -    if ( !v->fpu_dirtied )
>>>>>> -        return;
>>>>>> -
>>>>> 
>>>>> And the - afaict - the only changed needed to this function is
>>>>> the deletion above. 
>>>>> 
>>>> 
>>>> If I didn't misunderstand your meaning, it can not only delete
>>>> these 2 lines, say, when (!v->fpu_dirtied) and in old platform
>>>> that do fpu_fxsave/fpu_fsave?
>>> 
>>> Sorry, I don't understand what you're asking.
>>> 
>> 
>> The problem is I don't understand your last comments:
>> 'And the - afaict - the only changed needed to this function is the
>> deletion above.' 
>> 
>> Seems some misunderstanding here :)
>> So would you please give me the code of your thought based on the
>> patch below?
> 
> void vcpu_save_fpu(struct vcpu *v)
> {
>     ASSERT(!is_idle_vcpu(v));
> 
>     /* This can happen, if a paravirtualised guest OS has set its
>     CR0.TS. */ clts();
> 
>     if ( cpu_has_xsave )
>         fpu_xsave(v);
>     else if ( !v->fpu_dirtied )
>         /* nothing */;
>     else if ( cpu_has_fxsr )
>         fpu_fxsave(v);
>     else
>         fpu_fsave(v);
> 
>     v->fpu_dirtied = 0;
>     stts();
> }
> 

But that we need add logic at fpu_xsave(), like

if ( v->fpu_dirtied )
{
    if ( v->arch.nonlazy_xstate_used )
        mask = XSTATE_ALL;
    else
        mask = XSTATE_LAZY;
}
else
{
    if ( v->arch.nonlazy_xstate_used )
        mask = XSTATE_NONLAZY;
    else
        mask = 0;
}

xsave(v, mask);

This way (new vcpu_save_fpu + new fpu_xsave) is some obscure, and calling 
fpu_xsave may do nothing.
So how about keep old patch, use 'mask' to directly tell fpu_xsave what we want 
it to save?

Thanks,
Jinsong


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel