| Message ID | 20190109114744.10936-7-bigeasy@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6] x86: load FPU registers on return to userland | expand |
On Wed, Jan 09, 2019 at 12:47:28PM +0100, Sebastian Andrzej Siewior wrote: > Why does copy_fpstate_to_sigframe() do copy_fxregs_to_kernel() in the > ia32_fxstate case? I don't know. It just does. > Maybe it was required at some point, maybe it was added by accident and > nobody noticed it because it makes no difference. So 72a671ced66d ("x86, fpu: Unify signal handling code paths for x86 and x86_64 kernels") talks about some exclusion of legacy fsave state. > In copy_fpstate_to_sigframe() we stash the FPU state into the task's > stackframe. Then the CPU's FPU registers (and its fpu->state) are > cleared (handle_signal() does fpu__clear()). So that fpu__clear() name is not optimal. It should be fpu__reinitialize() or so. The comment above it says so too: /* * Clear the FPU state back to init state. > So it makes *no* difference > what happens to fpu->state after copy_fpregs_to_sigframe(). > > Remove copy_fxregs_to_kernel() since it does not matter what it does and > save a few cycles. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > arch/x86/kernel/fpu/signal.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c > index c136a4327659d..047390a45e016 100644 > --- a/arch/x86/kernel/fpu/signal.c > +++ b/arch/x86/kernel/fpu/signal.c > @@ -174,9 +174,6 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size) > /* 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); Need to get rid of that local "fpu" var too: arch/x86/kernel/fpu/signal.c: In function ‘copy_fpstate_to_sigframe’: arch/x86/kernel/fpu/signal.c:159:14: warning: unused variable ‘fpu’ [-Wunused-variable] struct fpu *fpu = ¤t->thread.fpu; ^~~ > /* Save the fsave header for the 32-bit frames. */ > if ((ia32_fxstate || !use_fxsr()) && save_fsave_header(tsk, buf)) > -- > 2.20.1 >
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c index c136a4327659d..047390a45e016 100644 --- a/arch/x86/kernel/fpu/signal.c +++ b/arch/x86/kernel/fpu/signal.c @@ -174,9 +174,6 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size) /* 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))
Why does copy_fpstate_to_sigframe() do copy_fxregs_to_kernel() in the ia32_fxstate case? I don't know. It just does. Maybe it was required at some point, maybe it was added by accident and nobody noticed it because it makes no difference. In copy_fpstate_to_sigframe() we stash the FPU state into the task's stackframe. Then the CPU's FPU registers (and its fpu->state) are cleared (handle_signal() does fpu__clear()). So it makes *no* difference what happens to fpu->state after copy_fpregs_to_sigframe(). Remove copy_fxregs_to_kernel() since it does not matter what it does and save a few cycles. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- arch/x86/kernel/fpu/signal.c | 3 --- 1 file changed, 3 deletions(-)