xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* XSAVE/XRSTOR crash resurgence in 4.3
@ 2013-07-03 14:02 Ben Guthro
  2013-07-04 13:21 ` Jan Beulich
  0 siblings, 1 reply; 38+ messages in thread
From: Ben Guthro @ 2013-07-03 14:02 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Mark Roddy, xen-devel

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

We (XenClient Enterprise) recently updated our mainline development to
xen 4.3, and noticed that the xsave/xrstor bug that crashes 32bit
windows guests with the driver verifier enabled has re-emerged.

>From Mark Roddy:
"The DOMU crash from last nights run  has this signature in the crash dump:

Interrupt Service Routine A30BC91C has changed extended thread context.
Context saved before executing ISR: 841C4380. Context saved after
executing ISR: 841C5040.

It looks like we lost the fix for the XSAVE/XRSTOR"

However, this tree is based on 4.3-rc6, and does include the following commit:

commit 10f969150025498fe27d985f9021a68f8c7acc31
Author: Jan Beulich <jbeulich@suse.com>
Date:   Tue Jun 4 17:23:11 2013 +0200

    x86: preserve FPU selectors for 32-bit guest code



Original patch against 4.2 (that does not fail) is attached.


Ben

[-- Attachment #2: x86-FPU-preserve-selectors.patch --]
[-- Type: application/octet-stream, Size: 8734 bytes --]

diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c
index 0ec2308..6d2ac71 100644
--- a/xen/arch/x86/i387.c
+++ b/xen/arch/x86/i387.c
@@ -55,34 +55,54 @@ static inline void fpu_fxrstor(struct vcpu *v)
      * possibility, which may occur if the block was passed to us by control
      * tools, by silently clearing the block.
      */
-    asm volatile (
-#ifdef __i386__
-        "1: fxrstor %0            \n"
-#else /* __x86_64__ */
-        /* See above for why the operands/constraints are this way. */
-        "1: " REX64_PREFIX "fxrstor (%2)\n"
-#endif
-        ".section .fixup,\"ax\"   \n"
-        "2: push %%"__OP"ax       \n"
-        "   push %%"__OP"cx       \n"
-        "   push %%"__OP"di       \n"
-        "   lea  %0,%%"__OP"di    \n"
-        "   mov  %1,%%ecx         \n"
-        "   xor  %%eax,%%eax      \n"
-        "   rep ; stosl           \n"
-        "   pop  %%"__OP"di       \n"
-        "   pop  %%"__OP"cx       \n"
-        "   pop  %%"__OP"ax       \n"
-        "   jmp  1b               \n"
-        ".previous                \n"
-        _ASM_EXTABLE(1b, 2b)
-        : 
-        : "m" (*fpu_ctxt),
-          "i" (sizeof(v->arch.xsave_area->fpu_sse)/4)
-#ifdef __x86_64__
-          ,"cdaSDb" (fpu_ctxt)
-#endif
-        );
+    switch ( __builtin_expect(fpu_ctxt[FPU_WORD_SIZE_OFFSET], 8) )
+    {
+    default:
+        asm volatile (
+            /* See below for why the operands/constraints are this way. */
+            "1: " REX64_PREFIX "fxrstor (%2)\n"
+            ".section .fixup,\"ax\"   \n"
+            "2: push %%"__OP"ax       \n"
+            "   push %%"__OP"cx       \n"
+            "   push %%"__OP"di       \n"
+            "   mov  %2,%%"__OP"di    \n"
+            "   mov  %1,%%ecx         \n"
+            "   xor  %%eax,%%eax      \n"
+            "   rep ; stosl           \n"
+            "   pop  %%"__OP"di       \n"
+            "   pop  %%"__OP"cx       \n"
+            "   pop  %%"__OP"ax       \n"
+            "   jmp  1b               \n"
+            ".previous                \n"
+            _ASM_EXTABLE(1b, 2b)
+            :
+            : "m" (*fpu_ctxt),
+              "i" (sizeof(v->arch.xsave_area->fpu_sse)/4),
+              "cdaSDb" (fpu_ctxt) );
+        break;
+    case 4: case 2:
+        asm volatile (
+            "1: fxrstor (%2)\n"
+            ".section .fixup,\"ax\"   \n"
+            "2: push %%"__OP"ax       \n"
+            "   push %%"__OP"cx       \n"
+            "   push %%"__OP"di       \n"
+            "   mov  %2,%%"__OP"di    \n"
+            "   mov  %1,%%ecx         \n"
+            "   xor  %%eax,%%eax      \n"
+            "   rep ; stosl           \n"
+            "   pop  %%"__OP"di       \n"
+            "   pop  %%"__OP"cx       \n"
+            "   pop  %%"__OP"ax       \n"
+            "   jmp  1b               \n"
+            ".previous                \n"
+            _ASM_EXTABLE(1b, 2b)
+            :
+            : "m" (*fpu_ctxt),
+              "i" (sizeof(v->arch.xsave_area->fpu_sse)/4),
+              "r" (fpu_ctxt) );
+        break;
+    }
 }
 
 /* Restore x87 extended state */
@@ -111,6 +131,7 @@ static inline void fpu_xsave(struct vcpu *v)
 static inline void fpu_fxsave(struct vcpu *v)
 {
     char *fpu_ctxt = v->arch.fpu_ctxt;
+    int word_size = guest_word_size(v);
 
 #ifdef __i386__
     asm volatile (
@@ -122,9 +143,23 @@ static inline void fpu_fxsave(struct vcpu *v)
      * older versions the rex64 prefix works only if we force an
      * addressing mode that doesn't require extended registers.
      */
-    asm volatile (
-        REX64_PREFIX "fxsave (%1)"
-        : "=m" (*fpu_ctxt) : "cdaSDb" (fpu_ctxt) );
+    switch ( __builtin_expect(word_size, 8) )
+    {
+    default:
+        /*
+         * The only way to force fxsaveq on a wide range of gas versions.
+         * On older versions the rex64 prefix works only if we force an
+         * addressing mode that doesn't require extended registers.
+         */
+        asm volatile ( REX64_PREFIX "fxsave (%1)"
+                       : "=m" (*fpu_ctxt) : "cdaSDb" (fpu_ctxt) );
+        break;
+    case 4: case 2:
+        asm volatile ( "fxsave %0" : "=m" (*fpu_ctxt) );
+        break;
+    }
+    fpu_ctxt[FPU_WORD_SIZE_OFFSET] = word_size;
+
 #endif
     
     /* Clear exception flags if FSW.ES is set. */
@@ -265,6 +300,39 @@ void vcpu_destroy_fpu(struct vcpu *v)
         xfree(v->arch.fpu_ctxt);
 }
 
+int guest_word_size(struct vcpu *v)
+{
+    int mode;
+
+    if ( !is_hvm_vcpu(v) )
+    {
+        if ( is_pv_32bit_vcpu(v) )
+            return 4;
+
+        asm ( "1: lar %1,%0          \n"
+              "   jnz 2f             \n"
+              "3:                    \n"
+              ".section .fixup,\"ax\"\n"
+              "2: xor %0,%0          \n"
+              "   jmp 3b             \n"
+              ".previous             \n"
+              _ASM_EXTABLE(1b, 2b)
+              : "=r" (mode)
+              : "m" (guest_cpu_user_regs()->cs) );
+
+        return !(mode & _SEGMENT_S) || (mode & _SEGMENT_L) ? 8 : 4;
+    }
+
+    switch ( mode = hvm_guest_x86_mode(v) )
+    {
+    case 0: /* real mode */
+    case 1: /* virtual mode */
+        return 2;
+    }
+
+    return mode;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 4d88638..2add85a 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -56,32 +56,53 @@ void xsave(struct vcpu *v, uint64_t mask)
     struct xsave_struct *ptr = v->arch.xsave_area;
     uint32_t hmask = mask >> 32;
     uint32_t lmask = mask;
+    int word_size = guest_word_size(v);
 
-    if ( cpu_has_xsaveopt )
-        asm volatile (
-            ".byte " REX_PREFIX "0x0f,0xae,0x37"
-            :
-            : "a" (lmask), "d" (hmask), "D"(ptr)
-            : "memory" );
-    else
-        asm volatile (
-            ".byte " REX_PREFIX "0x0f,0xae,0x27"
-            :
-            : "a" (lmask), "d" (hmask), "D"(ptr)
-            : "memory" );
+    switch ( __builtin_expect(word_size, 8) )
+    {
+    default:
+        if ( cpu_has_xsaveopt )
+            asm volatile ( ".byte 0x48,0x0f,0xae,0x37"
+                           : "=m" (*ptr)
+                           : "a" (lmask), "d" (hmask), "D" (ptr) );
+        else
+            asm volatile ( ".byte 0x48,0x0f,0xae,0x27"
+                           : "=m" (*ptr)
+                           : "a" (lmask), "d" (hmask), "D" (ptr) );
+        break;
+    case 4: case 2:
+        if ( cpu_has_xsaveopt )
+            asm volatile ( ".byte 0x0f,0xae,0x37"
+                           : "=m" (*ptr)
+                           : "a" (lmask), "d" (hmask), "D" (ptr) );
+        else
+            asm volatile ( ".byte 0x0f,0xae,0x27"
+                           : "=m" (*ptr)
+                           : "a" (lmask), "d" (hmask), "D" (ptr) );
+        break;
+    }
+    ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] = word_size;
 }
 
 void xrstor(struct vcpu *v, uint64_t mask)
 {
     uint32_t hmask = mask >> 32;
     uint32_t lmask = mask;
-
     struct xsave_struct *ptr = v->arch.xsave_area;
 
-    asm volatile (
-        ".byte " REX_PREFIX "0x0f,0xae,0x2f"
-        :
-        : "m" (*ptr), "a" (lmask), "d" (hmask), "D"(ptr) );
+    switch ( __builtin_expect(ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET], 8) )
+    {
+    default:
+        asm volatile ( ".byte 0x48,0x0f,0xae,0x2f"
+                       :
+                       : "m" (*ptr), "a" (lmask), "d" (hmask), "D" (ptr) );
+        break;
+    case 4: case 2:
+        asm volatile ( ".byte 0x0f,0xae,0x2f"
+                       :
+                       : "m" (*ptr), "a" (lmask), "d" (hmask), "D" (ptr) );
+        break;
+    }
 }
 
 bool_t xsave_enabled(const struct vcpu *v)
diff --git a/xen/include/asm-x86/xstate.h b/xen/include/asm-x86/xstate.h
index 90e405e..d5ac499 100644
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -34,12 +34,6 @@
 #define XSTATE_NONLAZY (XSTATE_LWP)
 #define XSTATE_LAZY    (XSTATE_ALL & ~XSTATE_NONLAZY)
 
-#ifdef CONFIG_X86_64
-#define REX_PREFIX     "0x48, "
-#else
-#define REX_PREFIX
-#endif
-
 /* extended state variables */
 DECLARE_PER_CPU(uint64_t, xcr0);
 
@@ -94,4 +88,14 @@ void xstate_free_save_area(struct vcpu *v);
 int xstate_alloc_save_area(struct vcpu *v);
 void xstate_init(void);
 
+/* Byte offset within the FXSAVE (portion) of the stored word size. */
+#define FPU_WORD_SIZE_OFFSET 511
+
+/*
+ * Used EXCLUSIVELY to determine the needed operand size override on
+ * XSAVE/FXSAVE. Any other use would need to make sure that the context
+ * is suitable for all operations this involves.
+ */
+int guest_word_size(struct vcpu *);
+
 #endif /* __ASM_XSTATE_H */

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

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

^ permalink raw reply related	[flat|nested] 38+ messages in thread
end of thread, other threads:[~2013-08-05 16:03 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-03 14:02 XSAVE/XRSTOR crash resurgence in 4.3 Ben Guthro
2013-07-04 13:21 ` Jan Beulich
2013-07-04 13:24   ` Ben Guthro
2013-07-04 18:19     ` Mark Roddy
2013-07-05  6:42       ` Jan Beulich
2013-07-05 10:30       ` Jan Beulich
2013-07-05 12:10         ` Ben Guthro
2013-07-05 12:15           ` Jan Beulich
2013-07-05 12:58             ` Ben Guthro
2013-07-08 14:13               ` Ben Guthro
2013-07-08 14:24                 ` Jan Beulich
2013-07-08 14:31                   ` Ben Guthro
2013-07-08 14:40                     ` Jan Beulich
2013-07-08 14:42                       ` Ben Guthro
2013-07-08 14:47                         ` Jan Beulich
2013-07-08 15:10                           ` Ben Guthro
2013-07-12 13:11                             ` Ben Guthro
2013-07-12 13:38                               ` Jan Beulich
2013-07-12 13:49                                 ` Ben Guthro
2013-07-12 14:34                                   ` Jan Beulich
2013-07-12 14:49                                     ` Ben Guthro
2013-07-12 14:55                                       ` Jan Beulich
2013-07-12 15:14                                         ` Ben Guthro
2013-07-15  6:41                                           ` Jan Beulich
2013-07-15 12:33                                           ` Jan Beulich
2013-07-15 12:43                                             ` Ben Guthro
2013-07-15 13:49                                               ` Ben Guthro
2013-07-15 14:06                                                 ` Jan Beulich
2013-07-16 16:23                                                 ` Jan Beulich
2013-07-16 16:57                                                   ` Ben Guthro
2013-07-17  6:38                                                     ` Jan Beulich
2013-07-17 13:07                                                       ` Ben Guthro
2013-07-22 12:25                                                         ` Ben Guthro
2013-08-05 13:05                                                           ` [PATCH] x86: refine FPU selector handling code for XSAVEOPT Jan Beulich
2013-08-05 16:03                                                             ` Keir Fraser
2013-07-08 14:44                       ` XSAVE/XRSTOR crash resurgence in 4.3 Andrew Cooper
2013-07-08 14:52                         ` Jan Beulich
2013-07-08 14:55                           ` Andrew Cooper

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).