KVM: PPC: Add FWNMI support for KVM guests on POWER

Enhance KVM to cause a guest exit with KVM_EXIT_NMI exit reason upon a machine check exception (MCE) in the guest address space if the KVM_CAP_PPC_FWNMI capability is enabled (instead of delivering a 0x200 interrupt to guest). This enables QEMU to build error log and deliver machine check exception to guest via guest registered machine check handler.

This approach simplifies the delivery of machine check exception to guest OS compared to the earlier approach of KVM directly invoking 0x200 guest interrupt vector.

This design/approach is based on the feedback for the QEMU patches to handle machine check exception. Details of earlier approach of handling machine check exception in QEMU and related discussions can be found at:

https://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg00813.html

Note:

This patch introduces a hook which is invoked at the time of guest exit to facilitate the host-side handling of machine check exception before the exception is passed on to the guest. Hence, the host-side handling which was performed earlier via machine_check_fwnmi is removed.

The reasons for this approach is (i) it is not possible to distinguish whether the exception occurred in the guest or the host from the pt_regs passed on the machine_check_exception(). Hence machine_check_exception() calls panic, instead of passing on the exception to the guest, if the machine check exception is not recoverable. (ii) the approach introduced in this patch gives opportunity to the host kernel to perform actions in virtual mode before passing on the exception to the guest. This approach does not require complex tweaks to machine_check_fwnmi and friends.

Signed-off-by Aravinda Prasad <[email protected]>

Reviewed-by David Gibson <[email protected]>

---
 arch/powerpc/kvm/book3s_hv.c            |   27 +++++++++++++-----
 arch/powerpc/kvm/book3s_hv_rmhandlers.S |   47 ++++++++++++++++---------------
 arch/powerpc/platforms/powernv/opal.c   |   10 +++++++
 3 files changed, 54 insertions(+), 30 deletions(-)

arch/powerpc/kvm/book3s_hv.c +4 -1

index 3686471..cae4921 100644
@@ -123,6 +123,7 @@ MODULE_PARM_DESC(halt_poll_ns_shrink, "Factor halt poll time is shrunk by");
 
 static void kvmppc_end_cede(struct kvm_vcpu *vcpu);
 static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
+static void kvmppc_machine_check_hook(void);
 
 static inline struct kvm_vcpu *next_runnable_thread(struct kvmppc_vcore *vc,
 		int *ip)

@@ -954,15 +955,14 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
 		r = RESUME_GUEST;
 		break;
 	case BOOK3S_INTERRUPT_MACHINE_CHECK:
+		/* Exit to guest with KVM_EXIT_NMI as exit reason */
+		run->exit_reason = KVM_EXIT_NMI;
+		r = RESUME_HOST;
 		/*
-		 * Deliver a machine check interrupt to the guest.

arch/powerpc/kvm/book3s_hv.c +2 -0

 		break;
 	case BOOK3S_INTERRUPT_PROGRAM:
 	{

@@ -3491,6 +3491,19 @@ static void kvmppc_irq_bypass_del_producer_hv(struct irq_bypass_consumer *cons,
 }
 #endif
 
+/*
+ * Hook to handle machine check exceptions occurred inside a guest.

Quoted 40 lines · Aravinda Prasad

Enhance KVM to cause a guest exit with KVM_EXIT_NMI exit reason upon a machine check exception (MCE) in the guest address space if the KVM_CAP_PPC_FWNMI capability is enabled (instead of delivering a 0x200 interrupt to guest). This enables QEMU to build error log and deliver machine check exception to guest via guest registered machine check handler.

This approach simplifies the delivery of machine check exception to guest OS compared to the earlier approach of KVM directly invoking 0x200 guest interrupt vector.

This design/approach is based on the feedback for the QEMU patches to handle machine check exception. Details of earlier approach of handling machine check exception in QEMU and related discussions can be found at:

https://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg00813.html

Note:

This patch introduces a hook which is invoked at the time of guest exit to facilitate the host-side handling of machine check exception before the exception is passed on to the guest. Hence, the host-side handling which was performed earlier via machine_check_fwnmi is removed.

The reasons for this approach is (i) it is not possible to distinguish whether the exception occurred in the guest or the host from the pt_regs passed on the machine_check_exception(). Hence machine_check_exception() calls panic, instead of passing on the exception to the guest, if the machine check exception is not recoverable. (ii) the approach introduced in this patch gives opportunity to the host kernel to perform actions in virtual mode before passing on the exception to the guest. This approach does not require complex tweaks to machine_check_fwnmi and friends.

Quoted 2 lines

Signed-off-by Aravinda Prasad <[email protected]>

Reviewed-by David Gibson <[email protected]>

This patch mostly looks OK. I have a few relatively minor comments below.

Quoted 35 lines

---
 arch/powerpc/kvm/book3s_hv.c            |   27 +++++++++++++-----
 arch/powerpc/kvm/book3s_hv_rmhandlers.S |   47 ++++++++++++++++---------------
 arch/powerpc/platforms/powernv/opal.c   |   10 +++++++
 3 files changed, 54 insertions(+), 30 deletions(-)

arch/powerpc/kvm/book3s_hv.c +6 -6

index 3686471..cae4921 100644
@@ -123,6 +123,7 @@ MODULE_PARM_DESC(halt_poll_ns_shrink, "Factor halt poll time is shrunk by");
 
 static void kvmppc_end_cede(struct kvm_vcpu *vcpu);
 static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
+static void kvmppc_machine_check_hook(void);
 
 static inline struct kvm_vcpu *next_runnable_thread(struct kvmppc_vcore *vc,
 		int *ip)

@@ -954,15 +955,14 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
 		r = RESUME_GUEST;
 		break;
 	case BOOK3S_INTERRUPT_MACHINE_CHECK:
+		/* Exit to guest with KVM_EXIT_NMI as exit reason */
+		run->exit_reason = KVM_EXIT_NMI;
+		r = RESUME_HOST;
 		/*
-		 * Deliver a machine check interrupt to the guest.
-		 * We have to do this, even if the host has handled the
-		 * machine check, because machine checks use SRR0/1 and
-		 * the interrupt might have trashed guest state in them.
+		 * Invoke host-kernel handler to perform any host-side
+		 * handling before exiting the guest.
 		 */
-		kvmppc_book3s_queue_irqprio(vcpu,
-					    BOOK3S_INTERRUPT_MACHINE_CHECK);

arch/powerpc/kvm/book3s_hv.c static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, · line 965

Quoted 2 lines

arch/powerpc/kvm/book3s_hv.c +1 -1

-		r = RESUME_GUEST;
+		kvmppc_machine_check_hook();

Note that this won't necessarily be called on the same CPU that received the machine check. This will be called on thread 0 of the core (or subcore), whereas the machine check could have occurred on some other thread. Are you sure that the machine check handling code will be OK with that?

Quoted 10 lines

arch/powerpc/kvm/book3s_hv.c +2 -0

 		break;
 	case BOOK3S_INTERRUPT_PROGRAM:
 	{

@@ -3491,6 +3491,19 @@ static void kvmppc_irq_bypass_del_producer_hv(struct irq_bypass_consumer *cons,
 }
 #endif
 
+/*
+ * Hook to handle machine check exceptions occurred inside a guest.

arch/powerpc/kvm/book3s_hv.c static void kvmppc_irq_bypass_del_producer_hv(struct irq_bypass_consumer *cons, · line 3505

Quoted 10 lines

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

+ * This hook is invoked from host virtual mode from KVM before exiting
+ * the guest with KVM_EXIT_NMI exit reason. This gives an opportunity
+ * for the host to take action (if any) before passing on the machine
+ * check exception to the guest kernel.
+ */
+static void kvmppc_machine_check_hook(void)
+{
+	if (ppc_md.machine_check_exception)
+		ppc_md.machine_check_exception(NULL);
+}

What is the advantage of having this as a separate function, as opposed to just putting those two lines of code in line in the one place where this gets called?

Paul.

Quoted 1 line · Paul Mackerras

On Fri, Jan 13, 2017 at 04:51:45PM +0530, Aravinda Prasad wrote:

Quoted 14 lines

Quoted 14 lines

arch/powerpc/kvm/book3s_hv.c +5 -6

 	case BOOK3S_INTERRUPT_MACHINE_CHECK:
+		/* Exit to guest with KVM_EXIT_NMI as exit reason */
+		run->exit_reason = KVM_EXIT_NMI;
+		r = RESUME_HOST;
 		/*
-		 * Deliver a machine check interrupt to the guest.
-		 * We have to do this, even if the host has handled the
-		 * machine check, because machine checks use SRR0/1 and
-		 * the interrupt might have trashed guest state in them.
+		 * Invoke host-kernel handler to perform any host-side
+		 * handling before exiting the guest.
 		 */
-		kvmppc_book3s_queue_irqprio(vcpu,
-					    BOOK3S_INTERRUPT_MACHINE_CHECK);

Quoted 8 lines

arch/powerpc/kvm/book3s_hv.c static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, · line 965

Quoted 2 lines

arch/powerpc/kvm/book3s_hv.c +1 -1

-		r = RESUME_GUEST;
+		kvmppc_machine_check_hook();

Note that this won't necessarily be called on the same CPU that received the machine check. This will be called on thread 0 of the core (or subcore), whereas the machine check could have occurred on some other thread. Are you sure that the machine check handling code will be OK with that?

That will have only one problem. get_mce_event() from opal_machine_check() may not be able to pull mce event for error on non-zero thread. We should hook the mce event into vcpu structure during kvmppc_realmode_machine_check() and then pass it to ppc_md.machine_check_exception() as an additional argument.

Quoted 2 lines · Mahesh Jagannath Salgaonkar

Quoted 1 line · Paul Mackerras

On Fri, Jan 13, 2017 at 04:51:45PM +0530, Aravinda Prasad wrote:

[snip]

Quoted 23 lines

Quoted 14 lines

Quoted 14 lines

(quoted diff) +5 -6

 	case BOOK3S_INTERRUPT_MACHINE_CHECK:
+		/* Exit to guest with KVM_EXIT_NMI as exit reason */
+		run->exit_reason = KVM_EXIT_NMI;
+		r = RESUME_HOST;
 		/*
-		 * Deliver a machine check interrupt to the guest.
-		 * We have to do this, even if the host has handled the
-		 * machine check, because machine checks use SRR0/1 and
-		 * the interrupt might have trashed guest state in them.
+		 * Invoke host-kernel handler to perform any host-side
+		 * handling before exiting the guest.
 		 */
-		kvmppc_book3s_queue_irqprio(vcpu,
-					    BOOK3S_INTERRUPT_MACHINE_CHECK);

Quoted 8 lines

Review

Quoted 2 lines

(quoted diff) +1 -1

-		r = RESUME_GUEST;
+		kvmppc_machine_check_hook();

Note that this won't necessarily be called on the same CPU that received the machine check. This will be called on thread 0 of the core (or subcore), whereas the machine check could have occurred on some other thread. Are you sure that the machine check handling code will be OK with that?

Quoted 5 lines

That will have only one problem. get_mce_event() from opal_machine_check() may not be able to pull mce event for error on non-zero thread. We should hook the mce event into vcpu structure during kvmppc_realmode_machine_check() and then pass it to ppc_md.machine_check_exception() as an additional argument.

To move things along...

Mahesh, how would we get hold of the mce event from real-mode assembly code? What function would we need to call to get the event? Could you write some code (or at least some pseudo-code) to illustrate how it would be done?