Re: [PATCH v2] KVM: nVMX: Fix exception injection

[Wanpeng Li]

2017-06-05 20:26 GMT+08:00 Paolo Bonzini <pbonzini@xxxxxxxxxx>:
>
>
> On 05/06/2017 14:19, Wanpeng Li wrote:
>> From: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
>>
>>  WARNING: CPU: 3 PID: 2840 at arch/x86/kvm/vmx.c:10966 nested_vmx_vmexit+0xdcd/0xde0 [kvm_intel]
>>  CPU: 3 PID: 2840 Comm: qemu-system-x86 Tainted: G           OE   4.12.0-rc3+ #23
>>  RIP: 0010:nested_vmx_vmexit+0xdcd/0xde0 [kvm_intel]
>>  Call Trace:
>>   ? kvm_check_async_pf_completion+0xef/0x120 [kvm]
>>   ? rcu_read_lock_sched_held+0x79/0x80
>>   vmx_queue_exception+0x104/0x160 [kvm_intel]
>>   ? vmx_queue_exception+0x104/0x160 [kvm_intel]
>>   kvm_arch_vcpu_ioctl_run+0x1171/0x1ce0 [kvm]
>>   ? kvm_arch_vcpu_load+0x47/0x240 [kvm]
>>   ? kvm_arch_vcpu_load+0x62/0x240 [kvm]
>>   kvm_vcpu_ioctl+0x384/0x7b0 [kvm]
>>   ? kvm_vcpu_ioctl+0x384/0x7b0 [kvm]
>>   ? __fget+0xf3/0x210
>>   do_vfs_ioctl+0xa4/0x700
>>   ? __fget+0x114/0x210
>>   SyS_ioctl+0x79/0x90
>>   do_syscall_64+0x81/0x220
>>   entry_SYSCALL64_slow_path+0x25/0x25
>>
>> This is triggered occasionally by running both win7 and win2016 in L2, in
>> addition, EPT is disabled on both L1 and L2. It can't be reproduced easily.
>>
>> Commit 0b6ac343fc (KVM: nVMX: Correct handling of exception injection) mentioned
>> that "KVM wants to inject page-faults which it got to the guest. This function
>> assumes it is called with the exit reason in vmcs02 being a #PF exception".
>> Commit e011c663 (KVM: nVMX: Check all exceptions for intercept during delivery to
>> L2) allows to check all exceptions for intercept during delivery to L2. However,
>> there is no guarantee the exit reason is exception currently, when there is an
>> external interrupt occurred on host, maybe a time interrupt for host which should
>> not be injected to guest, and somewhere queues an exception, then the function
>> nested_vmx_check_exception() will be called and the vmexit emulation codes will
>> try to emulate the "Acknowledge interrupt on exit" behavior, the warning is
>> triggered.
>>
>> This patch fixes it by confirming to inject exception to the guest when the exit
>> reason in vmcs02 is exception.
>
> I think the final part of the commit message needs to be reworded, like:
>
> Reusing the exit reason from the L2->L0 vmexit is wrong in this case,
> the reason must always be EXCEPTION_NMI when injecting an exception into
> L1 as a nested vmexit.
>
> Radim can fix it when applying.

Thanks for that. :)

Regards,
Wanpeng Li

>
> Paolo
>
>> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>> Cc: Radim KrÄmÃÅ <rkrcmar@xxxxxxxxxx>
>> Signed-off-by: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
>> ---
>> v1 -> v2:
>>  * pass EXIT_REASON_EXCEPTION_NMI instead of reusing to_vmx(vcpu)->exit_reason
>>
>>  arch/x86/kvm/vmx.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 9b4b5d6..ca5d2b9 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -2425,7 +2425,7 @@ static int nested_vmx_check_exception(struct kvm_vcpu *vcpu, unsigned nr)
>>       if (!(vmcs12->exception_bitmap & (1u << nr)))
>>               return 0;
>>
>> -     nested_vmx_vmexit(vcpu, to_vmx(vcpu)->exit_reason,
>> +     nested_vmx_vmexit(vcpu, EXIT_REASON_EXCEPTION_NMI,
>>                         vmcs_read32(VM_EXIT_INTR_INFO),
>>                         vmcs_readl(EXIT_QUALIFICATION));
>>       return 1;
>>