From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCHv3 RFC 0/2] kvm: direct msix injection Date: Mon, 9 Jul 2012 01:55:25 +0300 Message-ID: <20120708225525.GA26193@redhat.com> References: <4FE8303E.8080808@siemens.com> <1341248900.1207.452.camel@bling.home> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Jan Kiszka , "kvm@vger.kernel.org" , avi@redhat.com To: Alex Williamson Return-path: Received: from mx1.redhat.com ([209.132.183.28]:24160 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752390Ab2GHWzK (ORCPT ); Sun, 8 Jul 2012 18:55:10 -0400 Content-Disposition: inline In-Reply-To: <1341248900.1207.452.camel@bling.home> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, Jul 02, 2012 at 11:08:20AM -0600, Alex Williamson wrote: > On Mon, 2012-06-25 at 11:32 +0200, Jan Kiszka wrote: > > On 2012-06-11 13:19, Michael S. Tsirkin wrote: > > > We can deliver certain interrupts, notably MSIX, > > > from atomic context. > > > Here's an untested patch to do this (compiled only). > > > > > > Changes from v2: > > > Don't inject broadcast interrupts directly > > > Changes from v1: > > > Tried to address comments from v1, except unifying > > > with kvm_set_irq: passing flags to it looks too ugly. > > > Added a comment. > > > > > > Jan, you said you can test this? > > > > > > > > > Michael S. Tsirkin (2): > > > kvm: implement kvm_set_msi_inatomic > > > kvm: deliver msix interrupts from irq handler > > > > > > include/linux/kvm_host.h | 3 ++ > > > virt/kvm/assigned-dev.c | 31 ++++++++++++++++++-- > > > virt/kvm/irq_comm.c | 75 ++++++++++++++++++++++++++++++++++++++++++++---- > > > 3 files changed, 102 insertions(+), 7 deletions(-) > > > > > > > Finally-tested-by: Jan Kiszka > > Michael, we need either this or the simple oneshot patch to get device > assignment working again for 3.5. Are you planning to push this for > 3.5? Thanks, > > Alex > Avi wants this reworked using an injection cache. I agree with Jan though: oneshot looks too ugly. Just so you can make progress, can't we add a stub handler returning IRQ_WAKE_THREAD unconditionally for now? I.e. like the below (warning: completely untested). Signed-off-by: Michael S. Tsirkin -- diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c index b1e091a..18cc36e 100644 --- a/virt/kvm/assigned-dev.c +++ b/virt/kvm/assigned-dev.c @@ -334,6 +334,11 @@ static int assigned_device_enable_host_intx(struct kvm *kvm, } #ifdef __KVM_HAVE_MSI +static irqreturn_t kvm_assigned_dev_msi(int irq, void *dev_id) +{ + return IRQ_WAKE_THREAD; +} + static int assigned_device_enable_host_msi(struct kvm *kvm, struct kvm_assigned_dev_kernel *dev) { @@ -346,7 +351,7 @@ static int assigned_device_enable_host_msi(struct kvm *kvm, } dev->host_irq = dev->dev->irq; - if (request_threaded_irq(dev->host_irq, NULL, + if (request_threaded_irq(dev->host_irq, kvm_assigned_dev_msi, kvm_assigned_dev_thread_msi, 0, dev->irq_name, dev)) { pci_disable_msi(dev->dev); @@ -358,6 +363,11 @@ static int assigned_device_enable_host_msi(struct kvm *kvm, #endif #ifdef __KVM_HAVE_MSIX +static irqreturn_t kvm_assigned_dev_msix(int irq, void *dev_id) +{ + return IRQ_WAKE_THREAD; +} + static int assigned_device_enable_host_msix(struct kvm *kvm, struct kvm_assigned_dev_kernel *dev) { @@ -374,7 +384,8 @@ static int assigned_device_enable_host_msix(struct kvm *kvm, for (i = 0; i < dev->entries_nr; i++) { r = request_threaded_irq(dev->host_msix_entries[i].vector, - NULL, kvm_assigned_dev_thread_msix, + kvm_assigned_dev_msix, + kvm_assigned_dev_thread_msix, 0, dev->irq_name, dev); if (r) goto err; -- MST