Re: [PATCH v2] KVM: nVMX: Fix trying to cancel vmlauch/vmresume

[Wanpeng Li]

2017-08-23 15:13 GMT+08:00 Paolo Bonzini <pbonzini@xxxxxxxxxx>:
> On 21/08/2017 18:20, Radim KrÄmÃÅ wrote:
>> 2017-08-18 07:11-0700, Wanpeng Li:
>>> From: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
>>>
>>> ------------[ cut here ]------------
>>> WARNING: CPU: 7 PID: 3861 at /home/kernel/ssd/kvm/arch/x86/kvm//vmx.c:11299 nested_vmx_vmexit+0x176e/0x1980 [kvm_intel]
>>> CPU: 7 PID: 3861 Comm: qemu-system-x86 Tainted: G        W  OE   4.13.0-rc4+ #11
>>> RIP: 0010:nested_vmx_vmexit+0x176e/0x1980 [kvm_intel]
>>> Call Trace:
>>>  ? kvm_multiple_exception+0x149/0x170 [kvm]
>>>  ? handle_emulation_failure+0x79/0x230 [kvm]
>>>  ? load_vmcs12_host_state+0xa80/0xa80 [kvm_intel]
>>>  ? check_chain_key+0x137/0x1e0
>>>  ? reexecute_instruction.part.168+0x130/0x130 [kvm]
>>>  nested_vmx_inject_exception_vmexit+0xb7/0x100 [kvm_intel]
>>>  ? nested_vmx_inject_exception_vmexit+0xb7/0x100 [kvm_intel]
>>>  vmx_queue_exception+0x197/0x300 [kvm_intel]
>>>  kvm_arch_vcpu_ioctl_run+0x1b0c/0x2c90 [kvm]
>>>  ? kvm_arch_vcpu_runnable+0x220/0x220 [kvm]
>>>  ? preempt_count_sub+0x18/0xc0
>>>  ? restart_apic_timer+0x17d/0x300 [kvm]
>>>  ? kvm_lapic_restart_hv_timer+0x37/0x50 [kvm]
>>>  ? kvm_arch_vcpu_load+0x1d8/0x350 [kvm]
>>>  kvm_vcpu_ioctl+0x4e4/0x910 [kvm]
>>>  ? kvm_vcpu_ioctl+0x4e4/0x910 [kvm]
>>>  ? kvm_dev_ioctl+0xbe0/0xbe0 [kvm]
>>>
>>> The flag "nested_run_pending", which can override the decision of which should run
>>> next, L1 or L2. nested_run_pending=1 means that we *must* run L2 next, not L1. This
>>> is necessary in particular when L1 did a VMLAUNCH of L2 and therefore expects L2 to
>>> be run (and perhaps be injected with an event it specified, etc.). Nested_run_pending
>>> is especially intended to avoid switching  to L1 in the injection decision-point.
>>>
>>> I catch this in the queue exception path, this patch fixes it by requesting
>>> an immediate VM exit from L2 and keeping the exception for L1 pending for a
>>> subsequent nested VM exit.
>>>
>>> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>>> Cc: Radim KrÄmÃÅ <rkrcmar@xxxxxxxxxx>
>>> Signed-off-by: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
>>> ---
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> @@ -6356,8 +6356,8 @@ static int inject_pending_event(struct kvm_vcpu *vcpu, bool req_int_win)
>>>                      kvm_update_dr7(vcpu);
>>>              }
>>
>> Hm, we shouldn't execute the code above if exception won't be injected.
>
> True, the code should have been when the exception is injected first,
> not here.  But it's fine since in both cases (set RF and clear GD) it's
> idempotent.
>
>>>
>>> -            kvm_x86_ops->queue_exception(vcpu);
>>> -            return 0;
>>
>> vmx_complete_interrupts() assumes that the exception is always injected,
>> so it would be dropped by kvm_clear_exception_queue().
>
> That's a separate bug.  We need to separate exception.pending from
> exception.injected, similar to NMIs (what is now exception.pending would
> become exception.injected).
>
> For now, this patch would be better than nothing, but getting the right
> fix for 4.14 would be nice too.

Ok, I will figure out the exception.pending and exception.injected later. :)

Regards,
Wanpeng Li