Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1716931 > unrolled thread
| Started by | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| First post | 2017-08-22 01:10 +0200 |
| Last post | 2017-08-23 09:20 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH v3] KVM: nVMX: Fix trying to cancel vmlauch/vmresume Wanpeng Li <kernellwp@gmail.com> - 2017-08-22 01:10 +0200
Re: [PATCH v3] KVM: nVMX: Fix trying to cancel vmlauch/vmresume Paolo Bonzini <pbonzini@redhat.com> - 2017-08-22 18:20 +0200
Re: [PATCH v3] KVM: nVMX: Fix trying to cancel vmlauch/vmresume Wanpeng Li <kernellwp@gmail.com> - 2017-08-23 00:00 +0200
Re: [PATCH v3] KVM: nVMX: Fix trying to cancel vmlauch/vmresume Paolo Bonzini <pbonzini@redhat.com> - 2017-08-23 09:20 +0200
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-08-22 01:10 +0200 |
| Subject | [PATCH v3] KVM: nVMX: Fix trying to cancel vmlauch/vmresume |
| Message-ID | <uh3Zv-2fV-7@gated-at.bofh.it> |
From: Wanpeng Li <wanpeng.li@hotmail.com>
------------[ 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 running L2 next
instead of L1 in the queue exception path and injecting the pending exception to
L2 directly.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
v2 -> v3:
* move the nested_run_pending to the else branch
v1 -> v2:
* request an immediate VM exit from L2 and keep the exception for
L1 pending for a subsequent nested VM exit
arch/x86/kvm/vmx.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index e398946..685f51e 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2488,6 +2488,10 @@ static int nested_vmx_check_exception(struct kvm_vcpu *vcpu)
}
} else {
unsigned long exit_qual = 0;
+
+ if (to_vmx(vcpu)->nested.nested_run_pending)
+ return 0;
+
if (nr == DB_VECTOR)
exit_qual = vcpu->arch.dr6;
--
2.7.4
[toc] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2017-08-22 18:20 +0200 |
| Message-ID | <uhk4i-4NV-21@gated-at.bofh.it> |
| In reply to | #1716931 |
On 22/08/2017 01:08, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
>
> ------------[ 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 running L2 next
> instead of L1 in the queue exception path and injecting the pending exception to
> L2 directly.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
> v2 -> v3:
> * move the nested_run_pending to the else branch
> v1 -> v2:
> * request an immediate VM exit from L2 and keep the exception for
> L1 pending for a subsequent nested VM exit
>
> arch/x86/kvm/vmx.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index e398946..685f51e 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2488,6 +2488,10 @@ static int nested_vmx_check_exception(struct kvm_vcpu *vcpu)
> }
> } else {
> unsigned long exit_qual = 0;
> +
> + if (to_vmx(vcpu)->nested.nested_run_pending)
> + return 0;
> +
> if (nr == DB_VECTOR)
> exit_qual = vcpu->arch.dr6;
>
>
Hmm, why would this not apply to page faults? It doesn't make much sense...
Paolo
[toc] | [prev] | [next] | [standalone]
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-08-23 00:00 +0200 |
| Message-ID | <uhpnk-8fS-37@gated-at.bofh.it> |
| In reply to | #1717547 |
2017-08-23 0:09 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
> On 22/08/2017 01:08, Wanpeng Li wrote:
>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>
>> ------------[ 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 running L2 next
>> instead of L1 in the queue exception path and injecting the pending exception to
>> L2 directly.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>> v2 -> v3:
>> * move the nested_run_pending to the else branch
>> v1 -> v2:
>> * request an immediate VM exit from L2 and keep the exception for
>> L1 pending for a subsequent nested VM exit
>>
>> arch/x86/kvm/vmx.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index e398946..685f51e 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -2488,6 +2488,10 @@ static int nested_vmx_check_exception(struct kvm_vcpu *vcpu)
>> }
>> } else {
>> unsigned long exit_qual = 0;
>> +
>> + if (to_vmx(vcpu)->nested.nested_run_pending)
>> + return 0;
>> +
>> if (nr == DB_VECTOR)
>> exit_qual = vcpu->arch.dr6;
>>
>>
>
> Hmm, why would this not apply to page faults? It doesn't make much sense...
Agreed, I do this in v1, please have a look.
Regards,
Wanpeng Li
[toc] | [prev] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2017-08-23 09:20 +0200 |
| Message-ID | <uhy7g-5VC-17@gated-at.bofh.it> |
| In reply to | #1717867 |
On 22/08/2017 23:57, Wanpeng Li wrote:
> 2017-08-23 0:09 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>> On 22/08/2017 01:08, Wanpeng Li wrote:
>>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>>
>>> ------------[ 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 running L2 next
>>> instead of L1 in the queue exception path and injecting the pending exception to
>>> L2 directly.
>>>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>>> ---
>>> v2 -> v3:
>>> * move the nested_run_pending to the else branch
>>> v1 -> v2:
>>> * request an immediate VM exit from L2 and keep the exception for
>>> L1 pending for a subsequent nested VM exit
>>>
>>> arch/x86/kvm/vmx.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index e398946..685f51e 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -2488,6 +2488,10 @@ static int nested_vmx_check_exception(struct kvm_vcpu *vcpu)
>>> }
>>> } else {
>>> unsigned long exit_qual = 0;
>>> +
>>> + if (to_vmx(vcpu)->nested.nested_run_pending)
>>> + return 0;
>>> +
>>> if (nr == DB_VECTOR)
>>> exit_qual = vcpu->arch.dr6;
>>>
>>>
>>
>> Hmm, why would this not apply to page faults? It doesn't make much sense...
>
> Agreed, I do this in v1, please have a look.
On the other hand Radim was right in his review of v1 (and sort of wrong
in the review of v2). I replied there.
Paolo
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web