Xen devel - Aug 2010 - [PATCH 2/3 v2] XSAVE/XRSTOR: fix frozen states

If a guest sets a state and dirties the state, but later temporarily clears the
state, and at this time if this vcpu is scheduled out, then other vcpus may
corrupt the state before the vcpu is scheduled in again, thus the state cannot
be restored correctly. To solve this issue, this patch save/restore all states
unconditionally on vcpu context switch.

Signed-off-by: Weidong Han <weidong.han@intel.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> On 31.08.10 at 16:52, "Han, Weidong"
<weidong.han@intel.com> wrote:
>--- a/xen/include/asm-x86/i387.h	Tue Aug 31 18:12:03 2010 -0400
>+++ b/xen/include/asm-x86/i387.h	Tue Aug 31 18:20:46 2010 -0400
>@@ -113,9 +113,9 @@ static inline void setup_fpu(struct vcpu
>             if ( !v->fpu_initialised )
>                 v->fpu_initialised = 1;
> 
>-            set_xcr0(v->arch.hvm_vcpu.xcr0 | XSTATE_FP_SSE);
>+            /* Restore all supported feature states */
>+            set_xcr0(xfeature_mask);
>             xrstor(v);
>-            set_xcr0(v->arch.hvm_vcpu.xcr0);
>         }
>         else
>         {
Aren''t you taking away control from the guest OS of what part of the
state it wants/needs to save?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jan Beulich wrote:
>>>> On 31.08.10 at 16:52, "Han, Weidong"
<weidong.han@intel.com> wrote:
>>>>         
>> --- a/xen/include/asm-x86/i387.h	Tue Aug 31 18:12:03 2010 -0400
>> +++ b/xen/include/asm-x86/i387.h	Tue Aug 31 18:20:46 2010 -0400
>> @@ -113,9 +113,9 @@ static inline void setup_fpu(struct vcpu
>>             if ( !v->fpu_initialised )
>>                 v->fpu_initialised = 1;
>>
>> -            set_xcr0(v->arch.hvm_vcpu.xcr0 | XSTATE_FP_SSE);
>> +            /* Restore all supported feature states */
>> +            set_xcr0(xfeature_mask);
>>             xrstor(v);
>> -            set_xcr0(v->arch.hvm_vcpu.xcr0);
>>         }
>>         else
>>         {
>>     
>
> Aren''t you taking away control from the guest OS of what part of
the
> state it wants/needs to save?
>
>   
To fix ''frozen states'', it needs to saves/restores all
supported states.

I find an issue here. It should set_xcr0 to hvm_vcpu.xcr0 after xsave() 
and xrstor(), because applications in guest may read xcr0. I will fix it 
in next version.

Regards,
Weidong



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel