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

[Shuai Ruan <shuai.ruan@xxxxxxxxxxxxxxx>]

From: Shuai Ruan <shuai.ruan@xxxxxxxxx>

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@xxxxxxxxx>
---
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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel