KVM: PPC: Exit guest upon fatal machine check exception
Subsystems (3)
PATCH v2 1/1 KVM: PPC: Exit guest upon fatal machine check exception
This patch modifies KVM to cause a guest exit with KVM_EXIT_NMI instead of immediately delivering a 0x200 interrupt to guest upon machine check exception in guest address. Exiting the guest enables QEMU to build error log and deliver machine check exception to guest OS (either via guest OS registered machine check handler or via 0x200 guest OS interrupt vector).
This approach simplifies the delivering of machine check exception to guest OS compared to the earlier approach of KVM directly invoking 0x200 guest interrupt vector. In the earlier approach QEMU patched the 0x200 interrupt vector during boot. The patched code at 0x200 issued a private hcall to pass the control to QEMU to build the error log.
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
This patch also introduces a new KVM capability to control how KVM behaves on machine check exception. Without this capability, KVM redirects machine check exceptions to guest's 0x200 vector if the address in error belongs to guest. With this capability KVM causes a guest exit with NMI exit reason.
This is required to avoid problems if a new kernel/KVM is used with an old QEMU for guests that don't issue "ibm,nmi-register". As old QEMU does not understand the NMI exit type, it treats it as a fatal error. However, the guest could have handled the machine check error if the exception was delivered to guest's 0x200 interrupt vector instead of NMI exit in case of old QEMU.
Change Log v2:
- Added KVM capability
--- arch/powerpc/include/asm/kvm_host.h | 1 + arch/powerpc/kernel/asm-offsets.c | 1 + arch/powerpc/kvm/book3s_hv.c | 12 +++------- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 37 +++++++++++++++---------------- arch/powerpc/kvm/powerpc.c | 7 ++++++ include/uapi/linux/kvm.h | 1 + 6 files changed, 31 insertions(+), 28 deletions(-)
arch/powerpc/include/asm/kvm_host.h +1 -0
@@ -243,6 +243,7 @@ struct kvm_arch {int hpt_cma_alloc;struct dentry *debugfs_dir;struct dentry *htab_dentry; +u8 fwnmi_enabled;#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */ #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE struct mutex hpt_mutex;
arch/powerpc/kernel/asm-offsets.c +1 -0
@@ -506,6 +506,7 @@ int main(void) DEFINE(KVM_ENABLED_HCALLS, offsetof(struct kvm, arch.enabled_hcalls)); DEFINE(KVM_LPCR, offsetof(struct kvm, arch.lpcr)); DEFINE(KVM_VRMA_SLB_V, offsetof(struct kvm, arch.vrma_slb_v)); + DEFINE(KVM_FWNMI, offsetof(struct kvm, arch.fwnmi_enabled)); DEFINE(VCPU_DSISR, offsetof(struct kvm_vcpu, arch.shregs.dsisr)); DEFINE(VCPU_DAR, offsetof(struct kvm_vcpu, arch.shregs.dar)); DEFINE(VCPU_VPA, offsetof(struct kvm_vcpu, arch.vpa.pinned_addr));
arch/powerpc/kvm/book3s_hv.c +3 -9
@@ -859,15 +859,9 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, r =RESUME_GUEST ;break ;case BOOK3S_INTERRUPT_MACHINE_CHECK : -/* -* 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. -*/ -kvmppc_book3s_queue_irqprio (vcpu, -BOOK3S_INTERRUPT_MACHINE_CHECK ); - r =RESUME_GUEST ; +/* Exit to guest with KVM_EXIT_NMI as exit reason */ + run->exit_reason =KVM_EXIT_NMI ; + r =RESUME_HOST ;break ;case BOOK3S_INTERRUPT_PROGRAM : {
arch/powerpc/kvm/book3s_hv_rmhandlers.S +18 -19
@@ -147,7 +147,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) addi r1, r1, 112 ld r7, HSTATE_HOST_MSR(r13) - cmpwi cr1, r12, BOOK3S_INTERRUPT_MACHINE_CHECK cmpwi r12, BOOK3S_INTERRUPT_EXTERNAL beq 11f cmpwi cr2, r12, BOOK3S_INTERRUPT_HMI
@@ -160,7 +159,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) mtmsrd r6, 1 /* Clear RI in MSR */ mtsrr0 r8 mtsrr1 r7 - beq cr1, 13f /* machine check */ RFI /* On POWER7, we have external interrupts set to use HSRR0/1 */
@@ -168,8 +166,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) mtspr SPRN_HSRR1, r7 ba 0x500 -13: b machine_check_fwnmi - 14: mtspr SPRN_HSRR0, r8 mtspr SPRN_HSRR1, r7 b hmi_exception_after_realmode
@@ -2381,24 +2377,27 @@ machine_check_realmode: ld r9, HSTATE_KVM_VCPU(r13) li r12, BOOK3S_INTERRUPT_MACHINE_CHECK /* - * Deliver unhandled/fatal (e.g. UE) MCE errors to guest through - * machine check interrupt (set HSRR0 to 0x200). And for handled - * errors (no-fatal), just go back to guest execution with current - * HSRR0 instead of exiting guest. This new approach will inject - * machine check to guest for fatal error causing guest to crash. - * - * The old code used to return to host for unhandled errors which - * was causing guest to hang with soft lockups inside guest and - * makes it difficult to recover guest instance. + * Deliver unhandled/fatal (e.g. UE) MCE errors to guest + * by exiting the guest with KVM_EXIT_NMI exit reason (exit + * reason set later based on trap). For handled errors + * (no-fatal), go back to guest execution with current HSRR0 + * instead of exiting the guest. This approach will cause + * the guest to exit in case of fatal machine check error. */ - ld r10, VCPU_PC(r9) - ld r11, VCPU_MSR(r9) - bne 2f /* Continue guest execution. */ - /* If not, deliver a machine check. SRR0/1 are already set */ - li r10, BOOK3S_INTERRUPT_MACHINE_CHECK + bne 2f /* Continue guest execution? */ + /* If not, check if guest is capable of handling NMI exit */ + ld r3, VCPU_KVM(r9) + ld r3, KVM_FWNMI(r3) + cmpdi r3, 1 /* FWNMI capable? */ + bne 1f /* Deliver machine check via guest's 0x200 vector */ + b mc_cont +1: li r10, BOOK3S_INTERRUPT_MACHINE_CHECK ld r11, VCPU_MSR(r9) bl kvmppc_msr_interrupt -2: b fast_interrupt_c_return + b fast_interrupt_c_return +2: ld r10, VCPU_PC(r9) + ld r11, VCPU_MSR(r9) + b fast_interrupt_c_return /* * Check the reason we woke from nap, and take appropriate action.
arch/powerpc/kvm/powerpc.c +7 -0
@@ -567,6 +567,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) r =1 ;break ;#endif +case KVM_CAP_PPC_FWNMI : + r =1 ; +break ;default : r =0 ;break ;
@@ -1129,6 +1132,10 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu, break; }#endif /* CONFIG_KVM_XICS */ +case KVM_CAP_PPC_FWNMI : + r =0 ; + vcpu->kvm->arch.fwnmi_enabled = true; +break ;default : r = -EINVAL ;break ;
include/uapi/linux/kvm.h +1 -0
@@ -824,6 +824,7 @@ struct kvm_ppc_smmu_info {#define KVM_CAP_MULTI_ADDRESS_SPACE 118#define KVM_CAP_GUEST_DEBUG_HW_BPS 119#define KVM_CAP_GUEST_DEBUG_HW_WPS 120 +#define KVM_CAP_PPC_FWNMI 121#ifdef KVM_CAP_IRQ_ROUTING
Hi,
Quoted 15 lines
This patch also introduces a new KVM capability to control how KVM behaves on machine check exception. Without this capability, KVM redirects machine check exceptions to guest's
0x200vector if the address in error belongs to guest. With this capability KVM causes a guest exit with NMI exit reason.This is required to avoid problems if a new kernel/KVM is used with an old QEMU for guests that don't issue "ibm,nmi-register". As old QEMU does not understand the NMI exit type, it treats it as a fatal error. However, the guest could have handled the machine check error if the exception was delivered to guest's
0x200interrupt vector instead of NMI exit in case of old QEMU.
Quoted 2 lines
Change Log v2:
- Added KVM capability
I'm not really qualified to review the contents of this patch, but I'm happy that the changes in v2 address the concern I had for version 1:
thank you.
Regards,
Daniel
Quoted 172 lines
Signed-off-by Aravinda Prasad <[email protected]>--- arch/powerpc/include/asm/kvm_host.h | 1 + arch/powerpc/kernel/asm-offsets.c | 1 + arch/powerpc/kvm/book3s_hv.c | 12 +++------- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 37 +++++++++++++++---------------- arch/powerpc/kvm/powerpc.c | 7 ++++++ include/uapi/linux/kvm.h | 1 + 6 files changed, 31 insertions(+), 28 deletions(-)arch/powerpc/include/asm/kvm_host.h +1
-0@@ -243,6 +243,7 @@ struct kvm_arch {int hpt_cma_alloc;struct dentry *debugfs_dir;struct dentry *htab_dentry; +u8 fwnmi_enabled;#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */ #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE struct mutex hpt_mutex;arch/powerpc/kernel/asm-offsets.c +1
-0@@ -506,6 +506,7 @@ int main(void) DEFINE(KVM_ENABLED_HCALLS, offsetof(struct kvm, arch.enabled_hcalls)); DEFINE(KVM_LPCR, offsetof(struct kvm, arch.lpcr)); DEFINE(KVM_VRMA_SLB_V, offsetof(struct kvm, arch.vrma_slb_v)); + DEFINE(KVM_FWNMI, offsetof(struct kvm, arch.fwnmi_enabled)); DEFINE(VCPU_DSISR, offsetof(struct kvm_vcpu, arch.shregs.dsisr)); DEFINE(VCPU_DAR, offsetof(struct kvm_vcpu, arch.shregs.dar)); DEFINE(VCPU_VPA, offsetof(struct kvm_vcpu, arch.vpa.pinned_addr));arch/powerpc/kvm/book3s_hv.c +3
-9@@ -859,15 +859,9 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, r =RESUME_GUEST ;break ;case BOOK3S_INTERRUPT_MACHINE_CHECK : -/* -* 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. -*/ -kvmppc_book3s_queue_irqprio (vcpu, -BOOK3S_INTERRUPT_MACHINE_CHECK ); - r =RESUME_GUEST ; +/* Exit to guest with KVM_EXIT_NMI as exit reason */ + run->exit_reason =KVM_EXIT_NMI ; + r =RESUME_HOST ;break ;case BOOK3S_INTERRUPT_PROGRAM : {arch/powerpc/kvm/book3s_hv_rmhandlers.S +18
-19@@ -147,7 +147,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) addi r1, r1, 112 ld r7, HSTATE_HOST_MSR(r13) - cmpwi cr1, r12, BOOK3S_INTERRUPT_MACHINE_CHECK cmpwi r12, BOOK3S_INTERRUPT_EXTERNAL beq 11f cmpwi cr2, r12, BOOK3S_INTERRUPT_HMI@@ -160,7 +159,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) mtmsrd r6, 1 /* Clear RI in MSR */ mtsrr0 r8 mtsrr1 r7 - beq cr1, 13f /* machine check */ RFI /* On POWER7, we have external interrupts set to use HSRR0/1 */@@ -168,8 +166,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) mtspr SPRN_HSRR1, r7 ba 0x500 -13: b machine_check_fwnmi - 14: mtspr SPRN_HSRR0, r8 mtspr SPRN_HSRR1, r7 b hmi_exception_after_realmode@@ -2381,24 +2377,27 @@ machine_check_realmode: ld r9, HSTATE_KVM_VCPU(r13) li r12, BOOK3S_INTERRUPT_MACHINE_CHECK /* - * Deliver unhandled/fatal (e.g. UE) MCE errors to guest through - * machine check interrupt (set HSRR0 to 0x200). And for handled - * errors (no-fatal), just go back to guest execution with current - * HSRR0 instead of exiting guest. This new approach will inject - * machine check to guest for fatal error causing guest to crash. - * - * The old code used to return to host for unhandled errors which - * was causing guest to hang with soft lockups inside guest and - * makes it difficult to recover guest instance. + * Deliver unhandled/fatal (e.g. UE) MCE errors to guest + * by exiting the guest with KVM_EXIT_NMI exit reason (exit + * reason set later based on trap). For handled errors + * (no-fatal), go back to guest execution with current HSRR0 + * instead of exiting the guest. This approach will cause + * the guest to exit in case of fatal machine check error. */ - ld r10, VCPU_PC(r9) - ld r11, VCPU_MSR(r9) - bne 2f /* Continue guest execution. */ - /* If not, deliver a machine check. SRR0/1 are already set */ - li r10, BOOK3S_INTERRUPT_MACHINE_CHECK + bne 2f /* Continue guest execution? */ + /* If not, check if guest is capable of handling NMI exit */ + ld r3, VCPU_KVM(r9) + ld r3, KVM_FWNMI(r3) + cmpdi r3, 1 /* FWNMI capable? */ + bne 1f /* Deliver machine check via guest's 0x200 vector */ + b mc_cont +1: li r10, BOOK3S_INTERRUPT_MACHINE_CHECK ld r11, VCPU_MSR(r9) bl kvmppc_msr_interrupt -2: b fast_interrupt_c_return + b fast_interrupt_c_return +2: ld r10, VCPU_PC(r9) + ld r11, VCPU_MSR(r9) + b fast_interrupt_c_return /* * Check the reason we woke from nap, and take appropriate action.arch/powerpc/kvm/powerpc.c +7
-0@@ -567,6 +567,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) r =1 ;break ;#endif +case KVM_CAP_PPC_FWNMI : + r =1 ; +break ;default : r =0 ;break ;@@ -1129,6 +1132,10 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu, break; }#endif /* CONFIG_KVM_XICS */ +case KVM_CAP_PPC_FWNMI : + r =0 ; + vcpu->kvm->arch.fwnmi_enabled = true; +break ;default : r = -EINVAL ;break ;include/uapi/linux/kvm.h +1
-0@@ -824,6 +824,7 @@ struct kvm_ppc_smmu_info {#define KVM_CAP_MULTI_ADDRESS_SPACE 118#define KVM_CAP_GUEST_DEBUG_HW_BPS 119#define KVM_CAP_GUEST_DEBUG_HW_WPS 120 +#define KVM_CAP_PPC_FWNMI 121#ifdef KVM_CAP_IRQ_ROUTING _______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev
Quoted 56 lines · Aravinda Prasad
This patch modifies KVM to cause a guest exit with
KVM_EXIT_NMIinstead of immediately delivering a0x200interrupt to guest upon machine check exception in guest address. Exiting the guest enables QEMU to build error log and deliver machine check exception to guest OS (either via guest OS registered machine check handler or via0x200guest OS interrupt vector).This approach simplifies the delivering of machine check exception to guest OS compared to the earlier approach of KVM directly invoking
0x200guest interrupt vector. In the earlier approach QEMU patched the0x200interrupt vector during boot. The patched code at0x200issued a private hcall to pass the control to QEMU to build the error log.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
This patch also introduces a new KVM capability to control how KVM behaves on machine check exception. Without this capability, KVM redirects machine check exceptions to guest's
0x200vector if the address in error belongs to guest. With this capability KVM causes a guest exit with NMI exit reason.This is required to avoid problems if a new kernel/KVM is used with an old QEMU for guests that don't issue "ibm,nmi-register". As old QEMU does not understand the NMI exit type, it treats it as a fatal error. However, the guest could have handled the machine check error if the exception was delivered to guest's
0x200interrupt vector instead of NMI exit in case of old QEMU.Change Log v2:
- Added KVM capabilitySigned-off-by Aravinda Prasad <[email protected]>--- arch/powerpc/include/asm/kvm_host.h | 1 + arch/powerpc/kernel/asm-offsets.c | 1 + arch/powerpc/kvm/book3s_hv.c | 12 +++------- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 37 +++++++++++++++---------------- arch/powerpc/kvm/powerpc.c | 7 ++++++ include/uapi/linux/kvm.h | 1 + 6 files changed, 31 insertions(+), 28 deletions(-)arch/powerpc/include/asm/kvm_host.h +0
-0
arch/powerpc/include/asm/kvm_host.h struct kvm_arch { · line 246
Quoted 4 lines
arch/powerpc/include/asm/kvm_host.h +1
-0@@ -243,6 +243,7 @@ struct kvm_arch {int hpt_cma_alloc;struct dentry *debugfs_dir;struct dentry *htab_dentry; +u8 fwnmi_enabled;
Here you declare the variable as 8-bits ...
arch/powerpc/kernel/asm-offsets.c int main(void) · line 509
Quoted 12 lines
arch/powerpc/include/asm/kvm_host.h +0
-0#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */ #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE struct mutex hpt_mutex;arch/powerpc/kernel/asm-offsets.c +1
-0@@ -506,6 +506,7 @@ int main(void) DEFINE(KVM_ENABLED_HCALLS, offsetof(struct kvm, arch.enabled_hcalls)); DEFINE(KVM_LPCR, offsetof(struct kvm, arch.lpcr)); DEFINE(KVM_VRMA_SLB_V, offsetof(struct kvm, arch.vrma_slb_v)); + DEFINE(KVM_FWNMI, offsetof(struct kvm, arch.fwnmi_enabled));
... then define an asm-offset for it ...
Quoted 7 lines
arch/powerpc/kernel/asm-offsets.c +0
-0DEFINE(VCPU_DSISR, offsetof(struct kvm_vcpu, arch.shregs.dsisr)); DEFINE(VCPU_DAR, offsetof(struct kvm_vcpu, arch.shregs.dar)); DEFINE(VCPU_VPA, offsetof(struct kvm_vcpu, arch.vpa.pinned_addr));arch/powerpc/kvm/book3s_hv_rmhandlers.S +0
-0
[...]
Quoted 20 lines
arch/powerpc/kvm/book3s_hv_rmhandlers.S +6
-9@@ -2381,24 +2377,27 @@ machine_check_realmode: ld r9, HSTATE_KVM_VCPU(r13) li r12, BOOK3S_INTERRUPT_MACHINE_CHECK /* - * Deliver unhandled/fatal (e.g. UE) MCE errors to guest through - * machine check interrupt (set HSRR0 to 0x200). And for handled - * errors (no-fatal), just go back to guest execution with current - * HSRR0 instead of exiting guest. This new approach will inject - * machine check to guest for fatal error causing guest to crash. - * - * The old code used to return to host for unhandled errors which - * was causing guest to hang with soft lockups inside guest and - * makes it difficult to recover guest instance. + * Deliver unhandled/fatal (e.g. UE) MCE errors to guest + * by exiting the guest with KVM_EXIT_NMI exit reason (exit + * reason set later based on trap). For handled errors + * (no-fatal), go back to guest execution with current HSRR0 + * instead of exiting the guest. This approach will cause + * the guest to exit in case of fatal machine check error. */
arch/powerpc/kvm/book3s_hv_rmhandlers.S machine_check_realmode: · line 2391
Quoted 10 lines
arch/powerpc/kvm/book3s_hv_rmhandlers.S +5
-5- ld r10, VCPU_PC(r9) - ld r11, VCPU_MSR(r9) - bne 2f /* Continue guest execution. */ - /* If not, deliver a machine check. SRR0/1 are already set */ - li r10, BOOK3S_INTERRUPT_MACHINE_CHECK + bne 2f /* Continue guest execution? */ + /* If not, check if guest is capable of handling NMI exit */ + ld r3, VCPU_KVM(r9) + ld r3, KVM_FWNMI(r3) + cmpdi r3, 1 /* FWNMI capable? */
... and here you're accessing the 8-bit variable with "ld" and "cmpdi"! Is this really working as expected? Or did I miss something? Did you check your code on both, little and big endian hosts?
Thomas
Quoted 113 lines · Thomas Huth
Quoted 56 lines · Aravinda Prasad
This patch modifies KVM to cause a guest exit with
KVM_EXIT_NMIinstead of immediately delivering a0x200interrupt to guest upon machine check exception in guest address. Exiting the guest enables QEMU to build error log and deliver machine check exception to guest OS (either via guest OS registered machine check handler or via0x200guest OS interrupt vector).This approach simplifies the delivering of machine check exception to guest OS compared to the earlier approach of KVM directly invoking
0x200guest interrupt vector. In the earlier approach QEMU patched the0x200interrupt vector during boot. The patched code at0x200issued a private hcall to pass the control to QEMU to build the error log.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
This patch also introduces a new KVM capability to control how KVM behaves on machine check exception. Without this capability, KVM redirects machine check exceptions to guest's
0x200vector if the address in error belongs to guest. With this capability KVM causes a guest exit with NMI exit reason.This is required to avoid problems if a new kernel/KVM is used with an old QEMU for guests that don't issue "ibm,nmi-register". As old QEMU does not understand the NMI exit type, it treats it as a fatal error. However, the guest could have handled the machine check error if the exception was delivered to guest's
0x200interrupt vector instead of NMI exit in case of old QEMU.Change Log v2:
- Added KVM capabilitySigned-off-by Aravinda Prasad <[email protected]>--- arch/powerpc/include/asm/kvm_host.h | 1 + arch/powerpc/kernel/asm-offsets.c | 1 + arch/powerpc/kvm/book3s_hv.c | 12 +++------- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 37 +++++++++++++++---------------- arch/powerpc/kvm/powerpc.c | 7 ++++++ include/uapi/linux/kvm.h | 1 + 6 files changed, 31 insertions(+), 28 deletions(-)arch/powerpc/include/asm/kvm_host.h +0
-0arch/powerpc/include/asm/kvm_host.h struct kvm_arch { · line 246
Quoted 4 lines
arch/powerpc/include/asm/kvm_host.h +1
-0@@ -243,6 +243,7 @@ struct kvm_arch {int hpt_cma_alloc;struct dentry *debugfs_dir;struct dentry *htab_dentry; +u8 fwnmi_enabled;Here you declare the variable as 8-bits ...
arch/powerpc/kernel/asm-offsets.c int main(void) · line 509
Quoted 12 lines
arch/powerpc/include/asm/kvm_host.h +0
-0#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */ #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE struct mutex hpt_mutex;arch/powerpc/kernel/asm-offsets.c +1
-0@@ -506,6 +506,7 @@ int main(void) DEFINE(KVM_ENABLED_HCALLS, offsetof(struct kvm, arch.enabled_hcalls)); DEFINE(KVM_LPCR, offsetof(struct kvm, arch.lpcr)); DEFINE(KVM_VRMA_SLB_V, offsetof(struct kvm, arch.vrma_slb_v)); + DEFINE(KVM_FWNMI, offsetof(struct kvm, arch.fwnmi_enabled));... then define an asm-offset for it ...
Quoted 7 lines
arch/powerpc/kernel/asm-offsets.c +0
-0DEFINE(VCPU_DSISR, offsetof(struct kvm_vcpu, arch.shregs.dsisr)); DEFINE(VCPU_DAR, offsetof(struct kvm_vcpu, arch.shregs.dar)); DEFINE(VCPU_VPA, offsetof(struct kvm_vcpu, arch.vpa.pinned_addr));arch/powerpc/kvm/book3s_hv_rmhandlers.S +0
-0[...]
Quoted 26 lines
arch/powerpc/kvm/book3s_hv_rmhandlers.S +7
-14@@ -2381,24 +2377,27 @@ machine_check_realmode: ld r9, HSTATE_KVM_VCPU(r13) li r12, BOOK3S_INTERRUPT_MACHINE_CHECK /* - * Deliver unhandled/fatal (e.g. UE) MCE errors to guest through - * machine check interrupt (set HSRR0 to 0x200). And for handled - * errors (no-fatal), just go back to guest execution with current - * HSRR0 instead of exiting guest. This new approach will inject - * machine check to guest for fatal error causing guest to crash. - * - * The old code used to return to host for unhandled errors which - * was causing guest to hang with soft lockups inside guest and - * makes it difficult to recover guest instance. + * Deliver unhandled/fatal (e.g. UE) MCE errors to guest + * by exiting the guest with KVM_EXIT_NMI exit reason (exit + * reason set later based on trap). For handled errors + * (no-fatal), go back to guest execution with current HSRR0 + * instead of exiting the guest. This approach will cause + * the guest to exit in case of fatal machine check error. */ - ld r10, VCPU_PC(r9) - ld r11, VCPU_MSR(r9) - bne 2f /* Continue guest execution. */ - /* If not, deliver a machine check. SRR0/1 are already set */ - li r10, BOOK3S_INTERRUPT_MACHINE_CHECK + bne 2f /* Continue guest execution? */
Quoted 8 lines
arch/powerpc/kvm/book3s_hv_rmhandlers.S machine_check_realmode: · line 2391
Quoted 4 lines
arch/powerpc/kvm/book3s_hv_rmhandlers.S +4
-0+ /* If not, check if guest is capable of handling NMI exit */ + ld r3, VCPU_KVM(r9) + ld r3, KVM_FWNMI(r3) + cmpdi r3, 1 /* FWNMI capable? */... and here you're accessing the 8-bit variable with "ld" and "cmpdi"! Is this really working as expected? Or did I miss something? Did you check your code on both, little and big endian hosts?
Ah... I should have used lbz
Regards,
Aravinda
Quoted 8 lines
Thomas
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev
Signature · 2 lines
Regards, Aravinda
Quoted 103 lines · Aravinda Prasad
This patch modifies KVM to cause a guest exit with
KVM_EXIT_NMIinstead of immediately delivering a0x200interrupt to guest upon machine check exception in guest address. Exiting the guest enables QEMU to build error log and deliver machine check exception to guest OS (either via guest OS registered machine check handler or via0x200guest OS interrupt vector).This approach simplifies the delivering of machine check exception to guest OS compared to the earlier approach of KVM directly invoking
0x200guest interrupt vector. In the earlier approach QEMU patched the0x200interrupt vector during boot. The patched code at0x200issued a private hcall to pass the control to QEMU to build the error log.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
This patch also introduces a new KVM capability to control how KVM behaves on machine check exception. Without this capability, KVM redirects machine check exceptions to guest's
0x200vector if the address in error belongs to guest. With this capability KVM causes a guest exit with NMI exit reason.This is required to avoid problems if a new kernel/KVM is used with an old QEMU for guests that don't issue "ibm,nmi-register". As old QEMU does not understand the NMI exit type, it treats it as a fatal error. However, the guest could have handled the machine check error if the exception was delivered to guest's
0x200interrupt vector instead of NMI exit in case of old QEMU.Change Log v2:
- Added KVM capabilitySigned-off-by Aravinda Prasad <[email protected]>--- arch/powerpc/include/asm/kvm_host.h | 1 + arch/powerpc/kernel/asm-offsets.c | 1 + arch/powerpc/kvm/book3s_hv.c | 12 +++------- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 37 +++++++++++++++---------------- arch/powerpc/kvm/powerpc.c | 7 ++++++ include/uapi/linux/kvm.h | 1 + 6 files changed, 31 insertions(+), 28 deletions(-)arch/powerpc/include/asm/kvm_host.h +1
-0@@ -243,6 +243,7 @@ struct kvm_arch {int hpt_cma_alloc;struct dentry *debugfs_dir;struct dentry *htab_dentry; +u8 fwnmi_enabled;#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */ #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE struct mutex hpt_mutex;arch/powerpc/kernel/asm-offsets.c +1
-0@@ -506,6 +506,7 @@ int main(void) DEFINE(KVM_ENABLED_HCALLS, offsetof(struct kvm, arch.enabled_hcalls)); DEFINE(KVM_LPCR, offsetof(struct kvm, arch.lpcr)); DEFINE(KVM_VRMA_SLB_V, offsetof(struct kvm, arch.vrma_slb_v)); + DEFINE(KVM_FWNMI, offsetof(struct kvm, arch.fwnmi_enabled)); DEFINE(VCPU_DSISR, offsetof(struct kvm_vcpu, arch.shregs.dsisr)); DEFINE(VCPU_DAR, offsetof(struct kvm_vcpu, arch.shregs.dar)); DEFINE(VCPU_VPA, offsetof(struct kvm_vcpu, arch.vpa.pinned_addr));arch/powerpc/kvm/book3s_hv.c +3
-9@@ -859,15 +859,9 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, r =RESUME_GUEST ;break ;case BOOK3S_INTERRUPT_MACHINE_CHECK : -/* -* 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. -*/ -kvmppc_book3s_queue_irqprio (vcpu, -BOOK3S_INTERRUPT_MACHINE_CHECK ); - r =RESUME_GUEST ; +/* Exit to guest with KVM_EXIT_NMI as exit reason */ + run->exit_reason =KVM_EXIT_NMI ; + r =RESUME_HOST ;break ;case BOOK3S_INTERRUPT_PROGRAM : {arch/powerpc/kvm/book3s_hv_rmhandlers.S +0
-0
arch/powerpc/kvm/book3s_hv_rmhandlers.S END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) · line 149
Quoted 3 lines
arch/powerpc/kvm/book3s_hv_rmhandlers.S +0
-0@@ -147,7 +147,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) addi r1, r1, 112 ld r7, HSTATE_HOST_MSR(r13)
Seems like the comment a little above this should be updated to reflect the fact that this path no longer handles machine checks.
Apart from that and the access width bug Thomas spotted, it looks ok to me,.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 111 lines · David Gibson
Quoted 103 lines · Aravinda Prasad
This patch modifies KVM to cause a guest exit with
KVM_EXIT_NMIinstead of immediately delivering a0x200interrupt to guest upon machine check exception in guest address. Exiting the guest enables QEMU to build error log and deliver machine check exception to guest OS (either via guest OS registered machine check handler or via0x200guest OS interrupt vector).This approach simplifies the delivering of machine check exception to guest OS compared to the earlier approach of KVM directly invoking
0x200guest interrupt vector. In the earlier approach QEMU patched the0x200interrupt vector during boot. The patched code at0x200issued a private hcall to pass the control to QEMU to build the error log.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
This patch also introduces a new KVM capability to control how KVM behaves on machine check exception. Without this capability, KVM redirects machine check exceptions to guest's
0x200vector if the address in error belongs to guest. With this capability KVM causes a guest exit with NMI exit reason.This is required to avoid problems if a new kernel/KVM is used with an old QEMU for guests that don't issue "ibm,nmi-register". As old QEMU does not understand the NMI exit type, it treats it as a fatal error. However, the guest could have handled the machine check error if the exception was delivered to guest's
0x200interrupt vector instead of NMI exit in case of old QEMU.Change Log v2:
- Added KVM capabilitySigned-off-by Aravinda Prasad <[email protected]>--- arch/powerpc/include/asm/kvm_host.h | 1 + arch/powerpc/kernel/asm-offsets.c | 1 + arch/powerpc/kvm/book3s_hv.c | 12 +++------- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 37 +++++++++++++++---------------- arch/powerpc/kvm/powerpc.c | 7 ++++++ include/uapi/linux/kvm.h | 1 + 6 files changed, 31 insertions(+), 28 deletions(-)arch/powerpc/include/asm/kvm_host.h +1
-0@@ -243,6 +243,7 @@ struct kvm_arch {int hpt_cma_alloc;struct dentry *debugfs_dir;struct dentry *htab_dentry; +u8 fwnmi_enabled;#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */ #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE struct mutex hpt_mutex;arch/powerpc/kernel/asm-offsets.c +1
-0@@ -506,6 +506,7 @@ int main(void) DEFINE(KVM_ENABLED_HCALLS, offsetof(struct kvm, arch.enabled_hcalls)); DEFINE(KVM_LPCR, offsetof(struct kvm, arch.lpcr)); DEFINE(KVM_VRMA_SLB_V, offsetof(struct kvm, arch.vrma_slb_v)); + DEFINE(KVM_FWNMI, offsetof(struct kvm, arch.fwnmi_enabled)); DEFINE(VCPU_DSISR, offsetof(struct kvm_vcpu, arch.shregs.dsisr)); DEFINE(VCPU_DAR, offsetof(struct kvm_vcpu, arch.shregs.dar)); DEFINE(VCPU_VPA, offsetof(struct kvm_vcpu, arch.vpa.pinned_addr));arch/powerpc/kvm/book3s_hv.c +3
-9@@ -859,15 +859,9 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu, r =RESUME_GUEST ;break ;case BOOK3S_INTERRUPT_MACHINE_CHECK : -/* -* 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. -*/ -kvmppc_book3s_queue_irqprio (vcpu, -BOOK3S_INTERRUPT_MACHINE_CHECK ); - r =RESUME_GUEST ; +/* Exit to guest with KVM_EXIT_NMI as exit reason */ + run->exit_reason =KVM_EXIT_NMI ; + r =RESUME_HOST ;break ;case BOOK3S_INTERRUPT_PROGRAM : {arch/powerpc/kvm/book3s_hv_rmhandlers.S +0
-0arch/powerpc/kvm/book3s_hv_rmhandlers.S END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) · line 149
Quoted 3 lines
arch/powerpc/kvm/book3s_hv_rmhandlers.S +0
-0@@ -147,7 +147,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S) addi r1, r1, 112 ld r7, HSTATE_HOST_MSR(r13)Seems like the comment a little above this should be updated to reflect the fact that this path no longer handles machine checks.
Quoted 2 lines
Apart from that and the access width bug Thomas spotted, it looks ok to me,.
sure.
Regards,
Aravinda
Quoted 8 lines
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev
Signature · 2 lines
Regards, Aravinda