Skip to contentSkip to conversation

Introduce a mechanism to allocate PCI IRQ on demand

PATCH Superseded Jiang Liu 2 participants 9 messages Jun 2, 2015 – Jun 5, 2015

Subsystems (3)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Jiang Liu

author
#0 lore raw

v2 cover Introduce a mechanism to allocate PCI IRQ on demand

This patch set introduces a mechanism to allocate PCI IRQ on demand and free it when not used anymore by hooking pci_device_probe() and pci_device_remove().

It will be used to track IOAPIC pin usage on x86 so we could support IOAPIC hot-removal.

The patch set passes Fengguang's 0day test suite.

V1->V2:
1) Refine pci_device_probe() to optimize for mainline code as suggested by Bjorn
2) Reorder patch set to put optional patch as the last (Patch 4)

Thanks!
Gerry

Jiang Liu (4):
PCI: Add hooks to allocate/free IRQ resources when binding/unbinding
driver
PCI, x86: Allocate PCI IRQ on demand and free it when not used
anymore
PCI: Introduce helpers to manage pci_dev->irq and

    pci_dev->irq_managed

PCI, MSI: Optionally free legacy PCI IRQ when enabling MSI/MSI-X

 arch/x86/include/asm/pci_x86.h |    2 --
 arch/x86/pci/common.c          |   20 +++++++++-----------
 arch/x86/pci/intel_mid_pci.c   |    9 ++++++---
 arch/x86/pci/irq.c             |   23 ++++-------------------
 drivers/acpi/pci_irq.c         |   17 ++++-------------
 drivers/pci/msi.c              |    6 +++++-
 drivers/pci/pci-driver.c       |   26 ++++++++++++++++++++------
 include/linux/pci.h            |   19 +++++++++++++++++++
 8 files changed, 67 insertions(+), 55 deletions(-)
Signature · 1 line
1.7.10.4
author
#1 lore raw

PATCH v2 1/4 PCI: Add hooks to allocate/free IRQ resources when binding/unbinding driver

Add two hook points pcibios_{alloc|free}_irq() into PCI core, which will be called when binding/unbinding PCI device drivers. Then PCI arch code may hook into these two points to allocate IRQ resources on demand and free them when not used anymore.

---
 drivers/pci/pci-driver.c |   26 ++++++++++++++++++++------
 include/linux/pci.h      |    2 ++
 2 files changed, 22 insertions(+), 6 deletions(-)
drivers/pci/pci-driver.c +20 -6
index 3cb2210de553..450ad36ffc77 100644
@@ -388,18 +388,31 @@ static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
 	return error;
 }
 
+int __weak pcibios_alloc_irq(struct pci_dev *dev)
+{
+	return dev->irq;
+}
+
+void __weak pcibios_free_irq(struct pci_dev *dev)
+{
+}
+
 static int pci_device_probe(struct device *dev)
 {
-	int error = 0;
-	struct pci_driver *drv;
-	struct pci_dev *pci_dev;
+	int error;
+	struct pci_dev *pci_dev = to_pci_dev(dev);
+	struct pci_driver *drv = to_pci_driver(dev->driver);
+
+	error = pcibios_alloc_irq(pci_dev);
+	if (error < 0)
+		return error;
 
-	drv = to_pci_driver(dev->driver);
-	pci_dev = to_pci_dev(dev);
 	pci_dev_get(pci_dev);
 	error = __pci_device_probe(drv, pci_dev);
-	if (error)
+	if (error) {
+		pcibios_free_irq(pci_dev);
 		pci_dev_put(pci_dev);
+	}
 
 	return error;
 }
@@ -415,6 +428,7 @@ static int pci_device_remove(struct device *dev)
 			drv->remove(pci_dev);
 			pm_runtime_put_noidle(dev);
 		}
+		pcibios_free_irq(pci_dev);
 		pci_dev->driver = NULL;
 	}
 
include/linux/pci.h +2 -0
index 353db8dc4c6e..f50d16a04abc 100644
@@ -1656,6 +1656,8 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
 int pcibios_add_device(struct pci_dev *dev);
 void pcibios_release_device(struct pci_dev *dev);
 void pcibios_penalize_isa_irq(int irq, int active);
+int pcibios_alloc_irq(struct pci_dev *dev);
+void pcibios_free_irq(struct pci_dev *dev);
 
 #ifdef CONFIG_HIBERNATE_CALLBACKS
 extern struct dev_pm_ops pcibios_pm_ops;
Signature · 1 line
1.7.10.4
maintainer
#2 lore raw

Re: [Patch v2 1/4] PCI: Add hooks to allocate/free IRQ resources when binding/unbinding driver

Quoted 16 lines · Jiang Liu

Add two hook points pcibios_{alloc|free}_irq() into PCI core, which will be called when binding/unbinding PCI device drivers. Then PCI arch code may hook into these two points to allocate IRQ resources on demand and free them when not used anymore.

---
 drivers/pci/pci-driver.c |   26 ++++++++++++++++++++------
 include/linux/pci.h      |    2 ++
 2 files changed, 22 insertions(+), 6 deletions(-)
drivers/pci/pci-driver.c +0 -0
index 3cb2210de553..450ad36ffc77 100644

drivers/pci/pci-driver.c static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev) · line 393

Quoted 6 lines
drivers/pci/pci-driver.c +3 -0
@@ -388,18 +388,31 @@ static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
 	return error;
 }
 
+int __weak pcibios_alloc_irq(struct pci_dev *dev)
+{
+	return dev->irq;

Based on your usage, pcibios_alloc_irq() returns either zero or a negative errno. So I think you should return 0 here.

Quoted 55 lines
drivers/pci/pci-driver.c +17 -6
+}
+
+void __weak pcibios_free_irq(struct pci_dev *dev)
+{
+}
+
 static int pci_device_probe(struct device *dev)
 {
-	int error = 0;
-	struct pci_driver *drv;
-	struct pci_dev *pci_dev;
+	int error;
+	struct pci_dev *pci_dev = to_pci_dev(dev);
+	struct pci_driver *drv = to_pci_driver(dev->driver);
+
+	error = pcibios_alloc_irq(pci_dev);
+	if (error < 0)
+		return error;
 
-	drv = to_pci_driver(dev->driver);
-	pci_dev = to_pci_dev(dev);
 	pci_dev_get(pci_dev);
 	error = __pci_device_probe(drv, pci_dev);
-	if (error)
+	if (error) {
+		pcibios_free_irq(pci_dev);
 		pci_dev_put(pci_dev);
+	}
 
 	return error;
 }
@@ -415,6 +428,7 @@ static int pci_device_remove(struct device *dev)
 			drv->remove(pci_dev);
 			pm_runtime_put_noidle(dev);
 		}
+		pcibios_free_irq(pci_dev);
 		pci_dev->driver = NULL;
 	}
 
include/linux/pci.h +2 -0
index 353db8dc4c6e..f50d16a04abc 100644
@@ -1656,6 +1656,8 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
 int pcibios_add_device(struct pci_dev *dev);
 void pcibios_release_device(struct pci_dev *dev);
 void pcibios_penalize_isa_irq(int irq, int active);
+int pcibios_alloc_irq(struct pci_dev *dev);
+void pcibios_free_irq(struct pci_dev *dev);
 
 #ifdef CONFIG_HIBERNATE_CALLBACKS
 extern struct dev_pm_ops pcibios_pm_ops;
Signature · 1 line
1.7.10.4
author
#3 lore raw

PATCH v2 2/4 PCI, x86: Allocate PCI IRQ on demand and free it when not used anymore

In reply to Jiang Liu

To support IOAPIC hotplug, we need to correctly manage IOAPIC pin usage, which is to allocate IRQs on demand and free them when not used anymore. So use pcibios_alloc_irq() and pcibios_free_irq() to dynamically allocate and free PCI IRQs.

Also remove obseleted code mp_should_keep_irq().

---
 arch/x86/include/asm/pci_x86.h |    2 --
 arch/x86/pci/common.c          |   20 +++++++++-----------
 arch/x86/pci/intel_mid_pci.c   |    7 +++++--
 arch/x86/pci/irq.c             |   15 +--------------
 drivers/acpi/pci_irq.c         |    9 +--------
 5 files changed, 16 insertions(+), 37 deletions(-)
arch/x86/include/asm/pci_x86.h +0 -2
index 164e3f8d3c3d..fa1195dae425 100644
@@ -93,8 +93,6 @@ 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);
arch/x86/pci/common.c +9 -11
index 8fd6f44aee83..dc78a4a9a466 100644
@@ -673,24 +673,22 @@ int pcibios_add_device(struct pci_dev *dev)
 	return 0;
 }
 
-int pcibios_enable_device(struct pci_dev *dev, int mask)
+int pcibios_alloc_irq(struct pci_dev *dev)
 {
-	int err;
-
-	if ((err = pci_enable_resources(dev, mask)) < 0)
-		return err;
-
-	if (!pci_dev_msi_enabled(dev))
-		return pcibios_enable_irq(dev);
-	return 0;
+	return pcibios_enable_irq(dev);
 }
 
-void pcibios_disable_device (struct pci_dev *dev)
+void pcibios_free_irq(struct pci_dev *dev)
 {
-	if (!pci_dev_msi_enabled(dev) && pcibios_disable_irq)
+	if (pcibios_disable_irq)
 		pcibios_disable_irq(dev);
 }
 
+int pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+	return pci_enable_resources(dev, mask);
+}
+
 int pci_ext_cfg_avail(void)
 {
 	if (raw_pci_ext_ops)
arch/x86/pci/intel_mid_pci.c +5 -2
index 27062303c881..fb7a1f96d80c 100644
@@ -234,10 +234,13 @@ static int intel_mid_pci_irq_enable(struct pci_dev *dev)
 
 static void intel_mid_pci_irq_disable(struct pci_dev *dev)
 {
-	if (!mp_should_keep_irq(&dev->dev) && dev->irq_managed &&
-	    dev->irq > 0) {
+	if (dev->irq_managed && dev->irq > 0) {
 		mp_unmap_irq(dev->irq);
 		dev->irq_managed = 0;
+		/*
+		 * Don't reset dev->irq here, otherwise
+		 * intel_mid_pci_irq_enable() will fail on next call.
+		 */
 	}
 }
 
arch/x86/pci/irq.c +1 -14
index 9bd115484745..72108f0b66b1 100644
@@ -1257,22 +1257,9 @@ 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 && !mp_should_keep_irq(&dev->dev) &&
-	    dev->irq_managed && dev->irq) {
+	if (io_apic_assign_pci_irqs && dev->irq_managed && dev->irq) {
 		mp_unmap_irq(dev->irq);
 		dev->irq = 0;
 		dev->irq_managed = 0;
drivers/acpi/pci_irq.c +1 -8
index b1def411c0b8..e7f718d6918a 100644
@@ -485,14 +485,6 @@ 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;
@@ -513,5 +505,6 @@ void acpi_pci_irq_disable(struct pci_dev *dev)
 	if (gsi >= 0) {
 		acpi_unregister_gsi(gsi);
 		dev->irq_managed = 0;
+		dev->irq = 0;
 	}
 }
Signature · 1 line
1.7.10.4
author
#4 lore raw

PATCH v2 3/4 PCI: Introduce helpers to manage pci_dev->irq and pci_dev->irq_managed

In reply to Jiang Liu

Introduce three helpers to manage pci_dev->irq and pci_dev->irq_managed, which helps to improve maintenance.

---
 arch/x86/pci/intel_mid_pci.c |    4 ++--
 arch/x86/pci/irq.c           |   10 ++++------
 drivers/acpi/pci_irq.c       |   10 ++++------
 include/linux/pci.h          |   17 +++++++++++++++++
 4 files changed, 27 insertions(+), 14 deletions(-)
arch/x86/pci/intel_mid_pci.c +2 -2
index fb7a1f96d80c..22aaefb4f1ca 100644
@@ -211,7 +211,7 @@ static int intel_mid_pci_irq_enable(struct pci_dev *dev)
 	struct irq_alloc_info info;
 	int polarity;
 
-	if (dev->irq_managed && dev->irq > 0)
+	if (pci_has_managed_irq(dev))
 		return 0;
 
 	if (intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER)
@@ -234,7 +234,7 @@ 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 (pci_has_managed_irq(dev)) {
 		mp_unmap_irq(dev->irq);
 		dev->irq_managed = 0;
 		/*
arch/x86/pci/irq.c +4 -6
index 72108f0b66b1..32e70343e6fd 100644
@@ -1202,7 +1202,7 @@ static int pirq_enable_irq(struct pci_dev *dev)
 			struct pci_dev *temp_dev;
 			int irq;
 
-			if (dev->irq_managed && dev->irq > 0)
+			if (pci_has_managed_irq(dev))
 				return 0;
 
 			irq = IO_APIC_get_PCI_irq_vector(dev->bus->number,
@@ -1230,8 +1230,7 @@ static int pirq_enable_irq(struct pci_dev *dev)
 			}
 			dev = temp_dev;
 			if (irq >= 0) {
-				dev->irq_managed = 1;
-				dev->irq = irq;
+				pci_set_managed_irq(dev, irq);
 				dev_info(&dev->dev, "PCI->APIC IRQ transform: "
 					 "INT %c -> IRQ %d\n", 'A' + pin - 1, irq);
 				return 0;
@@ -1259,9 +1258,8 @@ static int pirq_enable_irq(struct pci_dev *dev)
 
 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 && pci_has_managed_irq(dev)) {
 		mp_unmap_irq(dev->irq);
-		dev->irq = 0;
-		dev->irq_managed = 0;
+		pci_reset_managed_irq(dev);
 	}
 }
drivers/acpi/pci_irq.c +4 -6
index e7f718d6918a..9d6343d02f7e 100644
@@ -413,7 +413,7 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
 		return 0;
 	}
 
-	if (dev->irq_managed && dev->irq > 0)
+	if (pci_has_managed_irq(dev))
 		return 0;
 
 	entry = acpi_pci_irq_lookup(dev, pin);
@@ -458,8 +458,7 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
 		kfree(entry);
 		return rc;
 	}
-	dev->irq = rc;
-	dev->irq_managed = 1;
+	pci_set_managed_irq(dev, rc);
 
 	if (link)
 		snprintf(link_desc, sizeof(link_desc), " -> Link[%s]", link);
@@ -482,7 +481,7 @@ void acpi_pci_irq_disable(struct pci_dev *dev)
 	u8 pin;
 
 	pin = dev->pin;
-	if (!pin || !dev->irq_managed || dev->irq <= 0)
+	if (!pin || !pci_has_managed_irq(dev))
 		return;
 
 	entry = acpi_pci_irq_lookup(dev, pin);
@@ -504,7 +503,6 @@ void acpi_pci_irq_disable(struct pci_dev *dev)
 	dev_dbg(&dev->dev, "PCI INT %c disabled\n", pin_name(pin));
 	if (gsi >= 0) {
 		acpi_unregister_gsi(gsi);
-		dev->irq_managed = 0;
-		dev->irq = 0;
+		pci_reset_managed_irq(dev);
 	}
 }
include/linux/pci.h +17 -0
index f50d16a04abc..4bc640eef76f 100644
@@ -958,6 +958,23 @@ static inline int pci_is_managed(struct pci_dev *pdev)
 	return pdev->is_managed;
 }
 
+static inline void pci_set_managed_irq(struct pci_dev *pdev, unsigned int irq)
+{
+	pdev->irq = irq;
+	pdev->irq_managed = 1;
+}
+
+static inline void pci_reset_managed_irq(struct pci_dev *pdev)
+{
+	pdev->irq = 0;
+	pdev->irq_managed = 0;
+}
+
+static inline bool pci_has_managed_irq(struct pci_dev *pdev)
+{
+	return pdev->irq_managed && pdev->irq > 0;
+}
+
 void pci_disable_device(struct pci_dev *dev);
 
 extern unsigned int pcibios_max_latency;
Signature · 1 line
1.7.10.4
maintainer
#5 lore raw

Re: [Patch v2 3/4] PCI: Introduce helpers to manage pci_dev->irq and pci_dev->irq_managed

I'm on a campaign to squeeze more useful information into changelog subjects. "Add" means the same as "Introduce" but leaves more space for other useful information.

Quoted 140 lines · Jiang Liu

Introduce three helpers to manage pci_dev->irq and pci_dev->irq_managed, which helps to improve maintenance.

---
 arch/x86/pci/intel_mid_pci.c |    4 ++--
 arch/x86/pci/irq.c           |   10 ++++------
 drivers/acpi/pci_irq.c       |   10 ++++------
 include/linux/pci.h          |   17 +++++++++++++++++
 4 files changed, 27 insertions(+), 14 deletions(-)
arch/x86/pci/intel_mid_pci.c +2 -2
index fb7a1f96d80c..22aaefb4f1ca 100644
@@ -211,7 +211,7 @@ static int intel_mid_pci_irq_enable(struct pci_dev *dev)
 	struct irq_alloc_info info;
 	int polarity;
 
-	if (dev->irq_managed && dev->irq > 0)
+	if (pci_has_managed_irq(dev))
 		return 0;
 
 	if (intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER)
@@ -234,7 +234,7 @@ 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 (pci_has_managed_irq(dev)) {
 		mp_unmap_irq(dev->irq);
 		dev->irq_managed = 0;
 		/*
arch/x86/pci/irq.c +4 -6
index 72108f0b66b1..32e70343e6fd 100644
@@ -1202,7 +1202,7 @@ static int pirq_enable_irq(struct pci_dev *dev)
 			struct pci_dev *temp_dev;
 			int irq;
 
-			if (dev->irq_managed && dev->irq > 0)
+			if (pci_has_managed_irq(dev))
 				return 0;
 
 			irq = IO_APIC_get_PCI_irq_vector(dev->bus->number,
@@ -1230,8 +1230,7 @@ static int pirq_enable_irq(struct pci_dev *dev)
 			}
 			dev = temp_dev;
 			if (irq >= 0) {
-				dev->irq_managed = 1;
-				dev->irq = irq;
+				pci_set_managed_irq(dev, irq);
 				dev_info(&dev->dev, "PCI->APIC IRQ transform: "
 					 "INT %c -> IRQ %d\n", 'A' + pin - 1, irq);
 				return 0;
@@ -1259,9 +1258,8 @@ static int pirq_enable_irq(struct pci_dev *dev)
 
 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 && pci_has_managed_irq(dev)) {
 		mp_unmap_irq(dev->irq);
-		dev->irq = 0;
-		dev->irq_managed = 0;
+		pci_reset_managed_irq(dev);
 	}
 }
drivers/acpi/pci_irq.c +4 -6
index e7f718d6918a..9d6343d02f7e 100644
@@ -413,7 +413,7 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
 		return 0;
 	}
 
-	if (dev->irq_managed && dev->irq > 0)
+	if (pci_has_managed_irq(dev))
 		return 0;
 
 	entry = acpi_pci_irq_lookup(dev, pin);
@@ -458,8 +458,7 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
 		kfree(entry);
 		return rc;
 	}
-	dev->irq = rc;
-	dev->irq_managed = 1;
+	pci_set_managed_irq(dev, rc);
 
 	if (link)
 		snprintf(link_desc, sizeof(link_desc), " -> Link[%s]", link);
@@ -482,7 +481,7 @@ void acpi_pci_irq_disable(struct pci_dev *dev)
 	u8 pin;
 
 	pin = dev->pin;
-	if (!pin || !dev->irq_managed || dev->irq <= 0)
+	if (!pin || !pci_has_managed_irq(dev))
 		return;
 
 	entry = acpi_pci_irq_lookup(dev, pin);
@@ -504,7 +503,6 @@ void acpi_pci_irq_disable(struct pci_dev *dev)
 	dev_dbg(&dev->dev, "PCI INT %c disabled\n", pin_name(pin));
 	if (gsi >= 0) {
 		acpi_unregister_gsi(gsi);
-		dev->irq_managed = 0;
-		dev->irq = 0;
+		pci_reset_managed_irq(dev);
 	}
 }
include/linux/pci.h +17 -0
index f50d16a04abc..4bc640eef76f 100644
@@ -958,6 +958,23 @@ static inline int pci_is_managed(struct pci_dev *pdev)
 	return pdev->is_managed;
 }
 
+static inline void pci_set_managed_irq(struct pci_dev *pdev, unsigned int irq)
+{
+	pdev->irq = irq;
+	pdev->irq_managed = 1;
+}
+
+static inline void pci_reset_managed_irq(struct pci_dev *pdev)
+{
+	pdev->irq = 0;
+	pdev->irq_managed = 0;
+}
+
+static inline bool pci_has_managed_irq(struct pci_dev *pdev)
+{
+	return pdev->irq_managed && pdev->irq > 0;
+}
+
 void pci_disable_device(struct pci_dev *dev);
 
 extern unsigned int pcibios_max_latency;
Signature · 1 line
1.7.10.4
author
#6 lore raw

PATCH v2 4/4 PCI, MSI: Optionally free legacy PCI IRQ when enabling MSI/MSI-X

In reply to Jiang Liu

Once PCI MSI/MSI-X is enabled by the device driver, PCI device won't make use of legacy PCI IRQ until PCI MSI/MSI-X is disabled again. So optionally free legacy PCI IRQ when enabling MSI/MSI-X and reallocate when disabling MSI/MSI-X.

---
 drivers/pci/msi.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/pci/msi.c +5 -1
index c3e7dfcf9ff5..47cf72c669f0 100644
@@ -686,6 +686,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
 	msi_set_enable(dev, 1);
 	dev->msi_enabled = 1;
 
+	pcibios_free_irq(dev);
 	dev->irq = entry->irq;
 	return 0;
 }
@@ -813,9 +814,10 @@ static int msix_capability_init(struct pci_dev *dev,
 	/* Set MSI-X enabled bits and unmask the function */
 	pci_intx_for_msi(dev, 0);
 	dev->msix_enabled = 1;
-
 	msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
 
+	pcibios_free_irq(dev);
+
 	return 0;
 
 out_avail:
@@ -930,6 +932,7 @@ void pci_msi_shutdown(struct pci_dev *dev)
 
 	/* Restore dev->irq to its default pin-assertion irq */
 	dev->irq = desc->msi_attrib.default_irq;
+	pcibios_alloc_irq(dev);
 }
 
 void pci_disable_msi(struct pci_dev *dev)
@@ -1030,6 +1033,7 @@ void pci_msix_shutdown(struct pci_dev *dev)
 	msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
 	pci_intx_for_msi(dev, 1);
 	dev->msix_enabled = 0;
+	pcibios_alloc_irq(dev);
 }
 
 void pci_disable_msix(struct pci_dev *dev)
Signature · 1 line
1.7.10.4
maintainer
#7 lore raw

Re: [Patch v2 4/4] PCI, MSI: Optionally free legacy PCI IRQ when enabling MSI/MSI-X

Quoted 1 line

From the point of view of the generic code in this patch, freeing the IRQ

isn't optional -- there's no "if" in this this patch.

It's true that the default pcibios_free_irq() implementation does nothing, but the changelog of this patch should describe what this patch does.

Quoted 53 lines · Jiang Liu

Once PCI MSI/MSI-X is enabled by the device driver, PCI device won't make use of legacy PCI IRQ until PCI MSI/MSI-X is disabled again. So optionally free legacy PCI IRQ when enabling MSI/MSI-X and reallocate when disabling MSI/MSI-X.

---
 drivers/pci/msi.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/pci/msi.c +5 -1
index c3e7dfcf9ff5..47cf72c669f0 100644
@@ -686,6 +686,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
 	msi_set_enable(dev, 1);
 	dev->msi_enabled = 1;
 
+	pcibios_free_irq(dev);
 	dev->irq = entry->irq;
 	return 0;
 }
@@ -813,9 +814,10 @@ static int msix_capability_init(struct pci_dev *dev,
 	/* Set MSI-X enabled bits and unmask the function */
 	pci_intx_for_msi(dev, 0);
 	dev->msix_enabled = 1;
-
 	msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
 
+	pcibios_free_irq(dev);
+
 	return 0;
 
 out_avail:
@@ -930,6 +932,7 @@ void pci_msi_shutdown(struct pci_dev *dev)
 
 	/* Restore dev->irq to its default pin-assertion irq */
 	dev->irq = desc->msi_attrib.default_irq;
+	pcibios_alloc_irq(dev);
 }
 
 void pci_disable_msi(struct pci_dev *dev)
@@ -1030,6 +1033,7 @@ void pci_msix_shutdown(struct pci_dev *dev)
 	msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
 	pci_intx_for_msi(dev, 1);
 	dev->msix_enabled = 0;
+	pcibios_alloc_irq(dev);
 }
 
 void pci_disable_msix(struct pci_dev *dev)
Signature · 1 line
1.7.10.4
maintainer Acked-by on the v2 series
#8 lore raw
In reply to Jiang Liu
Quoted 27 lines · Jiang Liu

This patch set introduces a mechanism to allocate PCI IRQ on demand and free it when not used anymore by hooking pci_device_probe() and pci_device_remove().

It will be used to track IOAPIC pin usage on x86 so we could support IOAPIC hot-removal.

The patch set passes Fengguang's 0day test suite.

V1->V2:
1) Refine pci_device_probe() to optimize for mainline code as suggested by Bjorn
2) Reorder patch set to put optional patch as the last (Patch 4)

Thanks!
Gerry

Jiang Liu (4):
PCI: Add hooks to allocate/free IRQ resources when binding/unbinding
driver
PCI, x86: Allocate PCI IRQ on demand and free it when not used
anymore
PCI: Introduce helpers to manage pci_dev->irq and

    pci_dev->irq_managed

PCI, MSI: Optionally free legacy PCI IRQ when enabling MSI/MSI-X

Quoted 9 lines
 arch/x86/include/asm/pci_x86.h |    2 --
 arch/x86/pci/common.c          |   20 +++++++++-----------
 arch/x86/pci/intel_mid_pci.c   |    9 ++++++---
 arch/x86/pci/irq.c             |   23 ++++-------------------
 drivers/acpi/pci_irq.c         |   17 ++++-------------
 drivers/pci/msi.c              |    6 +++++-
 drivers/pci/pci-driver.c       |   26 ++++++++++++++++++++------
 include/linux/pci.h            |   19 +++++++++++++++++++
 8 files changed, 67 insertions(+), 55 deletions(-)

I'm fine with these patches, and I can merge them, but I would like an ack from Thomas.

Or, if it makes more sense to route these along with other related patches through Thomas, here's my ack: