Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932371Ab2FHIBO (ORCPT ); Fri, 8 Jun 2012 04:01:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55380 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752611Ab2FHIBL (ORCPT ); Fri, 8 Jun 2012 04:01:11 -0400 Date: Fri, 8 Jun 2012 11:00:59 +0300 From: "Michael S. Tsirkin" To: Jan Kiszka Cc: Thomas Gleixner , Avi Kivity , Alex Williamson , "kvm@vger.kernel.org" , "mtosatti@redhat.com" , "linux-kernel@vger.kernel.org" , "yongjie.ren@intel.com" Subject: Re: [PATCH] KVM: Use IRQF_ONESHOT for assigned device MSI interrupts Message-ID: <20120608080058.GB524@redhat.com> References: <4FC8F042.5050600@siemens.com> <1338570192.23475.25.camel@bling.home> <4FC8F867.7080103@siemens.com> <1338573558.23475.41.camel@bling.home> <4FC90961.8030701@siemens.com> <4FCB2359.9020505@redhat.com> <4FCC9EAC.9090007@siemens.com> <20120608074744.GA524@redhat.com> <4FD1AFD5.70808@siemens.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4FD1AFD5.70808@siemens.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4370 Lines: 111 On Fri, Jun 08, 2012 at 09:55:01AM +0200, Jan Kiszka wrote: > On 2012-06-08 09:47, Michael S. Tsirkin wrote: > > On Mon, Jun 04, 2012 at 01:40:28PM +0200, Jan Kiszka wrote: > >> On 2012-06-04 13:21, Thomas Gleixner wrote: > >>> On Sun, 3 Jun 2012, Avi Kivity wrote: > >>> > >>>> On 06/01/2012 09:26 PM, Jan Kiszka wrote: > >>>>> > >>>>>> you suggesting we need a request_edge_threaded_only_irq() API? Thanks, > >>>>> > >>>>> I'm just wondering if that restriction for threaded IRQs is really > >>>>> necessary for all use cases we have. Threaded MSIs do not appear to me > >>>>> like have to be handled that conservatively, but maybe I'm missing some > >>>>> detail. > >>>>> > >>>> > >>>> btw, I'm hoping we can unthread assigned MSIs. If the delivery is > >>>> unicast, we can precalculate everything and all the handler has to do is > >>>> set the IRR, KVM_REQ_EVENT, and kick the vcpu. All of these can be done > >>>> from interrupt context with just RCU locking. > >>> > >>> There is really no need to run MSI/MSI-X interrupts threaded for > >>> KVM. I'm running the patch below for quite some time and it works like > >>> a charm. > >>> > >>> Thanks, > >>> > >>> tglx > >>> ---- > >>> Index: linux-2.6/virt/kvm/assigned-dev.c > >>> =================================================================== > >>> --- linux-2.6.orig/virt/kvm/assigned-dev.c > >>> +++ linux-2.6/virt/kvm/assigned-dev.c > >>> @@ -105,7 +105,7 @@ static irqreturn_t kvm_assigned_dev_thre > >>> } > >>> > >>> #ifdef __KVM_HAVE_MSI > >>> -static irqreturn_t kvm_assigned_dev_thread_msi(int irq, void *dev_id) > >>> +static irqreturn_t kvm_assigned_dev_msi_handler(int irq, void *dev_id) > >>> { > >>> struct kvm_assigned_dev_kernel *assigned_dev = dev_id; > >>> > >>> @@ -117,7 +117,7 @@ static irqreturn_t kvm_assigned_dev_thre > >>> #endif > >>> > >>> #ifdef __KVM_HAVE_MSIX > >>> -static irqreturn_t kvm_assigned_dev_thread_msix(int irq, void *dev_id) > >>> +static irqreturn_t kvm_assigned_dev_msix_handler(int irq, void *dev_id) > >>> { > >>> struct kvm_assigned_dev_kernel *assigned_dev = dev_id; > >>> int index = find_index_from_host_irq(assigned_dev, irq); > >>> @@ -346,9 +346,8 @@ static int assigned_device_enable_host_m > >>> } > >>> > >>> dev->host_irq = dev->dev->irq; > >>> - if (request_threaded_irq(dev->host_irq, NULL, > >>> - kvm_assigned_dev_thread_msi, 0, > >>> - dev->irq_name, dev)) { > >>> + if (request_irq(dev->host_irq, kvm_assigned_dev_msi_handler, 0, > >>> + dev->irq_name, dev)) { > >>> pci_disable_msi(dev->dev); > >>> return -EIO; > >>> } > >>> @@ -373,9 +372,9 @@ static int assigned_device_enable_host_m > >>> return r; > >>> > >>> for (i = 0; i < dev->entries_nr; i++) { > >>> - r = request_threaded_irq(dev->host_msix_entries[i].vector, > >>> - NULL, kvm_assigned_dev_thread_msix, > >>> - 0, dev->irq_name, dev); > >>> + r = request_irq(dev->host_msix_entries[i].vector, > >>> + kvm_assigned_dev_msix_handler, 0, > >>> + dev->irq_name, dev); > >>> if (r) > >>> goto err; > >>> } > >> > >> This may work in practice but has two conceptual problems: > >> - we do not want to run a potential broadcast to all VCPUs to run in > >> a host IRQ handler > >> - crazy user space could have configured the route to end up in the > >> PIC or IOAPIC, and both are not hard-IRQ safe (this should probably > >> be caught on setup) > >> > >> So this shortcut requires some checks before being applied to a specific > >> MSI/MSI-X vector. > > > > I did this in the past: > > https://lkml.org/lkml/2012/1/18/287 > > > > Have no hw to test this atm but if there are any takers > > wanting to play with it I can update and post. > > Just add check that allow only unicasts, and this should be fine. > > Jan If I code it up you can test it? > -- > Siemens AG, Corporate Technology, CT T DE IT 1 > Corporate Competence Center Embedded Linux > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/