diff mbox

KVM: nVMX: Fix try to emulate "Acknowledge interrupt on exit" for interrupt which belongs to L1

Message ID 1501399902-7325-1-git-send-email-wanpeng.li@hotmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wanpeng Li July 30, 2017, 7:31 a.m. UTC
From: Wanpeng Li <wanpeng.li@hotmail.com>

------------[ cut here ]------------
 WARNING: CPU: 5 PID: 2288 at arch/x86/kvm/vmx.c:11124 nested_vmx_vmexit+0xd64/0xd70 [kvm_intel]
 CPU: 5 PID: 2288 Comm: qemu-system-x86 Not tainted 4.13.0-rc2+ #7
 RIP: 0010:nested_vmx_vmexit+0xd64/0xd70 [kvm_intel]
Call Trace:
  vmx_check_nested_events+0x131/0x1f0 [kvm_intel]
  ? vmx_check_nested_events+0x131/0x1f0 [kvm_intel]
  kvm_arch_vcpu_ioctl_run+0x5dd/0x1be0 [kvm]
  ? vmx_vcpu_load+0x1be/0x220 [kvm_intel]
  ? kvm_arch_vcpu_load+0x62/0x230 [kvm]
  kvm_vcpu_ioctl+0x340/0x700 [kvm]
  ? kvm_vcpu_ioctl+0x340/0x700 [kvm]
  ? __fget+0xfc/0x210
  do_vfs_ioctl+0xa4/0x6a0
  ? __fget+0x11d/0x210
  SyS_ioctl+0x79/0x90
  do_syscall_64+0x8f/0x750
  ? trace_hardirqs_on_thunk+0x1a/0x1c
  entry_SYSCALL64_slow_path+0x25/0x25

This can be reproduced by booting L1 guest w/ 'noapic' grub parameter, which 
means that tells the kernel to not make use of any IOAPICs that may be present 
in the system.

There is a scenario when !kvm_cpu_has_interrupt(vcpu) && external_intr in 
vmx_check_nested_events() which means that there is a pending interrupt from 
L0's qemu device model which should be injected to L1, we should not try to 
emualte "Acknowledge interrupt on exit" for this interrupt.

This patch fixes it by trying to emulate "Acknowledge interrupt on exit" if L1 
asks us to.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/kvm/vmx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Paolo Bonzini July 31, 2017, 12:10 p.m. UTC | #1
On 30/07/2017 09:31, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> ------------[ cut here ]------------
>  WARNING: CPU: 5 PID: 2288 at arch/x86/kvm/vmx.c:11124 nested_vmx_vmexit+0xd64/0xd70 [kvm_intel]
>  CPU: 5 PID: 2288 Comm: qemu-system-x86 Not tainted 4.13.0-rc2+ #7
>  RIP: 0010:nested_vmx_vmexit+0xd64/0xd70 [kvm_intel]
> Call Trace:
>   vmx_check_nested_events+0x131/0x1f0 [kvm_intel]
>   ? vmx_check_nested_events+0x131/0x1f0 [kvm_intel]
>   kvm_arch_vcpu_ioctl_run+0x5dd/0x1be0 [kvm]
>   ? vmx_vcpu_load+0x1be/0x220 [kvm_intel]
>   ? kvm_arch_vcpu_load+0x62/0x230 [kvm]
>   kvm_vcpu_ioctl+0x340/0x700 [kvm]
>   ? kvm_vcpu_ioctl+0x340/0x700 [kvm]
>   ? __fget+0xfc/0x210
>   do_vfs_ioctl+0xa4/0x6a0
>   ? __fget+0x11d/0x210
>   SyS_ioctl+0x79/0x90
>   do_syscall_64+0x8f/0x750
>   ? trace_hardirqs_on_thunk+0x1a/0x1c
>   entry_SYSCALL64_slow_path+0x25/0x25
> 
> This can be reproduced by booting L1 guest w/ 'noapic' grub parameter, which 
> means that tells the kernel to not make use of any IOAPICs that may be present 
> in the system.
> 
> There is a scenario when !kvm_cpu_has_interrupt(vcpu) && external_intr in 
> vmx_check_nested_events() which means that there is a pending interrupt from 
> L0's qemu device model which should be injected to L1, we should not try to 
> emualte "Acknowledge interrupt on exit" for this interrupt.
> 
> This patch fixes it by trying to emulate "Acknowledge interrupt on exit" if L1 
> asks us to.

The commit message is not clear; can you rephrase?

> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  arch/x86/kvm/vmx.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 2737343..92eb0f75 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -11118,8 +11118,9 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
>  
>  	vmx_switch_vmcs(vcpu, &vmx->vmcs01);
>  
> -	if ((exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT)
> -	    && nested_exit_intr_ack_set(vcpu)) {
> +	if (exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT &&
> +		kvm_cpu_has_interrupt(vcpu) &&
> +		nested_exit_intr_ack_set(vcpu)) {

Maybe check nested_exit_intr_ack_set first.

Paolo

>  		int irq = kvm_cpu_get_interrupt(vcpu);
>  		WARN_ON(irq < 0);
>  		vmcs12->vm_exit_intr_info = irq |
>
Wanpeng Li July 31, 2017, 12:17 p.m. UTC | #2
2017-07-31 20:10 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
> On 30/07/2017 09:31, Wanpeng Li wrote:
>> From: Wanpeng Li <wanpeng.li@hotmail.com>
>>
>> ------------[ cut here ]------------
>>  WARNING: CPU: 5 PID: 2288 at arch/x86/kvm/vmx.c:11124 nested_vmx_vmexit+0xd64/0xd70 [kvm_intel]
>>  CPU: 5 PID: 2288 Comm: qemu-system-x86 Not tainted 4.13.0-rc2+ #7
>>  RIP: 0010:nested_vmx_vmexit+0xd64/0xd70 [kvm_intel]
>> Call Trace:
>>   vmx_check_nested_events+0x131/0x1f0 [kvm_intel]
>>   ? vmx_check_nested_events+0x131/0x1f0 [kvm_intel]
>>   kvm_arch_vcpu_ioctl_run+0x5dd/0x1be0 [kvm]
>>   ? vmx_vcpu_load+0x1be/0x220 [kvm_intel]
>>   ? kvm_arch_vcpu_load+0x62/0x230 [kvm]
>>   kvm_vcpu_ioctl+0x340/0x700 [kvm]
>>   ? kvm_vcpu_ioctl+0x340/0x700 [kvm]
>>   ? __fget+0xfc/0x210
>>   do_vfs_ioctl+0xa4/0x6a0
>>   ? __fget+0x11d/0x210
>>   SyS_ioctl+0x79/0x90
>>   do_syscall_64+0x8f/0x750
>>   ? trace_hardirqs_on_thunk+0x1a/0x1c
>>   entry_SYSCALL64_slow_path+0x25/0x25
>>
>> This can be reproduced by booting L1 guest w/ 'noapic' grub parameter, which
>> means that tells the kernel to not make use of any IOAPICs that may be present
>> in the system.
>>
>> There is a scenario when !kvm_cpu_has_interrupt(vcpu) && external_intr in
>> vmx_check_nested_events() which means that there is a pending interrupt from
>> L0's qemu device model which should be injected to L1, we should not try to
>> emualte "Acknowledge interrupt on exit" for this interrupt.
>>
>> This patch fixes it by trying to emulate "Acknowledge interrupt on exit" if L1
>> asks us to.
>
> The commit message is not clear; can you rephrase?

Actually external_intr here is the req_int_win passed from
vcpu_enter_guest() which means that the L0's qemu request an irq
window. I observed the secanrio !kvm_cpu_has_interrupt(vcpu) && L0's
qemu request an irq window is true. Which means that there is a
pending interrupt from L0's qemu device model which should be injected
to L1, we should not try to emualte "Acknowledge interrupt on exit"
for this interrupt.

Regards,
Wanpeng Li

>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>>  arch/x86/kvm/vmx.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 2737343..92eb0f75 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -11118,8 +11118,9 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
>>
>>       vmx_switch_vmcs(vcpu, &vmx->vmcs01);
>>
>> -     if ((exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT)
>> -         && nested_exit_intr_ack_set(vcpu)) {
>> +     if (exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT &&
>> +             kvm_cpu_has_interrupt(vcpu) &&
>> +             nested_exit_intr_ack_set(vcpu)) {
>
> Maybe check nested_exit_intr_ack_set first.
>
> Paolo
>
>>               int irq = kvm_cpu_get_interrupt(vcpu);
>>               WARN_ON(irq < 0);
>>               vmcs12->vm_exit_intr_info = irq |
>>
>
diff mbox

Patch

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 2737343..92eb0f75 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -11118,8 +11118,9 @@  static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
 
 	vmx_switch_vmcs(vcpu, &vmx->vmcs01);
 
-	if ((exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT)
-	    && nested_exit_intr_ack_set(vcpu)) {
+	if (exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT &&
+		kvm_cpu_has_interrupt(vcpu) &&
+		nested_exit_intr_ack_set(vcpu)) {
 		int irq = kvm_cpu_get_interrupt(vcpu);
 		WARN_ON(irq < 0);
 		vmcs12->vm_exit_intr_info = irq |