On Tue Feb 23, 2021 at 11:12 AM CST, Christophe Leroy wrote:
>
>
> Le 21/02/2021 à 02:23, Christopher M. Riedl a écrit :
> > Previously setup_sigcontext() performed a costly KUAP switch on every
> > uaccess operation. These repeated uaccess switches cause a significant
> > drop in signal handling performance.
> >
> > Rewrite setup_sigcontext() to assume that a userspace write access window
> > is open by replacing all uaccess functions with their 'unsafe' versions.
> > Modify the callers to first open, call unsafe_setup_sigcontext() and
> > then close the uaccess window.
>
> Do you plan to also convert setup_tm_sigcontexts() ?
> It would allow to then remove copy_fpr_to_user() and
> copy_ckfpr_to_user() and maybe other functions too.
I don't intend to convert the TM functions as part of this series.
Partially because I've been "threatened" with TM ownership for touching
the code :) and also because TM enhancements are a pretty low priority I
think.
>
> Christophe
>
> >
> > Signed-off-by: Christopher M. Riedl <c...@codefail.de>
> > ---
> > arch/powerpc/kernel/signal_64.c | 71 ++++++++++++++++++++-------------
> > 1 file changed, 44 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/signal_64.c
> > b/arch/powerpc/kernel/signal_64.c
> > index bd8d210c9115..3faaa736ed62 100644
> > --- a/arch/powerpc/kernel/signal_64.c
> > +++ b/arch/powerpc/kernel/signal_64.c
> > @@ -101,9 +101,13 @@ static void prepare_setup_sigcontext(struct
> > task_struct *tsk)
> > * Set up the sigcontext for the signal frame.
> > */
> >
> > -static long setup_sigcontext(struct sigcontext __user *sc,
> > - struct task_struct *tsk, int signr, sigset_t *set,
> > - unsigned long handler, int ctx_has_vsx_region)
> > +#define unsafe_setup_sigcontext(sc, tsk, signr, set, handler,
> > \
> > + ctx_has_vsx_region, e) \
> > + unsafe_op_wrap(__unsafe_setup_sigcontext(sc, tsk, signr, set, \
> > + handler, ctx_has_vsx_region), e)
> > +static long notrace __unsafe_setup_sigcontext(struct sigcontext __user *sc,
> > + struct task_struct *tsk, int signr,
> > sigset_t *set,
> > + unsigned long handler, int
> > ctx_has_vsx_region)
> > {
> > /* When CONFIG_ALTIVEC is set, we _always_ setup v_regs even if the
> > * process never used altivec yet (MSR_VEC is zero in pt_regs of
> > @@ -118,20 +122,19 @@ static long setup_sigcontext(struct sigcontext __user
> > *sc,
> > #endif
> > struct pt_regs *regs = tsk->thread.regs;
> > unsigned long msr = regs->msr;
> > - long err = 0;
> > /* Force usr to alway see softe as 1 (interrupts enabled) */
> > unsigned long softe = 0x1;
> >
> > BUG_ON(tsk != current);
> >
> > #ifdef CONFIG_ALTIVEC
> > - err |= __put_user(v_regs, &sc->v_regs);
> > + unsafe_put_user(v_regs, &sc->v_regs, efault_out);
> >
> > /* save altivec registers */
> > if (tsk->thread.used_vr) {
> > /* Copy 33 vec registers (vr0..31 and vscr) to the stack */
> > - err |= __copy_to_user(v_regs, &tsk->thread.vr_state,
> > - 33 * sizeof(vector128));
> > + unsafe_copy_to_user(v_regs, &tsk->thread.vr_state,
> > + 33 * sizeof(vector128), efault_out);
> > /* set MSR_VEC in the MSR value in the frame to indicate that
> > sc->v_reg)
> > * contains valid data.
> > */
> > @@ -140,12 +143,12 @@ static long setup_sigcontext(struct sigcontext __user
> > *sc,
> > /* We always copy to/from vrsave, it's 0 if we don't have or don't
> > * use altivec.
> > */
> > - err |= __put_user(tsk->thread.vrsave, (u32 __user *)&v_regs[33]);
> > + unsafe_put_user(tsk->thread.vrsave, (u32 __user *)&v_regs[33],
> > efault_out);
> > #else /* CONFIG_ALTIVEC */
> > - err |= __put_user(0, &sc->v_regs);
> > + unsafe_put_user(0, &sc->v_regs, efault_out);
> > #endif /* CONFIG_ALTIVEC */
> > /* copy fpr regs and fpscr */
> > - err |= copy_fpr_to_user(&sc->fp_regs, tsk);
> > + unsafe_copy_fpr_to_user(&sc->fp_regs, tsk, efault_out);
> >
> > /*
> > * Clear the MSR VSX bit to indicate there is no valid state attached
> > @@ -160,24 +163,27 @@ static long setup_sigcontext(struct sigcontext __user
> > *sc,
> > */
> > if (tsk->thread.used_vsr && ctx_has_vsx_region) {
> > v_regs += ELF_NVRREG;
> > - err |= copy_vsx_to_user(v_regs, tsk);
> > + unsafe_copy_vsx_to_user(v_regs, tsk, efault_out);
> > /* set MSR_VSX in the MSR value in the frame to
> > * indicate that sc->vs_reg) contains valid data.
> > */
> > msr |= MSR_VSX;
> > }
> > #endif /* CONFIG_VSX */
> > - err |= __put_user(&sc->gp_regs, &sc->regs);
> > + unsafe_put_user(&sc->gp_regs, &sc->regs, efault_out);
> > WARN_ON(!FULL_REGS(regs));
> > - err |= __copy_to_user(&sc->gp_regs, regs, GP_REGS_SIZE);
> > - err |= __put_user(msr, &sc->gp_regs[PT_MSR]);
> > - err |= __put_user(softe, &sc->gp_regs[PT_SOFTE]);
> > - err |= __put_user(signr, &sc->signal);
> > - err |= __put_user(handler, &sc->handler);
> > + unsafe_copy_to_user(&sc->gp_regs, regs, GP_REGS_SIZE, efault_out);
> > + unsafe_put_user(msr, &sc->gp_regs[PT_MSR], efault_out);
> > + unsafe_put_user(softe, &sc->gp_regs[PT_SOFTE], efault_out);
> > + unsafe_put_user(signr, &sc->signal, efault_out);
> > + unsafe_put_user(handler, &sc->handler, efault_out);
> > if (set != NULL)
> > - err |= __put_user(set->sig[0], &sc->oldmask);
> > + unsafe_put_user(set->sig[0], &sc->oldmask, efault_out);
> >
> > - return err;
> > + return 0;
> > +
> > +efault_out:
> > + return -EFAULT;
> > }
> >
> > #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> > @@ -670,12 +676,15 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user
> > *, old_ctx,
> >
> > if (old_ctx != NULL) {
> > prepare_setup_sigcontext(current);
> > - if (!access_ok(old_ctx, ctx_size)
> > - || setup_sigcontext(&old_ctx->uc_mcontext, current, 0,
> > NULL, 0,
> > - ctx_has_vsx_region)
> > - || __copy_to_user(&old_ctx->uc_sigmask,
> > - ¤t->blocked, sizeof(sigset_t)))
> > + if (!user_write_access_begin(old_ctx, ctx_size))
> > return -EFAULT;
> > +
> > + unsafe_setup_sigcontext(&old_ctx->uc_mcontext, current, 0, NULL,
> > + 0, ctx_has_vsx_region, efault_out);
> > + unsafe_copy_to_user(&old_ctx->uc_sigmask, ¤t->blocked,
> > + sizeof(sigset_t), efault_out);
> > +
> > + user_write_access_end();
> > }
> > if (new_ctx == NULL)
> > return 0;
> > @@ -704,6 +713,10 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *,
> > old_ctx,
> > /* This returns like rt_sigreturn */
> > set_thread_flag(TIF_RESTOREALL);
> > return 0;
> > +
> > +efault_out:
> > + user_write_access_end();
> > + return -EFAULT;
> > }
> >
> >
> > @@ -854,9 +867,13 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t
> > *set,
> > } else {
> > err |= __put_user(0, &frame->uc.uc_link);
> > prepare_setup_sigcontext(tsk);
> > - err |= setup_sigcontext(&frame->uc.uc_mcontext, tsk, ksig->sig,
> > - NULL, (unsigned
> > long)ksig->ka.sa.sa_handler,
> > - 1);
> > + if (!user_write_access_begin(&frame->uc.uc_mcontext,
> > + sizeof(frame->uc.uc_mcontext)))
> > + return -EFAULT;
> > + err |= __unsafe_setup_sigcontext(&frame->uc.uc_mcontext, tsk,
> > + ksig->sig, NULL,
> > + (unsigned
> > long)ksig->ka.sa.sa_handler, 1);
> > + user_write_access_end();
> > }
> > err |= __copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set));
> > if (err)
> >
