Re: [Xen-devel] [PATCH v2] x86emul: suppress memory writes after faulting FPU insns

[Andrew Cooper]

On 13/01/17 14:32, Jan Beulich wrote:
> FPU insns writing to memory must not touch memory if they latch #MF (to
> be delivered on the next waiting FPU insn). Note that inspecting FSW.ES
> needs to be avoided for all FNST* insns, as they don't raise exceptions
> themselves, but may instead be invoked with the bit already set.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2] x86emul: suppress memory writes after faulting FPU insns

[Jan Beulich]

FPU insns writing to memory must not touch memory if they latch #MF (to
be delivered on the next waiting FPU insn). Note that inspecting FSW.ES
needs to be avoided for all FNST* insns, as they don't raise exceptions
themselves, but may instead be invoked with the bit already set.

Signed-off-by: Jan Beulich 
---
v2: Add comments (hopefully) clarifying the data size checking.

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -463,6 +463,9 @@ typedef union {
 #define EFLG_MBS  (1<<1)
 #define EFLG_CF   (1<<0)
 
+/* Floating point status word definitions. */
+#define FSW_ES(1U << 7)
+
 /* MXCSR bit definitions. */
 #define MXCSR_MM  (1U << 17)
 
@@ -871,6 +874,15 @@ do {
   (_fic)->exn_raised);  \
 } while (0)
 
+static inline bool fpu_check_write(void)
+{
+uint16_t fsw;
+
+asm ( "fnstsw %0" : "=am" (fsw) );
+
+return !(fsw & FSW_ES);
+}
+
 #define emulate_fpu_insn(_op)   \
 asm volatile (  \
 "movb $2f-1f,%0 \n" \
@@ -3813,6 +3825,13 @@ x86_emulate(
 default:
 generate_exception(EXC_UD);
 }
+/*
+ * Control instructions can't raise FPU exceptions, so we need
+ * to consider suppressing writes only for non-control ones. All
+ * of them in this group have data width 4.
+ */
+if ( dst.type == OP_MEM && dst.bytes == 4 && !fpu_check_write() )
+dst.type = OP_NONE;
 }
 put_fpu();
 break;
@@ -3925,7 +3944,8 @@ x86_emulate(
 case 7: /* fstp m80fp */
 fail_if(!ops->write);
 emulate_fpu_insn_memdst("fstpt", *mmvalp);
-if ( (rc = ops->write(ea.mem.seg, ea.mem.off, mmvalp,
+if ( fpu_check_write() &&
+ (rc = ops->write(ea.mem.seg, ea.mem.off, mmvalp,
   10, ctxt)) != X86EMUL_OKAY )
 goto done;
 dst.type = OP_NONE;
@@ -3933,6 +3953,8 @@ x86_emulate(
 default:
 generate_exception(EXC_UD);
 }
+if ( dst.type == OP_MEM && !fpu_check_write() )
+dst.type = OP_NONE;
 }
 put_fpu();
 break;
@@ -4036,6 +4058,13 @@ x86_emulate(
 default:
 generate_exception(EXC_UD);
 }
+/*
+ * Control instructions can't raise FPU exceptions, so we need
+ * to consider suppressing writes only for non-control ones. All
+ * of them in this group have data width 8.
+ */
+if ( dst.type == OP_MEM && dst.bytes == 8 && !fpu_check_write() )
+dst.type = OP_NONE;
 }
 put_fpu();
 break;
@@ -4153,7 +4182,8 @@ x86_emulate(
 case 6: /* fbstp packed bcd */
 fail_if(!ops->write);
 emulate_fpu_insn_memdst("fbstp", *mmvalp);
-if ( (rc = ops->write(ea.mem.seg, ea.mem.off, mmvalp,
+if ( fpu_check_write() &&
+ (rc = ops->write(ea.mem.seg, ea.mem.off, mmvalp,
   10, ctxt)) != X86EMUL_OKAY )
 goto done;
 dst.type = OP_NONE;
@@ -4163,6 +4193,8 @@ x86_emulate(
 dst.bytes = 8;
 break;
 }
+if ( dst.type == OP_MEM && !fpu_check_write() )
+dst.type = OP_NONE;
 }
 put_fpu();
 break;



x86emul: suppress memory writes after faulting FPU insns

FPU insns writing to memory must not touch memory if they latch #MF (to
be delivered on the next waiting FPU insn). Note that inspecting FSW.ES
needs to be avoided for all FNST* insns, as they don't raise exceptions
themselves, but may instead be invoked with the bit already set.

Signed-off-by: Jan Beulich 
---
v2: Add comments (hopefully) clarifying the data size checking.

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -463,6 +463,9 @@ typedef union {
 #define EFLG_MBS  (1<<1)
 #define EFLG_CF   (1<<0)
 
+/* Floating point status word definitions. */
+#define FSW_ES(1U << 7)
+
 /* MXCSR bit definitions. */
 #define MXCSR_MM  (1U << 17)
 
@@ -871,6 +874,15 @@ do {
   (_fic)->exn_raised);  \
 } while (0)
 
+static inline bool fpu_check_write(void)
+{
+uint16_t fsw;
+
+asm ( "fnstsw %0" : "=am" (fsw) );
+
+return !(fsw & FSW_ES);
+}
+
 #define emulate_fpu_insn(_op)   \
 asm volatile (  \
 "movb $2f-1f,%0 \n" \
@@ -3813,6 +3825,13 @@ x86_emulate(
 default: