From patchwork Tue Oct 25 18:08:58 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Busch X-Patchwork-Id: 9395165 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4A8556077F for ; Tue, 25 Oct 2016 17:58:24 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4067F2970B for ; Tue, 25 Oct 2016 17:58:24 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3391E2970E; Tue, 25 Oct 2016 17:58:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AFF332970B for ; Tue, 25 Oct 2016 17:58:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933197AbcJYR6W (ORCPT ); Tue, 25 Oct 2016 13:58:22 -0400 Received: from mga06.intel.com ([134.134.136.31]:64940 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932267AbcJYR6V (ORCPT ); Tue, 25 Oct 2016 13:58:21 -0400 Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga104.jf.intel.com with ESMTP; 25 Oct 2016 10:58:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,546,1473145200"; d="scan'208";a="23502624" Received: from unknown (HELO localhost.localdomain) ([10.232.112.96]) by fmsmga006.fm.intel.com with ESMTP; 25 Oct 2016 10:58:20 -0700 Date: Tue, 25 Oct 2016 14:08:58 -0400 From: Keith Busch To: Prarit Bhargava Cc: linux-pci@vger.kernel.org, Alex Williamson , David Arcari , Myron Stowe , Bjorn Helgaas Subject: Re: [RFE PATCH] pci: Do not enable intx on MSI-capable devices on shutdown Message-ID: <20161025180858.GA22422@localhost.localdomain> References: <1477052083-13815-1-git-send-email-prarit@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1477052083-13815-1-git-send-email-prarit@redhat.com> User-Agent: Mutt/1.7.0 (2016-08-17) Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Oct 21, 2016 at 08:14:43AM -0400, Prarit Bhargava wrote: > We have seen this at Red Hat on various drivers: nouveau, ahci, and pcieport > (so far). Google search for "unhandled irq 16" yields many results reporting > similar behavior during shutdown indicating that this problem is widespread. > I can cause this to happen on a "stable" system by adding a 3 second delay in > pci_device_shutdown() which causes the number of spurious interrupts to exceed > the 100000 limit and display the warning above. Also note that by adding the > 3 second delay, NVIDIA devices with device ID 0x0FF* hit this problem 100% of > the time. > > darcari noticed that removing the pci_intx_for_msi() call resulted in a > stable system. After further discussions with Myron and Alex, Alex came up > idea of keeping the intx disabled during shutdown implemented below. > > ----8<---- > > The following unhandled IRQ warning is seen during shutdown: > > irq 16: nobody cared (try booting with the "irqpoll" option) > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.8.2-1.el7_UNSUPPORTED.x86_64 #1 > Hardware name: Hewlett-Packard HP Z820 Workstation/158B, BIOS J63 v03.90 06/01/2016 > 0000000000000000 ffff88041f803e70 ffffffff81333bd5 ffff88041cb78200 > ffff88041cb7829c ffff88041f803e98 ffffffff810d9465 ffff88041cb78200 > 0000000000000000 0000000000000028 ffff88041f803ed0 ffffffff810d97bf > Call Trace: > [] dump_stack+0x63/0x8e > [] __report_bad_irq+0x35/0xd0 > [] note_interrupt+0x20f/0x260 > [] handle_irq_event_percpu+0x45/0x60 > [] handle_irq_event+0x2c/0x50 > [] handle_fasteoi_irq+0x8a/0x150 > [] handle_irq+0xab/0x130 > [] ? _local_bh_enable+0x21/0x50 > [] do_IRQ+0x4d/0xd0 > [] common_interrupt+0x82/0x82 > [] ? cpuidle_enter_state+0xc1/0x280 > [] ? cpuidle_enter_state+0xb4/0x280 > [] cpuidle_enter+0x17/0x20 > [] cpu_startup_entry+0x220/0x3a0 > [] rest_init+0x77/0x80 > [] start_kernel+0x495/0x4a2 > [] ? set_init_arg+0x55/0x55 > [] ? early_idt_handler_array+0x120/0x120 > [] x86_64_start_reservations+0x2a/0x2c > [] x86_64_start_kernel+0x13d/0x14c > > This occurs because the pci_msi_shutdown() and pci_msix_shutdown() functions > enable the legacy intx interrupt even though the device and driver were not > configured for legacy intx. > > This patch blocks the enabling of intx during system shutdown or reboot. I am feeling a bit cautious to tie this behavior to the system_state. Is there better criteria to know we shouldn't enable INTx after disabling MSI/MSI-x? It sounds like we would never want to enable INTx if a driver still has IRQ actions tied to the MSI/MSI-x. Does this alternate proposal look okay? --- -- -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index bfdd074..90a4e84 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -357,19 +357,30 @@ void pci_write_msi_msg(unsigned int irq, struct msi_msg *msg) } EXPORT_SYMBOL_GPL(pci_write_msi_msg); +static bool msi_has_action(struct pci_dev *dev) +{ + struct msi_desc *entry; + int i; + + for_each_pci_msi_entry(entry, dev) { + if (entry->irq) { + for (i = 0; i < entry->nvec_used; i++) + if (irq_has_action(entry->irq + i)) + return true; + } + } + return false; +} + static void free_msi_irqs(struct pci_dev *dev) { struct list_head *msi_list = dev_to_msi_list(&dev->dev); struct msi_desc *entry, *tmp; struct attribute **msi_attrs; struct device_attribute *dev_attr; - int i, count = 0; - - for_each_pci_msi_entry(entry, dev) - if (entry->irq) - for (i = 0; i < entry->nvec_used; i++) - BUG_ON(irq_has_action(entry->irq + i)); + int count = 0; + BUG_ON(msi_has_action(dev)); pci_msi_teardown_msi_irqs(dev); list_for_each_entry_safe(entry, tmp, msi_list, list) { @@ -910,7 +921,8 @@ void pci_msi_shutdown(struct pci_dev *dev) desc = first_pci_msi_entry(dev); pci_msi_set_enable(dev, 0); - pci_intx_for_msi(dev, 1); + if (!msi_has_action(dev)) + pci_intx_for_msi(dev, 1); dev->msi_enabled = 0; /* Return the device with MSI unmasked as initial states */ @@ -1024,7 +1036,8 @@ void pci_msix_shutdown(struct pci_dev *dev) } pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0); - pci_intx_for_msi(dev, 1); + if (!msi_has_action(dev)) + pci_intx_for_msi(dev, 1); dev->msix_enabled = 0; pcibios_alloc_irq(dev); }