| Message ID | 20190420055059.16816-2-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: VMX: INTR, NMI and #MC cleanup | expand |
On 20/04/19 07:50, Sean Christopherson wrote: > A previous fix to prevent KVM from consuming stale VMCS state after a > failed VM-Entry inadvertantly blocked KVM's handling of machine checks > that occur during VM-Entry. > > Per Intel's SDM, a #MC during VM-Entry is handled in one of three ways, > depending on when the #MC is recognoized. As it pertains to this bug > fix, the third case explicitly states EXIT_REASON_MCE_DURING_VMENTRY > is handled like any other VM-Exit during VM-Entry, i.e. sets bit 31 to > indicate the VM-Entry failed. > > If a machine-check event occurs during a VM entry, one of the following occurs: > - The machine-check event is handled as if it occurred before the VM entry: > ... > - The machine-check event is handled after VM entry completes: > ... > - A VM-entry failure occurs as described in Section 26.7. The basic > exit reason is 41, for "VM-entry failure due to machine-check event". > > Explicitly handle EXIT_REASON_MCE_DURING_VMENTRY as a one-off case in > vmx_vcpu_run() instead of binning it into vmx_complete_atomic_exit(). > Doing so allows vmx_vcpu_run() to handle VMX_EXIT_REASONS_FAILED_VMENTRY > in a sane fashion and also simplifies vmx_complete_atomic_exit() since > VMCS.VM_EXIT_INTR_INFO is guaranteed to be fresh. > > Fixes: b060ca3b2e9e7 ("kvm: vmx: Handle VMLAUNCH/VMRESUME failure properly") > Cc: Jim Mattson <jmattson@google.com> > Cc: stable@vger.kernel.org > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > arch/x86/kvm/vmx/vmx.c | 20 ++++++++------------ > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index d8f101b58ab8..79ce9c7062f9 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -6103,28 +6103,21 @@ static void vmx_apicv_post_state_restore(struct kvm_vcpu *vcpu) > > static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx) > { > - u32 exit_intr_info = 0; > - u16 basic_exit_reason = (u16)vmx->exit_reason; > - > - if (!(basic_exit_reason == EXIT_REASON_MCE_DURING_VMENTRY > - || basic_exit_reason == EXIT_REASON_EXCEPTION_NMI)) > + if (vmx->exit_reason != EXIT_REASON_EXCEPTION_NMI) > return; > > - if (!(vmx->exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY)) > - exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO); > - vmx->exit_intr_info = exit_intr_info; > + vmx->exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO); > > /* if exit due to PF check for async PF */ > - if (is_page_fault(exit_intr_info)) > + if (is_page_fault(vmx->exit_intr_info)) > vmx->vcpu.arch.apf.host_apf_reason = kvm_read_and_reset_pf_reason(); > > /* Handle machine checks before interrupts are enabled */ > - if (basic_exit_reason == EXIT_REASON_MCE_DURING_VMENTRY || > - is_machine_check(exit_intr_info)) > + if (is_machine_check(vmx->exit_intr_info)) > kvm_machine_check(); > > /* We need to handle NMIs before interrupts are enabled */ > - if (is_nmi(exit_intr_info)) { > + if (is_nmi(vmx->exit_intr_info)) { > kvm_before_interrupt(&vmx->vcpu); > asm("int $2"); > kvm_after_interrupt(&vmx->vcpu); This is indeed cleaner in addition to fixing the bug. I'm also applying this -------------- 8< -------------- Subject: [PATCH] kvm: nVMX: small cleanup in handle_exception From: Paolo Bonzini <pbonzini@redhat.com> The reason for skipping handling of NMI and #MC in handle_exception is the same, namely they are handled earlier by vmx_complete_atomic_exit. Calling the machine check handler (which just returns 1) is misleading, don't do it. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 1b3ca0582a0c..da6c829bad9f 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -4455,11 +4455,8 @@ static int handle_exception(struct kvm_vcpu *vcpu) vect_info = vmx->idt_vectoring_info; intr_info = vmx->exit_intr_info; - if (is_machine_check(intr_info)) - return handle_machine_check(vcpu); - - if (is_nmi(intr_info)) - return 1; /* already handled by vmx_vcpu_run() */ + if (is_machine_check(intr_info) || is_nmi(intr_info)) + return 1; /* already handled by vmx_complete_atomic_exit */ if (is_invalid_opcode(intr_info)) return handle_ud(vcpu);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index d8f101b58ab8..79ce9c7062f9 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -6103,28 +6103,21 @@ static void vmx_apicv_post_state_restore(struct kvm_vcpu *vcpu) static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx) { - u32 exit_intr_info = 0; - u16 basic_exit_reason = (u16)vmx->exit_reason; - - if (!(basic_exit_reason == EXIT_REASON_MCE_DURING_VMENTRY - || basic_exit_reason == EXIT_REASON_EXCEPTION_NMI)) + if (vmx->exit_reason != EXIT_REASON_EXCEPTION_NMI) return; - if (!(vmx->exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY)) - exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO); - vmx->exit_intr_info = exit_intr_info; + vmx->exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO); /* if exit due to PF check for async PF */ - if (is_page_fault(exit_intr_info)) + if (is_page_fault(vmx->exit_intr_info)) vmx->vcpu.arch.apf.host_apf_reason = kvm_read_and_reset_pf_reason(); /* Handle machine checks before interrupts are enabled */ - if (basic_exit_reason == EXIT_REASON_MCE_DURING_VMENTRY || - is_machine_check(exit_intr_info)) + if (is_machine_check(vmx->exit_intr_info)) kvm_machine_check(); /* We need to handle NMIs before interrupts are enabled */ - if (is_nmi(exit_intr_info)) { + if (is_nmi(vmx->exit_intr_info)) { kvm_before_interrupt(&vmx->vcpu); asm("int $2"); kvm_after_interrupt(&vmx->vcpu); @@ -6527,6 +6520,9 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu) vmx->idt_vectoring_info = 0; vmx->exit_reason = vmx->fail ? 0xdead : vmcs_read32(VM_EXIT_REASON); + if ((u16)vmx->exit_reason == EXIT_REASON_MCE_DURING_VMENTRY) + kvm_machine_check(); + if (vmx->fail || (vmx->exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY)) return;
A previous fix to prevent KVM from consuming stale VMCS state after a failed VM-Entry inadvertantly blocked KVM's handling of machine checks that occur during VM-Entry. Per Intel's SDM, a #MC during VM-Entry is handled in one of three ways, depending on when the #MC is recognoized. As it pertains to this bug fix, the third case explicitly states EXIT_REASON_MCE_DURING_VMENTRY is handled like any other VM-Exit during VM-Entry, i.e. sets bit 31 to indicate the VM-Entry failed. If a machine-check event occurs during a VM entry, one of the following occurs: - The machine-check event is handled as if it occurred before the VM entry: ... - The machine-check event is handled after VM entry completes: ... - A VM-entry failure occurs as described in Section 26.7. The basic exit reason is 41, for "VM-entry failure due to machine-check event". Explicitly handle EXIT_REASON_MCE_DURING_VMENTRY as a one-off case in vmx_vcpu_run() instead of binning it into vmx_complete_atomic_exit(). Doing so allows vmx_vcpu_run() to handle VMX_EXIT_REASONS_FAILED_VMENTRY in a sane fashion and also simplifies vmx_complete_atomic_exit() since VMCS.VM_EXIT_INTR_INFO is guaranteed to be fresh. Fixes: b060ca3b2e9e7 ("kvm: vmx: Handle VMLAUNCH/VMRESUME failure properly") Cc: Jim Mattson <jmattson@google.com> Cc: stable@vger.kernel.org Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/kvm/vmx/vmx.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-)