mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: [PATCH 1/2] i387: use 'restore_fpu_checking()' directly in task switching code
Date: Sun, 19 Feb 2012 14:26:38 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.2.02.1202191423270.3898@i5.linux-foundation.org> (raw)
In-Reply-To: <alpine.LFD.2.02.1202191412060.3898@i5.linux-foundation.org>


From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sun, 19 Feb 2012 11:48:44 -0800

This inlines what is usually just a couple of instructions, but more
importantly it also fixes the theoretical error case (can that FPU
restore really ever fail? Maybe we should remove the checking).

We can't start sending signals from within the scheduler, we're much too
deep in the kernel and are holding the runqueue lock etc.  So don't
bother even trying.

Also, make sure that we increment the fpu_counter even when we preload
the FP state, since that is also how we eventually decide to try not
preloading (when the byte counter overflows and becomes zero again).

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

The fpu_counter part in switch_fpu_finish() is technically separate, but 
these are both bugs in the same preloading code, so I made it one patch.

Can that paranoid restore actually fail? We're loading from kernel state, 
stuff we saved there. But maybe you can mess things up on x86-32 by 
messing up the signal stack restore state, which gets copied by hand into 
the kernel buffer? Regardless, *if* it can fail, that signal sending from 
the scheduler is seriously wrong, afaik.

 arch/x86/include/asm/i387.h |   19 ++++++++++++++++---
 arch/x86/kernel/traps.c     |   40 ++++++++--------------------------------
 2 files changed, 24 insertions(+), 35 deletions(-)

diff --git a/arch/x86/include/asm/i387.h b/arch/x86/include/asm/i387.h
index a850b4d8d14d..251c366289b9 100644
--- a/arch/x86/include/asm/i387.h
+++ b/arch/x86/include/asm/i387.h
@@ -29,7 +29,6 @@ extern unsigned int sig_xstate_size;
 extern void fpu_init(void);
 extern void mxcsr_feature_mask_init(void);
 extern int init_fpu(struct task_struct *child);
-extern void __math_state_restore(struct task_struct *);
 extern void math_state_restore(void);
 extern int dump_fpu(struct pt_regs *, struct user_i387_struct *);
 
@@ -269,6 +268,16 @@ static inline int fpu_restore_checking(struct fpu *fpu)
 
 static inline int restore_fpu_checking(struct task_struct *tsk)
 {
+	/* AMD K7/K8 CPUs don't save/restore FDP/FIP/FOP unless an exception
+	   is pending.  Clear the x87 state here by setting it to fixed
+	   values. "m" is a random variable that should be in L1 */
+	alternative_input(
+		ASM_NOP8 ASM_NOP2,
+		"emms\n\t"	  	/* clear stack tags */
+		"fildl %P[addr]",	/* set F?P to defined value */
+		X86_FEATURE_FXSAVE_LEAK,
+		[addr] "m" (tsk->thread.has_fpu));
+
 	return fpu_restore_checking(&tsk->thread.fpu);
 }
 
@@ -377,8 +386,12 @@ static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct ta
  */
 static inline void switch_fpu_finish(struct task_struct *new, fpu_switch_t fpu)
 {
-	if (fpu.preload)
-		__math_state_restore(new);
+	if (fpu.preload) {
+		new->fpu_counter++;
+		if (unlikely(restore_fpu_checking(new))) {
+			__thread_fpu_end(new);
+		}
+	}
 }
 
 /*
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 77da5b475ad2..4bbe04d96744 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -571,37 +571,6 @@ asmlinkage void __attribute__((weak)) smp_threshold_interrupt(void)
 }
 
 /*
- * This gets called with the process already owning the
- * FPU state, and with CR0.TS cleared. It just needs to
- * restore the FPU register state.
- */
-void __math_state_restore(struct task_struct *tsk)
-{
-	/* We need a safe address that is cheap to find and that is already
-	   in L1. We've just brought in "tsk->thread.has_fpu", so use that */
-#define safe_address (tsk->thread.has_fpu)
-
-	/* AMD K7/K8 CPUs don't save/restore FDP/FIP/FOP unless an exception
-	   is pending.  Clear the x87 state here by setting it to fixed
-	   values. safe_address is a random variable that should be in L1 */
-	alternative_input(
-		ASM_NOP8 ASM_NOP2,
-		"emms\n\t"	  	/* clear stack tags */
-		"fildl %P[addr]",	/* set F?P to defined value */
-		X86_FEATURE_FXSAVE_LEAK,
-		[addr] "m" (safe_address));
-
-	/*
-	 * Paranoid restore. send a SIGSEGV if we fail to restore the state.
-	 */
-	if (unlikely(restore_fpu_checking(tsk))) {
-		__thread_fpu_end(tsk);
-		force_sig(SIGSEGV, tsk);
-		return;
-	}
-}
-
-/*
  * 'math_state_restore()' saves the current math information in the
  * old math state array, and gets the new ones from the current task
  *
@@ -631,7 +600,14 @@ void math_state_restore(void)
 	}
 
 	__thread_fpu_begin(tsk);
-	__math_state_restore(tsk);
+	/*
+	 * Paranoid restore. send a SIGSEGV if we fail to restore the state.
+	 */
+	if (unlikely(restore_fpu_checking(tsk))) {
+		__thread_fpu_end(tsk);
+		force_sig(SIGSEGV, tsk);
+		return;
+	}
 
 	tsk->fpu_counter++;
 }
-- 
1.7.9.188.g12766.dirty


  reply	other threads:[~2012-02-19 22:26 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-19 22:23 [PATCH 0/2] More i387 state save/restore work Linus Torvalds
2012-02-19 22:26 ` Linus Torvalds [this message]
2012-02-19 22:37   ` [PATCH 2/2] i387: support lazy restore of FPU state Linus Torvalds
2012-02-19 22:44     ` H. Peter Anvin
2012-02-19 23:18       ` H. Peter Anvin
2012-02-19 23:56       ` Linus Torvalds
2012-02-20  7:51     ` Ingo Molnar
2012-02-20  0:53 ` [PATCH 0/2] More i387 state save/restore work Michael Neuling
2012-02-20  1:03   ` Linus Torvalds
2012-02-20  1:06     ` Linus Torvalds
2012-02-20  1:11       ` Linus Torvalds
2012-03-01 11:30         ` Benjamin Herrenschmidt
2012-02-20  2:09     ` Indan Zupancic
2012-02-20 19:46 ` [PATCH v2 0/3] " Linus Torvalds
2012-02-20 19:47   ` [PATCH v2 1/3] i387: fix up some fpu_counter confusion Linus Torvalds
2012-02-20 19:48     ` [PATCH v2 2/3] i387: use 'restore_fpu_checking()' directly in task switching code Linus Torvalds
2012-02-20 19:48       ` [PATCH v2 3/3] i387: support lazy restore of FPU state Linus Torvalds
2012-02-21  1:50         ` Josh Boyer
2012-02-21  2:10           ` Linus Torvalds
2012-02-21  2:14             ` H. Peter Anvin
2012-02-21  5:27               ` Linus Torvalds
2012-02-21  5:35                 ` H. Peter Anvin
2012-02-21 14:19                 ` Josh Boyer
2012-02-21 17:59                 ` H. Peter Anvin
2012-02-21 18:06                   ` Ingo Molnar
2012-02-21 18:26                   ` Linus Torvalds
2012-02-21 21:14                     ` H. Peter Anvin
2012-02-21 21:39                       ` [PATCH 0/2] i387: FP state interface cleanups Linus Torvalds
2012-02-21 21:40                         ` [PATCH 1/2] i387: uninline the generic FP helpers that we expose to kernel modules Linus Torvalds
2012-02-21 21:41                           ` [PATCH 2/2] i387: split up <asm/i387.h> into exported and internal interfaces Linus Torvalds
2012-02-21 23:50                             ` [tip:x86/fpu] i387: Split " tip-bot for Linus Torvalds
2012-02-28 11:21                             ` [PATCH 2/2] i387: split " Avi Kivity
2012-02-28 16:05                               ` Linus Torvalds
2012-02-28 17:21                                 ` Avi Kivity
2012-02-28 17:37                                   ` Linus Torvalds
2012-02-28 18:08                                     ` Linus Torvalds
2012-02-28 18:29                                       ` Avi Kivity
2012-02-28 18:09                                     ` Avi Kivity
2012-02-28 18:34                                       ` Linus Torvalds
2012-02-28 19:06                                         ` Avi Kivity
2012-02-28 19:26                                           ` Linus Torvalds
2012-02-28 19:45                                             ` Avi Kivity
2012-02-21 23:49                           ` [tip:x86/fpu] i387: Uninline the generic FP helpers that we expose to kernel modules tip-bot for Linus Torvalds
2012-02-21  2:18             ` [PATCH v2 3/3] i387: support lazy restore of FPU state Linus Torvalds
2012-02-21  2:32               ` H. Peter Anvin
2012-02-21  2:11           ` H. Peter Anvin
2012-02-21 21:54         ` Suresh Siddha
2012-02-21 21:57           ` Linus Torvalds
2012-02-21 22:19             ` Suresh Siddha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.2.02.1202191423270.3898@i5.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®