[Xen-devel] [PATCH 5/6] x86/AMD: Fix handling of FPU pointer on Zen hardware

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

AMD hardware before Zen doesn't safe/restore the FPU error pointers
unless an unmasked FPU exception is pending.  Zen processors have a
feature bit indicating that this (mis)behaviour no longer exists.

Express the common logic in terms of cpu_bug_fpu_err_ptr as Hygon
processors (being Zen derivatives) won't inherit this behaviour.

While at it, fix a performance issue with the workaround, which I should
have noticed by now.  Looking at the FPU state slows the context switch
path down, as it is a moderately complicated unpredictable condition
which will evaluate to true for all 64bit OSes and most 32bit ones.

Leave the sole condition being the easily-predictable
cpu_bug_fpu_err_ptr as the asm sequence is needed in ~100% of cases
these days on affected hardware.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Pu Wen <puwen@xxxxxxxx>
---
 xen/arch/x86/cpu/amd.c                      | 7 +++++++
 xen/arch/x86/xstate.c                       | 6 ++----
 xen/include/asm-x86/cpufeature.h            | 1 +
 xen/include/asm-x86/cpufeatures.h           | 1 +
 xen/include/public/arch-x86/cpufeatureset.h | 1 +
 5 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 21c82bb..8c4521c 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -713,6 +713,13 @@ static void init_amd(struct cpuinfo_x86 *c)
        setup_force_cpu_cap(X86_BUG_NULL_SEG);
 
        /*
+        * Older AMD CPUs don't save/load FOP/FIP/FDP unless an FPU exception
+        * is pending.
+        */
+       if ( !cpu_has(c, X86_FEATURE_XSAVEERRPTR) )
+               setup_force_cpu_cap(X86_BUG_FPU_PTR_LEAK);
+
+       /*
         * Attempt to set lfence to be Dispatch Serialising.  This MSR almost
         * certainly isn't virtualised (and Xen at least will leak the real
         * value in but silently discard writes), as well as being per-core
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 15edd5d..7ca5684 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -369,15 +369,13 @@ void xrstor(struct vcpu *v, uint64_t mask)
     unsigned int faults, prev_faults;
 
     /*
-     * AMD CPUs don't save/restore FDP/FIP/FOP unless an exception
+     * Some CPUs don't save/restore FDP/FIP/FOP unless an exception
      * is pending. Clear the x87 state here by setting it to fixed
      * values. The hypervisor data segment can be sometimes 0 and
      * sometimes new user value. Both should be ok. Use the FPU saved
      * data block as a safe address because it should be in L1.
      */
-    if ( (mask & ptr->xsave_hdr.xstate_bv & X86_XCR0_FP) &&
-         !(ptr->fpu_sse.fsw & ~ptr->fpu_sse.fcw & 0x003f) &&
-         boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
+    if ( cpu_bug_fpu_err_ptr )
         asm volatile ( "fnclex\n\t"        /* clear exceptions */
                        "ffree %%st(7)\n\t" /* clear stack tag */
                        "fildl %0"          /* load to clear state */
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index de45b6c..0f3bb5a 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -125,6 +125,7 @@
 /* Bugs. */
 #define cpu_bug_amd_erratum_121 boot_cpu_has(X86_BUG_AMD_ERRATUM_121)
 #define cpu_bug_null_seg        boot_cpu_has(X86_BUG_NULL_SEG)
+#define cpu_bug_fpu_err_ptr     boot_cpu_has(X86_BUG_FPU_PTR_LEAK)
 
 enum _cache_type {
     CACHE_TYPE_NULL = 0,
diff --git a/xen/include/asm-x86/cpufeatures.h 
b/xen/include/asm-x86/cpufeatures.h
index 8a73a09..e7d4171 100644
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -40,6 +40,7 @@ XEN_CPUFEATURE(XEN_LBR,           X86_SYNTH(22)) /* Xen uses 
MSR_DEBUGCTL.LBR */
 
 #define X86_BUG_AMD_ERRATUM_121   X86_BUG( 0) /* Hang on fetch across 
non-canonical boundary. */
 #define X86_BUG_NULL_SEG          X86_BUG( 1) /* NULL-ing a selector preserves 
the base and limit. */
+#define X86_BUG_FPU_PTR_LEAK      X86_BUG( 2) /* (F)XRSTOR doesn't load 
FOP/FIP/FDP. */
 
 /* Total number of capability words, inc synth and bug words. */
 #define NCAPINTS (FSCAPINTS + X86_NR_SYNTH + X86_NR_BUG) /* N 32-bit words 
worth of info */
diff --git a/xen/include/public/arch-x86/cpufeatureset.h 
b/xen/include/public/arch-x86/cpufeatureset.h
index fbc68fa..6beaab9 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -237,6 +237,7 @@ XEN_CPUFEATURE(EFRO,          7*32+10) /*   APERF/MPERF 
Read Only interface */
 
 /* AMD-defined CPU features, CPUID level 0x80000008.ebx, word 8 */
 XEN_CPUFEATURE(CLZERO,        8*32+ 0) /*A  CLZERO instruction */
+XEN_CPUFEATURE(XSAVEERRPTR,   8*32+ 2) /*A  (F)XSAVE Error pointers always 
updated. */
 XEN_CPUFEATURE(IBPB,          8*32+12) /*A  IBPB support only (no IBRS, used 
by AMD) */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0.edx, word 9 */
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel