2020-08-18 17:24:03

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v2 00/25] powerpc: Switch signal 32 to using unsafe_put_user() and friends

This series leads to a reduction from 2.55s to 1.73s of
the system CPU time with the following microbench app
on an mpc832x with KUAP (approx 32%)

This series replaces copies to users by unsafe_put_user() and friends
with user_write_access_begin() dance in signal32.

The advantages are:
- No KUAP unlock/lock at every copy
- More readable code.
- Better generated code.

Without KUAP, the difference is in the noise.

void sigusr1(int sig) { }

int main(int argc, char **argv)
{
int i = 100000;

signal(SIGUSR1, sigusr1);
for (;i--;)
raise(SIGUSR1);
exit(0);
}

An additional 0.10s reduction is achieved by removing
CONFIG_PPC_FPU, as the mpc832x has no FPU.

A bit less spectacular on an 8xx as KUAP is less heavy, prior to
the series (with KUAP) it ran in 8.10 ms. Once applies the removal
of FPU regs handling, we get 7.05s. With the full series, we get 6.9s.
If artificially re-activating FPU regs handling with the full series,
we get 7.6s.

So for the 8xx, the removal of the FPU regs copy is what makes the
difference, but the rework of handle_signal also have a benefit.

Same as above, without KUAP the difference is in the noise.

Difference since v1(RFC):
- Almost copies to user are now replaced by unsafe_ alternative.
- Reworked a bit the FPU registers handling following feedback from Michael.
- Fixed a few build failures reported by Mr Robot on the RFC.

Christophe Leroy (25):
powerpc/signal: Move inline functions in signal.h
powerpc/ptrace: Move declaration of ptrace_get_reg() and
ptrace_set_reg()
powerpc/ptrace: Consolidate reg index calculation
powerpc/ptrace: Create ptrace_get_fpr() and ptrace_put_fpr()
powerpc/signal: Don't manage floating point regs when no FPU
powerpc/32s: Allow deselecting CONFIG_PPC_FPU on mpc832x
powerpc/signal: Remove BUG_ON() in handler_signal functions
powerpc/signal: Move access_ok() out of get_sigframe()
powerpc/signal: Remove get_clean_sp()
powerpc/signal: Call get_tm_stackpointer() from get_sigframe()
powerpc/signal: Refactor bad frame logging
powerpc/signal32: Simplify logging in handle_rt_signal32()
powerpc/signal32: Move handle_signal32() close to handle_rt_signal32()
powerpc/signal32: Rename local pointers in handle_rt_signal32()
powerpc/signal32: Misc changes to make handle_[rt_]_signal32() more
similar
powerpc/signal32: Move signal trampoline setup to handle_[rt_]signal32
powerpc/signal32: Switch handle_signal32() to user_access_begin()
logic
powerpc/signal32: Switch handle_rt_signal32() to user_access_begin()
logic
powerpc/signal32: Remove ifdefery in middle of if/else
signal: Add unsafe_put_compat_sigset()
powerpc/signal32: Add and use unsafe_put_sigset_t()
powerpc/signal32: Switch swap_context() to user_access_begin() logic
powerpc/signal: Create 'unsafe' versions of
copy_[ck][fpr/vsx]_to_user()
powerpc/signal32: Isolate non-copy actions in save_user_regs() and
save_tm_user_regs()
powerpc/signal32: Transform save_user_regs() and save_tm_user_regs()
in 'unsafe' version

arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/processor.h | 16 +-
arch/powerpc/include/asm/ptrace.h | 6 -
arch/powerpc/kernel/asm-offsets.c | 2 +
arch/powerpc/kernel/head_32.S | 4 +
arch/powerpc/kernel/process.c | 4 +
arch/powerpc/kernel/ptrace/Makefile | 3 +-
arch/powerpc/kernel/ptrace/ptrace-decl.h | 21 +
arch/powerpc/kernel/ptrace/ptrace-fpu.c | 40 ++
arch/powerpc/kernel/ptrace/ptrace-view.c | 2 +
arch/powerpc/kernel/ptrace/ptrace.c | 54 +-
arch/powerpc/kernel/ptrace/ptrace32.c | 2 +
arch/powerpc/kernel/signal.c | 59 +--
arch/powerpc/kernel/signal.h | 115 ++++-
arch/powerpc/kernel/signal_32.c | 598 +++++++++++------------
arch/powerpc/kernel/signal_64.c | 21 +-
arch/powerpc/kernel/traps.c | 2 +
arch/powerpc/platforms/Kconfig.cputype | 15 +-
include/linux/compat.h | 32 ++
19 files changed, 566 insertions(+), 431 deletions(-)
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-fpu.c

--
2.25.0


2020-08-18 17:24:08

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v2 07/25] powerpc/signal: Remove BUG_ON() in handler_signal functions

There is already the same BUG_ON() check in do_signal() which
is the only caller of handle_rt_signal64() handle_rt_signal32() and
handle_signal32().

Remove those three redundant BUG_ON().

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/signal_32.c | 4 ----
arch/powerpc/kernel/signal_64.c | 2 --
2 files changed, 6 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 7b291707eb31..8cbc9ac1343d 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -764,8 +764,6 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
unsigned long msr = regs->msr;
#endif

- BUG_ON(tsk != current);
-
/* Set up Signal Frame */
/* Put a Real Time Context onto stack */
rt_sf = get_sigframe(ksig, get_tm_stackpointer(tsk), sizeof(*rt_sf), 1);
@@ -1227,8 +1225,6 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
unsigned long msr = regs->msr;
#endif

- BUG_ON(tsk != current);
-
/* Set up Signal Frame */
frame = get_sigframe(ksig, get_tm_stackpointer(tsk), sizeof(*frame), 1);
if (unlikely(frame == NULL))
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index bfc939360bad..cae612bdde5f 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -822,8 +822,6 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
unsigned long msr = regs->msr;
#endif

- BUG_ON(tsk != current);
-
frame = get_sigframe(ksig, get_tm_stackpointer(tsk), sizeof(*frame), 0);
if (unlikely(frame == NULL))
goto badframe;
--
2.25.0

2020-08-18 17:24:10

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v2 21/25] powerpc/signal32: Add and use unsafe_put_sigset_t()

put_sigset_t() calls copy_to_user() for copying two words.

This is terribly inefficient for copying two words.

By switching to unsafe_put_user(), we end up with something as
simple as:

3cc: 81 3d 00 00 lwz r9,0(r29)
3d0: 91 26 00 b4 stw r9,180(r6)
3d4: 81 3d 00 04 lwz r9,4(r29)
3d8: 91 26 00 b8 stw r9,184(r6)

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/signal_32.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 310d3b8d9ad5..3f9f315dd036 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -87,6 +87,8 @@ static inline int put_sigset_t(compat_sigset_t __user *uset, sigset_t *set)
return put_compat_sigset(uset, set, sizeof(*uset));
}

+#define unsafe_put_sigset_t unsafe_put_compat_sigset
+
static inline int get_sigset_t(sigset_t *set,
const compat_sigset_t __user *uset)
{
@@ -141,6 +143,13 @@ static inline int put_sigset_t(sigset_t __user *uset, sigset_t *set)
return copy_to_user(uset, set, sizeof(*uset));
}

+#define unsafe_put_sigset_t(uset, set, label) do { \
+ sigset_t __user *__us = uset ; \
+ const sigset_t *__s = set; \
+ \
+ unsafe_copy_to_user(__us, __s, sizeof(*__us), label); \
+} while (0)
+
static inline int get_sigset_t(sigset_t *set, const sigset_t __user *uset)
{
return copy_from_user(set, uset, sizeof(*uset));
@@ -780,10 +789,10 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
failed);
unsafe_put_user(PPC_INST_SC, &mctx->mc_pad[1], failed);
}
+ unsafe_put_sigset_t(&frame->uc.uc_sigmask, oldset, failed);
+
user_write_access_end();

- if (put_sigset_t(&frame->uc.uc_sigmask, oldset))
- goto badframe;
if (copy_siginfo_to_user(&frame->info, &ksig->info))
goto badframe;

--
2.25.0

2020-08-18 17:24:18

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v2 13/25] powerpc/signal32: Move handle_signal32() close to handle_rt_signal32()

Those two functions are similar and serving the same purpose.
To ease refactorisation, move them close to each other.

This is pure move, no code change, no cosmetic. Yes, checkpatch is
not happy, most will clear later.

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/signal_32.c | 170 ++++++++++++++++----------------
1 file changed, 85 insertions(+), 85 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 44a46911ff98..2cc686b9f566 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -836,6 +836,91 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
return 1;
}

+/*
+ * OK, we're invoking a handler
+ */
+int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
+ struct task_struct *tsk)
+{
+ struct sigcontext __user *sc;
+ struct sigframe __user *frame;
+ struct mcontext __user *tm_mctx = NULL;
+ unsigned long newsp = 0;
+ int sigret;
+ unsigned long tramp;
+ struct pt_regs *regs = tsk->thread.regs;
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+ /* Save the thread's msr before get_tm_stackpointer() changes it */
+ unsigned long msr = regs->msr;
+#endif
+
+ /* Set up Signal Frame */
+ frame = get_sigframe(ksig, tsk, sizeof(*frame), 1);
+ if (!access_ok(frame, sizeof(*frame)))
+ goto badframe;
+ sc = (struct sigcontext __user *) &frame->sctx;
+
+#if _NSIG != 64
+#error "Please adjust handle_signal()"
+#endif
+ if (__put_user(to_user_ptr(ksig->ka.sa.sa_handler), &sc->handler)
+ || __put_user(oldset->sig[0], &sc->oldmask)
+#ifdef CONFIG_PPC64
+ || __put_user((oldset->sig[0] >> 32), &sc->_unused[3])
+#else
+ || __put_user(oldset->sig[1], &sc->_unused[3])
+#endif
+ || __put_user(to_user_ptr(&frame->mctx), &sc->regs)
+ || __put_user(ksig->sig, &sc->signal))
+ goto badframe;
+
+ if (vdso32_sigtramp && tsk->mm->context.vdso_base) {
+ sigret = 0;
+ tramp = tsk->mm->context.vdso_base + vdso32_sigtramp;
+ } else {
+ sigret = __NR_sigreturn;
+ tramp = (unsigned long) frame->mctx.tramp;
+ }
+
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+ tm_mctx = &frame->mctx_transact;
+ if (MSR_TM_ACTIVE(msr)) {
+ if (save_tm_user_regs(regs, &frame->mctx, &frame->mctx_transact,
+ sigret, msr))
+ goto badframe;
+ }
+ else
+#endif
+ {
+ if (save_user_regs(regs, &frame->mctx, tm_mctx, sigret, 1))
+ goto badframe;
+ }
+
+ regs->link = tramp;
+
+#ifdef CONFIG_PPC_FPU_REGS
+ tsk->thread.fp_state.fpscr = 0; /* turn off all fp exceptions */
+#endif
+
+ /* create a stack frame for the caller of the handler */
+ newsp = ((unsigned long)frame) - __SIGNAL_FRAMESIZE;
+ if (put_user(regs->gpr[1], (u32 __user *)newsp))
+ goto badframe;
+
+ regs->gpr[1] = newsp;
+ regs->gpr[3] = ksig->sig;
+ regs->gpr[4] = (unsigned long) sc;
+ regs->nip = (unsigned long) (unsigned long)ksig->ka.sa.sa_handler;
+ /* enter the signal handler in big-endian mode */
+ regs->msr &= ~MSR_LE;
+ return 0;
+
+badframe:
+ signal_fault(tsk, regs, "handle_signal32", frame);
+
+ return 1;
+}
+
static int do_setcontext(struct ucontext __user *ucp, struct pt_regs *regs, int sig)
{
sigset_t set;
@@ -1188,91 +1273,6 @@ SYSCALL_DEFINE3(debug_setcontext, struct ucontext __user *, ctx,
}
#endif

-/*
- * OK, we're invoking a handler
- */
-int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
- struct task_struct *tsk)
-{
- struct sigcontext __user *sc;
- struct sigframe __user *frame;
- struct mcontext __user *tm_mctx = NULL;
- unsigned long newsp = 0;
- int sigret;
- unsigned long tramp;
- struct pt_regs *regs = tsk->thread.regs;
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
- /* Save the thread's msr before get_tm_stackpointer() changes it */
- unsigned long msr = regs->msr;
-#endif
-
- /* Set up Signal Frame */
- frame = get_sigframe(ksig, tsk, sizeof(*frame), 1);
- if (!access_ok(frame, sizeof(*frame)))
- goto badframe;
- sc = (struct sigcontext __user *) &frame->sctx;
-
-#if _NSIG != 64
-#error "Please adjust handle_signal()"
-#endif
- if (__put_user(to_user_ptr(ksig->ka.sa.sa_handler), &sc->handler)
- || __put_user(oldset->sig[0], &sc->oldmask)
-#ifdef CONFIG_PPC64
- || __put_user((oldset->sig[0] >> 32), &sc->_unused[3])
-#else
- || __put_user(oldset->sig[1], &sc->_unused[3])
-#endif
- || __put_user(to_user_ptr(&frame->mctx), &sc->regs)
- || __put_user(ksig->sig, &sc->signal))
- goto badframe;
-
- if (vdso32_sigtramp && tsk->mm->context.vdso_base) {
- sigret = 0;
- tramp = tsk->mm->context.vdso_base + vdso32_sigtramp;
- } else {
- sigret = __NR_sigreturn;
- tramp = (unsigned long) frame->mctx.tramp;
- }
-
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
- tm_mctx = &frame->mctx_transact;
- if (MSR_TM_ACTIVE(msr)) {
- if (save_tm_user_regs(regs, &frame->mctx, &frame->mctx_transact,
- sigret, msr))
- goto badframe;
- }
- else
-#endif
- {
- if (save_user_regs(regs, &frame->mctx, tm_mctx, sigret, 1))
- goto badframe;
- }
-
- regs->link = tramp;
-
-#ifdef CONFIG_PPC_FPU_REGS
- tsk->thread.fp_state.fpscr = 0; /* turn off all fp exceptions */
-#endif
-
- /* create a stack frame for the caller of the handler */
- newsp = ((unsigned long)frame) - __SIGNAL_FRAMESIZE;
- if (put_user(regs->gpr[1], (u32 __user *)newsp))
- goto badframe;
-
- regs->gpr[1] = newsp;
- regs->gpr[3] = ksig->sig;
- regs->gpr[4] = (unsigned long) sc;
- regs->nip = (unsigned long) (unsigned long)ksig->ka.sa.sa_handler;
- /* enter the signal handler in big-endian mode */
- regs->msr &= ~MSR_LE;
- return 0;
-
-badframe:
- signal_fault(tsk, regs, "handle_signal32", frame);
-
- return 1;
-}
-
/*
* Do a signal return; undo the signal stack.
*/
--
2.25.0

2020-08-18 17:24:47

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v2 14/25] powerpc/signal32: Rename local pointers in handle_rt_signal32()

Rename pointers in handle_rt_signal32() to make it more similar to
handle_signal32()

tm_frame becomes tm_mctx
frame becomes mctx
rt_sf becomes frame

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/signal_32.c | 51 ++++++++++++++++-----------------
1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 2cc686b9f566..d0fcb3de66aa 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -751,9 +751,9 @@ static long restore_tm_user_regs(struct pt_regs *regs,
int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
struct task_struct *tsk)
{
- struct rt_sigframe __user *rt_sf;
- struct mcontext __user *frame;
- struct mcontext __user *tm_frame = NULL;
+ struct rt_sigframe __user *frame;
+ struct mcontext __user *mctx;
+ struct mcontext __user *tm_mctx = NULL;
unsigned long newsp = 0;
int sigret;
unsigned long tramp;
@@ -765,46 +765,45 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,

/* Set up Signal Frame */
/* Put a Real Time Context onto stack */
- rt_sf = get_sigframe(ksig, tsk, sizeof(*rt_sf), 1);
- if (!access_ok(rt_sf, sizeof(*rt_sf)))
+ frame = get_sigframe(ksig, tsk, sizeof(*frame), 1);
+ if (!access_ok(frame, sizeof(*frame)))
goto badframe;

/* Put the siginfo & fill in most of the ucontext */
- if (copy_siginfo_to_user(&rt_sf->info, &ksig->info)
- || __put_user(0, &rt_sf->uc.uc_flags)
- || __save_altstack(&rt_sf->uc.uc_stack, regs->gpr[1])
- || __put_user(to_user_ptr(&rt_sf->uc.uc_mcontext),
- &rt_sf->uc.uc_regs)
- || put_sigset_t(&rt_sf->uc.uc_sigmask, oldset))
+ if (copy_siginfo_to_user(&frame->info, &ksig->info) ||
+ __put_user(0, &frame->uc.uc_flags) ||
+ __save_altstack(&frame->uc.uc_stack, regs->gpr[1]) ||
+ __put_user(to_user_ptr(&frame->uc.uc_mcontext), &frame->uc.uc_regs) ||
+ put_sigset_t(&frame->uc.uc_sigmask, oldset))
goto badframe;

/* Save user registers on the stack */
- frame = &rt_sf->uc.uc_mcontext;
+ mctx = &frame->uc.uc_mcontext;
if (vdso32_rt_sigtramp && tsk->mm->context.vdso_base) {
sigret = 0;
tramp = tsk->mm->context.vdso_base + vdso32_rt_sigtramp;
} else {
sigret = __NR_rt_sigreturn;
- tramp = (unsigned long) frame->tramp;
+ tramp = (unsigned long)mctx->tramp;
}

#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
- tm_frame = &rt_sf->uc_transact.uc_mcontext;
+ tm_mctx = &frame->uc_transact.uc_mcontext;
if (MSR_TM_ACTIVE(msr)) {
- if (__put_user((unsigned long)&rt_sf->uc_transact,
- &rt_sf->uc.uc_link) ||
- __put_user((unsigned long)tm_frame,
- &rt_sf->uc_transact.uc_regs))
+ if (__put_user((unsigned long)&frame->uc_transact,
+ &frame->uc.uc_link) ||
+ __put_user((unsigned long)tm_mctx,
+ &frame->uc_transact.uc_regs))
goto badframe;
- if (save_tm_user_regs(regs, frame, tm_frame, sigret, msr))
+ if (save_tm_user_regs(regs, mctx, tm_mctx, sigret, msr))
goto badframe;
}
else
#endif
{
- if (__put_user(0, &rt_sf->uc.uc_link))
+ if (__put_user(0, &frame->uc.uc_link))
goto badframe;
- if (save_user_regs(regs, frame, tm_frame, sigret, 1))
+ if (save_user_regs(regs, mctx, tm_mctx, sigret, 1))
goto badframe;
}
regs->link = tramp;
@@ -814,16 +813,16 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
#endif

/* create a stack frame for the caller of the handler */
- newsp = ((unsigned long)rt_sf) - (__SIGNAL_FRAMESIZE + 16);
+ newsp = ((unsigned long)frame) - (__SIGNAL_FRAMESIZE + 16);
if (put_user(regs->gpr[1], (u32 __user *)newsp))
goto badframe;

/* Fill registers for signal handler */
regs->gpr[1] = newsp;
regs->gpr[3] = ksig->sig;
- regs->gpr[4] = (unsigned long) &rt_sf->info;
- regs->gpr[5] = (unsigned long) &rt_sf->uc;
- regs->gpr[6] = (unsigned long) rt_sf;
+ regs->gpr[4] = (unsigned long)&frame->info;
+ regs->gpr[5] = (unsigned long)&frame->uc;
+ regs->gpr[6] = (unsigned long)frame;
regs->nip = (unsigned long) ksig->ka.sa.sa_handler;
/* enter the signal handler in native-endian mode */
regs->msr &= ~MSR_LE;
@@ -831,7 +830,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
return 0;

badframe:
- signal_fault(tsk, regs, "handle_rt_signal32", rt_sf);
+ signal_fault(tsk, regs, "handle_rt_signal32", frame);

return 1;
}
--
2.25.0

2020-08-18 17:24:56

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v2 16/25] powerpc/signal32: Move signal trampoline setup to handle_[rt_]signal32

Move signal trampoline setup into handle_signal32()
and handle_rt_signal32().

At the same time, remove the define which hides the mc_pad field
used for trampoline.

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/signal_32.c | 61 ++++++++++++---------------------
1 file changed, 22 insertions(+), 39 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index ab8c8cb98b15..d8c3843102df 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -199,9 +199,6 @@ struct sigframe {
int abigap[56];
};

-/* We use the mc_pad field for the signal return trampoline. */
-#define tramp mc_pad
-
/*
* When we have rt signals to deliver, we set up on the
* user stack, going down from the original stack pointer:
@@ -236,8 +233,7 @@ struct rt_sigframe {
* altivec/spe instructions at some point.
*/
static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
- struct mcontext __user *tm_frame, int sigret,
- int ctx_has_vsx_region)
+ struct mcontext __user *tm_frame, int ctx_has_vsx_region)
{
unsigned long msr = regs->msr;

@@ -320,15 +316,6 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
if (tm_frame && __put_user(0, &tm_frame->mc_gregs[PT_MSR]))
return 1;

- if (sigret) {
- /* Set up the sigreturn trampoline: li 0,sigret; sc */
- if (__put_user(PPC_INST_ADDI + sigret, &frame->tramp[0])
- || __put_user(PPC_INST_SC, &frame->tramp[1]))
- return 1;
- flush_icache_range((unsigned long) &frame->tramp[0],
- (unsigned long) &frame->tramp[2]);
- }
-
return 0;
}

@@ -342,10 +329,8 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
*
* See save_user_regs() and signal_64.c:setup_tm_sigcontexts().
*/
-static int save_tm_user_regs(struct pt_regs *regs,
- struct mcontext __user *frame,
- struct mcontext __user *tm_frame, int sigret,
- unsigned long msr)
+static int save_tm_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
+ struct mcontext __user *tm_frame, unsigned long msr)
{
WARN_ON(tm_suspend_disabled);

@@ -461,14 +446,6 @@ static int save_tm_user_regs(struct pt_regs *regs,

if (__put_user(msr, &frame->mc_gregs[PT_MSR]))
return 1;
- if (sigret) {
- /* Set up the sigreturn trampoline: li 0,sigret; sc */
- if (__put_user(PPC_INST_ADDI + sigret, &frame->tramp[0])
- || __put_user(PPC_INST_SC, &frame->tramp[1]))
- return 1;
- flush_icache_range((unsigned long) &frame->tramp[0],
- (unsigned long) &frame->tramp[2]);
- }

return 0;
}
@@ -755,7 +732,6 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
struct mcontext __user *mctx;
struct mcontext __user *tm_mctx = NULL;
unsigned long newsp = 0;
- int sigret;
unsigned long tramp;
struct pt_regs *regs = tsk->thread.regs;
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
@@ -782,11 +758,15 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,

/* Save user registers on the stack */
if (vdso32_rt_sigtramp && tsk->mm->context.vdso_base) {
- sigret = 0;
tramp = tsk->mm->context.vdso_base + vdso32_rt_sigtramp;
} else {
- sigret = __NR_rt_sigreturn;
- tramp = (unsigned long)mctx->tramp;
+ tramp = (unsigned long)mctx->mc_pad;
+ /* Set up the sigreturn trampoline: li r0,sigret; sc */
+ if (__put_user(PPC_INST_ADDI + __NR_sigreturn, &mctx->mc_pad[0]))
+ goto badframe;
+ if (__put_user(PPC_INST_SC, &mctx->mc_pad[1]))
+ goto badframe;
+ flush_icache_range(tramp, tramp + 2 * sizeof(unsigned long));
}

#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
@@ -796,7 +776,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
__put_user((unsigned long)tm_mctx,
&frame->uc_transact.uc_regs))
goto badframe;
- if (save_tm_user_regs(regs, mctx, tm_mctx, sigret, msr))
+ if (save_tm_user_regs(regs, mctx, tm_mctx, msr))
goto badframe;
}
else
@@ -804,7 +784,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
{
if (__put_user(0, &frame->uc.uc_link))
goto badframe;
- if (save_user_regs(regs, mctx, tm_mctx, sigret, 1))
+ if (save_user_regs(regs, mctx, tm_mctx, 1))
goto badframe;
}
regs->link = tramp;
@@ -847,7 +827,6 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
struct mcontext __user *mctx;
struct mcontext __user *tm_mctx = NULL;
unsigned long newsp = 0;
- int sigret;
unsigned long tramp;
struct pt_regs *regs = tsk->thread.regs;
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
@@ -880,22 +859,26 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
goto badframe;

if (vdso32_sigtramp && tsk->mm->context.vdso_base) {
- sigret = 0;
tramp = tsk->mm->context.vdso_base + vdso32_sigtramp;
} else {
- sigret = __NR_sigreturn;
- tramp = (unsigned long)mctx->tramp;
+ tramp = (unsigned long)mctx->mc_pad;
+ /* Set up the sigreturn trampoline: li r0,sigret; sc */
+ if (__put_user(PPC_INST_ADDI + __NR_sigreturn, &mctx->mc_pad[0]))
+ goto badframe;
+ if (__put_user(PPC_INST_SC, &mctx->mc_pad[1]))
+ goto badframe;
+ flush_icache_range(tramp, tramp + 2 * sizeof(unsigned long));
}

#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
if (MSR_TM_ACTIVE(msr)) {
- if (save_tm_user_regs(regs, mctx, tm_mctx, sigret, msr))
+ if (save_tm_user_regs(regs, mctx, tm_mctx, msr))
goto badframe;
}
else
#endif
{
- if (save_user_regs(regs, mctx, tm_mctx, sigret, 1))
+ if (save_user_regs(regs, mctx, tm_mctx, 1))
goto badframe;
}

@@ -1047,7 +1030,7 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
mctx = (struct mcontext __user *)
((unsigned long) &old_ctx->uc_mcontext & ~0xfUL);
if (!access_ok(old_ctx, ctx_size)
- || save_user_regs(regs, mctx, NULL, 0, ctx_has_vsx_region)
+ || save_user_regs(regs, mctx, NULL, ctx_has_vsx_region)
|| put_sigset_t(&old_ctx->uc_sigmask, &current->blocked)
|| __put_user(to_user_ptr(mctx), &old_ctx->uc_regs))
return -EFAULT;
--
2.25.0

2020-12-10 14:34:52

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v2 00/25] powerpc: Switch signal 32 to using unsafe_put_user() and friends

On Tue, 18 Aug 2020 17:19:11 +0000 (UTC), Christophe Leroy wrote:
> This series leads to a reduction from 2.55s to 1.73s of
> the system CPU time with the following microbench app
> on an mpc832x with KUAP (approx 32%)
>
> This series replaces copies to users by unsafe_put_user() and friends
> with user_write_access_begin() dance in signal32.
>
> [...]

Applied to powerpc/next.

[01/25] powerpc/signal: Move inline functions in signal.h
https://git.kernel.org/powerpc/c/95593e930d7d067ca9bbee996c845248930a01f9
[02/25] powerpc/ptrace: Move declaration of ptrace_get_reg() and ptrace_set_reg()
https://git.kernel.org/powerpc/c/67e364b3295f9dbf3b820d0edde86fb7c95efc98
[03/25] powerpc/ptrace: Consolidate reg index calculation
https://git.kernel.org/powerpc/c/e009fa433542cd09d6279e361b767a1f44ffd29a
[04/25] powerpc/ptrace: Create ptrace_get_fpr() and ptrace_put_fpr()
https://git.kernel.org/powerpc/c/4d90eb97e292c7b14de8ba59fded35b340c73101
[05/25] powerpc/signal: Don't manage floating point regs when no FPU
https://git.kernel.org/powerpc/c/b6254ced4da6cf28d49fbffe24ee4b3286dcb3f4
[06/25] powerpc/32s: Allow deselecting CONFIG_PPC_FPU on mpc832x
https://git.kernel.org/powerpc/c/7d68c89169508064c460a1208f38ed0589d226fa
[07/25] powerpc/signal: Remove BUG_ON() in handler_signal functions
https://git.kernel.org/powerpc/c/3fcfb5d1bf731bdbd847c29df57a5372d8ea58d3
[08/25] powerpc/signal: Move access_ok() out of get_sigframe()
https://git.kernel.org/powerpc/c/454b1abb588b3942655638a8bcf1ea4501260579
[09/25] powerpc/signal: Remove get_clean_sp()
https://git.kernel.org/powerpc/c/0ecbc6ad18e324012234183e21805423f5e0cc79
[10/25] powerpc/signal: Call get_tm_stackpointer() from get_sigframe()
https://git.kernel.org/powerpc/c/c180cb305c9bba094657259487d563c8fbfb648b
[11/25] powerpc/signal: Refactor bad frame logging
https://git.kernel.org/powerpc/c/7fe8f773ee248c726cec2addcdb94056049d6e34
[12/25] powerpc/signal32: Simplify logging in handle_rt_signal32()
https://git.kernel.org/powerpc/c/debf122c777f361137a3114db7be8aecc65f6af2
[13/25] powerpc/signal32: Move handle_signal32() close to handle_rt_signal32()
https://git.kernel.org/powerpc/c/3eea688be0ccba2221e047b7df6f9ae87361cdd6
[14/25] powerpc/signal32: Rename local pointers in handle_rt_signal32()
https://git.kernel.org/powerpc/c/8e91cf8501f14d8b6727c71c98fd743e95e9b402
[15/25] powerpc/signal32: Misc changes to make handle_[rt_]_signal32() more similar
https://git.kernel.org/powerpc/c/91b8ecd419cb46058e99b3a574184883c02b7729
[16/25] powerpc/signal32: Move signal trampoline setup to handle_[rt_]signal32
https://git.kernel.org/powerpc/c/8d33001dd650b88e915a1a13e2ca807350e374df
[17/25] powerpc/signal32: Switch handle_signal32() to user_access_begin() logic
https://git.kernel.org/powerpc/c/ad65f4909fd3736d84533784cd9ab76905536b34
[18/25] powerpc/signal32: Switch handle_rt_signal32() to user_access_begin() logic
https://git.kernel.org/powerpc/c/9504db3e90b22dca19d8152ed5a82c68512dac0e
[19/25] powerpc/signal32: Remove ifdefery in middle of if/else
https://git.kernel.org/powerpc/c/f1cf4f93de2ff66313a091320d7683735816a0bc
[20/25] signal: Add unsafe_put_compat_sigset()
https://git.kernel.org/powerpc/c/14026b94ccfe626e512bc9fa01e0e72ee75c7a98
[21/25] powerpc/signal32: Add and use unsafe_put_sigset_t()
https://git.kernel.org/powerpc/c/de781ebdf6b8a256742da4fd6b0e39bb22ed9fe3
[22/25] powerpc/signal32: Switch swap_context() to user_access_begin() logic
https://git.kernel.org/powerpc/c/31147d7d6133ea17504b118114a191a8af85f3de
[23/25] powerpc/signal: Create 'unsafe' versions of copy_[ck][fpr/vsx]_to_user()
https://git.kernel.org/powerpc/c/b3484a1d4d1fb54ad7b615a13003d8bc11919c96
[24/25] powerpc/signal32: Isolate non-copy actions in save_user_regs() and save_tm_user_regs()
https://git.kernel.org/powerpc/c/968c4fccd1bb8b440326dac5078ad87d17af4a47
[25/25] powerpc/signal32: Transform save_user_regs() and save_tm_user_regs() in 'unsafe' version
https://git.kernel.org/powerpc/c/ef75e73182949a94bde169a774de1b62ae21fbbc

cheers