From: Laura Abbott <hidden> Date: 2015-10-02 15:43:35
Hi,
We received a report (https://bugzilla.redhat.com/show_bug.cgi?id=1267395) of bad assembly
when compiling on powerpc with little endian
[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 problem started happening after a binutils update:
[labbott@labbott-redhat-machine linux_upstream]$ powerpc64-linux-gnu-as --version
GNU assembler version 2.25.1-1.fc22
Copyright (C) 2014 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or later.
This program has absolutely no warranty.
This assembler was configured for a target of `powerpc64-linux-gnu'.
[labbott@labbott-redhat-machine linux_upstream]$
After some discussion with the binutils folks, it turns out that the tlbie
instruction actually requires another operand and binutils was updated to
check for this https://sourceware.org/ml/binutils/2015-05/msg00133.html .
The code sequence in arch/powerpc/include/asm/ppc_asm.h now needs to be updated:
#if !defined(CONFIG_4xx) && !defined(CONFIG_8xx)
#define tlbia \
li r4,1024; \
mtctr r4; \
lis r4,KERNELBASE@h; \
0: tlbie r4; \
addi r4,r4,0x1000; \
bdnz 0b
#endif
I don't know enough ppc assembly to properly fix this but I can test.
Thanks,
Laura
Hi,
We received a report (https://bugzilla.redhat.com/show_bug.cgi?id=1267395)
of bad assembly
when compiling on powerpc with little endian
[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 problem started happening after a binutils update:
[labbott@labbott-redhat-machine linux_upstream]$ powerpc64-linux-gnu-as
--version
GNU assembler version 2.25.1-1.fc22
Copyright (C) 2014 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or later.
This program has absolutely no warranty.
This assembler was configured for a target of `powerpc64-linux-gnu'.
[labbott@labbott-redhat-machine linux_upstream]$
After some discussion with the binutils folks, it turns out that the tlbie
instruction actually requires another operand and binutils was updated to
check for this https://sourceware.org/ml/binutils/2015-05/msg00133.html .
The code sequence in arch/powerpc/include/asm/ppc_asm.h now needs to be
updated:
#if !defined(CONFIG_4xx) && !defined(CONFIG_8xx)
#define tlbia \
li r4,1024; \
mtctr r4; \
lis r4,KERNELBASE@h; \
0: tlbie r4; \
addi r4,r4,0x1000; \
bdnz 0b
#endif
I don't know enough ppc assembly to properly fix this but I can test.
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.
Peter
I don't know enough ppc assembly to properly fix this but I can test.
Could you please test the patch attached?
[snip]
quoted
-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?
On Sat, Oct 03, 2015 at 12:37:35AM +0300, Denis Kirjanov wrote:
quoted
quoted
-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
From: Laura Abbott <hidden> Date: 2015-10-02 22:12:12
On 10/02/2015 03:00 PM, Segher Boessenkool wrote:
On Sat, Oct 03, 2015 at 12:37:35AM +0300, Denis Kirjanov wrote:
quoted
quoted
quoted
-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.
From: Peter Bergner <hidden> Date: 2015-10-03 02:25:56
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:
quoted
quoted
quoted
-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
On Fri, Oct 02, 2015 at 09:24:46PM -0500, Peter Bergner wrote:
quoted
quoted
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
From: Laura Abbott <hidden> Date: 2015-10-06 00:40:00
On 10/03/2015 05:00 PM, Segher Boessenkool wrote:
On Fri, Oct 02, 2015 at 09:24:46PM -0500, Peter Bergner wrote:
quoted
quoted
quoted
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
I can't speculate on why it is a macro but would something such as the
following work?
Alternatively, we could move the assembly into swusp_asm64.S which appears to be
the only 64-bit caller of tlbia
After some discussion with the binutils folks, it turns out that the tlbie
instruction actually requires another operand and binutils was updated to
check for this https://sourceware.org/ml/binutils/2015-05/msg00133.html .
The code sequence in arch/powerpc/include/asm/ppc_asm.h now needs to be updated:
#if !defined(CONFIG_4xx) && !defined(CONFIG_8xx)
#define tlbia \
li r4,1024; \
mtctr r4; \
lis r4,KERNELBASE@h; \
0: tlbie r4; \
addi r4,r4,0x1000; \
bdnz 0b
#endif
I don't know enough ppc assembly to properly fix this but I can test.
How are you testing? This code is fairly old and I'm dubious if it still works.
These days we have a ppc_md hook for flushing the TLB, ppc_md.flush_tlb().
Ideally the swsusp code would use that.
cheers
After some discussion with the binutils folks, it turns out that the tlbie
instruction actually requires another operand and binutils was updated to
check for this https://sourceware.org/ml/binutils/2015-05/msg00133.html .
The code sequence in arch/powerpc/include/asm/ppc_asm.h now needs to be updated:
#if !defined(CONFIG_4xx) && !defined(CONFIG_8xx)
#define tlbia \
li r4,1024; \
mtctr r4; \
lis r4,KERNELBASE@h; \
0: tlbie r4; \
addi r4,r4,0x1000; \
bdnz 0b
#endif
I don't know enough ppc assembly to properly fix this but I can test.
How are you testing? This code is fairly old and I'm dubious if it still works.
These days we have a ppc_md hook for flushing the TLB, ppc_md.flush_tlb().
Ideally the swsusp code would use that.
cheers
Testing would probably just be compile and maybe boot. I don't have regular
access to the hardware. This problem just showed up for me when someone
tried to compile Fedora rawhide with the latest binutils.
From what I can tell, it looks like the .flush_tlb of the cpu_spec is only
defined for power7 and power8 and I don't see a ppc_md.flush_tlb on the
master branch. It's not clear what to do for the case where there is no
flush_tlb function. Would filling in a .flush_tlb for all the PPC_BOOK3S_64
with the existing tlbia sequence work? It's also worth noting that the
__flush_power7 uses tlbiel instead of tlbie.
Thanks,
Laura
After some discussion with the binutils folks, it turns out that the tlbie
instruction actually requires another operand and binutils was updated to
check for this https://sourceware.org/ml/binutils/2015-05/msg00133.html .
The code sequence in arch/powerpc/include/asm/ppc_asm.h now needs to be updated:
#if !defined(CONFIG_4xx) && !defined(CONFIG_8xx)
#define tlbia \
li r4,1024; \
mtctr r4; \
lis r4,KERNELBASE@h; \
0: tlbie r4; \
addi r4,r4,0x1000; \
bdnz 0b
#endif
I don't know enough ppc assembly to properly fix this but I can test.
How are you testing? This code is fairly old and I'm dubious if it still works.
These days we have a ppc_md hook for flushing the TLB, ppc_md.flush_tlb().
Ideally the swsusp code would use that.
Testing would probably just be compile and maybe boot. I don't have regular
access to the hardware. This problem just showed up for me when someone
tried to compile Fedora rawhide with the latest binutils.
Right. The code in question is for software suspend, ie. hibernation, so that's
what needs testing if the code is going to change.
It was mostly written for G5 (543b9fd3528f6), though it later gained support
for 64-bit BookE (5a31057fc06c3).
I just tested it on a G5 here and amazingly it worked.
So it is working code, even if it is old and crufty.
From what I can tell, it looks like the .flush_tlb of the cpu_spec is only
defined for power7 and power8 and I don't see a ppc_md.flush_tlb on the
master branch.
Yes it's only defined for Power7 and Power8 at the moment. It definitely does
exist in Linus' master branch, but I'm not sure if that's the master branch
you're referring to.
It's not clear what to do for the case where there is no
flush_tlb function. Would filling in a .flush_tlb for all the PPC_BOOK3S_64
with the existing tlbia sequence work?
It might, but it's not much of an improvement. Ideally we'd have an actually
correct sequence for each cpu type.
It's also worth noting that the __flush_power7 uses tlbiel instead of tlbie.
Yeah that's a good point. It's not clear if the swsusp code wants to a local or
a global invalidate.
As an alternative, can you try adding a .machine push / .machine "power4" /
.machine pop, around the tlbie. That should tell the assembler to drop back to
power4 mode for that instruction, which should then do the right thing. There
are some examples in that file.
cheers
On Wed, Oct 07, 2015 at 05:00:49PM +1100, Michael Ellerman wrote:
quoted
It's also worth noting that the __flush_power7 uses tlbiel instead of tlbie.
Yeah that's a good point. It's not clear if the swsusp code wants to a local or
a global invalidate.
If I read the code right, this is called on the boot CPU when all the
non-boot CPUs are still (potentially) down, so if you would do a global
invalidate the non-boot CPUs might not even notice, so those need to do
a (local) invalidate after being brought up anyway? Or they probably
need it before being brought down at all? You figure it out, it makes
my brain hurt :-)
As an alternative, can you try adding a .machine push / .machine "power4" /
.machine pop, around the tlbie. That should tell the assembler to drop back to
power4 mode for that instruction, which should then do the right thing. There
are some examples in that file.
That will get the assembler to not complain, but it will assemble the wrong
instruction: the power7 instruction has the same opcode (but different
semantics). So if you assemble a "tlbie r4" in power4 mode, a newer CPU
will see it as a "tlbie r4,r0" and do the wrong thing.
Segher
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-10-07 09:13:37
On Wed, 2015-10-07 at 02:19 -0500, Segher Boessenkool wrote:
On Wed, Oct 07, 2015 at 05:00:49PM +1100, Michael Ellerman wrote:
quoted
quoted
It's also worth noting that the __flush_power7 uses tlbiel instead of tlbie.
Yeah that's a good point. It's not clear if the swsusp code wants to a local or
a global invalidate.
If I read the code right, this is called on the boot CPU when all the
non-boot CPUs are still (potentially) down, so if you would do a global
invalidate the non-boot CPUs might not even notice, so those need to do
a (local) invalidate after being brought up anyway? Or they probably
need it before being brought down at all? You figure it out, it makes
my brain hurt :-)
A good rule would be that every cpu does a local invalidate before turning on
the MMU. That would work for this case and also for kexec, kdump, junk left by
firmare etc. But I don't think we do that consistently in a way that works for
this code at the moment.
quoted
As an alternative, can you try adding a .machine push / .machine "power4" /
.machine pop, around the tlbie. That should tell the assembler to drop back to
power4 mode for that instruction, which should then do the right thing. There
are some examples in that file.
That will get the assembler to not complain, but it will assemble the wrong
instruction: the power7 instruction has the same opcode (but different
semantics). So if you assemble a "tlbie r4" in power4 mode, a newer CPU
will see it as a "tlbie r4,r0" and do the wrong thing.
Yeah, it would basically maintain the existing behaviour which is wrong but a
known quantity. I suspect no one has ever run this on Power7 or in fact
anything other than G5 or Book3E.
cheers
On Wed, Oct 7, 2015 at 5:13 AM, Michael Ellerman [off-list ref] wrote:
On Wed, 2015-10-07 at 02:19 -0500, Segher Boessenkool wrote:
quoted
On Wed, Oct 07, 2015 at 05:00:49PM +1100, Michael Ellerman wrote:
quoted
quoted
It's also worth noting that the __flush_power7 uses tlbiel instead of tlbie.
Yeah that's a good point. It's not clear if the swsusp code wants to a local or
a global invalidate.
If I read the code right, this is called on the boot CPU when all the
non-boot CPUs are still (potentially) down, so if you would do a global
invalidate the non-boot CPUs might not even notice, so those need to do
a (local) invalidate after being brought up anyway? Or they probably
need it before being brought down at all? You figure it out, it makes
my brain hurt :-)
A good rule would be that every cpu does a local invalidate before turning on
the MMU. That would work for this case and also for kexec, kdump, junk left by
firmare etc. But I don't think we do that consistently in a way that works for
this code at the moment.
quoted
quoted
As an alternative, can you try adding a .machine push / .machine "power4" /
.machine pop, around the tlbie. That should tell the assembler to drop back to
power4 mode for that instruction, which should then do the right thing. There
are some examples in that file.
That will get the assembler to not complain, but it will assemble the wrong
instruction: the power7 instruction has the same opcode (but different
semantics). So if you assemble a "tlbie r4" in power4 mode, a newer CPU
will see it as a "tlbie r4,r0" and do the wrong thing.
Yeah, it would basically maintain the existing behaviour which is wrong but a
known quantity. I suspect no one has ever run this on Power7 or in fact
anything other than G5 or Book3E.
Likely not, but leaving it broken just because it is known behavior
seems pretty weird to me. I think Fedora will look at simply
disabling hibernation on ppc64 so the file isn't built at all. Seems
to be a safer option.
josh
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-10-08 00:10:40
On Wed, 2015-10-07 at 10:31 -0400, Josh Boyer wrote:
On Wed, Oct 7, 2015 at 5:13 AM, Michael Ellerman [off-list ref] wrote:
quoted
On Wed, 2015-10-07 at 02:19 -0500, Segher Boessenkool wrote:
quoted
On Wed, Oct 07, 2015 at 05:00:49PM +1100, Michael Ellerman wrote:
quoted
quoted
It's also worth noting that the __flush_power7 uses tlbiel instead of tlbie.
Yeah that's a good point. It's not clear if the swsusp code wants to a local or
a global invalidate.
If I read the code right, this is called on the boot CPU when all the
non-boot CPUs are still (potentially) down, so if you would do a global
invalidate the non-boot CPUs might not even notice, so those need to do
a (local) invalidate after being brought up anyway? Or they probably
need it before being brought down at all? You figure it out, it makes
my brain hurt :-)
A good rule would be that every cpu does a local invalidate before turning on
the MMU. That would work for this case and also for kexec, kdump, junk left by
firmare etc. But I don't think we do that consistently in a way that works for
this code at the moment.
quoted
quoted
As an alternative, can you try adding a .machine push / .machine "power4" /
.machine pop, around the tlbie. That should tell the assembler to drop back to
power4 mode for that instruction, which should then do the right thing. There
are some examples in that file.
That will get the assembler to not complain, but it will assemble the wrong
instruction: the power7 instruction has the same opcode (but different
semantics). So if you assemble a "tlbie r4" in power4 mode, a newer CPU
will see it as a "tlbie r4,r0" and do the wrong thing.
Yeah, it would basically maintain the existing behaviour which is wrong but a
known quantity. I suspect no one has ever run this on Power7 or in fact
anything other than G5 or Book3E.
Likely not, but leaving it broken just because it is known behavior
seems pretty weird to me.
In a universe where I have infinite time to fix random things we would
obviously do a proper fix :)
I think Fedora will look at simply disabling hibernation on ppc64 so the file
isn't built at all. Seems to be a safer option.
It's safer for sure. Though you might have some G5 users who are using it and
notice it being disabled.
cheers
On Wed, Oct 7, 2015 at 8:10 PM, Michael Ellerman [off-list ref] wrote:
On Wed, 2015-10-07 at 10:31 -0400, Josh Boyer wrote:
quoted
On Wed, Oct 7, 2015 at 5:13 AM, Michael Ellerman [off-list ref] wrote:
quoted
On Wed, 2015-10-07 at 02:19 -0500, Segher Boessenkool wrote:
quoted
On Wed, Oct 07, 2015 at 05:00:49PM +1100, Michael Ellerman wrote:
quoted
quoted
It's also worth noting that the __flush_power7 uses tlbiel instead of tlbie.
Yeah that's a good point. It's not clear if the swsusp code wants to a local or
a global invalidate.
If I read the code right, this is called on the boot CPU when all the
non-boot CPUs are still (potentially) down, so if you would do a global
invalidate the non-boot CPUs might not even notice, so those need to do
a (local) invalidate after being brought up anyway? Or they probably
need it before being brought down at all? You figure it out, it makes
my brain hurt :-)
A good rule would be that every cpu does a local invalidate before turning on
the MMU. That would work for this case and also for kexec, kdump, junk left by
firmare etc. But I don't think we do that consistently in a way that works for
this code at the moment.
quoted
quoted
As an alternative, can you try adding a .machine push / .machine "power4" /
.machine pop, around the tlbie. That should tell the assembler to drop back to
power4 mode for that instruction, which should then do the right thing. There
are some examples in that file.
That will get the assembler to not complain, but it will assemble the wrong
instruction: the power7 instruction has the same opcode (but different
semantics). So if you assemble a "tlbie r4" in power4 mode, a newer CPU
will see it as a "tlbie r4,r0" and do the wrong thing.
Yeah, it would basically maintain the existing behaviour which is wrong but a
known quantity. I suspect no one has ever run this on Power7 or in fact
anything other than G5 or Book3E.
Likely not, but leaving it broken just because it is known behavior
seems pretty weird to me.
In a universe where I have infinite time to fix random things we would
obviously do a proper fix :)
quoted
I think Fedora will look at simply disabling hibernation on ppc64 so the file
isn't built at all. Seems to be a safer option.
It's safer for sure. Though you might have some G5 users who are using it and
notice it being disabled.
The 5 of them will notice it being disabled and then they'll realize
they either get a working kernel minus hibernation, or they get no
kernel at all because it doesn't compile.
josh
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2015-10-08 00:38:34
On Wed, 2015-10-07 at 20:15 -0400, Josh Boyer wrote:
On Wed, Oct 7, 2015 at 8:10 PM, Michael Ellerman [off-list ref] wrote:
quoted
On Wed, 2015-10-07 at 10:31 -0400, Josh Boyer wrote:
quoted
On Wed, Oct 7, 2015 at 5:13 AM, Michael Ellerman [off-list ref] wrote:
quoted
On Wed, 2015-10-07 at 02:19 -0500, Segher Boessenkool wrote:
quoted
On Wed, Oct 07, 2015 at 05:00:49PM +1100, Michael Ellerman wrote:
quoted
quoted
It's also worth noting that the __flush_power7 uses tlbiel instead of tlbie.
Yeah that's a good point. It's not clear if the swsusp code wants to a local or
a global invalidate.
If I read the code right, this is called on the boot CPU when all the
non-boot CPUs are still (potentially) down, so if you would do a global
invalidate the non-boot CPUs might not even notice, so those need to do
a (local) invalidate after being brought up anyway? Or they probably
need it before being brought down at all? You figure it out, it makes
my brain hurt :-)
A good rule would be that every cpu does a local invalidate before turning on
the MMU. That would work for this case and also for kexec, kdump, junk left by
firmare etc. But I don't think we do that consistently in a way that works for
this code at the moment.
quoted
quoted
As an alternative, can you try adding a .machine push / .machine "power4" /
.machine pop, around the tlbie. That should tell the assembler to drop back to
power4 mode for that instruction, which should then do the right thing. There
are some examples in that file.
That will get the assembler to not complain, but it will assemble the wrong
instruction: the power7 instruction has the same opcode (but different
semantics). So if you assemble a "tlbie r4" in power4 mode, a newer CPU
will see it as a "tlbie r4,r0" and do the wrong thing.
Yeah, it would basically maintain the existing behaviour which is wrong but a
known quantity. I suspect no one has ever run this on Power7 or in fact
anything other than G5 or Book3E.
Likely not, but leaving it broken just because it is known behavior
seems pretty weird to me.
In a universe where I have infinite time to fix random things we would
obviously do a proper fix :)
quoted
I think Fedora will look at simply disabling hibernation on ppc64 so the file
isn't built at all. Seems to be a safer option.
It's safer for sure. Though you might have some G5 users who are using it and
notice it being disabled.
The 5 of them will notice it being disabled and then they'll realize
they either get a working kernel minus hibernation, or they get no
kernel at all because it doesn't compile.
Sure. But if we do the machine push thing they'll get both :)
And I doubt it's 5, 2 is more likely.
cheers