[Patch v2 0/4] Introduce a mechanism to allocate PCI IRQ on demand

[Patch v2 0/4] Introduce a mechanism to allocate PCI IRQ on demand

[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

 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(-)

-- 
1.7.10.4

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

[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.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/pci/pci-driver.c |   26 ++++++++++++++++++++------
 include/linux/pci.h      |    2 ++
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 3cb2210de553..450ad36ffc77 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -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;
 	}
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 353db8dc4c6e..f50d16a04abc 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -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;
-- 
1.7.10.4

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

[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().

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 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(-)

diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
index 164e3f8d3c3d..fa1195dae425 100644
--- a/arch/x86/include/asm/pci_x86.h
+++ b/arch/x86/include/asm/pci_x86.h
@@ -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);
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 8fd6f44aee83..dc78a4a9a466 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -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)
diff --git a/arch/x86/pci/intel_mid_pci.c b/arch/x86/pci/intel_mid_pci.c
index 27062303c881..fb7a1f96d80c 100644
--- a/arch/x86/pci/intel_mid_pci.c
+++ b/arch/x86/pci/intel_mid_pci.c
@@ -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.
+		 */
 	}
 }
 
diff --git a/arch/x86/pci/irq.c b/arch/x86/pci/irq.c
index 9bd115484745..72108f0b66b1 100644
--- a/arch/x86/pci/irq.c
+++ b/arch/x86/pci/irq.c
@@ -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;
diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
index b1def411c0b8..e7f718d6918a 100644
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -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;
 	}
 }
-- 
1.7.10.4

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

[Jiang Liu]

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

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 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(-)

diff --git a/arch/x86/pci/intel_mid_pci.c b/arch/x86/pci/intel_mid_pci.c
index fb7a1f96d80c..22aaefb4f1ca 100644
--- a/arch/x86/pci/intel_mid_pci.c
+++ b/arch/x86/pci/intel_mid_pci.c
@@ -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;
 		/*
diff --git a/arch/x86/pci/irq.c b/arch/x86/pci/irq.c
index 72108f0b66b1..32e70343e6fd 100644
--- a/arch/x86/pci/irq.c
+++ b/arch/x86/pci/irq.c
@@ -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);
 	}
 }
diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
index e7f718d6918a..9d6343d02f7e 100644
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -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);
 	}
 }
diff --git a/include/linux/pci.h b/include/linux/pci.h
index f50d16a04abc..4bc640eef76f 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -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;
-- 
1.7.10.4

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

[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.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/pci/msi.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index c3e7dfcf9ff5..47cf72c669f0 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -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)
-- 
1.7.10.4

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

[Bjorn Helgaas]

On Tue, Jun 02, 2015 at 10:49:16AM +0800, Jiang Liu wrote:
> 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.
> 
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>  drivers/pci/pci-driver.c |   26 ++++++++++++++++++++------
>  include/linux/pci.h      |    2 ++
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 3cb2210de553..450ad36ffc77 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -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.

> +}
> +
> +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;
>  	}
>  
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 353db8dc4c6e..f50d16a04abc 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -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;
> -- 
> 1.7.10.4
> 

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

[Bjorn Helgaas]

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.

On Tue, Jun 02, 2015 at 10:49:18AM +0800, Jiang Liu wrote:
> Introduce three helpers to manage pci_dev->irq and pci_dev->irq_managed,
> which helps to improve maintenance.
> 
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>  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(-)
> 
> diff --git a/arch/x86/pci/intel_mid_pci.c b/arch/x86/pci/intel_mid_pci.c
> index fb7a1f96d80c..22aaefb4f1ca 100644
> --- a/arch/x86/pci/intel_mid_pci.c
> +++ b/arch/x86/pci/intel_mid_pci.c
> @@ -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;
>  		/*
> diff --git a/arch/x86/pci/irq.c b/arch/x86/pci/irq.c
> index 72108f0b66b1..32e70343e6fd 100644
> --- a/arch/x86/pci/irq.c
> +++ b/arch/x86/pci/irq.c
> @@ -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);
>  	}
>  }
> diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
> index e7f718d6918a..9d6343d02f7e 100644
> --- a/drivers/acpi/pci_irq.c
> +++ b/drivers/acpi/pci_irq.c
> @@ -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);
>  	}
>  }
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index f50d16a04abc..4bc640eef76f 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -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;
> -- 
> 1.7.10.4
> 

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

[Bjorn Helgaas]

>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.

On Tue, Jun 02, 2015 at 10:49:19AM +0800, Jiang Liu wrote:
> 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.
> 
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>  drivers/pci/msi.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index c3e7dfcf9ff5..47cf72c669f0 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -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)
> -- 
> 1.7.10.4
> 

Re: [Patch v2 0/4] Introduce a mechanism to allocate PCI IRQ on demand

[Bjorn Helgaas]

On Tue, Jun 02, 2015 at 10:49:15AM +0800, Jiang Liu wrote:
> 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(-)

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:

Acked-by: Bjorn Helgaas <bhelgaas@google.com>