Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 15 Jun 2018 11:43:17 -0700
From:   Paul Burton <paul.burton@mips.com>
To:     Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC:     linux-mips <linux-mips@linux-mips.org>,
        Peter Zijlstra <peterz@infradead.org>,
        James Hogan <jhogan@kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Boqun Feng <boqun.feng@gmail.com>,
        "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
        Ralf Baechle <ralf@linux-mips.org>
Subject: Re: [PATCH 2/4] MIPS: Add syscall detection for restartable sequences
Message-ID: <20180615184317.vogskh6rg53kfuzz@pburton-laptop>
References: <20180614235211.31357-1-paul.burton@mips.com>
 <20180614235211.31357-3-paul.burton@mips.com>
 <1881638448.14415.1529084485237.JavaMail.zimbra@efficios.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <1881638448.14415.1529084485237.JavaMail.zimbra@efficios.com>
User-Agent: NeoMutt/20180512
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Mathieu,

On Fri, Jun 15, 2018 at 01:41:25PM -0400, Mathieu Desnoyers wrote:
> > diff --git a/arch/mips/kernel/entry.S b/arch/mips/kernel/entry.S
> > index 38a302919e6b..d7de8adcfcc8 100644
> > --- a/arch/mips/kernel/entry.S
> > +++ b/arch/mips/kernel/entry.S
> > @@ -79,6 +79,10 @@ FEXPORT(ret_from_fork)
> > 	jal	schedule_tail		# a0 = struct task_struct *prev
> > 
> > FEXPORT(syscall_exit)
> > +#ifdef CONFIG_DEBUG_RSEQ
> > +	move	a0, sp
> > +	jal	rseq_syscall
> > +#endif
> > 	local_irq_disable		# make sure need_resched and
> > 					# signals dont change between
> > 					# sampling and return
> > @@ -141,6 +145,10 @@ work_notifysig:				# deal with pending signals and
> > 	j	resume_userspace_check
> > 
> > FEXPORT(syscall_exit_partial)
> > +#ifdef CONFIG_DEBUG_RSEQ
> > +	move	a0, sp
> > +	jal	rseq_syscall
> > +#endif
> > 	local_irq_disable		# make sure need_resched doesn't
> > 					# change between and return
> > 	LONG_L	a2, TI_FLAGS($28)	# current->work
> 
> Just to double-check: you did test with CONFIG_DEBUG_RSEQ=y, right ?

Yes, I did. Although I only ran the selftests, which I don't believe
would actually trigger the SIGSEGV condition.

Side-note: maybe it'd be useful to have a test that does intentionally
perform a syscall within a restartable sequence & checks that it
actually receives a SIGSEGV?.

> Are there any live registers that need to be saved before calling
> rseq_syscall ?

No - we just need gp/$28 & sp/$29, and the calling convention means
rseq_syscall() should return with those unmodified. Everything else that
we or userland care about is about to be loaded from the stack anyway.

Thanks,
    Paul