From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mark McLoughlin <markmc@redhat.com>
Subject: Re: PCI-passthrough: interrupt work structure per device
Date: Thu, 29 May 2008 19:09:37 +0100
Message-ID: <1212084577.21998.28.camel@muff>
References: <alpine.DEB.1.10.0805292004310.24732@cluwyn.haifa.ibm.com>
Reply-To: Mark McLoughlin <markmc@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
Cc: Amit Shah <amit.shah@qumranet.com>,
	"Kay, Allen M" <allen.m.kay@intel.com>,
	Muli Ben-Yehuda <muli@il.ibm.com>, kvm@vger.kernel.org
To: Ben-Ami Yassour <benami@il.ibm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([66.187.233.31]:37567 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1750932AbYE2SMj (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 29 May 2008 14:12:39 -0400
In-Reply-To: <alpine.DEB.1.10.0805292004310.24732@cluwyn.haifa.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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.