[PATCH 1/2] x86/math-emu: Add support for FCMOVcc and F[U]COMI[P] insns

[PATCH 1/2] x86/math-emu: Add support for FCMOVcc and F[U]COMI[P] insns

[Denys Vlasenko]

Run-tested by booting with "no387 nofxsr" and running test programs:

# ./test_FCMOV
[RUN]	Testing fcmovCC instructions
[OK]	fcmovCC
# ./test_FCOMI
[RUN]	Testing f[u]comi[p] instructions
[OK]	f[u]comi[p]

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---

The patches did not change since last submission.
The difference, now they are run-tested.

 arch/x86/math-emu/fpu_aux.c     |  70 ++++++++++++++++++++++
 arch/x86/math-emu/fpu_entry.c   |  49 +++++++++------
 arch/x86/math-emu/fpu_proto.h   |  12 ++++
 arch/x86/math-emu/reg_compare.c | 128 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 241 insertions(+), 18 deletions(-)

diff --git a/arch/x86/math-emu/fpu_aux.c b/arch/x86/math-emu/fpu_aux.c
index dd76a05..024f6e9 100644
--- a/arch/x86/math-emu/fpu_aux.c
+++ b/arch/x86/math-emu/fpu_aux.c
@@ -169,6 +169,76 @@ void fxch_i(void)
 	fpu_tag_word = tag_word;
 }
 
+static void fcmovCC(void)
+{
+	/* fcmovCC st(i) */
+	int i = FPU_rm;
+	FPU_REG *st0_ptr = &st(0);
+	FPU_REG *sti_ptr = &st(i);
+	long tag_word = fpu_tag_word;
+	int regnr = top & 7;
+	int regnri = (top + i) & 7;
+	u_char sti_tag = (tag_word >> (regnri * 2)) & 3;
+
+	if (sti_tag == TAG_Empty) {
+		FPU_stack_underflow();
+		clear_C1();
+		return;
+	}
+	reg_copy(sti_ptr, st0_ptr);
+	tag_word &= ~(3 << (regnr * 2));
+	tag_word |= (sti_tag << (regnr * 2));
+	fpu_tag_word = tag_word;
+}
+
+void fcmovb(void)
+{
+	if (FPU_EFLAGS & X86_EFLAGS_CF)
+		fcmovCC();
+}
+
+void fcmove(void)
+{
+	if (FPU_EFLAGS & X86_EFLAGS_ZF)
+		fcmovCC();
+}
+
+void fcmovbe(void)
+{
+	if (FPU_EFLAGS & (X86_EFLAGS_CF|X86_EFLAGS_ZF))
+		fcmovCC();
+}
+
+void fcmovu(void)
+{
+	if (FPU_EFLAGS & X86_EFLAGS_PF)
+		fcmovCC();
+}
+
+void fcmovnb(void)
+{
+	if (!(FPU_EFLAGS & X86_EFLAGS_CF))
+		fcmovCC();
+}
+
+void fcmovne(void)
+{
+	if (!(FPU_EFLAGS & X86_EFLAGS_ZF))
+		fcmovCC();
+}
+
+void fcmovnbe(void)
+{
+	if (!(FPU_EFLAGS & (X86_EFLAGS_CF|X86_EFLAGS_ZF)))
+		fcmovCC();
+}
+
+void fcmovnu(void)
+{
+	if (!(FPU_EFLAGS & X86_EFLAGS_PF))
+		fcmovCC();
+}
+
 void ffree_(void)
 {
 	/* ffree st(i) */
diff --git a/arch/x86/math-emu/fpu_entry.c b/arch/x86/math-emu/fpu_entry.c
index 929b1d5..77c6505 100644
--- a/arch/x86/math-emu/fpu_entry.c
+++ b/arch/x86/math-emu/fpu_entry.c
@@ -40,10 +40,10 @@
 
 #define __BAD__ FPU_illegal	/* Illegal on an 80486, causes SIGILL */
 
-/* WARNING: These codes are not documented by Intel in their 80486 manual
+/* WARNING: These codes are not all documented by Intel in their 80486 manual
    and may not work on FPU clones or later Intel FPUs. */
 
-/* Changes to support the un-doc codes provided by Linus Torvalds. */
+/* Changes to support the un-documented instructions provided by Linus Torvalds. */
 
 #define _d9_d8_ fstp_i		/* unofficial code (19) */
 #define _dc_d0_ fcom_st		/* unofficial code (14) */
@@ -55,15 +55,27 @@
 #define _df_d0_ fstp_i		/* unofficial code (17) */
 #define _df_d8_ fstp_i		/* unofficial code (1f) */
 
+/* fcmovCC and f(u)comi(p) are enabled if CPUID(1).EDX(15) "cmov" is set */
+
 static FUNC const st_instr_table[64] = {
-	fadd__, fld_i_, __BAD__, __BAD__, fadd_i, ffree_, faddp_, _df_c0_,
-	fmul__, fxch_i, __BAD__, __BAD__, fmul_i, _dd_c8_, fmulp_, _df_c8_,
-	fcom_st, fp_nop, __BAD__, __BAD__, _dc_d0_, fst_i_, _de_d0_, _df_d0_,
-	fcompst, _d9_d8_, __BAD__, __BAD__, _dc_d8_, fstp_i, fcompp, _df_d8_,
-	fsub__, FPU_etc, __BAD__, finit_, fsubri, fucom_, fsubrp, fstsw_,
-	fsubr_, fconst, fucompp, __BAD__, fsub_i, fucomp, fsubp_, __BAD__,
-	fdiv__, FPU_triga, __BAD__, __BAD__, fdivri, __BAD__, fdivrp, __BAD__,
-	fdivr_, FPU_trigb, __BAD__, __BAD__, fdiv_i, __BAD__, fdivp_, __BAD__,
+/* Opcode:	d8		d9		da		db */
+/*		dc		dd		de		df */
+/* c0..7 */	fadd__,		fld_i_,		fcmovb,		fcmovnb,
+/* c0..7 */	fadd_i,		ffree_,		faddp_,		_df_c0_,
+/* c8..f */	fmul__,		fxch_i,		fcmove,		fcmovne,
+/* c8..f */	fmul_i,		_dd_c8_,	fmulp_,		_df_c8_,
+/* d0..7 */	fcom_st,	fp_nop,		fcmovbe,	fcmovnbe,
+/* d0..7 */	_dc_d0_,	fst_i_,		_de_d0_,	_df_d0_,
+/* d8..f */	fcompst,	_d9_d8_,	fcmovu,		fcmovnu,
+/* d8..f */	_dc_d8_,	fstp_i,		fcompp,		_df_d8_,
+/* e0..7 */	fsub__,		FPU_etc,	__BAD__,	finit_,
+/* e0..7 */	fsubri,		fucom_,		fsubrp,		fstsw_,
+/* e8..f */	fsubr_,		fconst,		fucompp,	fucomi_,
+/* e8..f */	fsub_i,		fucomp,		fsubp_,		fucomip,
+/* f0..7 */	fdiv__,		FPU_triga,	__BAD__,	fcomi_,
+/* f0..7 */	fdivri,		__BAD__,	fdivrp,		fcomip,
+/* f8..f */	fdivr_,		FPU_trigb,	__BAD__,	__BAD__,
+/* f8..f */	fdiv_i,		__BAD__,	fdivp_,		__BAD__,
 };
 
 #define _NONE_ 0		/* Take no special action */
@@ -78,14 +90,15 @@ static FUNC const st_instr_table[64] = {
 #define _REGIn 0		/* Uses st(0) and st(rm), but handle checks later */
 
 static u_char const type_table[64] = {
-	_REGI_, _NONE_, _null_, _null_, _REGIi, _REGi_, _REGIp, _REGi_,
-	_REGI_, _REGIn, _null_, _null_, _REGIi, _REGI_, _REGIp, _REGI_,
-	_REGIc, _NONE_, _null_, _null_, _REGIc, _REG0_, _REGIc, _REG0_,
-	_REGIc, _REG0_, _null_, _null_, _REGIc, _REG0_, _REGIc, _REG0_,
-	_REGI_, _NONE_, _null_, _NONE_, _REGIi, _REGIc, _REGIp, _NONE_,
-	_REGI_, _NONE_, _REGIc, _null_, _REGIi, _REGIc, _REGIp, _null_,
-	_REGI_, _NONE_, _null_, _null_, _REGIi, _null_, _REGIp, _null_,
-	_REGI_, _NONE_, _null_, _null_, _REGIi, _null_, _REGIp, _null_
+/* Opcode:	d8	d9	da	db	dc	dd	de	df */
+/* c0..7 */	_REGI_, _NONE_, _REGIn, _REGIn, _REGIi, _REGi_, _REGIp, _REGi_,
+/* c8..f */	_REGI_, _REGIn, _REGIn, _REGIn, _REGIi, _REGI_, _REGIp, _REGI_,
+/* d0..7 */	_REGIc, _NONE_, _REGIn, _REGIn, _REGIc, _REG0_, _REGIc, _REG0_,
+/* d8..f */	_REGIc, _REG0_, _REGIn, _REGIn, _REGIc, _REG0_, _REGIc, _REG0_,
+/* e0..7 */	_REGI_, _NONE_, _null_, _NONE_, _REGIi, _REGIc, _REGIp, _NONE_,
+/* e8..f */	_REGI_, _NONE_, _REGIc, _REGIc, _REGIi, _REGIc, _REGIp, _REGIc,
+/* f0..7 */	_REGI_, _NONE_, _null_, _REGIc, _REGIi, _null_, _REGIp, _REGIc,
+/* f8..f */	_REGI_, _NONE_, _null_, _null_, _REGIi, _null_, _REGIp, _null_,
 };
 
 #ifdef RE_ENTRANT_CHECKING
diff --git a/arch/x86/math-emu/fpu_proto.h b/arch/x86/math-emu/fpu_proto.h
index 9779df4..caff438 100644
--- a/arch/x86/math-emu/fpu_proto.h
+++ b/arch/x86/math-emu/fpu_proto.h
@@ -46,6 +46,14 @@ extern void fstsw_(void);
 extern void fp_nop(void);
 extern void fld_i_(void);
 extern void fxch_i(void);
+extern void fcmovb(void);
+extern void fcmove(void);
+extern void fcmovbe(void);
+extern void fcmovu(void);
+extern void fcmovnb(void);
+extern void fcmovne(void);
+extern void fcmovnbe(void);
+extern void fcmovnu(void);
 extern void ffree_(void);
 extern void ffreep(void);
 extern void fst_i_(void);
@@ -108,6 +116,10 @@ extern void fcompp(void);
 extern void fucom_(void);
 extern void fucomp(void);
 extern void fucompp(void);
+extern void fcomi_(void);
+extern void fcomip(void);
+extern void fucomi_(void);
+extern void fucomip(void);
 /* reg_constant.c */
 extern void fconst(void);
 /* reg_ld_str.c */
diff --git a/arch/x86/math-emu/reg_compare.c b/arch/x86/math-emu/reg_compare.c
index ecce55f..b77360f 100644
--- a/arch/x86/math-emu/reg_compare.c
+++ b/arch/x86/math-emu/reg_compare.c
@@ -249,6 +249,54 @@ static int compare_st_st(int nr)
 	return 0;
 }
 
+static int compare_i_st_st(int nr)
+{
+	int f, c;
+	FPU_REG *st_ptr;
+
+	if (!NOT_EMPTY(0) || !NOT_EMPTY(nr)) {
+		FPU_EFLAGS |= (X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF);
+		/* Stack fault */
+		EXCEPTION(EX_StackUnder);
+		return !(control_word & CW_Invalid);
+	}
+
+	partial_status &= ~SW_C0;
+	st_ptr = &st(nr);
+	c = compare(st_ptr, FPU_gettagi(nr));
+	if (c & COMP_NaN) {
+		FPU_EFLAGS |= (X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF);
+		EXCEPTION(EX_Invalid);
+		return !(control_word & CW_Invalid);
+	}
+
+	switch (c & 7) {
+	case COMP_A_lt_B:
+		f = X86_EFLAGS_CF;
+		break;
+	case COMP_A_eq_B:
+		f = X86_EFLAGS_ZF;
+		break;
+	case COMP_A_gt_B:
+		f = 0;
+		break;
+	case COMP_No_Comp:
+		f = X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF;
+		break;
+#ifdef PARANOID
+	default:
+		EXCEPTION(EX_INTERNAL | 0x122);
+		f = 0;
+		break;
+#endif /* PARANOID */
+	}
+	FPU_EFLAGS = (FPU_EFLAGS & ~(X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF)) | f;
+	if (c & COMP_Denormal) {
+		return denormal_operand() < 0;
+	}
+	return 0;
+}
+
 static int compare_u_st_st(int nr)
 {
 	int f = 0, c;
@@ -299,6 +347,58 @@ static int compare_u_st_st(int nr)
 	return 0;
 }
 
+static int compare_ui_st_st(int nr)
+{
+	int f = 0, c;
+	FPU_REG *st_ptr;
+
+	if (!NOT_EMPTY(0) || !NOT_EMPTY(nr)) {
+		FPU_EFLAGS |= (X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF);
+		/* Stack fault */
+		EXCEPTION(EX_StackUnder);
+		return !(control_word & CW_Invalid);
+	}
+
+	partial_status &= ~SW_C0;
+	st_ptr = &st(nr);
+	c = compare(st_ptr, FPU_gettagi(nr));
+	if (c & COMP_NaN) {
+		FPU_EFLAGS |= (X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF);
+		if (c & COMP_SNaN) {	/* This is the only difference between
+					   un-ordered and ordinary comparisons */
+			EXCEPTION(EX_Invalid);
+			return !(control_word & CW_Invalid);
+		}
+		return 0;
+	}
+
+	switch (c & 7) {
+	case COMP_A_lt_B:
+		f = X86_EFLAGS_CF;
+		break;
+	case COMP_A_eq_B:
+		f = X86_EFLAGS_ZF;
+		break;
+	case COMP_A_gt_B:
+		f = 0;
+		break;
+	case COMP_No_Comp:
+		f = X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF;
+		break;
+#ifdef PARANOID
+	default:
+		EXCEPTION(EX_INTERNAL | 0x123);
+		f = 0;
+		break;
+#endif /* PARANOID */
+	}
+	FPU_EFLAGS = (FPU_EFLAGS & ~(X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF)) | f;
+	if (c & COMP_Denormal) {
+		return denormal_operand() < 0;
+	}
+	return 0;
+}
+
 /*---------------------------------------------------------------------------*/
 
 void fcom_st(void)
@@ -348,3 +448,31 @@ void fucompp(void)
 	} else
 		FPU_illegal();
 }
+
+/* P6+ compare-to-EFLAGS ops */
+
+void fcomi_(void)
+{
+	/* fcomi st(i) */
+	compare_i_st_st(FPU_rm);
+}
+
+void fcomip(void)
+{
+	/* fcomip st(i) */
+	if (!compare_i_st_st(FPU_rm))
+		FPU_pop();
+}
+
+void fucomi_(void)
+{
+	/* fucomi st(i) */
+	compare_ui_st_st(FPU_rm);
+}
+
+void fucomip(void)
+{
+	/* fucomip st(i) */
+	if (!compare_ui_st_st(FPU_rm))
+		FPU_pop();
+}
-- 
1.8.1.4

[PATCH 2/2] x86/math-emu: Remove define layer for undocumented opcodes

[Denys Vlasenko]

No code changes.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/math-emu/fpu_entry.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/arch/x86/math-emu/fpu_entry.c b/arch/x86/math-emu/fpu_entry.c
index 77c6505..0e86006 100644
--- a/arch/x86/math-emu/fpu_entry.c
+++ b/arch/x86/math-emu/fpu_entry.c
@@ -40,34 +40,23 @@
 
 #define __BAD__ FPU_illegal	/* Illegal on an 80486, causes SIGILL */
 
-/* WARNING: These codes are not all documented by Intel in their 80486 manual
-   and may not work on FPU clones or later Intel FPUs. */
-
-/* Changes to support the un-documented instructions provided by Linus Torvalds. */
-
-#define _d9_d8_ fstp_i		/* unofficial code (19) */
-#define _dc_d0_ fcom_st		/* unofficial code (14) */
-#define _dc_d8_ fcompst		/* unofficial code (1c) */
-#define _dd_c8_ fxch_i		/* unofficial code (0d) */
-#define _de_d0_ fcompst		/* unofficial code (16) */
-#define _df_c0_ ffreep		/* unofficial code (07) ffree + pop */
-#define _df_c8_ fxch_i		/* unofficial code (0f) */
-#define _df_d0_ fstp_i		/* unofficial code (17) */
-#define _df_d8_ fstp_i		/* unofficial code (1f) */
-
 /* fcmovCC and f(u)comi(p) are enabled if CPUID(1).EDX(15) "cmov" is set */
 
+/* WARNING: "u" entries are not documented by Intel in their 80486 manual
+   and may not work on FPU clones or later Intel FPUs.
+   Changes to support the un-documented instructions provided by Linus Torvalds. */
+
 static FUNC const st_instr_table[64] = {
 /* Opcode:	d8		d9		da		db */
 /*		dc		dd		de		df */
 /* c0..7 */	fadd__,		fld_i_,		fcmovb,		fcmovnb,
-/* c0..7 */	fadd_i,		ffree_,		faddp_,		_df_c0_,
+/* c0..7 */	fadd_i,		ffree_,		faddp_,		ffreep,/*u*/
 /* c8..f */	fmul__,		fxch_i,		fcmove,		fcmovne,
-/* c8..f */	fmul_i,		_dd_c8_,	fmulp_,		_df_c8_,
+/* c8..f */	fmul_i,		fxch_i,/*u*/	fmulp_,		fxch_i,/*u*/
 /* d0..7 */	fcom_st,	fp_nop,		fcmovbe,	fcmovnbe,
-/* d0..7 */	_dc_d0_,	fst_i_,		_de_d0_,	_df_d0_,
-/* d8..f */	fcompst,	_d9_d8_,	fcmovu,		fcmovnu,
-/* d8..f */	_dc_d8_,	fstp_i,		fcompp,		_df_d8_,
+/* d0..7 */	fcom_st,/*u*/	fst_i_,		fcompst,/*u*/	fstp_i,/*u*/
+/* d8..f */	fcompst,	fstp_i,/*u*/	fcmovu,		fcmovnu,
+/* d8..f */	fcompst,/*u*/	fstp_i,		fcompp,		fstp_i,/*u*/
 /* e0..7 */	fsub__,		FPU_etc,	__BAD__,	finit_,
 /* e0..7 */	fsubri,		fucom_,		fsubrp,		fstsw_,
 /* e8..f */	fsubr_,		fconst,		fucompp,	fucomi_,
-- 
1.8.1.4

Re: [PATCH 1/2] x86/math-emu: Add support for FCMOVcc and F[U]COMI[P] insns

[Denys Vlasenko]

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

On 09/16/2015 09:48 PM, Denys Vlasenko wrote:
> Run-tested by booting with "no387 nofxsr" and running test programs:
> 
> # ./test_FCMOV
> [RUN]	Testing fcmovCC instructions
> [OK]	fcmovCC
> # ./test_FCOMI
> [RUN]	Testing f[u]comi[p] instructions
> [OK]	f[u]comi[p]

The sources for these test programs are in this email.

Ingo, let me know if you want a patch which adds them
to tools/testing/selftests/x86/*


[-- Attachment #2: test_FCMOV.c --]
[-- Type: text/x-csrc, Size: 2101 bytes --]

#undef _GNU_SOURCE
#define _GNU_SOURCE 1
#undef __USE_GNU
#define __USE_GNU 1
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
#include <stdio.h>
#include <signal.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/select.h>
#include <sys/time.h>
//#include <sys/ptrace.h>
#include <sys/wait.h>

#define TEST(insn) \
long double __attribute__((noinline)) insn(long flags) \
{						\
	long double out;			\
	asm ("\n"				\
	"	push	%1""\n"			\
	"	popf""\n"			\
	"	fldpi""\n"			\
	"	fld1""\n"			\
 	"	" #insn " %%st(1), %%st" "\n"	\
 	"	ffree	%%st(1)" "\n"		\
	: "=t" (out)				\
	: "r" (flags)				\
	);					\
	return out;				\
}

TEST(fcmovb)
TEST(fcmove)
TEST(fcmovbe)
TEST(fcmovu)
TEST(fcmovnb)
TEST(fcmovne)
TEST(fcmovnbe)
TEST(fcmovnu)

enum {
	CF = 1 << 0,
	PF = 1 << 2,
	ZF = 1 << 6,
};

int main(int argc, char **argv, char **envp)
{
	int err = 0;

	printf("[RUN]\tTesting fcmovCC instructions\n");
	/* If fcmovCC() returns 1.0, the move wasn't done */
	err |= !(fcmovb(0)   == 1.0); err |= !(fcmovnb(0)  != 1.0);
	err |= !(fcmove(0)   == 1.0); err |= !(fcmovne(0)  != 1.0);
	err |= !(fcmovbe(0)  == 1.0); err |= !(fcmovnbe(0) != 1.0);
	err |= !(fcmovu(0)   == 1.0); err |= !(fcmovnu(0)  != 1.0);

	err |= !(fcmovb(CF)  != 1.0); err |= !(fcmovnb(CF)  == 1.0);
	err |= !(fcmove(CF)  == 1.0); err |= !(fcmovne(CF)  != 1.0);
	err |= !(fcmovbe(CF) != 1.0); err |= !(fcmovnbe(CF) == 1.0);
	err |= !(fcmovu(CF)  == 1.0); err |= !(fcmovnu(CF)  != 1.0);

	err |= !(fcmovb(ZF)  == 1.0); err |= !(fcmovnb(ZF)  != 1.0);
	err |= !(fcmove(ZF)  != 1.0); err |= !(fcmovne(ZF)  == 1.0);
	err |= !(fcmovbe(ZF) != 1.0); err |= !(fcmovnbe(ZF) == 1.0);
	err |= !(fcmovu(ZF)  == 1.0); err |= !(fcmovnu(ZF)  != 1.0);

	err |= !(fcmovb(PF)  == 1.0); err |= !(fcmovnb(PF)  != 1.0);
	err |= !(fcmove(PF)  == 1.0); err |= !(fcmovne(PF)  != 1.0);
	err |= !(fcmovbe(PF) == 1.0); err |= !(fcmovnbe(PF) != 1.0);
	err |= !(fcmovu(PF)  != 1.0); err |= !(fcmovnu(PF)  == 1.0);

        if (!err)
                printf("[OK]\tfcmovCC\n");
	else
		printf("[FAIL]\tfcmovCC errors: %d\n", err);

	return err;
}

[-- Attachment #3: test_FCOMI.c --]
[-- Type: text/x-csrc, Size: 7121 bytes --]

#undef _GNU_SOURCE
#define _GNU_SOURCE 1
#undef __USE_GNU
#define __USE_GNU 1
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
#include <stdio.h>
#include <signal.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/select.h>
#include <sys/time.h>
//#include <sys/ptrace.h>
#include <sys/wait.h>
#include <fenv.h>

enum {
	CF = 1 << 0,
	PF = 1 << 2,
	ZF = 1 << 6,
	ARITH = CF | PF | ZF,
};

long res_fcomi_pi_1;
long res_fcomi_1_pi;
long res_fcomi_1_1;
long res_fcomi_nan_1;
/* sNaN is s|111 1111 1|1xx xxxx xxxx xxxx xxxx xxxx */
/* qNaN is s|111 1111 1|0xx xxxx xxxx xxxx xxxx xxxx (some x must be nonzero) */
int snan = 0x7fc11111;
int qnan = 0x7f811111;
unsigned short snan1[5];
/* sNaN80 is s|111 1111 1111 1111 |10xx xx...xx (some x must be nonzero) */
unsigned short snan80[5] = { 0x1111, 0x1111, 0x1111, 0x8111, 0x7fff };

int test(long flags)
{
	feclearexcept(FE_DIVBYZERO|FE_INEXACT|FE_INVALID|FE_OVERFLOW|FE_UNDERFLOW);

	asm ("\n"

	"	push	%0""\n"
	"	popf""\n"
	"	fld1""\n"
	"	fldpi""\n"
 	"	fcomi	%%st(1), %%st" "\n"
 	"	ffree	%%st(0)" "\n"
 	"	ffree	%%st(1)" "\n"
	"	pushf""\n"
	"	pop	res_fcomi_1_pi""\n"

	"	push	%0""\n"
	"	popf""\n"
	"	fldpi""\n"
	"	fld1""\n"
 	"	fcomi	%%st(1), %%st" "\n"
 	"	ffree	%%st(0)" "\n"
 	"	ffree	%%st(1)" "\n"
	"	pushf""\n"
	"	pop	res_fcomi_pi_1""\n"

	"	push	%0""\n"
	"	popf""\n"
	"	fld1""\n"
	"	fld1""\n"
 	"	fcomi	%%st(1), %%st" "\n"
 	"	ffree	%%st(0)" "\n"
 	"	ffree	%%st(1)" "\n"
	"	pushf""\n"
	"	pop	res_fcomi_1_1""\n"
	:
	: "r" (flags)
	);
	if ((res_fcomi_1_pi & ARITH) != (0)) {
		printf("[BAD]\tfcomi_1_pi with flags:%lx\n", flags);
		return 1;
	}
	if ((res_fcomi_pi_1 & ARITH) != (CF)) {
		printf("[BAD]\tfcomi_pi_1 with flags:%lx->%lx\n", flags, res_fcomi_pi_1 & ARITH);
		return 1;
	}
	if ((res_fcomi_1_1 & ARITH) != (ZF)) {
		printf("[BAD]\tfcomi_1_1 with flags:%lx\n", flags);
		return 1;
	}
	if (fetestexcept(FE_INVALID) != 0) {
		printf("[BAD]\tFE_INVALID is set in %s\n", __func__);
		return 1;
	}
	return 0;
}

int test_qnan(long flags)
{
	feclearexcept(FE_DIVBYZERO|FE_INEXACT|FE_INVALID|FE_OVERFLOW|FE_UNDERFLOW);

	asm ("\n"
	"	push	%0""\n"
	"	popf""\n"
	"	flds	qnan""\n"
	"	fld1""\n"
	"	fnclex""\n"		// fld of a qnan raised FE_INVALID, clear it
 	"	fcomi	%%st(1), %%st" "\n"
 	"	ffree	%%st(0)" "\n"
 	"	ffree	%%st(1)" "\n"
	"	pushf""\n"
	"	pop	res_fcomi_nan_1""\n"
	:
	: "r" (flags)
	);
	if ((res_fcomi_nan_1 & ARITH) != (ZF|CF|PF)) {
		printf("[BAD]\tfcomi_qnan_1 with flags:%lx\n", flags);
		return 1;
	}
	if (fetestexcept(FE_INVALID) != FE_INVALID) {
		printf("[BAD]\tFE_INVALID is not set in %s\n", __func__);
		return 1;
	}
	return 0;
}

int testu_qnan(long flags)
{
	feclearexcept(FE_DIVBYZERO|FE_INEXACT|FE_INVALID|FE_OVERFLOW|FE_UNDERFLOW);

	asm ("\n"
	"	push	%0""\n"
	"	popf""\n"
	"	flds	qnan""\n"
	"	fld1""\n"
	"	fnclex""\n"		// fld of a qnan raised FE_INVALID, clear it
 	"	fucomi	%%st(1), %%st" "\n"
 	"	ffree	%%st(0)" "\n"
 	"	ffree	%%st(1)" "\n"
	"	pushf""\n"
	"	pop	res_fcomi_nan_1""\n"
	:
	: "r" (flags)
	);
	if ((res_fcomi_nan_1 & ARITH) != (ZF|CF|PF)) {
		printf("[BAD]\tfcomi_qnan_1 with flags:%lx\n", flags);
		return 1;
	}
	if (fetestexcept(FE_INVALID) != 0) {
		printf("[BAD]\tFE_INVALID is set in %s\n", __func__);
		return 1;
	}
	return 0;
}

int testu_snan(long flags)
{
	feclearexcept(FE_DIVBYZERO|FE_INEXACT|FE_INVALID|FE_OVERFLOW|FE_UNDERFLOW);

	asm ("\n"
	"	push	%0""\n"
	"	popf""\n"
//	"	flds	snan""\n"	// WRONG, this will convert 32-bit fp snan to a *qnan* in 80-bit fp register!
//	"	fstpt	snan1""\n"	// if uncommented, it prints "snan1:7fff c111 1100 0000 0000" - c111, not 8111!
//	"	fnclex""\n"		// flds of a snan raised FE_INVALID, clear it
	"	fldt	snan80""\n"	// fldt never raise FE_INVALID
	"	fld1""\n"
 	"	fucomi	%%st(1), %%st" "\n"
 	"	ffree	%%st(0)" "\n"
 	"	ffree	%%st(1)" "\n"
	"	pushf""\n"
	"	pop	res_fcomi_nan_1""\n"
	:
	: "r" (flags)
	);
	if ((res_fcomi_nan_1 & ARITH) != (ZF|CF|PF)) {
		printf("[BAD]\tfcomi_qnan_1 with flags:%lx\n", flags);
		return 1;
	}
//	printf("snan:%x snan1:%04x %04x %04x %04x %04x\n", snan, snan1[4], snan1[3], snan1[2], snan1[1], snan1[0]);
	if (fetestexcept(FE_INVALID) != FE_INVALID) {
		printf("[BAD]\tFE_INVALID is not set in %s\n", __func__);
		return 1;
	}
	return 0;
}

int testp(long flags)
{
	feclearexcept(FE_DIVBYZERO|FE_INEXACT|FE_INVALID|FE_OVERFLOW|FE_UNDERFLOW);

	asm ("\n"

	"	push	%0""\n"
	"	popf""\n"
	"	fld1""\n"
	"	fldpi""\n"
 	"	fcomip	%%st(1), %%st" "\n"
 	"	ffree	%%st(0)" "\n"
	"	pushf""\n"
	"	pop	res_fcomi_1_pi""\n"

	"	push	%0""\n"
	"	popf""\n"
	"	fldpi""\n"
	"	fld1""\n"
 	"	fcomip	%%st(1), %%st" "\n"
 	"	ffree	%%st(0)" "\n"
	"	pushf""\n"
	"	pop	res_fcomi_pi_1""\n"

	"	push	%0""\n"
	"	popf""\n"
	"	fld1""\n"
	"	fld1""\n"
 	"	fcomip	%%st(1), %%st" "\n"
 	"	ffree	%%st(0)" "\n"
	"	pushf""\n"
	"	pop	res_fcomi_1_1""\n"
	:
	: "r" (flags)
	);
	if ((res_fcomi_1_pi & ARITH) != (0)) {
		printf("[BAD]\tfcomi_1_pi with flags:%lx\n", flags);
		return 1;
	}
	if ((res_fcomi_pi_1 & ARITH) != (CF)) {
		printf("[BAD]\tfcomi_pi_1 with flags:%lx->%lx\n", flags, res_fcomi_pi_1 & ARITH);
		return 1;
	}
	if ((res_fcomi_1_1 & ARITH) != (ZF)) {
		printf("[BAD]\tfcomi_1_1 with flags:%lx\n", flags);
		return 1;
	}
	if (fetestexcept(FE_INVALID) != 0) {
		printf("[BAD]\tFE_INVALID is set in %s\n", __func__);
		return 1;
	}
	return 0;
}

int testp_qnan(long flags)
{
	feclearexcept(FE_DIVBYZERO|FE_INEXACT|FE_INVALID|FE_OVERFLOW|FE_UNDERFLOW);

	asm ("\n"
	"	push	%0""\n"
	"	popf""\n"
	"	flds	qnan""\n"
	"	fld1""\n"
	"	fnclex""\n"		// fld of a qnan raised FE_INVALID, clear it
 	"	fcomip	%%st(1), %%st" "\n"
 	"	ffree	%%st(0)" "\n"
	"	pushf""\n"
	"	pop	res_fcomi_nan_1""\n"
	:
	: "r" (flags)
	);
	if ((res_fcomi_nan_1 & ARITH) != (ZF|CF|PF)) {
		printf("[BAD]\tfcomi_qnan_1 with flags:%lx\n", flags);
		return 1;
	}
	if (fetestexcept(FE_INVALID) != FE_INVALID) {
		printf("[BAD]\tFE_INVALID is not set in %s\n", __func__);
		return 1;
	}
	return 0;
}

int testup_qnan(long flags)
{
	feclearexcept(FE_DIVBYZERO|FE_INEXACT|FE_INVALID|FE_OVERFLOW|FE_UNDERFLOW);

	asm ("\n"
	"	push	%0""\n"
	"	popf""\n"
	"	flds	qnan""\n"
	"	fld1""\n"
	"	fnclex""\n"		// fld of a qnan raised FE_INVALID, clear it
 	"	fucomip	%%st(1), %%st" "\n"
 	"	ffree	%%st(0)" "\n"
	"	pushf""\n"
	"	pop	res_fcomi_nan_1""\n"
	:
	: "r" (flags)
	);
	if ((res_fcomi_nan_1 & ARITH) != (ZF|CF|PF)) {
		printf("[BAD]\tfcomi_qnan_1 with flags:%lx\n", flags);
		return 1;
	}
	if (fetestexcept(FE_INVALID) != 0) {
		printf("[BAD]\tFE_INVALID is set in %s\n", __func__);
		return 1;
	}
	return 0;
}

int main(int argc, char **argv, char **envp)
{
	int err = 0;

        printf("[RUN]\tTesting f[u]comi[p] instructions\n");
	err |= test(0);
	err |= test_qnan(0);
	err |= testu_qnan(0);
	err |= testu_snan(0);
	err |= test(CF|ZF|PF);
	err |= test_qnan(CF|ZF|PF);
	err |= testu_qnan(CF|ZF|PF);
	err |= testu_snan(CF|ZF|PF);
	err |= testp(0);
	err |= testp_qnan(0);
	err |= testup_qnan(0);
	err |= testp(CF|ZF|PF);
	err |= testp_qnan(CF|ZF|PF);
	err |= testup_qnan(CF|ZF|PF);
	if (!err)
		printf("[OK]\tf[u]comi[p]\n");
	else
		printf("[FAIL]\tf[u]comi[p] errors: %d\n", err);

	return err;
}

Re: [PATCH 1/2] x86/math-emu: Add support for FCMOVcc and F[U]COMI[P] insns

[Ingo Molnar]

* Denys Vlasenko <dvlasenk@redhat.com> wrote:

> On 09/16/2015 09:48 PM, Denys Vlasenko wrote:
> > Run-tested by booting with "no387 nofxsr" and running test programs:
> > 
> > # ./test_FCMOV
> > [RUN]	Testing fcmovCC instructions
> > [OK]	fcmovCC
> > # ./test_FCOMI
> > [RUN]	Testing f[u]comi[p] instructions
> > [OK]	f[u]comi[p]
> 
> The sources for these test programs are in this email.
> 
> Ingo, let me know if you want a patch which adds them
> to tools/testing/selftests/x86/*

Absolutely, that would be very nice!

Thanks,

	Ingo

Re: [PATCH 1/2] x86/math-emu: Add support for FCMOVcc and F[U]COMI[P] insns

[Ingo Molnar]

* Denys Vlasenko <dvlasenk@redhat.com> wrote:

> Run-tested by booting with "no387 nofxsr" and running test programs:
> 
> # ./test_FCMOV
> [RUN]	Testing fcmovCC instructions
> [OK]	fcmovCC
> # ./test_FCOMI
> [RUN]	Testing f[u]comi[p] instructions
> [OK]	f[u]comi[p]
> 
> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> CC: Ingo Molnar <mingo@kernel.org>
> CC: Borislav Petkov <bp@alien8.de>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Kees Cook <keescook@chromium.org>
> CC: x86@kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
> 
> The patches did not change since last submission.
> The difference, now they are run-tested.
> 
>  arch/x86/math-emu/fpu_aux.c     |  70 ++++++++++++++++++++++
>  arch/x86/math-emu/fpu_entry.c   |  49 +++++++++------
>  arch/x86/math-emu/fpu_proto.h   |  12 ++++
>  arch/x86/math-emu/reg_compare.c | 128 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 241 insertions(+), 18 deletions(-)

I wanted to apply this patch, but noticed the following problem: why are two 
instruction families added in a single patch?

Note that regressing math-emu for real is a real concern here: we start 
interpreting real instructions. If anything breaks, and the failure isn't right at 
the instruction that is buggy (but some time later), it would be nice which new 
instruction causes the problem and make it all bisectable ...

So we should be finegrained and only add a single (family) of instructions in a 
single patch. Please split this patch into two:

> +extern void fcmovb(void);
> +extern void fcmove(void);
> +extern void fcmovbe(void);
> +extern void fcmovu(void);
> +extern void fcmovnb(void);
> +extern void fcmovne(void);
> +extern void fcmovnbe(void);
> +extern void fcmovnu(void);
>  extern void ffree_(void);
>  extern void ffreep(void);
>  extern void fst_i_(void);
> @@ -108,6 +116,10 @@ extern void fcompp(void);
>  extern void fucom_(void);
>  extern void fucomp(void);
>  extern void fucompp(void);
> +extern void fcomi_(void);
> +extern void fcomip(void);
> +extern void fucomi_(void);
> +extern void fucomip(void);

Thanks,

	Ingo

Re: [PATCH 2/2] x86/math-emu: Remove define layer for undocumented opcodes

[Ingo Molnar]

* Denys Vlasenko <dvlasenk@redhat.com> wrote:

> No code changes.
> 
> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> CC: Ingo Molnar <mingo@kernel.org>
> CC: Borislav Petkov <bp@alien8.de>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Kees Cook <keescook@chromium.org>
> CC: x86@kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
>  arch/x86/math-emu/fpu_entry.c | 29 +++++++++--------------------
>  1 file changed, 9 insertions(+), 20 deletions(-)

This patch should come before riskier patches introducing new instructions, to 
make eventual reverts and fixes easier.

The rule is generally: sort patches by risk, low risk first, high risk later. This 
means that cleanups, restructuring, refactoring patches always come first - beyond 
making bisection and reverts easier, those make subsequent patches cleaner as well 
and make any bugs in subsequent patches easier to see as well.

Thanks,

	Ingo

Re: [PATCH 1/2] x86/math-emu: Add support for FCMOVcc and F[U]COMI[P] insns

[Denys Vlasenko]

On 09/18/2015 09:33 AM, Ingo Molnar wrote:
> 
> * Denys Vlasenko <dvlasenk@redhat.com> wrote:
> 
>> Run-tested by booting with "no387 nofxsr" and running test programs:
>>
>> # ./test_FCMOV
>> [RUN]	Testing fcmovCC instructions
>> [OK]	fcmovCC
>> # ./test_FCOMI
>> [RUN]	Testing f[u]comi[p] instructions
>> [OK]	f[u]comi[p]
>>
>> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
>> CC: Ingo Molnar <mingo@kernel.org>
>> CC: Borislav Petkov <bp@alien8.de>
>> CC: "H. Peter Anvin" <hpa@zytor.com>
>> CC: Andy Lutomirski <luto@amacapital.net>
>> CC: Kees Cook <keescook@chromium.org>
>> CC: x86@kernel.org
>> CC: linux-kernel@vger.kernel.org
>> ---
>>
>> The patches did not change since last submission.
>> The difference, now they are run-tested.
>>
>>  arch/x86/math-emu/fpu_aux.c     |  70 ++++++++++++++++++++++
>>  arch/x86/math-emu/fpu_entry.c   |  49 +++++++++------
>>  arch/x86/math-emu/fpu_proto.h   |  12 ++++
>>  arch/x86/math-emu/reg_compare.c | 128 ++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 241 insertions(+), 18 deletions(-)
> 
> I wanted to apply this patch, but noticed the following problem: why are two 
> instruction families added in a single patch?

They were introduced at the same time in the CPU evolution
(when CMOV feature was added).

The idea is that there may be programs which assume that "cmov"
in /proc/cpuinfo means both these insns are safe to use,
which is true for all real CPUs.

> Note that regressing math-emu for real is a real concern here: we start 
> interpreting real instructions. If anything breaks, and the failure isn't right at 
> the instruction that is buggy (but some time later), it would be nice which new 
> instruction causes the problem and make it all bisectable ...
> 
> So we should be finegrained and only add a single (family) of instructions in a 
> single patch. Please split this patch into two

I'm sending a new patchset.

Re: [PATCH 1/2] x86/math-emu: Add support for FCMOVcc and F[U]COMI[P] insns

[Ingo Molnar]

* Denys Vlasenko <dvlasenk@redhat.com> wrote:

> >>  arch/x86/math-emu/fpu_aux.c     |  70 ++++++++++++++++++++++
> >>  arch/x86/math-emu/fpu_entry.c   |  49 +++++++++------
> >>  arch/x86/math-emu/fpu_proto.h   |  12 ++++
> >>  arch/x86/math-emu/reg_compare.c | 128 ++++++++++++++++++++++++++++++++++++++++
> >>  4 files changed, 241 insertions(+), 18 deletions(-)
> > 
> > I wanted to apply this patch, but noticed the following problem: why are two 
> > instruction families added in a single patch?
> 
> They were introduced at the same time in the CPU evolution
> (when CMOV feature was added).
> 
> The idea is that there may be programs which assume that "cmov"
> in /proc/cpuinfo means both these insns are safe to use,
> which is true for all real CPUs.

I see, that makes sense - and this makes bisection of any bugs in the emulator 
implementation harder.

Still, it cannot hurt I suppose.

Btw., has anyone explored the possibility to have an emulator 'runtime test' on 
FPU-capable CPUs: to run in FPU emulation and to run the emulation code, but to 
also run the real hardware instruction(s) against that context and double check 
that the two FPU register sets match up, bit for bit?

If the emulator is perfect then the two would always match: the hardware 
instruction generates the exact same result as software emulation.

This would slows down emulation a bit (but emulation is slow anyway due to trap 
overhead), but makes it a lot more obvious that the emulator is correct.

It's not a trivial feature though I suspect, exceptions would have to be taken 
care of, etc.

The motivation would be to make it much easier to extend the emulator, which would 
be useful to run modern Linux distros on future low-power x86 designs that have no 
FPU circuitry altogether.

Thanks,

	Ingo