From: Boqun Feng <boqun.feng@gmail.com> To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Peter Zijlstra <peterz@infradead.org>, "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>, Andy Lutomirski <luto@amacapital.net>, Dave Watson <davejwatson@fb.com>, linux-kernel <linux-kernel@vger.kernel.org>, linux-api <linux-api@vger.kernel.org>, Paul Turner <pjt@google.com>, Andrew Morton <akpm@linux-foundation.org>, Russell King <linux@arm.linux.org.uk>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>, Andrew Hunter <ahh@google.com>, Andi Kleen <andi@firstfloor.org>, Chris Lameter <cl@linux.com>, Ben Maurer <bmaurer@fb.com>, rostedt <rostedt@goodmis.org>, Josh Triplett <josh@joshtriplett.org>, Linus Torvalds <torvalds@linux-foundation.org>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, Michael Kerrisk <mtk.manpages@gmail.com>, Joel Fernandes <joelaf@google.com>, Benjamin Herrenschmidt <benh@kernel.crashing.org>, Paul Mackerras <paulus@samba.org>, Michael Ellerman <mpe@ellerman.id.au>, linuxppc-dev <linuxppc-dev@lists.ozlabs.org> Subject: Re: [PATCH 07/14] powerpc: Add support for restartable sequences Date: Thu, 17 May 2018 09:19:49 +0800 [thread overview] Message-ID: <20180517011949.GA1121@tardis> (raw) In-Reply-To: <112970629.1913.1526501596485.JavaMail.zimbra@efficios.com> [-- Attachment #1: Type: text/plain, Size: 3578 bytes --] On Wed, May 16, 2018 at 04:13:16PM -0400, Mathieu Desnoyers wrote: > ----- On May 16, 2018, at 12:18 PM, Peter Zijlstra peterz@infradead.org wrote: > > > On Mon, Apr 30, 2018 at 06:44:26PM -0400, Mathieu Desnoyers wrote: > >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > >> index c32a181a7cbb..ed21a777e8c6 100644 > >> --- a/arch/powerpc/Kconfig > >> +++ b/arch/powerpc/Kconfig > >> @@ -223,6 +223,7 @@ config PPC > >> select HAVE_SYSCALL_TRACEPOINTS > >> select HAVE_VIRT_CPU_ACCOUNTING > >> select HAVE_IRQ_TIME_ACCOUNTING > >> + select HAVE_RSEQ > >> select IRQ_DOMAIN > >> select IRQ_FORCED_THREADING > >> select MODULES_USE_ELF_RELA > >> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c > >> index 61db86ecd318..d3bb3aaaf5ac 100644 > >> --- a/arch/powerpc/kernel/signal.c > >> +++ b/arch/powerpc/kernel/signal.c > >> @@ -133,6 +133,8 @@ static void do_signal(struct task_struct *tsk) > >> /* Re-enable the breakpoints for the signal stack */ > >> thread_change_pc(tsk, tsk->thread.regs); > >> > >> + rseq_signal_deliver(tsk->thread.regs); > >> + > >> if (is32) { > >> if (ksig.ka.sa.sa_flags & SA_SIGINFO) > >> ret = handle_rt_signal32(&ksig, oldset, tsk); > >> @@ -164,6 +166,7 @@ void do_notify_resume(struct pt_regs *regs, unsigned long > >> thread_info_flags) > >> if (thread_info_flags & _TIF_NOTIFY_RESUME) { > >> clear_thread_flag(TIF_NOTIFY_RESUME); > >> tracehook_notify_resume(regs); > >> + rseq_handle_notify_resume(regs); > >> } > >> > >> user_enter(); > > > > Again no rseq_syscall(). > > Same question for PowerPC as for ARM: > > Considering that rseq_syscall is implemented as follows: > > +void rseq_syscall(struct pt_regs *regs) > +{ > + unsigned long ip = instruction_pointer(regs); > + struct task_struct *t = current; > + struct rseq_cs rseq_cs; > + > + if (!t->rseq) > + return; > + if (!access_ok(VERIFY_READ, t->rseq, sizeof(*t->rseq)) || > + rseq_get_rseq_cs(t, &rseq_cs) || in_rseq_cs(ip, &rseq_cs)) > + force_sig(SIGSEGV, t); > +} > > and that x86 calls it from syscall_return_slowpath() (which AFAIU is > now used in the fast-path since KPTI), I wonder where we should call So we actually detect this after the syscall takes effect, right? I wonder whether this could be problematic, because "disallowing syscall" in rseq areas may means the syscall won't take effect to some people, I guess? > this on PowerPC ? I was under the impression that PowerPC return to > userspace fast-path was not calling C code unless work flags were set, > but I might be wrong. > I think you're right. So we have to introduce callsite to rseq_syscall() in syscall path, something like: diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S index 51695608c68b..a25734a96640 100644 --- a/arch/powerpc/kernel/entry_64.S +++ b/arch/powerpc/kernel/entry_64.S @@ -222,6 +222,9 @@ system_call_exit: mtmsrd r11,1 #endif /* CONFIG_PPC_BOOK3E */ + addi r3,r1,STACK_FRAME_OVERHEAD + bl rseq_syscall + ld r9,TI_FLAGS(r12) li r11,-MAX_ERRNO andi. r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK) But I think it's important for us to first decide where (before or after the syscall) we do the detection. Regards, Boqun > Thoughts ? > > Thanks! > > Mathieu > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Boqun Feng <boqun.feng@gmail.com> To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Peter Zijlstra <peterz@infradead.org>, "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>, Andy Lutomirski <luto@amacapital.net>, Dave Watson <davejwatson@fb.com>, linux-kernel <linux-kernel@vger.kernel.org>, linux-api <linux-api@vger.kernel.org>, Paul Turner <pjt@google.com>, Andrew Morton <akpm@linux-foundation.org>, Russell King <linux@arm.linux.org.uk>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>, Andrew Hunter <ahh@google.com>, Andi Kleen <andi@firstfloor.org>, Chris Lameter <cl@linux.com>, Ben Maurer <bmaurer@fb.com>, rostedt <rostedt@goodmis.org>, Josh Triplett <josh@joshtriplett.org>, Linus Torvalds <torvalds@linux-foundation.org>, Catalin Marinas <catalin.marinas@arm.com> Subject: Re: [PATCH 07/14] powerpc: Add support for restartable sequences Date: Thu, 17 May 2018 09:19:49 +0800 [thread overview] Message-ID: <20180517011949.GA1121@tardis> (raw) In-Reply-To: <112970629.1913.1526501596485.JavaMail.zimbra@efficios.com> [-- Attachment #1: Type: text/plain, Size: 3578 bytes --] On Wed, May 16, 2018 at 04:13:16PM -0400, Mathieu Desnoyers wrote: > ----- On May 16, 2018, at 12:18 PM, Peter Zijlstra peterz@infradead.org wrote: > > > On Mon, Apr 30, 2018 at 06:44:26PM -0400, Mathieu Desnoyers wrote: > >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > >> index c32a181a7cbb..ed21a777e8c6 100644 > >> --- a/arch/powerpc/Kconfig > >> +++ b/arch/powerpc/Kconfig > >> @@ -223,6 +223,7 @@ config PPC > >> select HAVE_SYSCALL_TRACEPOINTS > >> select HAVE_VIRT_CPU_ACCOUNTING > >> select HAVE_IRQ_TIME_ACCOUNTING > >> + select HAVE_RSEQ > >> select IRQ_DOMAIN > >> select IRQ_FORCED_THREADING > >> select MODULES_USE_ELF_RELA > >> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c > >> index 61db86ecd318..d3bb3aaaf5ac 100644 > >> --- a/arch/powerpc/kernel/signal.c > >> +++ b/arch/powerpc/kernel/signal.c > >> @@ -133,6 +133,8 @@ static void do_signal(struct task_struct *tsk) > >> /* Re-enable the breakpoints for the signal stack */ > >> thread_change_pc(tsk, tsk->thread.regs); > >> > >> + rseq_signal_deliver(tsk->thread.regs); > >> + > >> if (is32) { > >> if (ksig.ka.sa.sa_flags & SA_SIGINFO) > >> ret = handle_rt_signal32(&ksig, oldset, tsk); > >> @@ -164,6 +166,7 @@ void do_notify_resume(struct pt_regs *regs, unsigned long > >> thread_info_flags) > >> if (thread_info_flags & _TIF_NOTIFY_RESUME) { > >> clear_thread_flag(TIF_NOTIFY_RESUME); > >> tracehook_notify_resume(regs); > >> + rseq_handle_notify_resume(regs); > >> } > >> > >> user_enter(); > > > > Again no rseq_syscall(). > > Same question for PowerPC as for ARM: > > Considering that rseq_syscall is implemented as follows: > > +void rseq_syscall(struct pt_regs *regs) > +{ > + unsigned long ip = instruction_pointer(regs); > + struct task_struct *t = current; > + struct rseq_cs rseq_cs; > + > + if (!t->rseq) > + return; > + if (!access_ok(VERIFY_READ, t->rseq, sizeof(*t->rseq)) || > + rseq_get_rseq_cs(t, &rseq_cs) || in_rseq_cs(ip, &rseq_cs)) > + force_sig(SIGSEGV, t); > +} > > and that x86 calls it from syscall_return_slowpath() (which AFAIU is > now used in the fast-path since KPTI), I wonder where we should call So we actually detect this after the syscall takes effect, right? I wonder whether this could be problematic, because "disallowing syscall" in rseq areas may means the syscall won't take effect to some people, I guess? > this on PowerPC ? I was under the impression that PowerPC return to > userspace fast-path was not calling C code unless work flags were set, > but I might be wrong. > I think you're right. So we have to introduce callsite to rseq_syscall() in syscall path, something like: diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S index 51695608c68b..a25734a96640 100644 --- a/arch/powerpc/kernel/entry_64.S +++ b/arch/powerpc/kernel/entry_64.S @@ -222,6 +222,9 @@ system_call_exit: mtmsrd r11,1 #endif /* CONFIG_PPC_BOOK3E */ + addi r3,r1,STACK_FRAME_OVERHEAD + bl rseq_syscall + ld r9,TI_FLAGS(r12) li r11,-MAX_ERRNO andi. r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK) But I think it's important for us to first decide where (before or after the syscall) we do the detection. Regards, Boqun > Thoughts ? > > Thanks! > > Mathieu > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2018-05-17 1:15 UTC|newest] Thread overview: 105+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-04-30 22:44 [RFC PATCH for 4.18 00/14] Restartable Sequences Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 01/14] uapi headers: Provide types_32_64.h (v2) Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 02/14] rseq: Introduce restartable sequences system call (v13) Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-05-16 16:24 ` Peter Zijlstra 2018-05-16 16:24 ` Peter Zijlstra 2018-05-16 20:18 ` Mathieu Desnoyers 2018-05-16 20:18 ` Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 03/14] arm: Add restartable sequences support Mathieu Desnoyers 2018-05-16 16:18 ` Peter Zijlstra 2018-05-16 16:18 ` Peter Zijlstra 2018-05-16 20:13 ` Mathieu Desnoyers 2018-05-16 20:13 ` Mathieu Desnoyers 2018-05-17 13:32 ` Will Deacon 2018-05-17 13:32 ` Will Deacon 2018-05-17 15:30 ` Mathieu Desnoyers 2018-05-17 15:30 ` Mathieu Desnoyers 2018-05-22 18:19 ` Mathieu Desnoyers 2018-05-22 18:19 ` Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 04/14] arm: Wire up restartable sequences system call Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 05/14] x86: Add support for restartable sequences (v2) Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 06/14] x86: Wire up restartable sequence system call Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 07/14] powerpc: Add support for restartable sequences Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-05-16 16:18 ` Peter Zijlstra 2018-05-16 16:18 ` Peter Zijlstra 2018-05-16 20:13 ` Mathieu Desnoyers 2018-05-16 20:13 ` Mathieu Desnoyers 2018-05-17 1:19 ` Boqun Feng [this message] 2018-05-17 1:19 ` Boqun Feng 2018-05-17 1:19 ` Boqun Feng 2018-05-17 7:43 ` Peter Zijlstra 2018-05-17 7:43 ` Peter Zijlstra 2018-05-17 15:28 ` Mathieu Desnoyers 2018-05-17 15:28 ` Mathieu Desnoyers 2018-05-17 23:50 ` Boqun Feng 2018-05-17 23:50 ` Boqun Feng 2018-05-18 18:17 ` Mathieu Desnoyers 2018-05-18 18:17 ` Mathieu Desnoyers 2018-05-20 14:08 ` Boqun Feng 2018-05-20 14:08 ` Boqun Feng 2018-05-20 14:08 ` Boqun Feng 2018-05-23 20:14 ` Mathieu Desnoyers 2018-05-23 20:14 ` Mathieu Desnoyers 2018-05-23 20:46 ` Paul E. McKenney 2018-05-23 20:46 ` Paul E. McKenney 2018-05-23 21:29 ` Mathieu Desnoyers 2018-05-23 21:29 ` Mathieu Desnoyers 2018-05-24 1:03 ` Michael Ellerman 2018-05-24 1:03 ` Michael Ellerman 2018-05-28 7:00 ` Mathieu Desnoyers 2018-05-28 7:00 ` Mathieu Desnoyers 2018-05-18 12:38 ` Michael Ellerman 2018-05-18 12:38 ` Michael Ellerman 2018-04-30 22:44 ` [PATCH 08/14] powerpc: Wire up restartable sequences system call Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` [PATCH 09/14] selftests: lib.mk: Introduce OVERRIDE_TARGETS Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` mathieu.desnoyers 2018-04-30 22:44 ` [PATCH 10/14] rseq: selftests: Provide rseq library (v5) Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` mathieu.desnoyers 2018-04-30 22:44 ` [PATCH 11/14] rseq: selftests: Provide basic test Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` mathieu.desnoyers 2018-04-30 22:44 ` [PATCH 12/14] rseq: selftests: Provide basic percpu ops test (v2) Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` mathieu.desnoyers 2018-04-30 22:44 ` [PATCH 13/14] rseq: selftests: Provide parametrized tests (v2) Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` mathieu.desnoyers 2018-04-30 22:44 ` [PATCH 14/14] rseq: selftests: Provide Makefile, scripts, gitignore (v2) Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` Mathieu Desnoyers 2018-04-30 22:44 ` mathieu.desnoyers 2018-05-02 3:53 ` [RFC PATCH for 4.18 00/14] Restartable Sequences Daniel Colascione 2018-05-02 8:43 ` Peter Zijlstra 2018-05-02 16:03 ` Mathieu Desnoyers 2018-05-02 16:03 ` Mathieu Desnoyers 2018-05-02 16:07 ` Daniel Colascione 2018-05-02 16:42 ` Steven Rostedt 2018-05-02 16:55 ` Daniel Colascione 2018-05-03 16:12 ` Mathieu Desnoyers 2018-05-03 16:12 ` Mathieu Desnoyers 2018-05-03 16:22 ` Daniel Colascione 2018-05-03 18:04 ` Mathieu Desnoyers 2018-05-03 18:04 ` Mathieu Desnoyers 2018-05-03 16:48 ` Joel Fernandes 2018-05-03 17:18 ` Daniel Colascione 2018-05-03 17:46 ` Joel Fernandes 2018-05-03 17:46 ` Joel Fernandes 2018-05-04 22:17 ` Ben Maurer 2018-05-04 22:17 ` Ben Maurer 2018-05-02 17:22 ` Peter Zijlstra 2018-05-02 18:27 ` Daniel Colascione 2018-05-02 20:22 ` Peter Zijlstra 2018-05-02 20:37 ` Daniel Colascione 2018-05-03 1:15 ` Steven Rostedt 2018-05-03 8:49 ` Peter Zijlstra 2018-05-06 10:03 ` Thomas Gleixner
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20180517011949.GA1121@tardis \ --to=boqun.feng@gmail.com \ --cc=ahh@google.com \ --cc=akpm@linux-foundation.org \ --cc=andi@firstfloor.org \ --cc=benh@kernel.crashing.org \ --cc=bmaurer@fb.com \ --cc=catalin.marinas@arm.com \ --cc=cl@linux.com \ --cc=davejwatson@fb.com \ --cc=hpa@zytor.com \ --cc=joelaf@google.com \ --cc=josh@joshtriplett.org \ --cc=linux-api@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=luto@amacapital.net \ --cc=mathieu.desnoyers@efficios.com \ --cc=mingo@redhat.com \ --cc=mpe@ellerman.id.au \ --cc=mtk.manpages@gmail.com \ --cc=paulmck@linux.vnet.ibm.com \ --cc=paulus@samba.org \ --cc=peterz@infradead.org \ --cc=pjt@google.com \ --cc=rostedt@goodmis.org \ --cc=tglx@linutronix.de \ --cc=torvalds@linux-foundation.org \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.