Revert "x86/PCI: Refine the way to release PCI IRQ resources"
authorRafael J. Wysocki <rafael.j.wysocki@intel.com>
Fri, 20 Mar 2015 13:56:19 +0000 (14:56 +0100)
committerRafael J. Wysocki <rafael.j.wysocki@intel.com>
Fri, 20 Mar 2015 13:56:19 +0000 (14:56 +0100)
Commit b4b55cda5874 (Refine the way to release PCI IRQ resources)
introduced a regression in the PCI IRQ resource management by causing
the IRQ resource of a device, established when pci_enabled_device()
is called on a fully disabled device, to be released when the driver
is unbound from the device, regardless of the enable_cnt.

This leads to 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().  That is a serious problem for secondary
drivers like vfio-pci, which are innocent of the transgressions of
the previous driver.

Since the solution of this problem is not immediate and requires
further discussion, revert commit b4b55cda5874 and the issue it was
supposed to address (a bug related to xen-pciback) will be taken
care of in a different way going forward.

Reported-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
arch/x86/include/asm/pci_x86.h
arch/x86/pci/common.c
arch/x86/pci/intel_mid_pci.c
arch/x86/pci/irq.c
drivers/acpi/pci_irq.c

index fa1195dae42541aaa1d836782a3a65aa25640e74..164e3f8d3c3dbb6eb4fc0ea60e01cae15fe5116e 100644 (file)
@@ -93,6 +93,8 @@ extern raw_spinlock_t pci_config_lock;
 extern int (*pcibios_enable_irq)(struct pci_dev *dev);
 extern void (*pcibios_disable_irq)(struct pci_dev *dev);
 
+extern bool mp_should_keep_irq(struct device *dev);
+
 struct pci_raw_ops {
        int (*read)(unsigned int domain, unsigned int bus, unsigned int devfn,
                                                int reg, int len, u32 *val);
index 3d2612b68694efd294ca214d478c7777030f4b03..2fb384724ebb52d1cf0ba6131b418e81609ca55c 100644 (file)
@@ -513,31 +513,6 @@ void __init pcibios_set_cache_line_size(void)
        }
 }
 
-/*
- * Some device drivers assume dev->irq won't change after calling
- * pci_disable_device(). So delay releasing of IRQ resource to driver
- * unbinding time. Otherwise it will break PM subsystem and drivers
- * like xen-pciback etc.
- */
-static int pci_irq_notifier(struct notifier_block *nb, unsigned long action,
-                           void *data)
-{
-       struct pci_dev *dev = to_pci_dev(data);
-
-       if (action != BUS_NOTIFY_UNBOUND_DRIVER)
-               return NOTIFY_DONE;
-
-       if (pcibios_disable_irq)
-               pcibios_disable_irq(dev);
-
-       return NOTIFY_OK;
-}
-
-static struct notifier_block pci_irq_nb = {
-       .notifier_call = pci_irq_notifier,
-       .priority = INT_MIN,
-};
-
 int __init pcibios_init(void)
 {
        if (!raw_pci_ops) {
@@ -550,9 +525,6 @@ int __init pcibios_init(void)
 
        if (pci_bf_sort >= pci_force_bf)
                pci_sort_breadthfirst();
-
-       bus_register_notifier(&pci_bus_type, &pci_irq_nb);
-
        return 0;
 }
 
@@ -711,6 +683,12 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
        return 0;
 }
 
+void pcibios_disable_device (struct pci_dev *dev)
+{
+       if (!pci_dev_msi_enabled(dev) && pcibios_disable_irq)
+               pcibios_disable_irq(dev);
+}
+
 int pci_ext_cfg_avail(void)
 {
        if (raw_pci_ext_ops)
index efb849323c745899f5e665364ceec36a1d15cb9d..852aa4c92da027cb07fb64c77c855aaf0877a1da 100644 (file)
@@ -234,10 +234,10 @@ static int intel_mid_pci_irq_enable(struct pci_dev *dev)
 
 static void intel_mid_pci_irq_disable(struct pci_dev *dev)
 {
-       if (dev->irq_managed && dev->irq > 0) {
+       if (!mp_should_keep_irq(&dev->dev) && dev->irq_managed &&
+           dev->irq > 0) {
                mp_unmap_irq(dev->irq);
                dev->irq_managed = 0;
-               dev->irq = 0;
        }
 }
 
index e71b3dbd87b8f688d3f2cbfa995421bd516ba581..5dc6ca5e174131d2c7208ea1ed86739ef4532d22 100644 (file)
@@ -1256,9 +1256,22 @@ static int pirq_enable_irq(struct pci_dev *dev)
        return 0;
 }
 
+bool mp_should_keep_irq(struct device *dev)
+{
+       if (dev->power.is_prepared)
+               return true;
+#ifdef CONFIG_PM
+       if (dev->power.runtime_status == RPM_SUSPENDING)
+               return true;
+#endif
+
+       return false;
+}
+
 static void pirq_disable_irq(struct pci_dev *dev)
 {
-       if (io_apic_assign_pci_irqs && dev->irq_managed && dev->irq) {
+       if (io_apic_assign_pci_irqs && !mp_should_keep_irq(&dev->dev) &&
+           dev->irq_managed && dev->irq) {
                mp_unmap_irq(dev->irq);
                dev->irq = 0;
                dev->irq_managed = 0;
index e7f718d6918a6a29b775aa97f7a40dc654d47136..b1def411c0b89cbf7847b767063c5c2ab528e8a8 100644 (file)
@@ -485,6 +485,14 @@ void acpi_pci_irq_disable(struct pci_dev *dev)
        if (!pin || !dev->irq_managed || dev->irq <= 0)
                return;
 
+       /* Keep IOAPIC pin configuration when suspending */
+       if (dev->dev.power.is_prepared)
+               return;
+#ifdef CONFIG_PM
+       if (dev->dev.power.runtime_status == RPM_SUSPENDING)
+               return;
+#endif
+
        entry = acpi_pci_irq_lookup(dev, pin);
        if (!entry)
                return;
@@ -505,6 +513,5 @@ void acpi_pci_irq_disable(struct pci_dev *dev)
        if (gsi >= 0) {
                acpi_unregister_gsi(gsi);
                dev->irq_managed = 0;
-               dev->irq = 0;
        }
 }