Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1226425 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/2] x86/math-emu: Add support for FCMOVcc and F[U]COMI[P] insns Denys Vlasenko <dvlasenk@redhat.com> - 2015-09-16 21:50 +0200
    [PATCH 2/2] x86/math-emu: Remove define layer for undocumented opcodes Denys Vlasenko <dvlasenk@redhat.com> - 2015-09-16 21:50 +0200
      Re: [PATCH 2/2] x86/math-emu: Remove define layer for undocumented  opcodes Ingo Molnar <mingo@kernel.org> - 2015-09-18 09:40 +0200
    Re: [PATCH 1/2] x86/math-emu: Add support for FCMOVcc and F[U]COMI[P]  insns Denys Vlasenko <dvlasenk@redhat.com> - 2015-09-16 22:00 +0200
      Re: [PATCH 1/2] x86/math-emu: Add support for FCMOVcc and  F[U]COMI[P] insns Ingo Molnar <mingo@kernel.org> - 2015-09-17 09:30 +0200
    Re: [PATCH 1/2] x86/math-emu: Add support for FCMOVcc and  F[U]COMI[P] insns Ingo Molnar <mingo@kernel.org> - 2015-09-18 09:40 +0200
      Re: [PATCH 1/2] x86/math-emu: Add support for FCMOVcc and F[U]COMI[P]  insns Denys Vlasenko <dvlasenk@redhat.com> - 2015-09-18 17:00 +0200
        Re: [PATCH 1/2] x86/math-emu: Add support for FCMOVcc and  F[U]COMI[P] insns Ingo Molnar <mingo@kernel.org> - 2015-09-20 10:20 +0200

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

* 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

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/*

[toc] | [prev] | [next] | [standalone]

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

* 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

* 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

* 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web