Re: [PATCH 4/8] KVM: PCIPT: fix interrupt handling

Amit Shah
2008-07-24T13:32:34Z

* On Thursday 24 Jul 2008 16:58:57 Ben-Ami Yassour wrote:
> On Wed, 2008-07-23 at 19:07 +0530, Amit Shah wrote:
> > * On Wednesday 16 Jul 2008 18:47:01 Ben-Ami Yassour wrote:
> > > This patch fixes a few problems with the interrupt handling for
> > > passthrough devices.
> > >
> > > 1. Pass the interrupt handler the pointer to the device, so we do not
> > > need to lock the pcipt lock in the interrupt handler.
> > >
> > > 2. Remove the pt_irq_handled bitmap - it is no longer needed.
> > >
> > > 3. Split kvm_pci_pt_work_fn into two functions, one for interrupt
> > > injection and another for the ack - is much simpler code this way.
> > >
> > > 4. Change the passthrough initialization order - add the device
> > > structure to the list, before registering the interrupt handler.
> > >
> > > 5. On passthrough destruction path, free the interrupt handler before
> > > cleaning queued work.
> > >
> > > Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
> > > ---
> > >
> > > if (irqchip_in_kernel(kvm)) {
> > > + match->pt_dev.guest.irq = pci_pt_dev->guest.irq;
> > > + match->pt_dev.host.irq = dev->irq;
> > > + if (kvm->arch.vioapic)
> > > + kvm->arch.vioapic->ack_notifier = kvm_pci_pt_ack_irq;
> > > + if (kvm->arch.vpic)
> > > + kvm->arch.vpic->ack_notifier = kvm_pci_pt_ack_irq;
> > > +
> >
> > We shouldn't register this notifier unless we get the irq below to avoid
> > unneeded function calls and checks.
>
> Note: This code was changed in the last version of the code but the
> comment is still relevant.
>
> Do you mean that we need to postpone registering the notification?

I mean we can register these function pointers after the request_irq succeeds.

> In the case of an assigned device this is means the we postpone it for a
> few seconds, and implementing it like above it cleaner. So I don't see
> the real value in postponing it.

Sorry, don't get what you mean here.