lkml.org 
[lkml]   [2018]   [May]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: arm64: add missing early clobber in atomic64_dec_if_positive()
On Mon, May 21, 2018 at 01:18:39PM -0400, Mark Salter wrote:
> On Mon, 2018-05-21 at 18:00 +0100, Will Deacon wrote:
> > Hi Mark,
> >
> > Thanks for reporting this.
> >
> > On Sat, May 19, 2018 at 08:17:26PM -0400, Mark Salter wrote:
> > > When running a kernel compiled with gcc8 on a machine using LSE, I
> > > get:
> > >
> > > Unable to handle kernel paging request at virtual address 11111122222221
> >
> > [...]
> >
> > > The fault happens at the casal insn of inlined atomic64_dec_if_positive().
> > > The inline asm code in that function has:
> > >
> > > "1: ldr x30, %[v]\n"
> > > " subs %[ret], x30, #1\n"
> > > " b.lt 2f\n"
> > > " casal x30, %[ret], %[v]\n"
> > > " sub x30, x30, #1\n"
> > > " sub x30, x30, %[ret]\n"
> > > " cbnz x30, 1b\n"
> > > "2:")
> > > : [ret] "+r" (x0), [v] "+Q" (v->counter)
> > >
> > > gcc8 used register x0 for both [ret] and [v] and the subs was
> > > clobbering [v] before it was used for casal. Gcc is free to do
> > > this because [ret] lacks an early clobber modifier. So add one
> > > to tell gcc a separate register is needed for [v].
> >
> > Oh blimey, it looks like GCC is realising that counter is at offset 0
> > of atomic_t and therefore assigns the same register for [ret] and [v],
> > which is actually forced to be x0 by the 'register' local variable in
> > C code. The "+Q" constraint only says that the memory is read/write, so
> > the pointer is fair game.
> >
> > I agree with your fix, but we also need to fix up the other places relying
> > on this. Patch below -- please yell if you think I missed any.
>
> I looked at the other places but figured they were okay because we're
> explicitly using separate registers. But I suppose the early clobber
> is the right thing to do in any case.

I was worried about silly things like a caller doing:

atomic64_and((long)v, v);

and then GCC figuring out that the two values were equal and allocating
the same register..

Will

\
 
 \ /
  Last update: 2018-05-21 19:21    [W:0.046 / U:1.260 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site