[PATCH V8 0/2] xsaves bug fix

[PATCH V8 0/2] xsaves bug fix

[Shuai Ruan]

From: Shuai Ruan <shuai.ruan@intel.com>

This patchset fix xsaves related bugs.

Shuai Ruan (2):
  x86/xsaves: fix two miscellaneous issues
  x86/xsaves: ebx may return wrong value using CPUID eax=0xd,ecx =1

 xen/arch/x86/hvm/hvm.c       | 12 ++++++++----
 xen/arch/x86/xstate.c        | 13 ++++++-------
 xen/include/asm-x86/xstate.h |  1 +
 3 files changed, 15 insertions(+), 11 deletions(-)

-- 
1.9.1


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

[PATCH V8 1/2] x86/xsaves: fix two miscellaneous issues

[Shuai Ruan]

From: Shuai Ruan <shuai.ruan@intel.com>

1. get_xsave_addr() will only be called when
xsave_area_compressed(xsave) is true. So drop the
conditional expression.

2. expand_xsave_states() will memset the area when
get NULL from get_xsave_addr().

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Shuai Ruan <shuai.ruan@intel.com>
---
v2: Address comments from Jan:
1. Add assert in get_xsave_addr.

 xen/arch/x86/xstate.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 8c652bc..047ac74 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -164,12 +164,9 @@ static void *get_xsave_addr(struct xsave_struct *xsave,
                             const uint16_t *comp_offsets,
                             unsigned int xfeature_idx)
 {
-    if ( !((1ul << xfeature_idx) & xsave->xsave_hdr.xstate_bv) )
-        return NULL;
-
-    return (void *)xsave + (xsave_area_compressed(xsave) ?
-                            comp_offsets[xfeature_idx] :
-                            xstate_offsets[xfeature_idx]);
+    ASSERT(xsave_area_compressed(xsave));
+    return (1ul << xfeature_idx) & xsave->xsave_hdr.xstate_bv ?
+           (void *)xsave + comp_offsets[xfeature_idx] : NULL;
 }
 
 void expand_xsave_states(struct vcpu *v, void *dest, unsigned int size)
@@ -211,6 +208,8 @@ void expand_xsave_states(struct vcpu *v, void *dest, unsigned int size)
             ASSERT((xstate_offsets[index] + xstate_sizes[index]) <= size);
             memcpy(dest + xstate_offsets[index], src, xstate_sizes[index]);
         }
+        else
+            memset(dest + xstate_offsets[index], 0, xstate_sizes[index]);
 
         valid &= ~feature;
     }
-- 
1.9.1


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

[PATCH V8 2/2] x86/xsaves: ebx may return wrong value using CPUID eax=0xd, ecx =1

[Shuai Ruan]

From: Shuai Ruan <shuai.ruan@intel.com>

Refer to SDM Volume 1 Extended Region of an XSAVE Area. The value returned
by ecx[1] with cpuid function 0xd and sub-function i (i>1) indicates
the alignment of the state component i when the compacted format of the
extended region of an xsave area is used.

So when hvm guest using CPUID eax=0xd, ecx=1 to get the size of area
used for compacted format, we need to take alignment into consideration.

tools side is fixed by
"tools/libxc: Calculate xstate cpuid leaf from guest information"
by Andrew Cooper

Signed-off-by: Shuai Ruan <shuai.ruan@intel.com>
---
v3: Address comments from Jan:
1. fix some code error.
drop pv_cpuid related code.

v2: Address comments from Jan:
1. take alignment into consideration in pv_cpuid.
2. fix coding style issues
 xen/arch/x86/hvm/hvm.c       | 12 ++++++++----
 xen/arch/x86/xstate.c        |  2 +-
 xen/include/asm-x86/xstate.h |  1 +
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index b239f74..8313cf9 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3432,14 +3432,18 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
         }
         if ( count == 1 )
         {
-            if ( cpu_has_xsaves && cpu_has_vmx_xsaves )
+            uint64_t xfeatures = v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss;
+            if ( cpu_has_xsaves && cpu_has_vmx_xsaves && xfeatures )
             {
                 *ebx = XSTATE_AREA_MIN_SIZE;
-                if ( v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss )
+                if ( xfeatures & ~XSTATE_FP_SSE )
                     for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ )
-                        if ( (v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss) &
-                             (1ULL << sub_leaf) )
+                        if ( xfeatures & (1ULL << sub_leaf) )
+                        {
+                            if ( test_bit(sub_leaf, &xstate_align) )
+                                *ebx = ROUNDUP(*ebx, 64);
                             *ebx += xstate_sizes[sub_leaf];
+                        }
             }
             else
                 *ebx = *ecx = *edx = 0;
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 047ac74..48c0195 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -26,7 +26,7 @@ u64 __read_mostly xfeature_mask;
 
 static unsigned int *__read_mostly xstate_offsets;
 unsigned int *__read_mostly xstate_sizes;
-static u64 __read_mostly xstate_align;
+u64 __read_mostly xstate_align;
 static unsigned int __read_mostly xstate_features;
 
 static uint32_t __read_mostly mxcsr_mask = 0x0000ffbf;
diff --git a/xen/include/asm-x86/xstate.h b/xen/include/asm-x86/xstate.h
index 91d1c39..535443a 100644
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -50,6 +50,7 @@
 #define XSTATE_ALIGN64 (1U << 1)
 
 extern u64 xfeature_mask;
+extern u64 xstate_align;
 extern unsigned int *xstate_sizes;
 
 /* extended state save area */
-- 
1.9.1


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