[PATCH 2/3 v2] XSAVE/XRSTOR: fix frozen states

[PATCH 2/3 v2] XSAVE/XRSTOR: fix frozen states

[Han, Weidong]

[-- Attachment #1: Type: text/plain, Size: 409 bytes --]

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>

[-- Attachment #2: fix-frozen-state.patch --]
[-- Type: application/octet-stream, Size: 982 bytes --]

diff -r 977efa62d329 xen/arch/x86/i387.c
--- a/xen/arch/x86/i387.c	Tue Aug 31 18:12:03 2010 -0400
+++ b/xen/arch/x86/i387.c	Tue Aug 31 18:19:54 2010 -0400
@@ -35,6 +35,8 @@ void save_init_fpu(struct vcpu *v)
 
     if ( cpu_has_xsave && is_hvm_vcpu(v) )
     {
+        /* Save all supported feature states */
+        set_xcr0(xfeature_mask);
         xsave(v);
     }
     else if ( cpu_has_fxsr )
diff -r 977efa62d329 xen/include/asm-x86/i387.h
--- 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
         {

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH 2/3 v2] XSAVE/XRSTOR: fix frozen states

[Jan Beulich]

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

Re: [PATCH 2/3 v2] XSAVE/XRSTOR: fix frozen states

[Weidong Han]

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