From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH v3 2/5] KVM: Switch assigned device IRQ forwarding to threaded handler Date: Wed, 3 Nov 2010 20:13:25 -0200 Message-ID: <20101103221325.GA1147@amt.cnet> References: <128511f28870098dbd57bdaa081dd30ac2af70df.1288771873.git.jan.kiszka@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , kvm , Alex Williamson , "Michael S. Tsirkin" , Jan Kiszka To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:20657 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752960Ab0KCWOC (ORCPT ); Wed, 3 Nov 2010 18:14:02 -0400 Content-Disposition: inline In-Reply-To: <128511f28870098dbd57bdaa081dd30ac2af70df.1288771873.git.jan.kiszka@web.de> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Nov 03, 2010 at 09:11:13AM +0100, Jan Kiszka wrote: > From: Jan Kiszka > > This improves the IRQ forwarding for assigned devices: By using the > kernel's threaded IRQ scheme, we can get rid of the latency-prone work > queue and simplify the code in the same run. > > Moreover, we no longer have to hold assigned_dev_lock while raising the > guest IRQ, which can be a lenghty operation as we may have to iterate > over all VCPUs. The lock is now only used for synchronizing masking vs. > unmasking of INTx-type IRQs, thus is renames to intx_lock. > > Signed-off-by: Jan Kiszka > --- > include/linux/kvm_host.h | 12 +---- > virt/kvm/assigned-dev.c | 106 ++++++++++++++------------------------------- > 2 files changed, 35 insertions(+), 83 deletions(-) > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index bcf71c7..eaacb5d 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -457,16 +457,8 @@ struct kvm_irq_ack_notifier { > void (*irq_acked)(struct kvm_irq_ack_notifier *kian); > }; > > -#define KVM_ASSIGNED_MSIX_PENDING 0x1 > -struct kvm_guest_msix_entry { > - u32 vector; > - u16 entry; > - u16 flags; > -}; > - > struct kvm_assigned_dev_kernel { > struct kvm_irq_ack_notifier ack_notifier; > - struct work_struct interrupt_work; > struct list_head list; > int assigned_dev_id; > int host_segnr; > @@ -477,13 +469,13 @@ struct kvm_assigned_dev_kernel { > bool host_irq_disabled; > struct msix_entry *host_msix_entries; > int guest_irq; > - struct kvm_guest_msix_entry *guest_msix_entries; > + struct msix_entry *guest_msix_entries; > unsigned long irq_requested_type; > int irq_source_id; > int flags; > struct pci_dev *dev; > struct kvm *kvm; > - spinlock_t assigned_dev_lock; > + spinlock_t intx_lock; > }; > > struct kvm_irq_mask_notifier { > diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c > index ecc4419..d0b07ea 100644 > --- a/virt/kvm/assigned-dev.c > +++ b/virt/kvm/assigned-dev.c > @@ -55,58 +55,31 @@ static int find_index_from_host_irq(struct kvm_assigned_dev_kernel > return index; > } > > -static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work) > +static irqreturn_t kvm_assigned_dev_thread(int irq, void *dev_id) > { > - struct kvm_assigned_dev_kernel *assigned_dev; > - int i; > + struct kvm_assigned_dev_kernel *assigned_dev = dev_id; > + u32 vector; > + int index; > > - assigned_dev = container_of(work, struct kvm_assigned_dev_kernel, > - interrupt_work); > + if (assigned_dev->irq_requested_type & KVM_DEV_IRQ_HOST_INTX) { > + spin_lock(&assigned_dev->intx_lock); > + disable_irq_nosync(irq); > + assigned_dev->host_irq_disabled = true; > + spin_unlock(&assigned_dev->intx_lock); > + } Shouldnt this happen in the hardirq handler? Otherwise host will receive interrupts between ->eoi and this point.