diff mbox

Missing operand for tlbie instruction on Power7

Message ID CAOJe8K2GOsBk=CcWaZqq9KN=McLad97BP6ZhEdRB7i2Ag4csOg@mail.gmail.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Denis Kirjanov Oct. 2, 2015, 9:37 p.m. UTC
On 10/2/15, Peter Bergner <bergner@vnet.ibm.com> wrote:
> On Fri, 2015-10-02 at 22:03 +0300, Denis Kirjanov wrote:
>> arch/powerpc/kernel/swsusp_asm64.S: Assembler messages:
>>> arch/powerpc/kernel/swsusp_asm64.S:188: Error: missing operand
>>> scripts/Makefile.build:294: recipe for target
>>> 'arch/powerpc/kernel/swsusp_asm64.o' failed
>>> make[1]: *** [arch/powerpc/kernel/swsusp_asm64.o] Error 1
>>> Makefile:941: recipe for target 'arch/powerpc/kernel' failed
>>> make: *** [arch/powerpc/kernel] Error 2
> [snip]
>>> I don't know enough ppc assembly to properly fix this but I can test.
>>
>> Could you please test the patch attached?
> [snip]
>> -0:     tlbie   r4;                             \
>> +0:     tlbie   r4, 0;                          \
>
> This isn't correct.  With POWER7 and later (which this compile
> is, since it's on LE), the tlbie instruction takes two register
> operands:
>
>     tlbie RB, RS
>
> The tlbie instruction on pre POWER7 cpus had one required register
> operand (RB) and an optional second L operand, where if you omitted
> it, it was the same as using "0":
>
>     tlbie RB, L
>
> This is a POWER7 and later build, so your change which adds the "0"
> above is really adding r0 for RS.  The new tlbie instruction doesn't
> treat r0 specially, so you'll be using whatever random bits which
> happen to be in r0 which I don't think that is what you want.

Ok, than we can just zero out r5 for example and use it in tlbie as RS,
right?


>
>
> Peter
>
>
>
>

Comments

Segher Boessenkool Oct. 2, 2015, 10 p.m. UTC | #1
On Sat, Oct 03, 2015 at 12:37:35AM +0300, Denis Kirjanov wrote:
> >> -0:     tlbie   r4;                             \
> >> +0:     tlbie   r4, 0;                          \
> >
> > This isn't correct.  With POWER7 and later (which this compile
> > is, since it's on LE), the tlbie instruction takes two register
> > operands:
> >
> >     tlbie RB, RS
> >
> > The tlbie instruction on pre POWER7 cpus had one required register
> > operand (RB) and an optional second L operand, where if you omitted
> > it, it was the same as using "0":
> >
> >     tlbie RB, L
> >
> > This is a POWER7 and later build, so your change which adds the "0"
> > above is really adding r0 for RS.  The new tlbie instruction doesn't
> > treat r0 specially, so you'll be using whatever random bits which
> > happen to be in r0 which I don't think that is what you want.
> 
> Ok, than we can just zero out r5 for example and use it in tlbie as RS,
> right?

That won't assemble _unless_ your assembler is in POWER7 mode.  It also
won't do the right thing at run time on older machines.

Where is this tlbia macro used at all, for 64-bit machines?


Segher
Laura Abbott Oct. 2, 2015, 10:12 p.m. UTC | #2
On 10/02/2015 03:00 PM, Segher Boessenkool wrote:
> On Sat, Oct 03, 2015 at 12:37:35AM +0300, Denis Kirjanov wrote:
>>>> -0:     tlbie   r4;                             \
>>>> +0:     tlbie   r4, 0;                          \
>>>
>>> This isn't correct.  With POWER7 and later (which this compile
>>> is, since it's on LE), the tlbie instruction takes two register
>>> operands:
>>>
>>>      tlbie RB, RS
>>>
>>> The tlbie instruction on pre POWER7 cpus had one required register
>>> operand (RB) and an optional second L operand, where if you omitted
>>> it, it was the same as using "0":
>>>
>>>      tlbie RB, L
>>>
>>> This is a POWER7 and later build, so your change which adds the "0"
>>> above is really adding r0 for RS.  The new tlbie instruction doesn't
>>> treat r0 specially, so you'll be using whatever random bits which
>>> happen to be in r0 which I don't think that is what you want.
>>
>> Ok, than we can just zero out r5 for example and use it in tlbie as RS,
>> right?
>
> That won't assemble _unless_ your assembler is in POWER7 mode.  It also
> won't do the right thing at run time on older machines.
>
> Where is this tlbia macro used at all, for 64-bit machines?
>


[labbott@labbott-redhat-machine linux_upstream]$ make ARCH=powerpc CROSS_COMPILE=powerpc64-linux-gnu-
   CHK     include/config/kernel.release
   CHK     include/generated/uapi/linux/version.h
   CHK     include/generated/utsrelease.h
   CHK     include/generated/bounds.h
   CHK     include/generated/timeconst.h
   CHK     include/generated/asm-offsets.h
   CALL    scripts/checksyscalls.sh
   CHK     include/generated/compile.h
   CALL    arch/powerpc/kernel/systbl_chk.sh
   AS      arch/powerpc/kernel/swsusp_asm64.o
arch/powerpc/kernel/swsusp_asm64.S: Assembler messages:
arch/powerpc/kernel/swsusp_asm64.S:188: Error: missing operand
scripts/Makefile.build:294: recipe for target 'arch/powerpc/kernel/swsusp_asm64.o' failed
make[1]: *** [arch/powerpc/kernel/swsusp_asm64.o] Error 1
Makefile:941: recipe for target 'arch/powerpc/kernel' failed
make: *** [arch/powerpc/kernel] Error 2

This is piece of code protected by CONFIG_PPC_BOOK3S_64.
  
>
> Segher
>
Peter Bergner Oct. 3, 2015, 2:24 a.m. UTC | #3
On Fri, 2015-10-02 at 17:00 -0500, Segher Boessenkool wrote:
> On Sat, Oct 03, 2015 at 12:37:35AM +0300, Denis Kirjanov wrote:
> > >> -0:     tlbie   r4;                             \
> > >> +0:     tlbie   r4, 0;                          \
> > >
> > > This isn't correct.  With POWER7 and later (which this compile
> > > is, since it's on LE), the tlbie instruction takes two register
> > > operands:
> > >
> > >     tlbie RB, RS
> > >
> > > The tlbie instruction on pre POWER7 cpus had one required register
> > > operand (RB) and an optional second L operand, where if you omitted
> > > it, it was the same as using "0":
> > >
> > >     tlbie RB, L
> > >
> > > This is a POWER7 and later build, so your change which adds the "0"
> > > above is really adding r0 for RS.  The new tlbie instruction doesn't
> > > treat r0 specially, so you'll be using whatever random bits which
> > > happen to be in r0 which I don't think that is what you want.
> > 
> > Ok, than we can just zero out r5 for example and use it in tlbie as RS,
> > right?
> 
> That won't assemble _unless_ your assembler is in POWER7 mode.  It also
> won't do the right thing at run time on older machines.

Correct, getting this to work on both pre-power7 and power7 and later
is tricky.  One really horrible hack would be to do:

  li r0,0
  tlbie r4,0

On pre-power7, the "0" will be taken as a zero L operand and on
power7 and later, it'll be r0, but with a zero value we loaded in
the insn before.  I know, really ugly. :-)

Peter
Segher Boessenkool Oct. 4, 2015, midnight UTC | #4
On Fri, Oct 02, 2015 at 09:24:46PM -0500, Peter Bergner wrote:
> > > Ok, than we can just zero out r5 for example and use it in tlbie as RS,
> > > right?
> > 
> > That won't assemble _unless_ your assembler is in POWER7 mode.  It also
> > won't do the right thing at run time on older machines.
> 
> Correct, getting this to work on both pre-power7 and power7 and later
> is tricky.  One really horrible hack would be to do:
> 
>   li r0,0
>   tlbie r4,0
> 
> On pre-power7, the "0" will be taken as a zero L operand and on
> power7 and later, it'll be r0, but with a zero value we loaded in
> the insn before.  I know, really ugly. :-)

Hide the "li 0,0" somewhere earlier, and write it as "tlbie 4,0", and
don't write a comment -- we *like* tricky!

It should really be a separate macro define for power7 and 4xx etc.;
and the macro should not be called "tlbia", but something that makes
it obvious at the usage sites that it is in fact a macro; and why a
macro anyway, a function call might be better here?


Segher
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index dd0fc18..cb0f627 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -443,9 +443,10 @@  END_FTR_SECTION_NESTED(CPU_FTR_HAS_PPR,CPU_FTR_HAS_PPR,945)
 #if !defined(CONFIG_4xx) && !defined(CONFIG_8xx)
 #define tlbia					\
 	li	r4,1024;			\
+	li  r5,0;				\
 	mtctr	r4;				\
 	lis	r4,KERNELBASE@h;		\
-0:	tlbie	r4;				\
+0:	tlbie	r4, r5;				\
 	addi	r4,r4,0x1000;			\
 	bdnz	0b
 #endif