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@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
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@xxxxxxxxxx>
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);
