All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gleb Natapov <gleb@redhat.com>
To: Marcelo Tosatti <mtosatti@redhat.com>
Cc: avi@redhat.com, kvm@vger.kernel.org
Subject: Re: [PATCH v3 1/8] Do not call ack notifiers on PIC reset.
Date: Thu, 13 Aug 2009 12:39:34 +0300	[thread overview]
Message-ID: <20090813093934.GA23736@redhat.com> (raw)
In-Reply-To: <20090813091105.GA17022@amt.cnet>

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 <gleb@redhat.com>
> > ---
> >  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.

  reply	other threads:[~2009-08-13  9:39 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-12 12:17 [PATCH v3 0/8] make interrupt injection lockless (almost) Gleb Natapov
2009-08-12 12:17 ` [PATCH v3 1/8] Do not call ack notifiers on PIC reset Gleb Natapov
2009-08-13  9:11   ` Marcelo Tosatti
2009-08-13  9:39     ` Gleb Natapov [this message]
2009-08-12 12:17 ` [PATCH v3 2/8] Change irq routing table to use gsi indexed array Gleb Natapov
2009-08-12 12:17 ` [PATCH v3 3/8] Maintain back mapping from irqchip/pin to gsi Gleb Natapov
2009-08-17  9:43   ` Avi Kivity
2009-08-12 12:17 ` [PATCH v3 4/8] Move irq routing data structure to rcu locking Gleb Natapov
2009-08-12 12:17 ` [PATCH v3 5/8] Move irq ack notifier list to arch independent code Gleb Natapov
2009-08-12 12:17 ` [PATCH v3 6/8] Convert irq notifiers lists to RCU locking Gleb Natapov
2009-08-12 12:17 ` [PATCH v3 7/8] Move IO APIC to its own lock Gleb Natapov
2009-08-13  9:13   ` Marcelo Tosatti
2009-08-13  9:48     ` Gleb Natapov
2009-08-13  9:49       ` Avi Kivity
2009-08-13 10:09         ` Gleb Natapov
2009-08-13 10:44           ` Avi Kivity
2009-08-13 15:15             ` Paul E. McKenney
2009-08-12 12:17 ` [PATCH v3 8/8] Drop kvm->irq_lock lock from irq injection path Gleb Natapov
2009-08-17  9:46 ` [PATCH v3 0/8] make interrupt injection lockless (almost) Avi Kivity

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090813093934.GA23736@redhat.com \
    --to=gleb@redhat.com \
    --cc=avi@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.