From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH v3 1/8] Do not call ack notifiers on PIC reset. Date: Thu, 13 Aug 2009 12:39:34 +0300 Message-ID: <20090813093934.GA23736@redhat.com> References: <1250079442-5163-1-git-send-email-gleb@redhat.com> <1250079442-5163-2-git-send-email-gleb@redhat.com> <20090813091105.GA17022@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: avi@redhat.com, kvm@vger.kernel.org To: Marcelo Tosatti Return-path: Received: from mx2.redhat.com ([66.187.237.31]:56749 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751418AbZHMJjg (ORCPT ); Thu, 13 Aug 2009 05:39:36 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n7D9dbVY010187 for ; Thu, 13 Aug 2009 05:39:37 -0400 Content-Disposition: inline In-Reply-To: <20090813091105.GA17022@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Aug 13, 2009 at 06:11:05AM -0300, Marcelo Tosatti wrote: > On Wed, Aug 12, 2009 at 03:17:15PM +0300, Gleb Natapov wrote: > > For device assigned it may cause host hang since ack notifier callback > > enables host interrupt and guest not necessary cleared interrupt > > condition in an assigned device. For PIT we should not call ack notifier > > here since interrupt was not acked by a guest and should be redelivered. > > > > Signed-off-by: Gleb Natapov > > --- > > arch/x86/kvm/i8259.c | 16 ---------------- > > 1 files changed, 0 insertions(+), 16 deletions(-) > > > > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c > > index 01f1516..eb2b8b7 100644 > > --- a/arch/x86/kvm/i8259.c > > +++ b/arch/x86/kvm/i8259.c > > @@ -225,22 +225,6 @@ int kvm_pic_read_irq(struct kvm *kvm) > > > > void kvm_pic_reset(struct kvm_kpic_state *s) > > { > > - int irq, irqbase, n; > > - struct kvm *kvm = s->pics_state->irq_request_opaque; > > - struct kvm_vcpu *vcpu0 = kvm->bsp_vcpu; > > - > > - if (s == &s->pics_state->pics[0]) > > - irqbase = 0; > > - else > > - irqbase = 8; > > - > > - for (irq = 0; irq < PIC_NUM_PINS/2; irq++) { > > - if (vcpu0 && kvm_apic_accept_pic_intr(vcpu0)) > > - if (s->irr & (1 << irq) || s->isr & (1 << irq)) { > > - n = irq + irqbase; > > - kvm_notify_acked_irq(kvm, SELECT_PIC(n), n); > > - } > > - } > > s->last_irr = 0; > > s->irr = 0; > > s->imr = 0; > > -- > > 1.6.3.3 > > This used to be necessary to clear pending state from i8254.c > "irq_acked" logic. I think it'll break it. This is just a hack then and it does not exists in ioapic so if it is really needed ioapic+pit combination is broken. But the problem should be solved inside i8254.c not somewhere else. Setting irq_acked to 1 in pit_load_count() seems like a right thing to do. Something like the patch below. Ideally pending should be scaled instead of reset. Also may be the problem exists because PIC doesn't call mask notifiers? diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c index b857ca3..aa7f68e 100644 --- a/arch/x86/kvm/i8254.c +++ b/arch/x86/kvm/i8254.c @@ -325,6 +325,9 @@ static void pit_load_count(struct kvm *kvm, int channel, u32 val) return; } + atomic_set(&pt->pending, 0); + ps->irq_ack = 1; + /* Two types of timer * mode 1 is one shot, mode 2 is period, otherwise del timer */ switch (ps->channels[0].mode) { -- Gleb.