KVM: PPC: Book3S HV: send kvmppc_bad_interrupt NMIs to Linux handlers

RFC PATCH In review Nicholas Piggin 2 participants 3 messages Apr 6, 2018 – May 17, 2018

Subsystems (2)

PowerPC KVM PowerPC

lore

Share & tools

lore

Nicholas Piggin

author reviewer

2018-04-06 04:14 UTC #0 lore raw

RFC PATCH 1/1 KVM: PPC: Book3S HV: send kvmppc_bad_interrupt NMIs to Linux handlers

It's possible to take a SRESET or MCE in these paths due to a bug in the host code or a NMI IPI etc. A recent bug attempting to load a virtual address gave a complete but cryptic error, abridged:

      Oops: Bad interrupt in KVM entry/exit code, sig: 6 [#1]
      LE SMP NR_CPUS 48 NUMA PowerNV
      CPU: 53 PID: 6582 Comm: qemu-system-ppc Not tainted
      NIP:  c0000000000155ac LR: c0000000000c2430 CTR: c000000000015580
      REGS: c000000fff76dd80 TRAP: 0200   Not tainted
      MSR:  9000000000201003 <SF,HV,ME,RI,LE>  CR: 48082222  XER: 00000000
      CFAR: 0000000102900ef0 DAR: d00017fffd941a28 DSISR: 00000040 SOFTE: 3
      NIP [c0000000000155ac] perf_trace_tlbie+0x2c/0x1a0
      LR [c0000000000c2430] do_tlbies+0x230/0x2f0

Sending NMIs through the Linux handlers gives a nicer output:

      Severe Machine check interrupt [Not recovered]
        NIP [c0000000000155ac]: perf_trace_tlbie+0x2c/0x1a0
        Initiator: CPU
        Error type: Real address [Load (bad)]
          Effective address: d00017fffcc01a28
      opal: Machine check interrupt unrecoverable: MSR(RI=0)
      opal: Hardware platform error: Unrecoverable Machine Check exception
      CPU: 0 PID: 6700 Comm: qemu-system-ppc Tainted: G   M
      NIP:  c0000000000155ac LR: c0000000000c23c0 CTR: c000000000015580
      REGS: c000000fff9e9d80 TRAP: 0200   Tainted: G   M
      MSR:  9000000000201001 <SF,HV,ME,LE>  CR: 48082222  XER: 00000000
      CFAR: 000000010cbc1a30 DAR: d00017fffcc01a28 DSISR: 00000040 SOFTE: 3
      NIP [c0000000000155ac] perf_trace_tlbie+0x2c/0x1a0
      LR [c0000000000c23c0] do_tlbies+0x1c0/0x280

Signed-off-by Nicholas Piggin <[email protected]>

---
This is lightly tested only, not sure if there would be a better way to do it?

 arch/powerpc/kvm/book3s_hv_builtin.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

arch/powerpc/kvm/book3s_hv_builtin.c +12 -0

index de18299f92b7..0b9b8e188bfa 100644
@@ -18,6 +18,7 @@
 #include <linux/cma.h>
 #include <linux/bitops.h>
 
+#include <asm/asm-prototypes.h>
 #include <asm/cputable.h>
 #include <asm/kvm_ppc.h>
 #include <asm/kvm_book3s.h>

@@ -633,6 +634,17 @@ int kvmppc_rm_h_eoi(struct kvm_vcpu *vcpu, unsigned long xirr)
 
 void kvmppc_bad_interrupt(struct pt_regs *regs)
 {
+	regs->msr &= ~MSR_RI;
+	/*
+	 * 100 could happen at any time, 200 can happen due to
+	 * invalid real address access for example.
+	 */
+	if (TRAP(regs) = 0x100) {
+		get_paca()->in_nmi++;
+		system_reset_exception(regs);
+	}
+	if (TRAP(regs) = 0x200)
+		machine_check_exception(regs);
 	die("Bad interrupt in KVM entry/exit code", regs, SIGABRT);
 	panic("Bad KVM trap");
 }

Signature · 1 line

2.16.3

Paul Mackerras

2018-05-17 06:33 UTC #1 lore raw

Quoted 31 lines · Nicholas Piggin

It's possible to take a SRESET or MCE in these paths due to a bug in the host code or a NMI IPI etc. A recent bug attempting to load a virtual address gave a complete but cryptic error, abridged:

      Oops: Bad interrupt in KVM entry/exit code, sig: 6 [#1]
      LE SMP NR_CPUS 48 NUMA PowerNV
      CPU: 53 PID: 6582 Comm: qemu-system-ppc Not tainted
      NIP:  c0000000000155ac LR: c0000000000c2430 CTR: c000000000015580
      REGS: c000000fff76dd80 TRAP: 0200   Not tainted
      MSR:  9000000000201003 <SF,HV,ME,RI,LE>  CR: 48082222  XER: 00000000
      CFAR: 0000000102900ef0 DAR: d00017fffd941a28 DSISR: 00000040 SOFTE: 3
      NIP [c0000000000155ac] perf_trace_tlbie+0x2c/0x1a0
      LR [c0000000000c2430] do_tlbies+0x230/0x2f0

Sending NMIs through the Linux handlers gives a nicer output:

      Severe Machine check interrupt [Not recovered]
        NIP [c0000000000155ac]: perf_trace_tlbie+0x2c/0x1a0
        Initiator: CPU
        Error type: Real address [Load (bad)]
          Effective address: d00017fffcc01a28
      opal: Machine check interrupt unrecoverable: MSR(RI=0)
      opal: Hardware platform error: Unrecoverable Machine Check exception
      CPU: 0 PID: 6700 Comm: qemu-system-ppc Tainted: G   M
      NIP:  c0000000000155ac LR: c0000000000c23c0 CTR: c000000000015580
      REGS: c000000fff9e9d80 TRAP: 0200   Tainted: G   M
      MSR:  9000000000201001 <SF,HV,ME,LE>  CR: 48082222  XER: 00000000
      CFAR: 000000010cbc1a30 DAR: d00017fffcc01a28 DSISR: 00000040 SOFTE: 3
      NIP [c0000000000155ac] perf_trace_tlbie+0x2c/0x1a0
      LR [c0000000000c23c0] do_tlbies+0x1c0/0x280

Quoted 4 lines

Signed-off-by Nicholas Piggin <[email protected]>
---
This is lightly tested only, not sure if there would be a better way to do it?

The patch seems reasonable. I have just one comment below. Do you want to re-send it without the RFC tag, or should I just take it?

Quoted 16 lines

 arch/powerpc/kvm/book3s_hv_builtin.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

arch/powerpc/kvm/book3s_hv_builtin.c +1 -0

index de18299f92b7..0b9b8e188bfa 100644
@@ -18,6 +18,7 @@
 #include <linux/cma.h>
 #include <linux/bitops.h>
 
+#include <asm/asm-prototypes.h>
 #include <asm/cputable.h>
 #include <asm/kvm_ppc.h>
 #include <asm/kvm_book3s.h>

arch/powerpc/kvm/book3s_hv_builtin.c int kvmppc_rm_h_eoi(struct kvm_vcpu *vcpu, unsigned long xirr) · line 637

Quoted 4 lines

arch/powerpc/kvm/book3s_hv_builtin.c +1 -0

@@ -633,6 +634,17 @@ int kvmppc_rm_h_eoi(struct kvm_vcpu *vcpu, unsigned long xirr)
 
 void kvmppc_bad_interrupt(struct pt_regs *regs)
 {
+	regs->msr &= ~MSR_RI;

This is worth mentioning in the commit message. I assume you're doing it to make sure we won't try to continue execution under any circumstances?

Quoted 15 lines

arch/powerpc/kvm/book3s_hv_builtin.c +10 -0

+	/*
+	 * 100 could happen at any time, 200 can happen due to
+	 * invalid real address access for example.
+	 */
+	if (TRAP(regs) = 0x100) {
+		get_paca()->in_nmi++;
+		system_reset_exception(regs);
+	}
+	if (TRAP(regs) = 0x200)
+		machine_check_exception(regs);
 	die("Bad interrupt in KVM entry/exit code", regs, SIGABRT);
 	panic("Bad KVM trap");
 }

Signature · 1 line

2.16.3

Paul.

Nicholas Piggin

author reviewer

2018-05-17 07:24 UTC #2 lore raw

In reply to Nicholas Piggin

Quoted 32 lines · On Thu, 17 May 2018 16:33:13 +1000 Paul Mackerras <[email protected]> wrote:

Quoted 31 lines · Nicholas Piggin
It's possible to take a SRESET or MCE in these paths due to a bug in the host code or a NMI IPI etc. A recent bug attempting to load a virtual address gave a complete but cryptic error, abridged:
      Oops: Bad interrupt in KVM entry/exit code, sig: 6 [#1]
      LE SMP NR_CPUS 48 NUMA PowerNV
      CPU: 53 PID: 6582 Comm: qemu-system-ppc Not tainted
      NIP:  c0000000000155ac LR: c0000000000c2430 CTR:
c000000000015580 REGS: c000000fff76dd80 TRAP: 0200   Not tainted
      MSR:  9000000000201003 <SF,HV,ME,RI,LE>  CR: 48082222  XER:
00000000 CFAR: 0000000102900ef0 DAR: d00017fffd941a28 DSISR:
00000040 SOFTE: 3 NIP [c0000000000155ac] perf_trace_tlbie+0x2c/0x1a0
      LR [c0000000000c2430] do_tlbies+0x230/0x2f0
Sending NMIs through the Linux handlers gives a nicer output:
Severe Machine check interrupt [Not recovered]
NIP [c0000000000155ac]: perf_trace_tlbie+0x2c/0x1a0
Initiator: CPU
Error type: Real address [Load (bad)]
Effective address: d00017fffcc01a28
opal: Machine check interrupt unrecoverable: MSR(RI=0)
opal: Hardware platform error: Unrecoverable Machine Check
exception CPU: 0 PID: 6700 Comm: qemu-system-ppc Tainted: G M
      NIP:  c0000000000155ac LR: c0000000000c23c0 CTR:
c000000000015580 REGS: c000000fff9e9d80 TRAP: 0200   Tainted: G   M
      MSR:  9000000000201001 <SF,HV,ME,LE>  CR: 48082222  XER:
00000000 CFAR: 000000010cbc1a30 DAR: d00017fffcc01a28 DSISR:
00000040 SOFTE: 3 NIP [c0000000000155ac] perf_trace_tlbie+0x2c/0x1a0
      LR [c0000000000c23c0] do_tlbies+0x1c0/0x280

Quoted 7 lines

Quoted 4 lines
Signed-off-by Nicholas Piggin <[email protected]>
---
This is lightly tested only, not sure if there would be a better way to do it?
The patch seems reasonable. I have just one comment below. Do you want to re-send it without the RFC tag, or should I just take it?

I can re-send if you'd like an updated changelog and comment for that MSR_RI bit.

Quoted 12 lines

Quoted 12 lines
 arch/powerpc/kvm/book3s_hv_builtin.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
(quoted diff) +0 -0
b/arch/powerpc/kvm/book3s_hv_builtin.c index de18299f92b7..0b9b8e188bfa 100644 --- a/arch/powerpc/kvm/book3s_hv_builtin.c +++ b/arch/powerpc/kvm/book3s_hv_builtin.c @@ -18,6 +18,7 @@
#include <linux/cma.h>
#include <linux/bitops.h>

Quoted 13 lines

arch/powerpc/kvm/book3s_hv_builtin.c int kvmppc_rm_h_eoi(struct kvm_vcpu *vcpu, · line 637
Quoted 9 lines
arch/powerpc/kvm/book3s_hv_builtin.c +2 -0
+#include <asm/asm-prototypes.h>
 #include <asm/cputable.h>
 #include <asm/kvm_ppc.h>
 #include <asm/kvm_book3s.h>
@@ -633,6 +634,17 @@ int kvmppc_rm_h_eoi(struct kvm_vcpu *vcpu,
 unsigned long xirr) 
 void kvmppc_bad_interrupt(struct pt_regs *regs)
 {
+	regs->msr &= ~MSR_RI;  
This is worth mentioning in the commit message. I assume you're doing it to make sure we won't try to continue execution under any circumstances?

That's right, sure I'll mention it. Probably a comment here wouldn't hurt either.

Thanks,
Nick