From patchwork Tue May 12 13:03:32 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 6388091 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 69375BEEE1 for ; Tue, 12 May 2015 13:04:18 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 566C12013A for ; Tue, 12 May 2015 13:04:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0C35C2041D for ; Tue, 12 May 2015 13:04:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933453AbbELND4 (ORCPT ); Tue, 12 May 2015 09:03:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56393 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933444AbbELNDy (ORCPT ); Tue, 12 May 2015 09:03:54 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t4CD3bDH026892 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 12 May 2015 09:03:38 -0400 Received: from redhat.com (ovpn-116-82.ams2.redhat.com [10.36.116.82]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id t4CD3WeY007964; Tue, 12 May 2015 09:03:33 -0400 Date: Tue, 12 May 2015 15:03:32 +0200 From: "Michael S. Tsirkin" To: linux-kernel@vger.kernel.org Cc: Bjorn Helgaas , linux-pci@vger.kernel.org, Fam Zheng , Yinghai Lu , Yijing Wang , "Eric W. Biederman" , Ulrich Obergfell , Rusty Russell Subject: [PATCH v6 1/2] PCI/MSI: Don't disable MSI/MSI-X at shutdown Message-ID: <1431431730-25164-2-git-send-email-mst@redhat.com> References: <1431431730-25164-1-git-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1431431730-25164-1-git-send-email-mst@redhat.com> X-Mutt-Fcc: =sent X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP d52877c7b1af ("pci/irq: let pci_device_shutdown to call pci_msi_shutdown v2") disabled MSI/MSI-X at device shutdown to address a kexec problem. The change made by the above commit is no longer necessary: it was superceded by commit d5dea7d95c48 ("PCI: msi: Disable msi interrupts when we initialize a pci device") which makes sure the original kexec problem is solved in the new kernel, and commit b566a22c23 ("PCI: disable Bus Master on PCI device shutdown") which makes sure device does not send MSI interrupts (MSI is a memory write and so is suppressed when bus master is cleared). On the contrary, disabling MSI makes it *more* likely that the device will cause mischief since unlike MSI, INT#x interrupts are not suppressed by clearing bus mastering. One example of such mischief is that after we disable MSI, the device may assert INT#x (remember, cleaning bus mastering does not disable INT#x interrupts), and if the driver hasn't registered an interrupt handler for it, the interrupt is never deasserted which causes an "irq %d: nobody cared" message, with irq being subsequently disabled. This is actually not hard to observe with virtio devices. Not a huge problem, but ugly, and might in theory cause other problems, e.g. if the irq being disabled is shared with another device that attempts to use it in its shutdown callback, or if irq debugging was explicitly disabled on the kernel command line. Another theoretical problem is that if the driver does not flush MSI interrupts in the shutdown callback, MSI interrupt handler will run at the same time as the INT#x handler, which doesn't normally happen outside the shutdown path; Depending on the driver, the two handlers might conflict. I did not go looking for such driver bugs but this seems plausible. virtio specifically has another issue: register functions change between msi enabled and disabled modes, so disabling MSI while driver is doing things (e.g. from a kthread) will make the device confused. Of course, some of the above specific issues can be solved by implementing a shutdown callback in the driver: this callback would have to suppress further activity from both the driver and the device, and flush outstanding handlers. This is a non-trivial amount of code that can trigger at any time, so needs careful thought to avoid race conditions causing bugs, and that's only running on shutdown, so isn't well tested. Instead, stop disabling MSIs in PCI core at shutdown: it's simple, safe, removes code instead of adding more code, and needs no driver support. Reported-by: Fam Zheng Tested-by: Fam Zheng Signed-off-by: Michael S. Tsirkin Signed-off-by: Bjorn Helgaas CC: Yinghai Lu CC: Ulrich Obergfell CC: Rusty Russell --- drivers/pci/pci-driver.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 3cb2210..38a602c 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -450,8 +450,6 @@ static void pci_device_shutdown(struct device *dev) if (drv && drv->shutdown) drv->shutdown(pci_dev); - pci_msi_shutdown(pci_dev); - pci_msix_shutdown(pci_dev); #ifdef CONFIG_KEXEC /*