* [PATCH] powerpc/mm/radix: Fix checkstops caused by invalid tlbiel
@ 2018-04-12 5:53 Michael Ellerman
2018-04-12 6:37 ` Nicholas Piggin
2018-04-12 21:39 ` Michael Ellerman
0 siblings, 2 replies; 3+ messages in thread
From: Michael Ellerman @ 2018-04-12 5:53 UTC (permalink / raw)
To: linuxppc-dev; +Cc: joel, npiggin
In tlbiel_radix_set_isa300() we use the PPC_TLBIEL() macro to
construct tlbiel instructions. The instruction takes 5 fields, two of
which are registers, and the others are constants. But because it's
constructed with inline asm the compiler doesn't know that.
We got the constraint wrong on the 'r' field, using "r" tells the
compiler to put the value in a register. The value we then get in the
macro is the *register number*, not the value of the field.
That means when we mask the register number with 0x1 we get 0 or 1
depending on which register the compiler happens to put the constant
in, eg:
li r10,1
tlbiel r8,r9,2,0,0
li r7,1
tlbiel r10,r6,0,0,1
If we're unlucky we might generate an invalid instruction form, for
example RIC=0, PRS=1 and R=0, tlbiel r8,r7,0,1,0, this has been
observed to cause machine checks:
Oops: Machine check, sig: 7 [#1]
CPU: 24 PID: 0 Comm: swapper
NIP: 00000000000385f4 LR: 000000000100ed00 CTR: 000000000000007f
REGS: c00000000110bb40 TRAP: 0200
MSR: 9000000000201003 <SF,HV,ME,RI,LE> CR: 48002222 XER: 20040000
CFAR: 00000000000385d0 DAR: 0000000000001c00 DSISR: 00000200 SOFTE: 1
If the machine check happens early in boot while we have MSR_ME=0 it
will escalate into a checkstop and kill the box entirely.
To fix it we could change the inline asm constraint to "i" which
tells the compiler the value is a constant. But a better fix is to just
pass a literal 1 into the macro, which bypasses any problems with inline
asm constraints.
Fixes: d4748276ae14 ("powerpc/64s: Improve local TLB flush for boot and MCE on POWER9")
Cc: stable@vger.kernel.org # v4.16+
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/mm/tlb-radix.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
index 2fba6170ab3f..a5d7309c2d05 100644
--- a/arch/powerpc/mm/tlb-radix.c
+++ b/arch/powerpc/mm/tlb-radix.c
@@ -33,13 +33,12 @@ static inline void tlbiel_radix_set_isa300(unsigned int set, unsigned int is,
{
unsigned long rb;
unsigned long rs;
- unsigned int r = 1; /* radix format */
rb = (set << PPC_BITLSHIFT(51)) | (is << PPC_BITLSHIFT(53));
rs = ((unsigned long)pid << PPC_BITLSHIFT(31));
- asm volatile(PPC_TLBIEL(%0, %1, %2, %3, %4)
- : : "r"(rb), "r"(rs), "i"(ric), "i"(prs), "r"(r)
+ asm volatile(PPC_TLBIEL(%0, %1, %2, %3, 1)
+ : : "r"(rb), "r"(rs), "i"(ric), "i"(prs)
: "memory");
}
--
2.14.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] powerpc/mm/radix: Fix checkstops caused by invalid tlbiel
2018-04-12 5:53 [PATCH] powerpc/mm/radix: Fix checkstops caused by invalid tlbiel Michael Ellerman
@ 2018-04-12 6:37 ` Nicholas Piggin
2018-04-12 21:39 ` Michael Ellerman
1 sibling, 0 replies; 3+ messages in thread
From: Nicholas Piggin @ 2018-04-12 6:37 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, joel
On Thu, 12 Apr 2018 15:53:52 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:
> In tlbiel_radix_set_isa300() we use the PPC_TLBIEL() macro to
> construct tlbiel instructions. The instruction takes 5 fields, two of
> which are registers, and the others are constants. But because it's
> constructed with inline asm the compiler doesn't know that.
>
> We got the constraint wrong on the 'r' field, using "r" tells the
> compiler to put the value in a register. The value we then get in the
> macro is the *register number*, not the value of the field.
>
> That means when we mask the register number with 0x1 we get 0 or 1
> depending on which register the compiler happens to put the constant
> in, eg:
>
> li r10,1
> tlbiel r8,r9,2,0,0
>
> li r7,1
> tlbiel r10,r6,0,0,1
>
> If we're unlucky we might generate an invalid instruction form, for
> example RIC=0, PRS=1 and R=0, tlbiel r8,r7,0,1,0, this has been
> observed to cause machine checks:
>
> Oops: Machine check, sig: 7 [#1]
> CPU: 24 PID: 0 Comm: swapper
> NIP: 00000000000385f4 LR: 000000000100ed00 CTR: 000000000000007f
> REGS: c00000000110bb40 TRAP: 0200
> MSR: 9000000000201003 <SF,HV,ME,RI,LE> CR: 48002222 XER: 20040000
> CFAR: 00000000000385d0 DAR: 0000000000001c00 DSISR: 00000200 SOFTE: 1
>
> If the machine check happens early in boot while we have MSR_ME=0 it
> will escalate into a checkstop and kill the box entirely.
>
> To fix it we could change the inline asm constraint to "i" which
> tells the compiler the value is a constant. But a better fix is to just
> pass a literal 1 into the macro, which bypasses any problems with inline
> asm constraints.
>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> Fixes: d4748276ae14 ("powerpc/64s: Improve local TLB flush for boot and MCE on POWER9")
> Cc: stable@vger.kernel.org # v4.16+
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/mm/tlb-radix.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
> index 2fba6170ab3f..a5d7309c2d05 100644
> --- a/arch/powerpc/mm/tlb-radix.c
> +++ b/arch/powerpc/mm/tlb-radix.c
> @@ -33,13 +33,12 @@ static inline void tlbiel_radix_set_isa300(unsigned int set, unsigned int is,
> {
> unsigned long rb;
> unsigned long rs;
> - unsigned int r = 1; /* radix format */
>
> rb = (set << PPC_BITLSHIFT(51)) | (is << PPC_BITLSHIFT(53));
> rs = ((unsigned long)pid << PPC_BITLSHIFT(31));
>
> - asm volatile(PPC_TLBIEL(%0, %1, %2, %3, %4)
> - : : "r"(rb), "r"(rs), "i"(ric), "i"(prs), "r"(r)
> + asm volatile(PPC_TLBIEL(%0, %1, %2, %3, 1)
> + : : "r"(rb), "r"(rs), "i"(ric), "i"(prs)
> : "memory");
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: powerpc/mm/radix: Fix checkstops caused by invalid tlbiel
2018-04-12 5:53 [PATCH] powerpc/mm/radix: Fix checkstops caused by invalid tlbiel Michael Ellerman
2018-04-12 6:37 ` Nicholas Piggin
@ 2018-04-12 21:39 ` Michael Ellerman
1 sibling, 0 replies; 3+ messages in thread
From: Michael Ellerman @ 2018-04-12 21:39 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: joel, npiggin
On Thu, 2018-04-12 at 05:53:52 UTC, Michael Ellerman wrote:
> In tlbiel_radix_set_isa300() we use the PPC_TLBIEL() macro to
> construct tlbiel instructions. The instruction takes 5 fields, two of
> which are registers, and the others are constants. But because it's
> constructed with inline asm the compiler doesn't know that.
>
> We got the constraint wrong on the 'r' field, using "r" tells the
> compiler to put the value in a register. The value we then get in the
> macro is the *register number*, not the value of the field.
>
> That means when we mask the register number with 0x1 we get 0 or 1
> depending on which register the compiler happens to put the constant
> in, eg:
>
> li r10,1
> tlbiel r8,r9,2,0,0
>
> li r7,1
> tlbiel r10,r6,0,0,1
>
> If we're unlucky we might generate an invalid instruction form, for
> example RIC=0, PRS=1 and R=0, tlbiel r8,r7,0,1,0, this has been
> observed to cause machine checks:
>
> Oops: Machine check, sig: 7 [#1]
> CPU: 24 PID: 0 Comm: swapper
> NIP: 00000000000385f4 LR: 000000000100ed00 CTR: 000000000000007f
> REGS: c00000000110bb40 TRAP: 0200
> MSR: 9000000000201003 <SF,HV,ME,RI,LE> CR: 48002222 XER: 20040000
> CFAR: 00000000000385d0 DAR: 0000000000001c00 DSISR: 00000200 SOFTE: 1
>
> If the machine check happens early in boot while we have MSR_ME=0 it
> will escalate into a checkstop and kill the box entirely.
>
> To fix it we could change the inline asm constraint to "i" which
> tells the compiler the value is a constant. But a better fix is to just
> pass a literal 1 into the macro, which bypasses any problems with inline
> asm constraints.
>
> Fixes: d4748276ae14 ("powerpc/64s: Improve local TLB flush for boot and MCE on POWER9")
> Cc: stable@vger.kernel.org # v4.16+
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Applied to powerpc fixes.
https://git.kernel.org/powerpc/c/2675c13b293a007b7b7f8229514126
cheers
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-04-12 21:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-12 5:53 [PATCH] powerpc/mm/radix: Fix checkstops caused by invalid tlbiel Michael Ellerman
2018-04-12 6:37 ` Nicholas Piggin
2018-04-12 21:39 ` Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).