x86/fpu: improve check for XSAVE* not writing FIP/FDP fields

The hardware may not write the FIP/FDP fields with a XSAVE*
instruction.  e.g., with XSAVEOPT/XSAVES if the state hasn't changed
or on AMD CPUs when a floating point exception is not pending.  We
need to identify this case so we can correctly apply the check for
whether to save/restore FCS/FDS.

By poisoning FIP in the saved state we can check if the hardware
writes to this field.  The poison value is both: a) non-canonical; and
b) random with a vanishingly small probability of matching a value
written by the hardware (1 / (2^63) = 10^-19).

The poison value is fixed and thus knowable by a guest (or guest
userspace).  This could allow the guest to cause Xen to incorrectly
detect that the field has not been written.  But: a) this requires the
FIP register to be a full 64 bits internally which is not the case for
all current AMD and Intel CPUs; and b) this only allows the guest (or
a guest userspace process) to corrupt its own state (i.e., it cannot
affect the state of another guest or another user space process).

This results in smaller code with fewer branches and is more
understandable.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Intel confirmed that 64-bit {F,}XRSTOR sign-extend FIP from bit 47.
While leaving the description above intact, modify the code comment
accordingly.

Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index a5ed9e5465b8c7e20b7687d2f441f438da24a7fb..f64940538ef62bf783a650d71cc553b276f9aa17 100644 (file)
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -271,40 +271,29 @@ void xsave(struct vcpu *v, uint64_t mask)
     }
     else
     {
-        typeof(ptr->fpu_sse.fip.sel) fcs = ptr->fpu_sse.fip.sel;
-        typeof(ptr->fpu_sse.fdp.sel) fds = ptr->fpu_sse.fdp.sel;
+        /*
+         * FIP/FDP may not be written in some cases (e.g., if XSAVEOPT/XSAVES
+         * is used, or on AMD CPUs if an exception isn't pending).
+         *
+         * To tell if the hardware writes these fields, poison the FIP field.
+         * The poison is
+         * a) non-canonical
+         * b) non-zero for the reserved part of a 32-bit FCS:FIP
+         * c) random with a vanishingly small probability to match a value the
+         *    hardware may write (1e-19) even if it did not canonicalize the
+         *    64-bit FIP or zero-extend the 16-bit FCS.
+         */
+        uint64_t orig_fip = ptr->fpu_sse.fip.addr;
+        const uint64_t bad_fip = 0x6a3f5c4b13a533f6;
 
-        if ( cpu_has_xsaveopt || cpu_has_xsaves )
-        {
-            /*
-             * XSAVEOPT/XSAVES may not write the FPU portion even when the
-             * respective mask bit is set. For the check further down to work
-             * we hence need to put the save image back into the state that
-             * it was in right after the previous XSAVEOPT.
-             */
-            if ( ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] == 4 ||
-                 ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] == 2 )
-            {
-                ptr->fpu_sse.fip.sel = 0;
-                ptr->fpu_sse.fdp.sel = 0;
-            }
-        }
+        ptr->fpu_sse.fip.addr = bad_fip;
 
         XSAVE("0x48,");
 
-        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) )
+        /* FIP/FDP not updated? Restore the old FIP value. */
+        if ( ptr->fpu_sse.fip.addr == bad_fip )
         {
-            if ( cpu_has_xsaveopt || cpu_has_xsaves )
-            {
-                ptr->fpu_sse.fip.sel = fcs;
-                ptr->fpu_sse.fdp.sel = fds;
-            }
+            ptr->fpu_sse.fip.addr = orig_fip;
             return;
         }