Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1200870 > unrolled thread

[PATCH 5/9] KVM: x86: unify handling of interrupt window

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 5/9] KVM: x86: unify handling of interrupt window Paolo Bonzini <pbonzini@redhat.com> - 2015-08-05 17:30 +0200
    RE: [PATCH 5/9] KVM: x86: unify handling of interrupt window "Wu, Feng" <feng.wu@intel.com> - 2015-08-06 09:00 +0200
      Re: [PATCH 5/9] KVM: x86: unify handling of interrupt window Paolo Bonzini <pbonzini@redhat.com> - 2015-08-06 14:00 +0200

#1200870 — [PATCH 5/9] KVM: x86: unify handling of interrupt window

The interrupt window is currently checked twice, once in vmx.c/svm.c and
once in dm_request_for_irq_injection.  The only difference is the extra
check for kvm_arch_interrupt_allowed in dm_request_for_irq_injection,
and the different return value (EINTR/KVM_EXIT_INTR for vmx.c/svm.c vs.
0/KVM_EXIT_IRQ_WINDOW_OPEN for dm_request_for_irq_injection).

However, dm_request_for_irq_injection is basically dead code!  Revive it
by removing the checks in vmx.c and svm.c's vmexit handlers, and
fixing the returned values for the dm_request_for_irq_injection case.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/svm.c | 13 -------------
 arch/x86/kvm/vmx.c | 11 -----------
 arch/x86/kvm/x86.c |  4 ++--
 3 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 222439fd73d4..189e46479dd5 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3396,24 +3396,11 @@ static int msr_interception(struct vcpu_svm *svm)
 
 static int interrupt_window_interception(struct vcpu_svm *svm)
 {
-	struct kvm_run *kvm_run = svm->vcpu.run;
-
 	kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);
 	svm_clear_vintr(svm);
 	svm->vmcb->control.int_ctl &= ~V_IRQ_MASK;
 	mark_dirty(svm->vmcb, VMCB_INTR);
 	++svm->vcpu.stat.irq_window_exits;
-	/*
-	 * If the user space waits to inject interrupts, exit as soon as
-	 * possible
-	 */
-	if (!lapic_in_kernel(&svm->vcpu) &&
-	    kvm_run->request_interrupt_window &&
-	    !kvm_cpu_has_interrupt(&svm->vcpu)) {
-		kvm_run->exit_reason = KVM_EXIT_IRQ_WINDOW_OPEN;
-		return 0;
-	}
-
 	return 1;
 }
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ef15dc72284b..4cf25b90dbe0 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5524,17 +5524,6 @@ static int handle_interrupt_window(struct kvm_vcpu *vcpu)
 	kvm_make_request(KVM_REQ_EVENT, vcpu);
 
 	++vcpu->stat.irq_window_exits;
-
-	/*
-	 * If the user space waits to inject interrupts, exit as soon as
-	 * possible
-	 */
-	if (!lapic_in_kernel(vcpu) &&
-	    vcpu->run->request_interrupt_window &&
-	    !kvm_cpu_has_interrupt(vcpu)) {
-		vcpu->run->exit_reason = KVM_EXIT_IRQ_WINDOW_OPEN;
-		return 0;
-	}
 	return 1;
 }
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index daa6d4fe97fe..62362fed4169 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6475,8 +6475,8 @@ static int vcpu_run(struct kvm_vcpu *vcpu)
 			kvm_inject_pending_timer_irqs(vcpu);
 
 		if (dm_request_for_irq_injection(vcpu)) {
-			r = -EINTR;
-			vcpu->run->exit_reason = KVM_EXIT_INTR;
+			r = 0;
+			vcpu->run->exit_reason = KVM_EXIT_IRQ_WINDOW_OPEN;
 			++vcpu->stat.request_irq_exits;
 			break;
 		}
-- 
1.8.3.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1201474

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org
> [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Paolo Bonzini
> Sent: Wednesday, August 05, 2015 11:24 PM
> To: linux-kernel@vger.kernel.org; kvm@vger.kernel.org
> Cc: Steve Rutherford; rkrcmar@redhat.com
> Subject: [PATCH 5/9] KVM: x86: unify handling of interrupt window
> 
> The interrupt window is currently checked twice, once in vmx.c/svm.c and
> once in dm_request_for_irq_injection.  The only difference is the extra
> check for kvm_arch_interrupt_allowed in dm_request_for_irq_injection,
> and the different return value (EINTR/KVM_EXIT_INTR for vmx.c/svm.c vs.
> 0/KVM_EXIT_IRQ_WINDOW_OPEN for dm_request_for_irq_injection).
> 
> However, dm_request_for_irq_injection is basically dead code!  Revive it
> by removing the checks in vmx.c and svm.c's vmexit handlers, and
> fixing the returned values for the dm_request_for_irq_injection case.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/svm.c | 13 -------------
>  arch/x86/kvm/vmx.c | 11 -----------
>  arch/x86/kvm/x86.c |  4 ++--
>  3 files changed, 2 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 222439fd73d4..189e46479dd5 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -3396,24 +3396,11 @@ static int msr_interception(struct vcpu_svm
> *svm)
> 
>  static int interrupt_window_interception(struct vcpu_svm *svm)
>  {
> -	struct kvm_run *kvm_run = svm->vcpu.run;
> -
>  	kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);
>  	svm_clear_vintr(svm);
>  	svm->vmcb->control.int_ctl &= ~V_IRQ_MASK;
>  	mark_dirty(svm->vmcb, VMCB_INTR);
>  	++svm->vcpu.stat.irq_window_exits;
> -	/*
> -	 * If the user space waits to inject interrupts, exit as soon as
> -	 * possible
> -	 */
> -	if (!lapic_in_kernel(&svm->vcpu) &&
> -	    kvm_run->request_interrupt_window &&
> -	    !kvm_cpu_has_interrupt(&svm->vcpu)) {
> -		kvm_run->exit_reason = KVM_EXIT_IRQ_WINDOW_OPEN;
> -		return 0;
> -	}
> -
>  	return 1;
>  }
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ef15dc72284b..4cf25b90dbe0 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5524,17 +5524,6 @@ static int handle_interrupt_window(struct
> kvm_vcpu *vcpu)
>  	kvm_make_request(KVM_REQ_EVENT, vcpu);
> 
>  	++vcpu->stat.irq_window_exits;
> -
> -	/*
> -	 * If the user space waits to inject interrupts, exit as soon as
> -	 * possible
> -	 */
> -	if (!lapic_in_kernel(vcpu) &&
> -	    vcpu->run->request_interrupt_window &&
> -	    !kvm_cpu_has_interrupt(vcpu)) {
> -		vcpu->run->exit_reason = KVM_EXIT_IRQ_WINDOW_OPEN;
> -		return 0;
> -	}
>  	return 1;
>  }

Is it possible to adjust the code here and remove the later checking in x86.c?
In that case, we can avoid calling dm_request_for_irq_injection(vcpu) for
VM exit. Basically dm_request_for_irq_injection() did the following
checks:

- The same as in handle_interrupt_window() below:

        if (!irqchip_in_kernel(vcpu->kvm) &&
            vcpu->run->request_interrupt_window &&
            !kvm_cpu_has_interrupt(vcpu))

- kvm_arch_interrupt_allowed(vcpu), in which, most of the conditions
are guaranteed by interrupt window exits, the only one I am not sure
how to handle is ' to_vmx(vcpu)->nested.nested_run_pending'.

Thanks,
Feng

> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index daa6d4fe97fe..62362fed4169 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6475,8 +6475,8 @@ static int vcpu_run(struct kvm_vcpu *vcpu)
>  			kvm_inject_pending_timer_irqs(vcpu);
> 
>  		if (dm_request_for_irq_injection(vcpu)) {
> -			r = -EINTR;
> -			vcpu->run->exit_reason = KVM_EXIT_INTR;
> +			r = 0;
> +			vcpu->run->exit_reason = KVM_EXIT_IRQ_WINDOW_OPEN;
>  			++vcpu->stat.request_irq_exits;
>  			break;
>  		}
> --
> 1.8.3.1
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201672

On 06/08/2015 08:50, Wu, Feng wrote:
> Is it possible to adjust the code here and remove the later checking in x86.c?
> In that case, we can avoid calling dm_request_for_irq_injection(vcpu) for
> VM exit.

No, see the dm_request_for_irq_injection() changes in patch 9.  In that
patch, you can get a IRQ window open vmexit under broader conditions
(e.g. writing to the local APIC's LVT0 register).

dm_request_for_irq_injection() is called just once, so it is inlined.
After patch 9, it boils down to simply

	if (!vcpu->run->request_interrupt_window)
		return false;

in the common case of in-kernel irqchip.  So it costs just one memory
access and a well-predicted branch.

Paolo

> Basically dm_request_for_irq_injection() did the following checks:
> 
> - The same as in handle_interrupt_window() below:
> 
>         if (!irqchip_in_kernel(vcpu->kvm) &&
>             vcpu->run->request_interrupt_window &&
>             !kvm_cpu_has_interrupt(vcpu))
> 
> - kvm_arch_interrupt_allowed(vcpu), in which, most of the conditions
> are guaranteed by interrupt window exits, the only one I am not sure
> how to handle is ' to_vmx(vcpu)->nested.nested_run_pending'.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web