From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sheng Yang Subject: Re: [PATCH 3/3] KVM: Enable MSI-X for KVM assigned device Date: Wed, 18 Feb 2009 20:52:42 +0800 Message-ID: <200902182052.42925.sheng@linux.intel.com> References: <1234950262-4142-1-git-send-email-sheng@linux.intel.com> <20090218122420.GA5055@syang10-desktop> <499C00BA.2040008@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from mga07.intel.com ([143.182.124.22]:31087 "EHLO azsmga101.ch.intel.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751451AbZBRMwr (ORCPT ); Wed, 18 Feb 2009 07:52:47 -0500 In-Reply-To: <499C00BA.2040008@redhat.com> Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: On Wednesday 18 February 2009 20:36:10 Avi Kivity wrote: > Sheng Yang wrote: > >>> struct kvm_assigned_msix_nr { > >>> __u32 assigned_dev_id; > >>> __u16 entry_nr; > >>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > >>> index 4010802..d3acb37 100644 > >>> --- a/virt/kvm/kvm_main.c > >>> +++ b/virt/kvm/kvm_main.c > >>> @@ -280,13 +280,33 @@ static void kvm_free_assigned_irq(struct kvm > >>> *kvm, * now, the kvm state is still legal for probably we also have to > >>> wait * interrupt_work done. > >>> */ > >>> - disable_irq_nosync(assigned_dev->host_irq); > >>> - cancel_work_sync(&assigned_dev->interrupt_work); > >>> + if (assigned_dev->irq_requested_type & KVM_ASSIGNED_DEV_MSIX) { > >>> + int i; > >>> + for (i = 0; i < assigned_dev->entries_nr; i++) > >>> + disable_irq_nosync(assigned_dev-> > >>> + host_msix_entries[i].vector); > >>> + > >>> + cancel_work_sync(&assigned_dev->interrupt_work); > >>> + > >>> + for (i = 0; i < assigned_dev->entries_nr; i++) > >>> + free_irq(assigned_dev->host_msix_entries[i].vector, > >>> + (void *)assigned_dev); > >>> + > >>> + assigned_dev->entries_nr = 0; > >>> + kfree(assigned_dev->host_msix_entries); > >>> + kfree(assigned_dev->guest_msix_entries); > >>> + pci_disable_msix(assigned_dev->dev); > >>> + } else { > >>> + /* Deal with MSI and INTx */ > >>> + disable_irq_nosync(assigned_dev->host_irq); > >>> + cancel_work_sync(&assigned_dev->interrupt_work); > >> > >> How about always have an array? That will also allow us to deal with > >> INTx where x=B,C,D. > >> > >> Currently for MSI and INTx the array will hold just one active element. > > > > So array, or bitmap? I remember I changed it to bitmap accounding to your > > first comment... > > Which bitmap? I'm confused. > > I'm talking about unifying the existing array > (assigned_dev->host_msix_entries[]) with ->host_irq. Also since we need > an array for INTx when a function uses INT[BCD]. > > So we'll have assigned_dev->host_irqs[], each entry can be INTx or MSI > or MSIx. > > > OK. I think array is reasonable, but the length is a problem, as I did > > before. How long would you like? > > MAX(4, KVM_MAX_MSIX_ENTRIES), no? Oh, yeah, I misunderstood it(wrong context)... Need more adjustment on the type, for host_msix_entries is used with pci_enable_msix. So I'd like to put it a bit later. -- regards Yang, Sheng