On Mon, 2008-07-28 at 15:27 +0800, Yang, Sheng wrote:
> On Tuesday 22 July 2008 20:13:53 Ben-Ami Yassour wrote:
> >
> > -int kvm_pic_read_irq(struct kvm_pic *s)
> > +int kvm_pic_read_irq(struct kvm *kvm)
> > {
> > int irq, irq2, intno;
> > + struct kvm_pic *s = pic_irqchip(kvm);
> >
> > irq = pic_get_irq(&s->pics[0]);
> > if (irq >= 0) {
> > @@ -186,6 +187,8 @@ int kvm_pic_read_irq(struct kvm_pic *s)
> > irq = 7;
> > intno = s->pics[0].irq_base + irq;
> > }
> > + kvm_notify_acked_irq(kvm, irq);
> > +
> > pic_update_irq(s);
> >
> > return intno;
>
> One coding style suggestion,
>
> struct kvm_pic has *irq_request_opaque which is struct kvm indeed. How
> about contain kvm in kvm_pic explicitly? (seems cleaner, though needs
> more modification).
Your suggestion is applied in the next set of patches that I sent.
>
>
> Another thing is about host share IRQ: Do we want follow the straight
> forward way to add it? That's it, return IRQ_HANDLED from irq handler
> and wait for EOI of guest?
That's what the code in the current patches doing now. It also disables
the irq until the EOI of the guest. If we don't do that, then the
interrupt handler keeps getting called (which causes many guest exit)
until the guest finally interacts with the device to release the
interrupt line.
Thanks,
Ben
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
