Re: [PATCH 4/8] KVM: PCIPT: fix interrupt handling
On Thu, 2008-07-24 at 19:01 +0530, Amit Shah wrote:
> * 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:
> > > >
> > > > 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.
request_irq should be the last initialization operation, since every
thing should be ready when in case an interrupt is received
>
> > 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.
never mind... see the answer is above.
--
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
Re: [PATCH 4/8] KVM: PCIPT: fix interrupt handling
* 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 <[EMAIL PROTECTED]>
> > > ---
> > >
> > > 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.
--
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
Re: [PATCH 4/8] KVM: PCIPT: fix interrupt handling
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 <[EMAIL PROTECTED]>
> > ---
>
> > 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?
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.
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
Re: [PATCH 4/8] KVM: PCIPT: fix interrupt handling
* 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 <[EMAIL PROTECTED]>
> ---
> 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.
--
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
Re: [PATCH 4/8] KVM: PCIPT: fix interrupt handling
Ben-Ami Yassour wrote: This patch fixes a few problems with the interrupt handling for passthrough devices. Well, fold it into the patch it fixes. There is no point in sending a buggy patch and a fix in the same patchset. -- error compiling committee.c: too many arguments to function -- 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
