x86: preserve FPU selectors for 32-bit guest code

Doing {,F}X{SAVE,RSTOR} unconditionally with 64-bit operand size leads
to the selector values associated with the last instruction/data
pointers getting lost. This, besides being inconsistent and not
compatible with native hardware behavior especially for 32-bit guests,
leads to bug checks in 32-bit Windows when running with "Driver
verifier" (see e.g. http://support.microsoft.com/kb/244617).

In a first try I made the code figure out the current guest mode, but
that has the disadvantage of only taking care of the issue when the
guest executes in the mode for which the state currently is (i.e.
namely not in the case of a 64-bit guest running a 32-bit application,
but being in kernle [64-bit] mode).

The solution presented here is to conditionally execute an auxiliary
FNSTENV and use the selectors from there.

In either case the determined word size gets stored in the last byte
of the FPU/SSE save area, which is available for software use (and I
verified is being cleared to zero by all versions of Xen, i.e. will not
present a problem when migrating guests from older to newer hosts), and
evaluated for determining the operand size {,F}XRSTOR is to be issued
with.

Note that I did check whether using a second FXSAVE or a partial second
XSAVE would be faster than FNSTENV - neither on my Westmere (FXSAVE)
nor on my Romley (XSAVE) they are.

I was really tempted to use branches into the middle of instructions
(i.e. past the REX64 prefixes) here, as that would have allowed to
collapse the otherwise identical fault recovery blocks. I stayed away
from doing so just because I expect others to dislike such slightly
subtle/tricky code.

Reported-by: Ben Guthro <Benjamin.Guthro@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

1 files changed, 81 insertions, 26 deletions

diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index a0d036d6c2..3ca856c6a9 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -10,6 +10,7 @@
 #include <asm/current.h>
 #include <asm/processor.h>
 #include <asm/hvm/support.h>
+#include <asm/i387.h>
 #include <asm/xstate.h>
 #include <asm/asm_defns.h>
 
@@ -56,26 +57,59 @@ 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 = mask & XSTATE_FP ? (cpu_has_fpu_sel ? 8 : 0) : -1;
 
-    if ( cpu_has_xsaveopt )
-        asm volatile (
-            ".byte " REX_PREFIX "0x0f,0xae,0x37"
-            :
-            : "a" (lmask), "d" (hmask), "D"(ptr)
-            : "memory" );
+    if ( word_size <= 0 || !is_pv_32bit_vcpu(v) )
+    {
+        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) );
+
+        if ( !(mask & ptr->xsave_hdr.xstate_bv & XSTATE_FP) ||
+             /*
+              * AMD CPUs don't save/restore FDP/FIP/FOP unless an exception
+              * is pending.
+              */
+             (!(ptr->fpu_sse.fsw & 0x0080) &&
+              boot_cpu_data.x86_vendor == X86_VENDOR_AMD) )
+            return;
+
+        if ( word_size > 0 &&
+             !((ptr->fpu_sse.fip.addr | ptr->fpu_sse.fdp.addr) >> 32) )
+        {
+            struct ix87_env fpu_env;
+
+            asm volatile ( "fnstenv %0" : "=m" (fpu_env) );
+            ptr->fpu_sse.fip.sel = fpu_env.fcs;
+            ptr->fpu_sse.fdp.sel = fpu_env.fds;
+            word_size = 4;
+        }
+    }
     else
-        asm volatile (
-            ".byte " REX_PREFIX "0x0f,0xae,0x27"
-            :
-            : "a" (lmask), "d" (hmask), "D"(ptr)
-            : "memory" );
+    {
+        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) );
+        word_size = 4;
+    }
+    if ( word_size >= 0 )
+        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;
 
     /*
@@ -98,20 +132,41 @@ void xrstor(struct vcpu *v, uint64_t mask)
      * possibility, which may occur if the block was passed to us by control
      * tools or through VCPUOP_initialise, by silently clearing the block.
      */
-    asm volatile ( "1: .byte " REX_PREFIX "0x0f,0xae,0x2f\n"
-                   ".section .fixup,\"ax\"\n"
-                   "2: mov %5,%%ecx       \n"
-                   "   xor %1,%1          \n"
-                   "   rep stosb          \n"
-                   "   lea %2,%0          \n"
-                   "   mov %3,%1          \n"
-                   "   jmp 1b             \n"
-                   ".previous             \n"
-                   _ASM_EXTABLE(1b, 2b)
-                   : "+&D" (ptr), "+&a" (lmask)
-                   : "m" (*ptr), "g" (lmask), "d" (hmask),
-                     "m" (xsave_cntxt_size)
-                   : "ecx" );
+    switch ( __builtin_expect(ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET], 8) )
+    {
+    default:
+        asm volatile ( "1: .byte 0x48,0x0f,0xae,0x2f\n"
+                       ".section .fixup,\"ax\"      \n"
+                       "2: mov %5,%%ecx             \n"
+                       "   xor %1,%1                \n"
+                       "   rep stosb                \n"
+                       "   lea %2,%0                \n"
+                       "   mov %3,%1                \n"
+                       "   jmp 1b                   \n"
+                       ".previous                   \n"
+                       _ASM_EXTABLE(1b, 2b)
+                       : "+&D" (ptr), "+&a" (lmask)
+                       : "m" (*ptr), "g" (lmask), "d" (hmask),
+                         "m" (xsave_cntxt_size)
+                       : "ecx" );
+        break;
+    case 4: case 2:
+        asm volatile ( "1: .byte 0x0f,0xae,0x2f\n"
+                       ".section .fixup,\"ax\" \n"
+                       "2: mov %5,%%ecx        \n"
+                       "   xor %1,%1           \n"
+                       "   rep stosb           \n"
+                       "   lea %2,%0           \n"
+                       "   mov %3,%1           \n"
+                       "   jmp 1b              \n"
+                       ".previous              \n"
+                       _ASM_EXTABLE(1b, 2b)
+                       : "+&D" (ptr), "+&a" (lmask)
+                       : "m" (*ptr), "g" (lmask), "d" (hmask),
+                         "m" (xsave_cntxt_size)
+                       : "ecx" );
+        break;
+    }
 }
 
 bool_t xsave_enabled(const struct vcpu *v)