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;
if (irq == -1)
return;
- read_lock_irqsave(&kvm_pci_pt_lock, flags);
+ 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_ACK);
if (!pci_pt_dev) {
- read_unlock_irqrestore(&kvm_pci_pt_lock, flags);
+ read_unlock(&kvm_pci_pt_lock);
return;
}
- pci_pt_dev->pt_dev.ack_work.irq = irq;
- pci_pt_dev->pt_dev.ack_work.kvm = kvm;
- pci_pt_dev->pt_dev.ack_work.source = 1;
-
kvm_get_kvm(kvm);
+ read_unlock(&kvm_pci_pt_lock);
schedule_work(&pci_pt_dev->pt_dev.ack_work.work);
- read_unlock_irqrestore(&kvm_pci_pt_lock, flags);
}
static int kvm_vm_ioctl_pci_pt_dev(struct kvm *kvm,
@@ -252,10 +218,9 @@ static int kvm_vm_ioctl_pci_pt_dev(struct kvm *kvm,
{
int r = 0;
struct kvm_pci_pt_dev_list *match;
- unsigned long flags;
struct pci_dev *dev;
- write_lock_irqsave(&kvm_pci_pt_lock, flags);
+ write_lock(&kvm_pci_pt_lock);
/* Check if this is a request to update the irq of the device
* in the guest (BIOS/ kernels can dynamically reprogram irq
@@ -266,10 +231,10 @@ static int kvm_vm_ioctl_pci_pt_dev(struct kvm *kvm,
&pci_pt_dev->host, 0, KVM_PT_SOURCE_UPDATE);
if (match) {
match->pt_dev.guest.irq = pci_pt_dev->guest.irq;
- write_unlock_irqrestore(&kvm_pci_pt_lock, flags);
+ write_unlock(&kvm_pci_pt_lock);
goto out;
}
- write_unlock_irqrestore(&kvm_pci_pt_lock, flags);
+ write_unlock(&kvm_pci_pt_lock);
match = kzalloc(sizeof(struct kvm_pci_pt_dev_list), GFP_KERNEL);
if (match == NULL) {
@@ -298,42 +263,49 @@ static int kvm_vm_ioctl_pci_pt_dev(struct kvm *kvm,
}
match->pt_dev.guest.busnr = pci_pt_dev->guest.busnr;
match->pt_dev.guest.devfn = pci_pt_dev->guest.devfn;
- match->pt_dev.host.busnr = pci_pt_dev->host.busnr;
- match->pt_dev.host.devfn = pci_pt_dev->host.devfn;
- match->pt_dev.dev = dev;
+ match->pt_dev.host.busnr = pci_pt_dev->host.busnr;
+ match->pt_dev.host.devfn = pci_pt_dev->host.devfn;
+ match->pt_dev.dev = dev;
+
+ write_lock(&kvm_pci_pt_lock);
+
+ INIT_WORK(&match->pt_dev.int_work.work, kvm_pci_pt_int_work_fn);
+ INIT_WORK(&match->pt_dev.ack_work.work, kvm_pci_pt_ack_work_fn);
+
+ match->pt_dev.kvm = kvm;
+ match->pt_dev.int_work.pt_dev = &match->pt_dev;
+ match->pt_dev.ack_work.pt_dev = &match->pt_dev;
+
+ list_add(&match->list, &kvm->arch.pci_pt_dev_head);
+
+ write_unlock(&kvm_pci_pt_lock);
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;
+
/* Even though this is PCI, we don't want to use shared
* interrupts. Sharing host devices with guest-assigned devices
* on the same interrupt line is not a happy situation: there
* are going to be long delays in accepting, acking, etc.
*/
if (request_irq(dev->irq, kvm_pci_pt_dev_intr, 0,
- "kvm_pt_device", (void *)kvm)) {
+ "kvm_pt_device", (void *)&match->pt_dev)) {
printk(KERN_INFO "%s: couldn't allocate irq for pv "
"device\n", __func__);
r = -EIO;
- goto out_put;
+ goto out_list_del;
}
- 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;
}
- write_lock_irqsave(&kvm_pci_pt_lock, flags);
-
- INIT_WORK(&match->pt_dev.int_work.work, kvm_pci_pt_work_fn);
- INIT_WORK(&match->pt_dev.ack_work.work, kvm_pci_pt_work_fn);
- list_add(&match->list, &kvm->arch.pci_pt_dev_head);
-
- if (irqchip_in_kernel(kvm))
- set_bit(dev->irq, pt_irq_handled);
- write_unlock_irqrestore(&kvm_pci_pt_lock, flags);
out:
return r;
+out_list_del:
+ list_del(&match->list);
out_put:
pci_dev_put(dev);
out_free:
@@ -345,11 +317,15 @@ static void kvm_free_pci_passthrough(struct kvm *kvm)
{
struct list_head *ptr, *ptr2;
struct kvm_pci_pt_dev_list *pci_pt_dev;
- unsigned long flags;
- write_lock_irqsave(&kvm_pci_pt_lock, flags);
+ write_lock(&kvm_pci_pt_lock);
list_for_each_safe(ptr, ptr2, &kvm->arch.pci_pt_dev_head) {
pci_pt_dev = list_entry(ptr, struct kvm_pci_pt_dev_list, list);
+
+ if (irqchip_in_kernel(kvm) && pci_pt_dev->pt_dev.host.irq)
+ free_irq(pci_pt_dev->pt_dev.host.irq,
+ (void *)&pci_pt_dev->pt_dev);
+
if (cancel_work_sync(&pci_pt_dev->pt_dev.int_work.work))
/* We had pending work. That means we will have to take
* care of kvm_put_kvm.
@@ -366,8 +342,6 @@ static void kvm_free_pci_passthrough(struct kvm *kvm)
list_for_each_safe(ptr, ptr2, &kvm->arch.pci_pt_dev_head) {
pci_pt_dev = list_entry(ptr, struct kvm_pci_pt_dev_list, list);
- if (irqchip_in_kernel(kvm) && pci_pt_dev->pt_dev.host.irq)
- free_irq(pci_pt_dev->pt_dev.host.irq, kvm);
/* Search for this device got us a refcount */
pci_dev_put(pci_pt_dev->pt_dev.dev);
pci_release_regions(pci_pt_dev->pt_dev.dev);
@@ -376,7 +350,7 @@ static void kvm_free_pci_passthrough(struct kvm *kvm)
list_del(&pci_pt_dev->list);
kfree(pci_pt_dev);
}
- write_unlock_irqrestore(&kvm_pci_pt_lock, flags);
+ write_unlock(&kvm_pci_pt_lock);
}
unsigned long segment_base(u16 selector)
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 2346c9f..f6973e0 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -331,9 +331,7 @@ struct kvm_mem_alias {
*/
struct kvm_pci_pt_work {
struct work_struct work;
- struct kvm *kvm;
- int irq;
- bool source;
+ struct kvm_pci_passthrough_dev_kernel *pt_dev;
};
struct kvm_pci_passthrough_dev_kernel {
@@ -342,6 +340,7 @@ struct kvm_pci_passthrough_dev_kernel {
struct kvm_pci_pt_work int_work;
struct kvm_pci_pt_work ack_work;
struct pci_dev *dev;
+ struct kvm *kvm;
};
/* This list is to store the guest bus:device:function-irq and host
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 0b5bc40..6ec99fd 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -289,17 +289,16 @@ static void __kvm_ioapic_update_eoi(struct kvm_ioapic
*ioapic, int gsi)
{
union ioapic_redir_entry *ent;
struct kvm_pci_pt_dev_list *match;
- unsigned long flags;
ent = &ioapic->redirtbl[gsi];
ASSERT(ent->fields.trig_mode == IOAPIC_LEVEL_TRIG);
ent->fields.remote_irr = 0;
- read_lock_irqsave(&kvm_pci_pt_lock, flags);
+ read_lock(&kvm_pci_pt_lock);
match = kvm_find_pci_pt_dev(&ioapic->kvm->arch.pci_pt_dev_head, NULL,
gsi, KVM_PT_SOURCE_IRQ_ACK);
- read_unlock_irqrestore(&kvm_pci_pt_lock, flags);
+ read_unlock(&kvm_pci_pt_lock);
if (!match) {
if (!ent->fields.mask && (ioapic->irr & (1 << gsi)))
ioapic_service(ioapic, gsi);
--
1.5.6
--
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
