From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 3/3] KVM: Reset PIT irq injection logic when the PIT IRQ is unmasked Date: Mon, 05 Jan 2009 22:59:01 +0200 Message-ID: <49627495.9020203@redhat.com> References: <1231085685-32201-1-git-send-email-avi@redhat.com> <1231085685-32201-4-git-send-email-avi@redhat.com> <20090105183159.GC5592@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Sheng Yang , kvm@vger.kernel.org To: Marcelo Tosatti Return-path: Received: from mx2.redhat.com ([66.187.237.31]:45903 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753204AbZAEU6o (ORCPT ); Mon, 5 Jan 2009 15:58:44 -0500 In-Reply-To: <20090105183159.GC5592@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: Marcelo Tosatti wrote: > On Sun, Jan 04, 2009 at 06:14:45PM +0200, Avi Kivity wrote: > >> While the PIT is masked the guest cannot ack the irq, so the reinject logic >> will never allow the interrupt to be injected. >> >> Fix by resetting the reinjection counters on unmask. >> >> Unbreaks Xen. >> >> diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c >> index 528daad..d78d430 100644 >> --- a/arch/x86/kvm/i8254.c >> +++ b/arch/x86/kvm/i8254.c >> @@ -539,6 +539,16 @@ void kvm_pit_reset(struct kvm_pit *pit) >> pit->pit_state.irq_ack = 1; >> } >> >> +static void pit_mask_notifer(struct kvm_irq_mask_notifier *kimn, int mask) >> +{ >> + struct kvm_pit *pit = container_of(kimn, struct kvm_pit, mask_notifier); >> + >> + if (!mask) { >> + atomic_set(&pit->pit_state.pit_timer.pending, 0); >> + pit->pit_state.irq_ack = 1; >> + } >> +} >> > > I'm not sure about zeroing the counter here. The guest can mask the > interrupt during normal operation, and in such cases you want the > pending count to be retained (and reinjected later). > > I suppose setting irq_ack to one is enough. > I'm worried about: - boot guest using local apic timer - reset - boot with pit timer - a zillion interrupts So at the very least, we need a limiter. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.