From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44846) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TBht9-0003SA-HF for qemu-devel@nongnu.org; Wed, 12 Sep 2012 04:01:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TBht8-0007tR-FI for qemu-devel@nongnu.org; Wed, 12 Sep 2012 04:01:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35271) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TBht8-0007tN-6d for qemu-devel@nongnu.org; Wed, 12 Sep 2012 04:01:34 -0400 Message-ID: <50504151.40704@redhat.com> Date: Wed, 12 Sep 2012 11:01:21 +0300 From: Avi Kivity MIME-Version: 1.0 References: <1347240563-6212-1-git-send-email-mmogilvi_qemu@miniinfo.net> In-Reply-To: <1347240563-6212-1-git-send-email-mmogilvi_qemu@miniinfo.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] KVM: fix i8259 interrupt high to low transition logic List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Matthew Ogilvie Cc: Paolo Bonzini , Jan Kiszka , qemu-devel@nongnu.org, "Maciej W. Rozycki" , kvm@vger.kernel.org On 09/10/2012 04:29 AM, Matthew Ogilvie wrote: > Intel's definition of "edge triggered" means: "asserted with a > low-to-high transition at the time an interrupt is registered > and then kept high until the interrupt is served via one of the > EOI mechanisms or goes away unhandled." > > So the only difference between edge triggered and level triggered > is in the leading edge, with no difference in the trailing edge. > > This bug manifested itself when the guest was Microport UNIX > System V/386 v2.1 (ca. 1987), because it would sometimes mask > off IRQ14 in the slave IMR after it had already been asserted. > The master would still try to deliver an interrupt even though > IRQ2 had dropped again, resulting in a spurious interupt > (IRQ15) and a panicked UNIX kernel. > diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c > index adba28f..5cbba99 100644 > --- a/arch/x86/kvm/i8254.c > +++ b/arch/x86/kvm/i8254.c > @@ -302,8 +302,12 @@ static void pit_do_work(struct kthread_work *work) > } > spin_unlock(&ps->inject_lock); > if (inject) { > - kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1); > + /* Clear previous interrupt, then create a rising > + * edge to request another interupt, and leave it at > + * level=1 until time to inject another one. > + */ > kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0); > + kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1); > > /* I thought I understood this, now I'm not sure. How can this be correct? Real hardware doesn't act like this. What if the PIT is disabled after this? You're injecting a spurious interrupt then. -- error compiling committee.c: too many arguments to function