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:24:20 +0800 Message-ID: <20090218122420.GA5055@syang10-desktop> References: <1234950262-4142-1-git-send-email-sheng@linux.intel.com> <1234950262-4142-4-git-send-email-sheng@linux.intel.com> <499BE6BF.2050904@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Marcelo Tosatti , kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from mga02.intel.com ([134.134.136.20]:42741 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751403AbZBRMY2 (ORCPT ); Wed, 18 Feb 2009 07:24:28 -0500 Content-Disposition: inline In-Reply-To: <499BE6BF.2050904@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Feb 18, 2009 at 10:45:19AM +0000, Avi Kivity wrote: > Sheng Yang wrote: >> index a2dfbe0..78480d0 100644 >> --- a/include/linux/kvm.h >> +++ b/include/linux/kvm.h >> @@ -440,6 +440,9 @@ struct kvm_irq_routing { >> }; >> #endif >> +#if defined(CONFIG_X86) >> +#define KVM_CAP_DEVICE_MSIX 26 >> +#endif >> > > We switched to a different way of depending on CONFIG_X86, see the other > KVM_CAP defines. Thanks to point it out. :) > >> 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... OK. I think array is reasonable, but the length is a problem, as I did before. How long would you like? -- regards Yang, Sheng