From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH 3/3] KVM: x86: fix ready_for_interrupt reporting in split IRQ chip case Date: Fri, 13 Nov 2015 09:52:21 +0100 Message-ID: <5645A4C5.9070304@redhat.com> References: 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-f44.google.com ([74.125.82.44]:34279 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753441AbbKMIwZ (ORCPT ); Fri, 13 Nov 2015 03:52:25 -0500 Received: by wmvv187 with SMTP id v187so70599428wmv.1 for ; Fri, 13 Nov 2015 00:52:24 -0800 (PST) In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 12/11/2015 20:07, Matt Gingell wrote: > This patch adds a call to kvm_arch_interrupt_allowed to ensure ready for > interrupt is reported to user space correctly. This addresses a problem > observed in QEMU when kvm->ready_for_interrupt is set but the x86 > interrupt flag is clear. > > Additionally, test that the APIC is ready to accept an interrupt before > reporting we are ready for injection. > > Reviewed-by: Andy Honig > Signed-off-by: Matt Gingell I think you need to add the same call to dm_request_for_irq_injection, like - return (irqchip_split(vcpu->kvm) - ? kvm_apic_accept_pic_intr(vcpu) - : kvm_arch_interrupt_allowed(vcpu)); + if (!kvm_arch_interrupt_allowed(vcpu)) + return false; + + return !lapic_in_kernel(vcpu) || kvm_apic_accept_pic_intr(vcpu); At this point, just to err on the safe side, we probably should test kvm_event_needs_reinjection(vcpu) as well in dm_request_for_irq_injection. We can then make a new function kvm_vcpu_ready_for_interrupt_injection with the sequence of tests (kvm_cpu_has_interrupt, kvm_arch_interrupt_allowed, kvm_event_needs_reinjection, possibly kvm_apic_accept_pic_intr) so that: - dm_request_for_irq_injection becomes simply return (vcpu->run->request_interrupt_window && likely(!pic_in_kernel(vcpu->kvm)); - the caller of dm_request_for_irq_injection does if (dm_request_for_irq_injection(vcpu) && kvm_vcpu_ready_for_interrupt_injection(vcpu)) - post_kvm_run_save's assignment becomes kvm_run->ready_for_interrupt_injection = !pic_in_kernel(vcpu->kvm) || kvm_vcpu_ready_for_interrupt_injection(vcpu); The code would make a lot of sense then; I hope it will work too. :) Paolo "ceci n'est pas une patch"