[1/4] x86/xstate: fix xcomp_bv initialization

Commit Message

Jan Beulich Jan. 29, 2016, 10:26 a.m. UTC

We must not clear the compaction bit when using XSAVES/XRSTORS. And
we need to guarantee that xcomp_bv never has any bits clear which
are set in xstate_bv (which requires partly undoing commit 83ae0bb226
["x86/xsave: simplify xcomp_bv initialization"]). Split initialization
of xcomp_bv from the other FPU/SSE/AVX related state setup in
arch_set_info_guest() and hvm_load_cpu_ctxt().

Reported-by: Harmandeep Kaur <[e-mail]>
Signed-off-by: Jan Beulich <[e-mail]>
x86/xstate: fix xcomp_bv initialization

We must not clear the compaction bit when using XSAVES/XRSTORS. And
we need to guarantee that xcomp_bv never has any bits clear which
are set in xstate_bv (which requires partly undoing commit 83ae0bb226
["x86/xsave: simplify xcomp_bv initialization"]). Split initialization
of xcomp_bv from the other FPU/SSE/AVX related state setup in
arch_set_info_guest() and hvm_load_cpu_ctxt().

Reported-by: Harmandeep Kaur <[e-mail]>
Signed-off-by: Jan Beulich <[e-mail]>

--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -923,15 +923,13 @@ int arch_set_info_guest(
     {
         memcpy(v->arch.fpu_ctxt, &c.nat->fpu_ctxt, sizeof(c.nat->fpu_ctxt));
         if ( v->arch.xsave_area )
-        {
             v->arch.xsave_area->xsave_hdr.xstate_bv = XSTATE_FP_SSE;
-            v->arch.xsave_area->xsave_hdr.xcomp_bv =
-                cpu_has_xsaves ? XSTATE_COMPACTION_ENABLED : 0;
-        }
     }
     else if ( v->arch.xsave_area )
-        memset(&v->arch.xsave_area->xsave_hdr, 0,
-               sizeof(v->arch.xsave_area->xsave_hdr));
+    {
+        v->arch.xsave_area->xsave_hdr.xstate_bv = 0;
+        v->arch.xsave_area->fpu_sse.mxcsr = MXCSR_DEFAULT;
+    }
     else
     {
         typeof(v->arch.xsave_area->fpu_sse) *fpu_sse = v->arch.fpu_ctxt;
@@ -940,6 +938,14 @@ int arch_set_info_guest(
         fpu_sse->fcw = FCW_DEFAULT;
         fpu_sse->mxcsr = MXCSR_DEFAULT;
     }
+    if ( cpu_has_xsaves )
+    {
+        ASSERT(v->arch.xsave_area);
+        v->arch.xsave_area->xsave_hdr.xcomp_bv = XSTATE_COMPACTION_ENABLED |
+            v->arch.xsave_area->xsave_hdr.xstate_bv;
+    }
+    else if ( v->arch.xsave_area )
+        v->arch.xsave_area->xsave_hdr.xcomp_bv = 0;
 
     if ( !compat )
     {
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1971,6 +1971,7 @@ static int hvm_load_cpu_ctxt(struct doma
     struct hvm_hw_cpu ctxt;
     struct segment_register seg;
     const char *errstr;
+    struct xsave_struct *xsave_area;
 
     /* Which vcpu is this? */
     vcpuid = hvm_load_instance(h);
@@ -2097,20 +2098,24 @@ static int hvm_load_cpu_ctxt(struct doma
     seg.attr.bytes = ctxt.ldtr_arbytes;
     hvm_set_segment_register(v, x86_seg_ldtr, &seg);
 
+    /* Cover xsave-absent save file restoration on xsave-capable host. */
+    xsave_area = xsave_enabled(v) ? NULL : v->arch.xsave_area;
+
     v->fpu_initialised = !!(ctxt.flags & XEN_X86_FPU_INITIALISED);
     if ( v->fpu_initialised )
     {
         memcpy(v->arch.fpu_ctxt, ctxt.fpu_regs, sizeof(ctxt.fpu_regs));
-        /* In case xsave-absent save file is restored on a xsave-capable host */
-        if ( cpu_has_xsave && !xsave_enabled(v) )
-        {
-            struct xsave_struct *xsave_area = v->arch.xsave_area;
-
+        if ( xsave_area )
             xsave_area->xsave_hdr.xstate_bv = XSTATE_FP_SSE;
-            xsave_area->xsave_hdr.xcomp_bv =
-                cpu_has_xsaves ? XSTATE_COMPACTION_ENABLED : 0;
-        }
     }
+    else if ( xsave_area )
+    {
+        xsave_area->xsave_hdr.xstate_bv = 0;
+        xsave_area->fpu_sse.mxcsr = MXCSR_DEFAULT;
+    }
+    if ( cpu_has_xsaves && xsave_area )
+        xsave_area->xsave_hdr.xcomp_bv = XSTATE_COMPACTION_ENABLED |
+            xsave_area->xsave_hdr.xstate_bv;
 
     v->arch.user_regs.eax = ctxt.rax;
     v->arch.user_regs.ebx = ctxt.rbx;
@@ -5468,8 +5473,8 @@ void hvm_vcpu_reset_state(struct vcpu *v
     if ( v->arch.xsave_area )
     {
         v->arch.xsave_area->xsave_hdr.xstate_bv = XSTATE_FP;
-        v->arch.xsave_area->xsave_hdr.xcomp_bv =
-            cpu_has_xsaves ? XSTATE_COMPACTION_ENABLED : 0;
+        v->arch.xsave_area->xsave_hdr.xcomp_bv = cpu_has_xsaves
+            ? XSTATE_COMPACTION_ENABLED | XSTATE_FP : 0;
     }
 
     v->arch.vgc_flags = VGCF_online;

Comments

Andrew Cooper Jan. 29, 2016, 3:07 p.m. UTC | #1

On 29/01/16 10:26, Jan Beulich wrote:
> We must not clear the compaction bit when using XSAVES/XRSTORS. And
> we need to guarantee that xcomp_bv never has any bits clear which
> are set in xstate_bv (which requires partly undoing commit 83ae0bb226
> ["x86/xsave: simplify xcomp_bv initialization"]). Split initialization
> of xcomp_bv from the other FPU/SSE/AVX related state setup in
> arch_set_info_guest() and hvm_load_cpu_ctxt().
>
> Reported-by: Harmandeep Kaur <[e-mail]>
> Signed-off-by: Jan Beulich <[e-mail]>

Reviewed-by: Andrew Cooper <[e-mail]>

Patch

--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -923,15 +923,13 @@  int arch_set_info_guest(
     {
         memcpy(v->arch.fpu_ctxt, &c.nat->fpu_ctxt, sizeof(c.nat->fpu_ctxt));
         if ( v->arch.xsave_area )
-        {
             v->arch.xsave_area->xsave_hdr.xstate_bv = XSTATE_FP_SSE;
-            v->arch.xsave_area->xsave_hdr.xcomp_bv =
-                cpu_has_xsaves ? XSTATE_COMPACTION_ENABLED : 0;
-        }
     }
     else if ( v->arch.xsave_area )
-        memset(&v->arch.xsave_area->xsave_hdr, 0,
-               sizeof(v->arch.xsave_area->xsave_hdr));
+    {
+        v->arch.xsave_area->xsave_hdr.xstate_bv = 0;
+        v->arch.xsave_area->fpu_sse.mxcsr = MXCSR_DEFAULT;
+    }
     else
     {
         typeof(v->arch.xsave_area->fpu_sse) *fpu_sse = v->arch.fpu_ctxt;
@@ -940,6 +938,14 @@  int arch_set_info_guest(
         fpu_sse->fcw = FCW_DEFAULT;
         fpu_sse->mxcsr = MXCSR_DEFAULT;
     }
+    if ( cpu_has_xsaves )
+    {
+        ASSERT(v->arch.xsave_area);
+        v->arch.xsave_area->xsave_hdr.xcomp_bv = XSTATE_COMPACTION_ENABLED |
+            v->arch.xsave_area->xsave_hdr.xstate_bv;
+    }
+    else if ( v->arch.xsave_area )
+        v->arch.xsave_area->xsave_hdr.xcomp_bv = 0;
 
     if ( !compat )
     {
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1971,6 +1971,7 @@  static int hvm_load_cpu_ctxt(struct doma
     struct hvm_hw_cpu ctxt;
     struct segment_register seg;
     const char *errstr;
+    struct xsave_struct *xsave_area;
 
     /* Which vcpu is this? */
     vcpuid = hvm_load_instance(h);
@@ -2097,20 +2098,24 @@  static int hvm_load_cpu_ctxt(struct doma
     seg.attr.bytes = ctxt.ldtr_arbytes;
     hvm_set_segment_register(v, x86_seg_ldtr, &seg);
 
+    /* Cover xsave-absent save file restoration on xsave-capable host. */
+    xsave_area = xsave_enabled(v) ? NULL : v->arch.xsave_area;
+
     v->fpu_initialised = !!(ctxt.flags & XEN_X86_FPU_INITIALISED);
     if ( v->fpu_initialised )
     {
         memcpy(v->arch.fpu_ctxt, ctxt.fpu_regs, sizeof(ctxt.fpu_regs));
-        /* In case xsave-absent save file is restored on a xsave-capable host */
-        if ( cpu_has_xsave && !xsave_enabled(v) )
-        {
-            struct xsave_struct *xsave_area = v->arch.xsave_area;
-
+        if ( xsave_area )
             xsave_area->xsave_hdr.xstate_bv = XSTATE_FP_SSE;
-            xsave_area->xsave_hdr.xcomp_bv =
-                cpu_has_xsaves ? XSTATE_COMPACTION_ENABLED : 0;
-        }
     }
+    else if ( xsave_area )
+    {
+        xsave_area->xsave_hdr.xstate_bv = 0;
+        xsave_area->fpu_sse.mxcsr = MXCSR_DEFAULT;
+    }
+    if ( cpu_has_xsaves && xsave_area )
+        xsave_area->xsave_hdr.xcomp_bv = XSTATE_COMPACTION_ENABLED |
+            xsave_area->xsave_hdr.xstate_bv;
 
     v->arch.user_regs.eax = ctxt.rax;
     v->arch.user_regs.ebx = ctxt.rbx;
@@ -5468,8 +5473,8 @@  void hvm_vcpu_reset_state(struct vcpu *v
     if ( v->arch.xsave_area )
     {
         v->arch.xsave_area->xsave_hdr.xstate_bv = XSTATE_FP;
-        v->arch.xsave_area->xsave_hdr.xcomp_bv =
-            cpu_has_xsaves ? XSTATE_COMPACTION_ENABLED : 0;
+        v->arch.xsave_area->xsave_hdr.xcomp_bv = cpu_has_xsaves
+            ? XSTATE_COMPACTION_ENABLED | XSTATE_FP : 0;
     }
 
     v->arch.vgc_flags = VGCF_online;