Re: PCI-passthrough: interrupt work structure per device

[Mark McLoughlin <markmc@redhat.com>]

On Thu, 2008-05-29 at 20:17 +0300, Ben-Ami Yassour wrote:

> Attached is a patch for the pci-passthrough tree.
> This patch changes the workq structure of the interrupt handling to be per 
> device.

Heh, I just happened to notice this myself yesterday.

...
> >From b847cef27c6c6dfff15b8fc9682e4c6563e997f3 Mon Sep 17 00:00:00 2001
> From: Ben-Ami Yassour <benami@il.ibm.com>
> Date: Thu, 29 May 2008 19:39:14 +0300
> Subject: [PATCH] KVM: PCIPT: interrupt work structure per device
> 
> Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
> ---
>   arch/x86/kvm/x86.c         |   90 ++++++++++++++++++++++++++++----------------
>   include/asm-x86/kvm_host.h |   23 +++++++----
>   include/asm-x86/kvm_para.h |    1 +
>   3 files changed, 72 insertions(+), 42 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index d1cc582..cf3a47c 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
...
> @@ -184,16 +190,27 @@ 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;
> 
> -	kvm->arch.pci_pt_int_work.irq = irq;
> -	kvm->arch.pci_pt_int_work.kvm = kvm;
> -	kvm->arch.pci_pt_int_work.source = 0;
> +	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);

Won't we leak this reference if we get another interrupt before the
workqueue is scheduled?

> -	printk(KERN_INFO "kvm: Handling hypercalls for device %02x:%02x.%1x\n",
> -	       pci_pt_dev->host.busnr, PCI_SLOT(pci_pt_dev->host.devfn),
> -	       PCI_FUNC(pci_pt_dev->host.devfn));

Spurious change.

...

> diff --git a/include/asm-x86/kvm_para.h b/include/asm-x86/kvm_para.h
> index 5f93b78..5813ed0 100644
> --- a/include/asm-x86/kvm_para.h
> +++ b/include/asm-x86/kvm_para.h
> @@ -171,4 +171,5 @@ struct kvm_pci_passthrough_dev {
>   	struct kvm_pci_pt_info guest;
>   	struct kvm_pci_pt_info host;
>   };
> +
>   #endif

Ditto.

Cheers,
Mark.