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 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 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 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
[PATCH 4/8] KVM: PCIPT: fix interrupt handling
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]
---
arch/x86/kvm/x86.c | 156 ++-
include/asm-x86/kvm_host.h |5 +-
virt/kvm/ioapic.c |5 +-
3 files changed, 69 insertions(+), 97 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c07ca2b..8d25b4a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -145,49 +145,37 @@ kvm_find_pci_pt_dev(struct list_head *head,
return NULL;
}
-static DECLARE_BITMAP(pt_irq_handled, NR_IRQS);
-
-static void kvm_pci_pt_work_fn(struct work_struct *work)
+static void kvm_pci_pt_int_work_fn(struct work_struct *work)
{
- struct kvm_pci_pt_dev_list *match;
struct kvm_pci_pt_work *int_work;
- int source;
- unsigned long flags;
- int guest_irq;
- int host_irq;
int_work = container_of(work, struct kvm_pci_pt_work, work);
- source = int_work-source ? KVM_PT_SOURCE_IRQ_ACK : KVM_PT_SOURCE_IRQ;
-
/* This is taken to safely inject irq inside the guest. When
* the interrupt injection (or the ioapic code) uses a
* finer-grained lock, update this
*/
- mutex_lock(int_work-kvm-lock);
- read_lock_irqsave(kvm_pci_pt_lock, flags);
- match = kvm_find_pci_pt_dev(int_work-kvm-arch.pci_pt_dev_head, NULL,
- int_work-irq, source);
- if (!match) {
- printk(KERN_ERR %s: no matching device assigned to guest
- found for irq %d, source = %d!\n,
- __func__, int_work-irq, int_work-source);
- read_unlock_irqrestore(kvm_pci_pt_lock, flags);
- goto out;
- }
- guest_irq = match-pt_dev.guest.irq;
- host_irq = match-pt_dev.host.irq;
- read_unlock_irqrestore(kvm_pci_pt_lock, flags);
+ mutex_lock(int_work-pt_dev-kvm-lock);
+ kvm_set_irq(int_work-pt_dev-kvm, int_work-pt_dev-guest.irq, 1);
+ mutex_unlock(int_work-pt_dev-kvm-lock);
+ kvm_put_kvm(int_work-pt_dev-kvm);
+}
- if (source == KVM_PT_SOURCE_IRQ)
- kvm_set_irq(int_work-kvm, guest_irq, 1);
- else {
- kvm_set_irq(int_work-kvm, int_work-irq, 0);
- enable_irq(host_irq);
- }
-out:
- mutex_unlock(int_work-kvm-lock);
- kvm_put_kvm(int_work-kvm);
+static void kvm_pci_pt_ack_work_fn(struct work_struct *work)
+{
+ struct kvm_pci_pt_work *ack_work;
+
+ ack_work = container_of(work, struct kvm_pci_pt_work, work);
+
+ /* This is taken to safely inject irq inside the guest. When
+* the interrupt injection (or the ioapic code) uses a
+* finer-grained lock, update this
+*/
+ mutex_lock(ack_work-pt_dev-kvm-lock);
+ kvm_set_irq(ack_work-pt_dev-kvm, ack_work-pt_dev-guest.irq, 0);
+ enable_irq(ack_work-pt_dev-host.irq);
+ mutex_unlock(ack_work-pt_dev-kvm-lock);
+ kvm_put_kvm(ack_work-pt_dev-kvm);
}
/* FIXME: Implement the OR logic needed to make shared interrupts on
@@ -195,28 +183,11 @@ out:
*/
static irqreturn_t kvm_pci_pt_dev_intr(int irq, void *dev_id)
{
- struct kvm *kvm = (struct kvm *) dev_id;
- struct kvm_pci_pt_dev_list *pci_pt_dev;
-
- if (!test_bit(irq, pt_irq_handled))
- return IRQ_NONE;
-
- read_lock(kvm_pci_pt_lock);
- pci_pt_dev = kvm_find_pci_pt_dev(kvm-arch.pci_pt_dev_head, NULL,
-irq, KVM_PT_SOURCE_IRQ);
- if (!pci_pt_dev) {
- read_unlock(kvm_pci_pt_lock);
- return IRQ_NONE;
- }
-
- pci_pt_dev-pt_dev.int_work.irq = irq;
- pci_pt_dev-pt_dev.int_work.kvm = kvm;
- pci_pt_dev-pt_dev.int_work.source = 0;
-
- kvm_get_kvm(kvm);
- schedule_work(pci_pt_dev-pt_dev.int_work.work);
- read_unlock(kvm_pci_pt_lock);
+ struct kvm_pci_passthrough_dev_kernel *pt_dev =
+ (struct kvm_pci_passthrough_dev_kernel *) dev_id;
+ kvm_get_kvm(pt_dev-kvm);
+ schedule_work(pt_dev-int_work.work);
disable_irq_nosync(irq);
return IRQ_HANDLED;
}
@@ -226,25 +197,20 @@ static void kvm_pci_pt_ack_irq(void *opaque, int irq)
{
struct kvm *kvm = opaque;
struct kvm_pci_pt_dev_list *pci_pt_dev;
- unsigned long flags;
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
