[PATCH v3 RESEND] kvm: vmx: Properly handle machine check during VM-entry

[PATCH v3 RESEND] kvm: vmx: Properly handle machine check during VM-entry

[Jim Mattson]

vmx_complete_atomic_exit should call kvm_machine_check for any
VM-entry failure due to a machine-check event. Such an exit should be
recognized solely by its basic exit reason (i.e. the low 16 bits of
the VMCS exit reason field). None of the other VMCS exit information
fields contain valid information when the VM-exit is due to "VM-entry
failure due to machine-check event".

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ca5d2b93385c..42db3eb2d13b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8591,17 +8591,20 @@ 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;
+	u32 exit_intr_info = 0;
+	u16 basic_exit_reason = (u16)vmx->exit_reason;
 
-	if (!(vmx->exit_reason == EXIT_REASON_MCE_DURING_VMENTRY
-	      || vmx->exit_reason == EXIT_REASON_EXCEPTION_NMI))
+	if (!(basic_exit_reason == EXIT_REASON_MCE_DURING_VMENTRY
+	      || basic_exit_reason == EXIT_REASON_EXCEPTION_NMI))
 		return;
 
-	vmx->exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
-	exit_intr_info = vmx->exit_intr_info;
+	if (basic_exit_reason != EXIT_REASON_MCE_DURING_VMENTRY)
+		exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
+	vmx->exit_intr_info = exit_intr_info;
 
 	/* Handle machine checks before interrupts are enabled */
-	if (is_machine_check(exit_intr_info))
+	if (basic_exit_reason == EXIT_REASON_MCE_DURING_VMENTRY ||
+	    is_machine_check(exit_intr_info))
 		kvm_machine_check();
 
 	/* We need to handle NMIs before interrupts are enabled */
-- 
2.13.2.725.g09c95d1e9-goog

Re: [PATCH v3 RESEND] kvm: vmx: Properly handle machine check during VM-entry

[Xiao Guangrong]

On 06/30/2017 04:01 AM, Jim Mattson wrote:
> vmx_complete_atomic_exit should call kvm_machine_check for any
> VM-entry failure due to a machine-check event. Such an exit should be
> recognized solely by its basic exit reason (i.e. the low 16 bits of
> the VMCS exit reason field). None of the other VMCS exit information
> fields contain valid information when the VM-exit is due to "VM-entry
> failure due to machine-check event".

Reviewed-by: Xiao Guangrong <xiaoguangrong@tencent.com>

Re: [PATCH v3 RESEND] kvm: vmx: Properly handle machine check during VM-entry

[Radim Krčmář]

2017-06-29 13:01-0700, Jim Mattson:
> vmx_complete_atomic_exit should call kvm_machine_check for any
> VM-entry failure due to a machine-check event. Such an exit should be
> recognized solely by its basic exit reason (i.e. the low 16 bits of
> the VMCS exit reason field). None of the other VMCS exit information
> fields contain valid information when the VM-exit is due to "VM-entry
> failure due to machine-check event".
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/vmx.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ca5d2b93385c..42db3eb2d13b 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -8591,17 +8591,20 @@ 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;
> +	u32 exit_intr_info = 0;
> +	u16 basic_exit_reason = (u16)vmx->exit_reason;
>  
> -	if (!(vmx->exit_reason == EXIT_REASON_MCE_DURING_VMENTRY
> -	      || vmx->exit_reason == EXIT_REASON_EXCEPTION_NMI))
> +	if (!(basic_exit_reason == EXIT_REASON_MCE_DURING_VMENTRY
> +	      || basic_exit_reason == EXIT_REASON_EXCEPTION_NMI))
>  		return;
>  
> -	vmx->exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
> -	exit_intr_info = vmx->exit_intr_info;
> +	if (basic_exit_reason != EXIT_REASON_MCE_DURING_VMENTRY)

I think this would be clearer as

  if (!(vmx->exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY))

because that is the actual reason why VM_EXIT_INTR_INFO is not useable.
I'm ok with the original too, and will change that while applying if you
agree,

thanks.

> +		exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
> +	vmx->exit_intr_info = exit_intr_info;
>  
>  	/* Handle machine checks before interrupts are enabled */
> -	if (is_machine_check(exit_intr_info))
> +	if (basic_exit_reason == EXIT_REASON_MCE_DURING_VMENTRY ||
> +	    is_machine_check(exit_intr_info))
>  		kvm_machine_check();
>  
>  	/* We need to handle NMIs before interrupts are enabled */
> -- 
> 2.13.2.725.g09c95d1e9-goog
> 

Re: [PATCH v3 RESEND] kvm: vmx: Properly handle machine check during VM-entry

[Jim Mattson]

SGTM.  Thanks!

On Tue, Jul 11, 2017 at 7:30 AM, Radim Krčmář <rkrcmar@redhat.com> wrote:
> 2017-06-29 13:01-0700, Jim Mattson:
>> vmx_complete_atomic_exit should call kvm_machine_check for any
>> VM-entry failure due to a machine-check event. Such an exit should be
>> recognized solely by its basic exit reason (i.e. the low 16 bits of
>> the VMCS exit reason field). None of the other VMCS exit information
>> fields contain valid information when the VM-exit is due to "VM-entry
>> failure due to machine-check event".
>>
>> Signed-off-by: Jim Mattson <jmattson@google.com>
>> ---
>>  arch/x86/kvm/vmx.c | 15 +++++++++------
>>  1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index ca5d2b93385c..42db3eb2d13b 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -8591,17 +8591,20 @@ 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;
>> +     u32 exit_intr_info = 0;
>> +     u16 basic_exit_reason = (u16)vmx->exit_reason;
>>
>> -     if (!(vmx->exit_reason == EXIT_REASON_MCE_DURING_VMENTRY
>> -           || vmx->exit_reason == EXIT_REASON_EXCEPTION_NMI))
>> +     if (!(basic_exit_reason == EXIT_REASON_MCE_DURING_VMENTRY
>> +           || basic_exit_reason == EXIT_REASON_EXCEPTION_NMI))
>>               return;
>>
>> -     vmx->exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
>> -     exit_intr_info = vmx->exit_intr_info;
>> +     if (basic_exit_reason != EXIT_REASON_MCE_DURING_VMENTRY)
>
> I think this would be clearer as
>
>   if (!(vmx->exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY))
>
> because that is the actual reason why VM_EXIT_INTR_INFO is not useable.
> I'm ok with the original too, and will change that while applying if you
> agree,
>
> thanks.
>
>> +             exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
>> +     vmx->exit_intr_info = exit_intr_info;
>>
>>       /* Handle machine checks before interrupts are enabled */
>> -     if (is_machine_check(exit_intr_info))
>> +     if (basic_exit_reason == EXIT_REASON_MCE_DURING_VMENTRY ||
>> +         is_machine_check(exit_intr_info))
>>               kvm_machine_check();
>>
>>       /* We need to handle NMIs before interrupts are enabled */
>> --
>> 2.13.2.725.g09c95d1e9-goog
>>