Patchwork [06/22,v2] x86/fpu: Don't save fxregs for ia32 frames in

mail settings
Submitter Sebastian Andrzej Siewior
Date Feb. 5, 2019, 4:43 p.m.
Message ID <>
Download mbox | patch
Permalink /patch/718469/
State New
Headers show


Sebastian Andrzej Siewior - Feb. 5, 2019, 4:43 p.m.
In commit 

  72a671ced66db ("x86, fpu: Unify signal handling code paths for x86 and x86_64 kernels")

the 32bit and 64bit path of the signal delivery code were merged. The 32bit version:
|int save_i387_xstate_ia32(void __user *buf)
|       if (cpu_has_xsave)
|               return save_i387_xsave(fp);
|       if (cpu_has_fxsr)
|               return save_i387_fxsave(fp);

The 64bit version:
|int save_i387_xstate(void __user *buf)
|       if (user_has_fpu()) {
|               if (use_xsave())
|                       err = xsave_user(buf);
|               else
|                       err = fxsave_user(buf);
|               if (unlikely(err)) {
|                       __clear_user(buf, xstate_size);
|                       return err;

The merge:
|int save_xstate_sig(void __user *buf, void __user *buf_fx, int size)
|       if (user_has_fpu()) {
|               /* Save the live register state to the user directly. */
|               if (save_user_xstate(buf_fx))
|                       return -1;
|               /* Update the thread's fxstate to save the fsave header. */
|               if (ia32_fxstate)
|                       fpu_fxsave(&tsk->thread.fpu);

I don't think that we needed to save the FPU registers to ->thread.fpu because
the registers were stored in `buf_fx'. Today the state will be restored from
`buf_fx' after the signal was handled (I assume that this was also the case
with lazy-FPU). Since commit

  66463db4fc560 ("x86, fpu: shift drop_init_fpu() from save_xstate_sig() to handle_signal()")

it is ensured that the signal handler starts with clear/fresh set of FPU
registers which means that the previous store is futile.

Remove copy_fxregs_to_kernel() because task's FPU state is cleared later in
handle_signal() via fpu__clear().

Signed-off-by: Sebastian Andrzej Siewior <>
 - reworte description. Replaced the "I don't know why it is like it is
   makes no sense buh" part with some pointer which might explain why
   copy_fxregs_to_kernel() ended there and since when it definitely is a
 - removed `fpu' since it unused after that change.

 arch/x86/kernel/fpu/signal.c | 3 ---
 1 file changed, 3 deletions(-)


Index: staging/arch/x86/kernel/fpu/signal.c
--- staging.orig/arch/x86/kernel/fpu/signal.c
+++ staging/arch/x86/kernel/fpu/signal.c
@@ -155,7 +155,6 @@  static inline int copy_fpregs_to_sigfram
 int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
-	struct fpu *fpu = &current->thread.fpu;
 	struct task_struct *tsk = current;
 	int ia32_fxstate = (buf != buf_fx);
@@ -173,9 +172,6 @@  int copy_fpstate_to_sigframe(void __user
 	/* Save the live register state to the user directly. */
 	if (copy_fpregs_to_sigframe(buf_fx))
 		return -1;
-	/* Update the thread's fxstate to save the fsave header. */
-	if (ia32_fxstate)
-		copy_fxregs_to_kernel(fpu);
 	/* Save the fsave header for the 32-bit frames. */
 	if ((ia32_fxstate || !use_fxsr()) && save_fsave_header(tsk, buf))