From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH v2 3/4] KVM: x86: request interrupt window when IRQ chip is split Date: Tue, 17 Nov 2015 18:00:25 +0100 Message-ID: <564B5D29.7040006@redhat.com> References: <284DE6CB-71F9-4168-8887-7BA3D1AC3C39@google.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Cc: Steve Rutherford To: Matt Gingell , kvm@vger.kernel.org Return-path: Received: from mail-wm0-f47.google.com ([74.125.82.47]:35892 "EHLO mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753151AbbKQRA3 (ORCPT ); Tue, 17 Nov 2015 12:00:29 -0500 Received: by wmww144 with SMTP id w144so162755327wmw.1 for ; Tue, 17 Nov 2015 09:00:28 -0800 (PST) In-Reply-To: <284DE6CB-71F9-4168-8887-7BA3D1AC3C39@google.com> Sender: kvm-owner@vger.kernel.org List-ID: On 17/11/2015 00:26, Matt Gingell wrote: > Before this patch, we incorrectly enter the guest without requesting an > interrupt window if the IRQ chip is split between user space and the > kernel. > > Because lapic_in_kernel no longer implies the PIC is in the kernel, this > patch tests pic_in_kernel to determining whether an interrupt window > should be requested when entering the guest. > > If the APIC is in the kernel and we request an interrupt window the > guest will return immediately. If the APIC is masked the guest will not > not make forward progress and unmask it, leading to a loop when KVM > reenters and requests again. This patch adds a check to ensure the APIC > is ready to accept an interrupt before requesting a window. > > Reviewed-by: Steve Rutherford > Signed-off-by: Matt Gingell > --- > arch/x86/kvm/x86.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index c370eef..d57bdd9 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6259,8 +6259,11 @@ void kvm_arch_mmu_notifier_invalidate_page(struct kvm *kvm, > static int vcpu_enter_guest(struct kvm_vcpu *vcpu) > { > int r; > - bool req_int_win = !lapic_in_kernel(vcpu) && > - vcpu->run->request_interrupt_window; > + bool req_int_win = > + vcpu->run->request_interrupt_window && > + likely(!pic_in_kernel(vcpu->kvm)) && > + (!lapic_in_kernel(vcpu) || kvm_apic_accept_pic_intr(vcpu)); > + This can be bool req_int_win = dm_request_for_irq_injection(vcpu) && (!lapic_in_kernel(vcpu) || kvm_apic_accept_pic_intr(vcpu)); I'll apply the patches and send them to Linus for 4.4-rc2, thanks! These cleanups can go on top: diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2e16068bba51..0bca1ec199df 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2661,12 +2661,24 @@ static int kvm_vcpu_ioctl_set_lapic(struct kvm_vcpu *vcpu, return 0; } -static int kvm_vcpu_ready_for_interrupt_injection(struct kvm_vcpu *vcpu) { +static int kvm_cpu_accept_dm_intr(struct kvm_vcpu *vcpu) +{ + return (!lapic_in_kernel(vcpu) || + kvm_apic_accept_pic_intr(vcpu)); +} + +/* + * if userspace requested an interrupt window, check that the + * interrupt window is open. + * + * No need to exit to userspace if we already have an interrupt queued. + */ +static int kvm_vcpu_ready_for_interrupt_injection(struct kvm_vcpu *vcpu) +{ return kvm_arch_interrupt_allowed(vcpu) && !kvm_cpu_has_interrupt(vcpu) && !kvm_event_needs_reinjection(vcpu) && - (!lapic_in_kernel(vcpu) || - kvm_apic_accept_pic_intr(vcpu)); + kvm_cpu_accept_dm_intr(vcpu); } static int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, @@ -5817,12 +5829,6 @@ static int emulator_fix_hypercall(struct x86_emulate_ctxt *ctxt) return emulator_write_emulated(ctxt, rip, instruction, 3, NULL); } -/* - * Check if userspace requested an interrupt window, and that the - * interrupt window is open. - * - * No need to exit to userspace if we already have an interrupt queued. - */ static int dm_request_for_irq_injection(struct kvm_vcpu *vcpu) { return vcpu->run->request_interrupt_window && @@ -6253,9 +6259,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) { int r; bool req_int_win = - vcpu->run->request_interrupt_window && - likely(!pic_in_kernel(vcpu->kvm)) && - (!lapic_in_kernel(vcpu) || kvm_apic_accept_pic_intr(vcpu)); + dm_request_for_irq_injection(vcpu) && + kvm_cpu_accept_dm_intr(vcpu); bool req_immediate_exit = false; A couple questions, that can be fixed by separate patches: - should kvm_cpu_accept_dm_intr check that pending_external_vector == -1? - should kvm_vcpu_ioctl_interrupt then use kvm_cpu_accept_dm_intr instead of checking pending_external_vector == -1 directly? Paolo