From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753032AbbIRHeE (ORCPT <rfc822;w@1wt.eu>);
	Fri, 18 Sep 2015 03:34:04 -0400
Received: from mail-wi0-f172.google.com ([209.85.212.172]:36807 "EHLO
	mail-wi0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752721AbbIRHeC (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 18 Sep 2015 03:34:02 -0400
Date: Fri, 18 Sep 2015 09:33:57 +0200
From: Ingo Molnar <mingo@kernel.org>
To: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>, "H. Peter Anvin" <hpa@zytor.com>,
        Andy Lutomirski <luto@amacapital.net>,
        Kees Cook <keescook@chromium.org>, x86@kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] x86/math-emu: Add support for FCMOVcc and
 F[U]COMI[P] insns
Message-ID: <20150918073357.GA2691@gmail.com>
References: <1442432914-27022-1-git-send-email-dvlasenk@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1442432914-27022-1-git-send-email-dvlasenk@redhat.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


* 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