Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761109Ab2FHHzM (ORCPT ); Fri, 8 Jun 2012 03:55:12 -0400 Received: from david.siemens.de ([192.35.17.14]:31318 "EHLO david.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760468Ab2FHHzK (ORCPT ); Fri, 8 Jun 2012 03:55:10 -0400 Message-ID: <4FD1AFD5.70808@siemens.com> Date: Fri, 08 Jun 2012 09:55:01 +0200 From: Jan Kiszka User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 MIME-Version: 1.0 To: "Michael S. Tsirkin" 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 References: <20120601161521.26935.25606.stgit@bling.home> <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> In-Reply-To: <20120608074744.GA524@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3885 Lines: 104 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 -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- 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/