[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [V3] x86/xsaves: calculate the xstate_comp_offsets base on xcomp_bv



Previous patch using all available features calculate xstate_comp_offsets.
This is wrong.This patch fix this bug by calculating the xstate_comp_offset
based on xcomp_bv of current guest.
Also, the xstate_comp_offset should take alignment into consideration.

Signed-off-by: Shuai Ruan <shuai.ruan@xxxxxxxxxxxxxxx>
Reported-by: Jan Beulich <jbeulich@xxxxxxxx>
---
V3: Address comments from Jan:
1. fix xstate_comp_offsets used as static array problem.
2. change xstate_align from array to u64 and used as bitmap.
3. change calculating xstate_comp_offsets into three step.
        1) whether component is set in xsavearea
        2) whether component need align
        3) add xstate_size[i-1]

V2: Address comments from Jan:
1. code style fix.
2. setup_xstate_comp take xcomp_bv as param.

 xen/arch/x86/domctl.c        | 21 +++++++++++---
 xen/arch/x86/xstate.c        | 66 +++++++++++++++++++++++++++++++-------------
 xen/include/asm-x86/xstate.h |  6 ++--
 3 files changed, 68 insertions(+), 25 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 55aecdc..3f891b4 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -934,8 +934,14 @@ long arch_do_domctl(
                     goto vcpuextstate_out;
                 }
 
-                expand_xsave_states(v, xsave_area,
-                                    size - 2 * sizeof(uint64_t));
+                ret = expand_xsave_states(v, xsave_area,
+                                          size - 2 * sizeof(uint64_t));
+                if ( ret )
+                {
+                    xfree(xsave_area);
+                    vcpu_unpause(v);
+                    goto vcpuextstate_out;
+                }
 
                 if ( copy_to_guest_offset(evc->buffer, offset, xsave_area,
                                           size - 2 * sizeof(uint64_t)) )
@@ -1000,8 +1006,15 @@ long arch_do_domctl(
                 v->arch.xcr0_accum = _xcr0_accum;
                 if ( _xcr0_accum & XSTATE_NONLAZY )
                     v->arch.nonlazy_xstate_used = 1;
-                compress_xsave_states(v, _xsave_area,
-                                      evc->size - 2 * sizeof(uint64_t));
+                ret = compress_xsave_states(v, _xsave_area,
+                                            evc->size - 2 * sizeof(uint64_t));
+                if ( ret )
+                {
+                    xfree(receive_buf);
+                    vcpu_unpause(v);
+                    goto vcpuextstate_out;
+                }
+
                 vcpu_unpause(v);
             }
             else
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 8316bd9..d25f045 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -26,8 +26,8 @@ u64 __read_mostly xfeature_mask;
 
 static unsigned int *__read_mostly xstate_offsets;
 unsigned int *__read_mostly xstate_sizes;
+static u64 __read_mostly xstate_align;
 static unsigned int __read_mostly xstate_features;
-static unsigned int __read_mostly xstate_comp_offsets[sizeof(xfeature_mask)*8];
 
 static uint32_t __read_mostly mxcsr_mask = 0x0000ffbf;
 
@@ -94,7 +94,7 @@ static bool_t xsave_area_compressed(const struct xsave_struct 
*xsave_area)
 
 static int setup_xstate_features(bool_t bsp)
 {
-    unsigned int leaf, tmp, eax, ebx;
+    unsigned int leaf, eax, ebx, ecx, edx;
 
     if ( bsp )
     {
@@ -111,21 +111,27 @@ static int setup_xstate_features(bool_t bsp)
     for ( leaf = 2; leaf < xstate_features; leaf++ )
     {
         if ( bsp )
+        {
             cpuid_count(XSTATE_CPUID, leaf, &xstate_sizes[leaf],
-                        &xstate_offsets[leaf], &tmp, &tmp);
+                        &xstate_offsets[leaf], &ecx, &edx);
+            if ( ecx & XSTATE_ALIGN64 )
+                set_bit(leaf, &xstate_align);
+        }
         else
         {
             cpuid_count(XSTATE_CPUID, leaf, &eax,
-                        &ebx, &tmp, &tmp);
+                        &ebx, &ecx, &edx);
             BUG_ON(eax != xstate_sizes[leaf]);
             BUG_ON(ebx != xstate_offsets[leaf]);
+            BUG_ON((ecx & XSTATE_ALIGN64) != test_bit(leaf, &xstate_align));
         }
     }
 
     return 0;
 }
 
-static void __init setup_xstate_comp(void)
+static void setup_xstate_comp(unsigned int *xstate_comp_offsets,
+                              const u64 xcomp_bv)
 {
     unsigned int i;
 
@@ -134,22 +140,26 @@ static void __init setup_xstate_comp(void)
      * in the fixed offsets in the xsave area in either compacted form
      * or standard form.
      */
-    xstate_comp_offsets[0] = 0;
     xstate_comp_offsets[1] = XSAVE_SSE_OFFSET;
 
     xstate_comp_offsets[2] = FXSAVE_SIZE + XSAVE_HDR_SIZE;
 
     for ( i = 3; i < xstate_features; i++ )
     {
-        xstate_comp_offsets[i] = xstate_comp_offsets[i - 1] +
-                                 (((1ul << i) & xfeature_mask)
-                                  ? xstate_sizes[i - 1] : 0);
-        ASSERT(xstate_comp_offsets[i] + xstate_sizes[i] <= xsave_cntxt_size);
+        if ( (1ul << i) & xcomp_bv )
+        {
+            xstate_comp_offsets[i] = test_bit(i, &xstate_align) ?
+                                     ROUNDUP(xstate_comp_offsets[i - 1], 64) :
+                                     xstate_comp_offsets[i -1];
+            xstate_comp_offsets[i] += xstate_sizes[i - 1];
+            ASSERT(xstate_comp_offsets[i] + xstate_sizes[i] <= 
xsave_cntxt_size);
+        }
     }
 }
 
 static void *get_xsave_addr(struct xsave_struct *xsave,
-        unsigned int xfeature_idx)
+                            unsigned int *xstate_comp_offsets,
+                            unsigned int xfeature_idx)
 {
     if ( !((1ul << xfeature_idx) & xsave->xsave_hdr.xstate_bv) )
         return NULL;
@@ -159,19 +169,26 @@ static void *get_xsave_addr(struct xsave_struct *xsave,
             : xstate_offsets)[xfeature_idx];
 }
 
-void expand_xsave_states(struct vcpu *v, void *dest, unsigned int size)
+int expand_xsave_states(struct vcpu *v, void *dest, unsigned int size)
 {
     struct xsave_struct *xsave = v->arch.xsave_area;
+    unsigned int *xstate_comp_offsets;
     u64 xstate_bv = xsave->xsave_hdr.xstate_bv;
     u64 valid;
 
     if ( !cpu_has_xsaves && !cpu_has_xsavec )
     {
         memcpy(dest, xsave, size);
-        return;
+        return 0;
     }
 
+    xstate_comp_offsets = xzalloc_array(unsigned int, xstate_features);
+    if ( !xstate_comp_offsets )
+        return -ENOMEM;
+
     ASSERT(xsave_area_compressed(xsave));
+    setup_xstate_comp(xstate_comp_offsets, xsave->xsave_hdr.xcomp_bv);
+
     /*
      * Copy legacy XSAVE area and XSAVE hdr area.
      */
@@ -188,7 +205,7 @@ void expand_xsave_states(struct vcpu *v, void *dest, 
unsigned int size)
     {
         u64 feature = valid & -valid;
         unsigned int index = fls(feature) - 1;
-        const void *src = get_xsave_addr(xsave, index);
+        const void *src = get_xsave_addr(xsave, xstate_comp_offsets, index);
 
         if ( src )
         {
@@ -198,20 +215,28 @@ void expand_xsave_states(struct vcpu *v, void *dest, 
unsigned int size)
 
         valid &= ~feature;
     }
+
+    xfree(xstate_comp_offsets);
+    return 0;
 }
 
-void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
+int compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
 {
     struct xsave_struct *xsave = v->arch.xsave_area;
+    unsigned int *xstate_comp_offsets;
     u64 xstate_bv = ((const struct xsave_struct *)src)->xsave_hdr.xstate_bv;
     u64 valid;
 
     if ( !cpu_has_xsaves && !cpu_has_xsavec )
     {
         memcpy(xsave, src, size);
-        return;
+        return 0;
     }
 
+    xstate_comp_offsets = xzalloc_array(unsigned int, xstate_features);
+    if ( !xstate_comp_offsets )
+        return -ENOMEM;
+
     ASSERT(!xsave_area_compressed(src));
     /*
      * Copy legacy XSAVE area, to avoid complications with CPUID
@@ -223,6 +248,8 @@ void compress_xsave_states(struct vcpu *v, const void *src, 
unsigned int size)
     xsave->xsave_hdr.xstate_bv = xstate_bv;
     xsave->xsave_hdr.xcomp_bv = v->arch.xcr0_accum | XSTATE_COMPACTION_ENABLED;
 
+    setup_xstate_comp(xstate_comp_offsets, xsave->xsave_hdr.xcomp_bv);
+
     /*
      * Copy each region from the non-compacted offset to the
      * possibly compacted offset.
@@ -232,7 +259,7 @@ void compress_xsave_states(struct vcpu *v, const void *src, 
unsigned int size)
     {
         u64 feature = valid & -valid;
         unsigned int index = fls(feature) - 1;
-        void *dest = get_xsave_addr(xsave, index);
+        void *dest = get_xsave_addr(xsave, xstate_comp_offsets, index);
 
         if ( dest )
         {
@@ -242,6 +269,9 @@ void compress_xsave_states(struct vcpu *v, const void *src, 
unsigned int size)
 
         valid &= ~feature;
     }
+
+    xfree(xstate_comp_offsets);
+    return 0;
 }
 
 void xsave(struct vcpu *v, uint64_t mask)
@@ -575,8 +605,6 @@ void xstate_init(struct cpuinfo_x86 *c)
 
     if ( setup_xstate_features(bsp) && bsp )
         BUG();
-    if ( bsp && (cpu_has_xsaves || cpu_has_xsavec) )
-        setup_xstate_comp();
 }
 
 static bool_t valid_xcr0(u64 xcr0)
diff --git a/xen/include/asm-x86/xstate.h b/xen/include/asm-x86/xstate.h
index c28cea5..782f9bf 100644
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -46,6 +46,8 @@
 #define XSTATE_LAZY    (XSTATE_ALL & ~XSTATE_NONLAZY)
 #define XSTATE_COMPACTION_ENABLED  (1ULL << 63)
 
+#define XSTATE_ALIGN64 (1U << 1)
+
 extern u64 xfeature_mask;
 extern unsigned int *xstate_sizes;
 
@@ -94,8 +96,8 @@ bool_t xsave_enabled(const struct vcpu *v);
 int __must_check validate_xstate(u64 xcr0, u64 xcr0_accum,
                                  const struct xsave_hdr *);
 int __must_check handle_xsetbv(u32 index, u64 new_bv);
-void expand_xsave_states(struct vcpu *v, void *dest, unsigned int size);
-void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size);
+int expand_xsave_states(struct vcpu *v, void *dest, unsigned int size);
+int compress_xsave_states(struct vcpu *v, const void *src, unsigned int size);
 
 /* extended state init and cleanup functions */
 void xstate_free_save_area(struct vcpu *v);
-- 
1.9.1


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.