Re: [PATCH 06/22] x86/fpu: Don't save fxregs for ia32 frames in copy_fpstate_to_sigframe()

Borislav Petkov
2019-01-24T11:17:24Z

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 = &current->thread.fpu;
^~~

> /* Save the fsave header for the 32-bit frames. */
> if ((ia32_fxstate || !use_fxsr()) && save_fsave_header(tsk, buf))
> --
> 2.20.1
>

--
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.