Message ID | 20190109114744.10936-17-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:38PM +0100, Sebastian Andrzej Siewior wrote: > From: Rik van Riel <riel@surriel.com> > > copy_fpstate_to_sigframe() stores the registers directly to user space. > This is okay because the FPU register are valid and saving it directly > avoids saving it into kernel memory and making a copy. > However… We can't keep doing this if we are going to restore the FPU > registers on the return to userland. It is possible that the FPU > registers will be invalidated in the middle of the save operation and > this should be done with disabled preemption / BH. > > Save the FPU registers to task's FPU struct and copy them to the user > memory later on. > > This code is extracted from an earlier version of the patchset while > there still was lazy-FPU on x86. > > Signed-off-by: Rik van Riel <riel@surriel.com> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > arch/x86/include/asm/fpu/internal.h | 45 ----------------------------- > arch/x86/kernel/fpu/signal.c | 29 +++++++------------ > 2 files changed, 10 insertions(+), 64 deletions(-) ... > @@ -171,9 +156,15 @@ 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; > > - /* Save the live register state to the user directly. */ > - if (copy_fpregs_to_sigframe(buf_fx)) > - return -1; > + copy_fpregs_to_fpstate(fpu); > + > + if (using_compacted_format()) { > + copy_xstate_to_user(buf_fx, xsave, 0, size); > + } else { > + fpstate_sanitize_xstate(fpu); > + if (__copy_to_user(buf_fx, xsave, fpu_user_xstate_size)) > + return -1; > + } > > /* Save the fsave header for the 32-bit frames. */ > if ((ia32_fxstate || !use_fxsr()) && save_fsave_header(tsk, buf)) Comments above that function need updating.
On 2019-01-30 12:43:22 [+0100], Borislav Petkov wrote: > > @@ -171,9 +156,15 @@ 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; > > > > - /* Save the live register state to the user directly. */ > > - if (copy_fpregs_to_sigframe(buf_fx)) > > - return -1; > > + copy_fpregs_to_fpstate(fpu); > > + > > + if (using_compacted_format()) { > > + copy_xstate_to_user(buf_fx, xsave, 0, size); > > + } else { > > + fpstate_sanitize_xstate(fpu); > > + if (__copy_to_user(buf_fx, xsave, fpu_user_xstate_size)) > > + return -1; > > + } > > > > /* Save the fsave header for the 32-bit frames. */ > > if ((ia32_fxstate || !use_fxsr()) && save_fsave_header(tsk, buf)) > > Comments above that function need updating. Did: - * Save the state directly to the user frame pointed by the aligned pointer - * 'buf_fx'. + * Save the state to task's fpu->state and then copy it to the user frame + * pointed by the aligned pointer 'buf_fx'. Sebastian
diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h index 7191eb9686827..16ea30235b025 100644 --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -126,22 +126,6 @@ extern void fpstate_sanitize_xstate(struct fpu *fpu); _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_fprestore) \ : output : input) -static inline int copy_fregs_to_user(struct fregs_state __user *fx) -{ - return user_insn(fnsave %[fx]; fwait, [fx] "=m" (*fx), "m" (*fx)); -} - -static inline int copy_fxregs_to_user(struct fxregs_state __user *fx) -{ - if (IS_ENABLED(CONFIG_X86_32)) - return user_insn(fxsave %[fx], [fx] "=m" (*fx), "m" (*fx)); - else if (IS_ENABLED(CONFIG_AS_FXSAVEQ)) - return user_insn(fxsaveq %[fx], [fx] "=m" (*fx), "m" (*fx)); - - /* See comment in copy_fxregs_to_kernel() below. */ - return user_insn(rex64/fxsave (%[fx]), "=m" (*fx), [fx] "R" (fx)); -} - static inline void copy_kernel_to_fxregs(struct fxregs_state *fx) { if (IS_ENABLED(CONFIG_X86_32)) { @@ -352,35 +336,6 @@ static inline void copy_kernel_to_xregs(struct xregs_state *xstate, u64 mask) XSTATE_XRESTORE(xstate, lmask, hmask); } -/* - * Save xstate to user space xsave area. - * - * We don't use modified optimization because xrstor/xrstors might track - * a different application. - * - * We don't use compacted format xsave area for - * backward compatibility for old applications which don't understand - * compacted format of xsave area. - */ -static inline int copy_xregs_to_user(struct xregs_state __user *buf) -{ - int err; - - /* - * Clear the xsave header first, so that reserved fields are - * initialized to zero. - */ - err = __clear_user(&buf->header, sizeof(buf->header)); - if (unlikely(err)) - return -EFAULT; - - stac(); - XSTATE_OP(XSAVE, buf, -1, -1, err); - clac(); - - return err; -} - /* * Restore xstate from user space xsave area. */ diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c index 555c469878874..bf4e6caad305e 100644 --- a/arch/x86/kernel/fpu/signal.c +++ b/arch/x86/kernel/fpu/signal.c @@ -118,22 +118,6 @@ static inline int save_xstate_epilog(void __user *buf, int ia32_frame) return err; } -static inline int copy_fpregs_to_sigframe(struct xregs_state __user *buf) -{ - int err; - - if (use_xsave()) - err = copy_xregs_to_user(buf); - else if (use_fxsr()) - err = copy_fxregs_to_user((struct fxregs_state __user *) buf); - else - err = copy_fregs_to_user((struct fregs_state __user *) buf); - - if (unlikely(err) && __clear_user(buf, fpu_user_xstate_size)) - err = -EFAULT; - return err; -} - /* * Save the fpu, extended register state to the user signal frame. * @@ -157,6 +141,7 @@ static inline int copy_fpregs_to_sigframe(struct xregs_state __user *buf) int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size) { struct fpu *fpu = ¤t->thread.fpu; + struct xregs_state *xsave = &fpu->state.xsave; struct task_struct *tsk = current; int ia32_fxstate = (buf != buf_fx); @@ -171,9 +156,15 @@ 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; - /* Save the live register state to the user directly. */ - if (copy_fpregs_to_sigframe(buf_fx)) - return -1; + copy_fpregs_to_fpstate(fpu); + + if (using_compacted_format()) { + copy_xstate_to_user(buf_fx, xsave, 0, size); + } else { + fpstate_sanitize_xstate(fpu); + if (__copy_to_user(buf_fx, xsave, fpu_user_xstate_size)) + return -1; + } /* Save the fsave header for the 32-bit frames. */ if ((ia32_fxstate || !use_fxsr()) && save_fsave_header(tsk, buf))