On Thu, 21 Mar 2019, Sebastian Andrzej Siewior wrote:

> From: Rik van Riel <[email protected]>
> 
> The FPU registers need only to be saved if TIF_NEED_FPU_LOAD is not set.
> Otherwise this has been already done and can be skipped.
> 
> Signed-off-by: Rik van Riel <[email protected]>
> Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
> ---
>  arch/x86/kernel/fpu/signal.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
> index f55f16d9e7e4e..97ea6909ede1f 100644
> --- a/arch/x86/kernel/fpu/signal.c
> +++ b/arch/x86/kernel/fpu/signal.c
> @@ -155,7 +155,16 @@ int copy_fpstate_to_sigframe(void __user *buf, void 
> __user *buf_fx, int size)
>                       sizeof(struct user_i387_ia32_struct), NULL,
>                       (struct _fpstate_32 __user *) buf) ? -1 : 1;
>  
> -     copy_fpregs_to_fpstate(fpu);
> +     fpregs_lock();
> +     /*
> +      * If we do not need to load the FPU registers at return to userspace
> +      * then the CPU has the current state and we need to save it. Otherwise
> +      * it is already done and we can skip it.
> +      */
> +     if (!test_thread_flag(TIF_NEED_FPU_LOAD))
> +             copy_fpregs_to_fpstate(fpu);

I think this should do the following:

        fpregs_lock();
        if (!test_thread_flag(TIF_NEED_FPU_LOAD)) {
                pagefault_disable();
                ret = copy_fpu_to_user(...);
                pagefault_enable();
                if (!res)
                        return 0;
                copy_fpregs_to_fpstate(fpu); 
        }
        fpregs_unlock();

The point is that in most cases the direct store from the FPU registers to
user space will succeed simply because the stack is accessible and you only
do the store in kernel memory and copy when that fails.

Thanks,

        tglx


Reply via email to