From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiang Liu Subject: Re: [PATCH] x86/PCI: Fully disable devices before releasing IRQ resource Date: Fri, 06 Mar 2015 09:49:04 +0800 Message-ID: <54F90790.7060204@linux.intel.com> References: <20150305210529.6393.29546.stgit@gimli.home> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: Received: from mga01.intel.com ([192.55.52.88]:64872 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753992AbbCFBtI (ORCPT ); Thu, 5 Mar 2015 20:49:08 -0500 In-Reply-To: <20150305210529.6393.29546.stgit@gimli.home> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Alex Williamson , x86@kernel.org, rjw@rjwysocki.net, mingo@redhat.com, bp@alien8.de, lv.zheng@intel.com, hpa@zytor.com, bhelgaas@google.com, tglx@linutronix.de, yinghai@kernel.org, lenb@kernel.org Cc: linux-pci@vger.kernel.org, tony.luck@intel.com, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org On 2015/3/6 5:06, Alex Williamson wrote: > The IRQ resource for a device is established when pci_enabled_device() > is called on a fully disabled device (ie. enable_cnt == 0). With > commit b4b55cda5874 ("x86/PCI: Refine the way to release PCI IRQ > resources") this same IRQ resource is released when the driver is > unbound from the device, regardless of the enable_cnt. This presents > the situation that an ill-behaved driver can now make a device > unusable to subsequent drivers by an imbalance in their use of > pci_enable/disable_device(). It's one thing to break your own device > if you're one of these ill-behaved drivers, but it's a serious > regression for secondary drivers like vfio-pci, which are innocent > of the transgressions of the previous driver. > > Resolve by pushing the device to a fully disabled state before > releasing the IRQ resource. > > Fixes: b4b55cda5874 ("x86/PCI: Refine the way to release PCI IRQ resources") > Signed-off-by: Alex Williamson > Cc: Jiang Liu > --- > arch/x86/pci/common.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c > index 3d2612b..4810194 100644 > --- a/arch/x86/pci/common.c > +++ b/arch/x86/pci/common.c > @@ -527,8 +527,19 @@ static int pci_irq_notifier(struct notifier_block *nb, unsigned long action, > if (action != BUS_NOTIFY_UNBOUND_DRIVER) > return NOTIFY_DONE; > > - if (pcibios_disable_irq) > + if (pcibios_disable_irq) { > + /* > + * Broken drivers may allow a device to be .remove()'d while > + * still enabled. pci_enable_device() will only re-establish > + * dev->irq if the devices is fully disabled. So if we want > + * to release the IRQ, we need to make sure the next driver > + * can re-establish it using pci_enable_device(). > + */ > + while (pci_is_enabled(dev)) > + pci_disable_device(dev); > + > pcibios_disable_irq(dev); > + } Hi Alex, Thanks for debugging and fixing it. Will it be feasible to give a debug message to remind those driver authors to correctly disable PCI when unbinding? Regards! Gerry > > return NOTIFY_OK; > } >