[Use the new kvm list address]
On Thursday 22 May 2008 18:22:43 Ben-Ami Yassour wrote:
> Amit,
>
> Attached is a fix for pvdma interrupt injection, and it seems to work fine
> with an e1000 NIC.
The interrupt injection is generic for pci-passthrough and not just for pvdma
(it'll work the same for vt-d as well).
> From d4c3326ae55f9b474673e5308394182551bb4fee Mon Sep 17 00:00:00 2001
> From: Ben-Ami Yassour <[EMAIL PROTECTED]>
> Date: Thu, 22 May 2008 15:33:19 +0300
> Subject: [PATCH 16/16] KVM: PCI Passthrough: fix interrupt injection
>
> Signed-off-by: Ben-Ami Yassour <[EMAIL PROTECTED]>
> ---
> arch/x86/kvm/x86.c | 48
> +++++++++++++++++++++++++++---------------- include/asm-x86/kvm_host.h |
> 7 +++++-
> virt/kvm/ioapic.c | 11 ++++++++-
> 3 files changed, 45 insertions(+), 21 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ac807a0..cee5be1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -276,7 +276,7 @@ static int pv_unmap_hypercall(struct kvm_vcpu *vcpu,
> dma_addr_t dma) * Used to find a registered host PCI device (a
> "passthrough" device) * during hypercalls, ioctls or interrupts or EOI
> */
> -static struct kvm_pci_pt_dev_list *
> +struct kvm_pci_pt_dev_list *
> find_pci_pt_dev(struct list_head *head,
> struct kvm_pci_pt_info *pt_pci_info, int irq, int source)
> {
> @@ -348,7 +348,7 @@ static int pv_mapped_pci_device_hypercall(struct
> kvm_vcpu *vcpu, static DECLARE_BITMAP(pt_irq_pending, NR_IRQS);
> static DECLARE_BITMAP(pt_irq_handled, NR_IRQS);
>
> -static void kvm_pci_pt_work_fn(struct work_struct *work)
> +static void kvm_pci_pt_intr_work_fn(struct work_struct *work)
> {
> struct kvm_pci_pt_dev_list *match;
> struct kvm_pci_pt_work *int_work;
> @@ -359,18 +359,16 @@ static void kvm_pci_pt_work_fn(struct work_struct
> *work) source = int_work->source ? KVM_PT_SOURCE_IRQ_ACK :
> KVM_PT_SOURCE_IRQ; match =
> find_pci_pt_dev(&int_work->kvm->arch.pci_pt_dev_head, NULL, int_work->irq,
> source);
> - if (!match)
> - return;
> mutex_lock(&int_work->kvm->lock);
> if (source == KVM_PT_SOURCE_IRQ)
> kvm_set_irq(int_work->kvm, match->pt_dev.guest.irq, 1);
> else if (test_bit(match->pt_dev.host.irq, pt_irq_pending)) {
> - kvm_set_irq(int_work->kvm, int_work->irq, 0);
> - clear_bit(match->pt_dev.host.irq, pt_irq_pending);
> - enable_irq(match->pt_dev.host.irq);
> - } else
> - goto out;
> + kvm_set_irq(int_work->kvm, int_work->irq, 0);
> + clear_bit(match->pt_dev.host.irq, pt_irq_pending);
> + enable_irq(match->pt_dev.host.irq);
> + } else
> + goto out;
With this rework, the else should also call kvm_put_kvm. Avi also says the
kvm_put_kvm should happen after the mutex_unlock.
> /* We only put the reference to the kvm struct if we
> * successfully found the assigned device and injected the
> @@ -387,6 +385,7 @@ 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 *match;
>
> if (!test_bit(irq, pt_irq_handled))
> return IRQ_NONE;
> @@ -394,13 +393,18 @@ static irqreturn_t kvm_pci_pt_dev_intr(int irq, void
> *dev_id) if (test_bit(irq, pt_irq_pending))
> return IRQ_HANDLED;
>
> + match = find_pci_pt_dev(&kvm->arch.pci_pt_dev_head, NULL,
> + irq, KVM_PT_SOURCE_IRQ);
> + if (!match)
> + return IRQ_NONE;
> +
> set_bit(irq, pt_irq_pending);
> - kvm->arch.pci_pt_int_work.irq = irq;
> - kvm->arch.pci_pt_int_work.kvm = kvm;
> - kvm->arch.pci_pt_int_work.source = 0;
> + kvm->arch.pci_pt_int_work_intr.irq = irq;
> + kvm->arch.pci_pt_int_work_intr.kvm = kvm;
> + kvm->arch.pci_pt_int_work_intr.source = 0;
>
> kvm_get_kvm(kvm);
> - schedule_work(&kvm->arch.pci_pt_int_work.work);
> + schedule_work(&kvm->arch.pci_pt_int_work_intr.work);
>
> disable_irq_nosync(irq);
> return IRQ_HANDLED;
> @@ -410,15 +414,21 @@ static irqreturn_t kvm_pci_pt_dev_intr(int irq, void
> *dev_id) void kvm_pci_pt_ack_irq(struct kvm *kvm, int vector)
> {
> int irq;
> + struct kvm_pci_pt_dev_list *match;
>
> irq = kvm_get_eoi_gsi(kvm->arch.vioapic, vector);
>
> - kvm->arch.pci_pt_int_work.irq = irq;
> - kvm->arch.pci_pt_int_work.kvm = kvm;
> - kvm->arch.pci_pt_int_work.source = 1;
> + match = find_pci_pt_dev(&kvm->arch.pci_pt_dev_head, NULL,
> + irq, KVM_PT_SOURCE_IRQ_ACK);
> + if (!match)
> + return;
> +
> + kvm->arch.pci_pt_int_work_ack.irq = irq;
> + kvm->arch.pci_pt_int_work_ack.kvm = kvm;
> + kvm->arch.pci_pt_int_work_ack.source = 1;
>
> kvm_get_kvm(kvm);
> - schedule_work(&kvm->arch.pci_pt_int_work.work);
> + schedule_work(&kvm->arch.pci_pt_int_work_ack.work);
> }
>
> static int kvm_vm_ioctl_pci_pt_dev(struct kvm *kvm,
> @@ -4322,7 +4332,8 @@ struct kvm *kvm_arch_create_vm(void)
> INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
> INIT_LIST_HEAD(&kvm->arch.pci_pt_dev_head);
> INIT_LIST_HEAD(&kvm->arch.pci_pv_dmap_head);
> - INIT_WORK(&kvm->arch.pci_pt_int_work.work, kvm_pci_pt_work_fn);
> + INIT_WORK(&kvm->arch.pci_pt_int_work_intr.work,
> kvm_pci_pt_intr_work_fn);
> + INIT_WORK(&kvm->arch.pci_pt_int_work_ack.work, kvm_pci_pt_intr_work_fn);
They can be called pci_pt_int_work and pci_pt_ack_work.
>
> return kvm;
> }
> @@ -4449,3 +4460,4 @@ void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
> smp_call_function_single(ipi_pcpu, vcpu_kick_intr, vcpu, 0, 0);
> put_cpu();
> }
> +
Extra whitespace
> diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
> index dc46287..ef11f48 100644
> --- a/include/asm-x86/kvm_host.h
> +++ b/include/asm-x86/kvm_host.h
> @@ -345,7 +345,8 @@ struct kvm_arch{
> struct list_head active_mmu_pages;
> struct list_head pci_pt_dev_head;
> struct list_head pci_pv_dmap_head;
> - struct kvm_pci_pt_work pci_pt_int_work;
> + struct kvm_pci_pt_work pci_pt_int_work_intr;
> + struct kvm_pci_pt_work pci_pt_int_work_ack;
> struct kvm_pic *vpic;
> struct kvm_ioapic *vioapic;
> struct kvm_pit *vpit;
> @@ -593,6 +594,10 @@ void kvm_enable_tdp(void);
> int load_pdptrs(struct kvm_vcpu *vcpu, unsigned long cr3);
> int complete_pio(struct kvm_vcpu *vcpu);
>
> +struct kvm_pci_pt_dev_list *
> +find_pci_pt_dev(struct list_head *head,
> + struct kvm_pci_pt_info *pt_pci_info, int irq, int source);
> +
This can go into include/asm-x86/kvm_host.h
> static inline struct kvm_mmu_page *page_header(hpa_t shadow_page)
> {
> struct page *page = pfn_to_page(shadow_page >> PAGE_SHIFT);
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index 4c41a00..da041c0 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -299,6 +299,7 @@ void kvm_ioapic_update_eoi(struct kvm *kvm, int vector)
> struct kvm_ioapic *ioapic = kvm->arch.vioapic;
> union ioapic_redir_entry *ent;
> int gsi;
> + struct kvm_pci_pt_dev_list *match;
This too can go into the header
> gsi = kvm_get_eoi_gsi(ioapic, vector);
> if (gsi == -1) {
> @@ -311,8 +312,14 @@ void kvm_ioapic_update_eoi(struct kvm *kvm, int
> vector) ASSERT(ent->fields.trig_mode == IOAPIC_LEVEL_TRIG);
>
> ent->fields.remote_irr = 0;
> - if (!ent->fields.mask && (ioapic->irr & (1 << gsi)))
> - ioapic_deliver(ioapic, gsi);
> +
> + match = find_pci_pt_dev(&kvm->arch.pci_pt_dev_head, NULL,
> + gsi, KVM_PT_SOURCE_IRQ_ACK);
> +
> + if (!match) {
> + if (!ent->fields.mask && (ioapic->irr & (1 << gsi)))
> + ioapic_deliver(ioapic, gsi);
> + }
The problem with all this is we'll have to introduce a new spinlock for the
pci_pt structures. We need to take this lock in all the various contexts we
get called, which means we might have to worry about lock ordering.
--
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
