----- On May 24, 2018, at 3:03 AM, Michael Ellerman mpe@xxxxxxxxxxxxxx wrote: > Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> writes: >> ----- On May 23, 2018, at 4:14 PM, Mathieu Desnoyers >> mathieu.desnoyers@xxxxxxxxxxxx wrote: > ... >>> >>> Hi Boqun, >>> >>> I tried your patch in a ppc64 le environment, and it does not survive boot >>> with CONFIG_DEBUG_RSEQ=y. init gets killed right away. > > > Sorry this code is super gross and hard to deal with. > >> The following fixup gets ppc64 to work: >> >> --- a/arch/powerpc/kernel/entry_64.S >> +++ b/arch/powerpc/kernel/entry_64.S >> @@ -208,6 +208,7 @@ system_call_exit: >> /* Check whether the syscall is issued inside a restartable sequence */ >> addi r3,r1,STACK_FRAME_OVERHEAD >> bl rseq_syscall >> + ld r3,RESULT(r1) >> #endif >> /* >> * Disable interrupts so current_thread_info()->flags can't change, > > I don't think that's safe. > > If you look above that, we have r3, r8 and r12 all live: > > .Lsyscall_exit: > std r3,RESULT(r1) > CURRENT_THREAD_INFO(r12, r1) > > ld r8,_MSR(r1) > #ifdef CONFIG_PPC_BOOK3S > /* No MSR:RI on BookE */ > andi. r10,r8,MSR_RI > beq- .Lunrecov_restore > #endif > > > They're all volatile across function calls: > > http://openpowerfoundation.org/wp-content/uploads/resources/leabi/content/dbdoclet.50655240_68174.html > > > The system_call_exit symbol is actually there for kprobes and cosmetic > purposes. The actual syscall return flow starts at .Lsyscall_exit. > > So I think this would work: > > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S > index db4df061c33a..e19f377a25e0 100644 > --- a/arch/powerpc/kernel/entry_64.S > +++ b/arch/powerpc/kernel/entry_64.S > @@ -184,6 +184,14 @@ system_call: /* label this so stack traces look sane */ > > .Lsyscall_exit: > std r3,RESULT(r1) > + > +#ifdef CONFIG_DEBUG_RSEQ > + /* Check whether the syscall is issued inside a restartable sequence */ > + addi r3,r1,STACK_FRAME_OVERHEAD > + bl rseq_syscall > + ld r3,RESULT(r1) > +#endif > + > CURRENT_THREAD_INFO(r12, r1) > > ld r8,_MSR(r1) > > > I'll try and get this series into my test setup at some point, been a > bit busy lately :) Yes, this was needed. I had this in my tree already, but there is still a kernel OOPS when running the rseq selftests on ppc64 with CONFIG_DEBUG_RSEQ=y. My current dev tree is at: https://github.com/compudj/linux-percpu-dev/tree/rseq/dev-local So considering we are at rc7 now, should I plan to removing the powerpc bits for merge window submission, or is there someone planning to spend time on fixing and testing ppc integration before the merge window opens ? Thanks, Mathieu > > cheers -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
