[PATCH v5 0/4] Introduce pci_enable_msi/msix_range() interfaces

[PATCH v5 0/4] Introduce pci_enable_msi/msix_range() interfaces

[Alexander Gordeev]

This series is against "next" branch in Bjorn's repo:
git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git

Changes from v4 to v5:
  - pci_auto_enable_msi* functions renamed to pci_enable_msi* ones;
  - pci_enable_msi_range() and pci_enable_msix_range() are the only
    basic interfaces left to enable MSI/MSI-X operation;
  - pci_enable_msi(), pci_enable_msi_block() and pci_enable_msix() old
    interfaces deprecated in favour of pci_enable_msi/msix_range() ones;
    description of the old interfaces is removed from the documentation;
  - pci_get_msi_vec_count()  renamed to pci_msi_vec_count() and
    pci_get_msix_vec_count() renamed to pci_msix_vec_count();

Changes from v3 to v4:
  - pcim_enable_msi* functions renamed to pci_auto_enable_msi* ones;
  - PowerPC patches dropped;
  - pci_get_msi_cap()     renamed to pci_get_msi_vec_count() and
    pci_msix_table_size() renamed to pci_get_msix_vec_count();

Changes from v2 to v3:
  - new public interfaces commented in drivers/pci/msi.c;
  - patch "Make quota traversing and requesting race-safe" explained;
  - pci_enable_msi/msix() 'nvec' arg type changed from 'unsigned int' to 'int';
  - pcim_enable_msi*() arg 'nvec' renamed to 'maxvec' when upper limit passed;
  - pcim_enable_msi*(..., maxvec, minvec) arg order swapped to minvec, maxvec;
  - "PCI: Fail MSI/MSI-X initialization if device is not in PCI_D0" commit
    869a161 and "PCI/MSI: Factor out pci_get_msi_cap() interface" patch
    conflicts resolved;

This update introduces pci_enable_msi_range() function which
supersedes pci_enable_msi() and pci_enable_msi_block()
interfaces for MSI and pci_enable_msi_range() function which
supersedes pci_enable_msix() interface for MSI-X interrupts.

The plan is to convert all drivers to the new interfaces and
then remove existing pci_enable_msi(), pci_enable_msi_block()
and pci_enable_msix() functions.

The newly introduced functions return either a negative error
code or a number of successfully allocated MSI/MSI-X interrupts.

Unlike the old interfaces where the "third" state indicates
a possible number of MSI interrupts that could have been
allocated, the new interfaces allow device drivers not to deal
with tri-state return values and hide fallback logic behind
the implementation.

As result, device drivers are expected to have more clearer
and straight code.

The tree could be found in "pci-next-msi-v5" branch in repo:
https://github.com/a-gordeev/linux.git

Alexander Gordeev (4):
  PCI/MSI: Factor out pci_msi_vec_count() interface
  PCI/MSI: Get rid of pci_enable_msi_block_auto() interface
  PCI/MSI: Introduce pci_msix_vec_count() interface
  PCI/MSI: Introduce pci_enable_msi/msix_range() interfaces

 Documentation/PCI/MSI-HOWTO.txt |  308 ++++++++++++++++++++++++++-------------
 drivers/ata/ahci.c              |   56 +++++---
 drivers/pci/msi.c               |  150 +++++++++++++++-----
 drivers/pci/pcie/portdrv_core.c |    7 +-
 include/linux/pci.h             |   27 +++-
 5 files changed, 385 insertions(+), 163 deletions(-)

-- 
1.7.7.6

[PATCH v5 1/4] PCI/MSI: Factor out pci_msi_vec_count() interface

[Alexander Gordeev]

Device drivers can use this interface to obtain maximum number
of MSI interrupts the device supports and use that number i.e.
in a following call to pci_enable_msi_block() interface.

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Reviewed-by: Tejun Heo <tj@kernel.org>
---
 Documentation/PCI/MSI-HOWTO.txt |   15 ++++++++++++++
 drivers/pci/msi.c               |   41 +++++++++++++++++++++++++++++++-------
 include/linux/pci.h             |    6 +++++
 3 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
index a4d174e..a8b4178 100644
--- a/Documentation/PCI/MSI-HOWTO.txt
+++ b/Documentation/PCI/MSI-HOWTO.txt
@@ -169,6 +169,21 @@ on any interrupt for which it previously called request_irq().
 Failure to do so results in a BUG_ON(), leaving the device with
 MSI enabled and thus leaking its vector.
 
+4.2.5 pci_msi_vec_count
+
+int pci_msi_vec_count(struct pci_dev *dev)
+
+This function could be used to retrieve the number of MSI vectors the
+device requested (via the Multiple Message Capable register). The MSI
+specification only allows the returned value to be a power of two,
+up to a maximum of 2^5 (32).
+
+If this function returns a negative number, it indicates the device is
+not capable of sending MSIs.
+
+If this function returns a positive number, it indicates the maximum
+number of MSI interrupt vectors that could be allocated.
+
 4.3 Using MSI-X
 
 The MSI-X capability is much more flexible than the MSI capability.
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 22957aa..5233e28 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -852,6 +852,31 @@ static int pci_msi_check_device(struct pci_dev *dev, int nvec, int type)
 }
 
 /**
+ * pci_msi_vec_count - Return the number of MSI vectors a device can send
+ * @dev: device to report about
+ *
+ * This function returns the number of MSI vectors a device requested via
+ * Multiple Message Capable register. It returns a negative errno if the
+ * device is not capable sending MSI interrupts. Otherwise, the call succeeds
+ * and returns a power of two, up to a maximum of 2^5 (32), according to the
+ * MSI specification.
+ **/
+int pci_msi_vec_count(struct pci_dev *dev)
+{
+	int ret;
+	u16 msgctl;
+
+	if (!dev->msi_cap)
+		return -EINVAL;
+
+	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &msgctl);
+	ret = 1 << ((msgctl & PCI_MSI_FLAGS_QMASK) >> 1);
+
+	return ret;
+}
+EXPORT_SYMBOL(pci_msi_vec_count);
+
+/**
  * pci_enable_msi_block - configure device's MSI capability structure
  * @dev: device to configure
  * @nvec: number of interrupts to configure
@@ -867,13 +892,13 @@ static int pci_msi_check_device(struct pci_dev *dev, int nvec, int type)
 int pci_enable_msi_block(struct pci_dev *dev, int nvec)
 {
 	int status, maxvec;
-	u16 msgctl;
 
-	if (!dev->msi_cap || dev->current_state != PCI_D0)
+	if (dev->current_state != PCI_D0)
 		return -EINVAL;
 
-	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &msgctl);
-	maxvec = 1 << ((msgctl & PCI_MSI_FLAGS_QMASK) >> 1);
+	maxvec = pci_msi_vec_count(dev);
+	if (maxvec < 0)
+		return maxvec;
 	if (nvec > maxvec)
 		return maxvec;
 
@@ -898,13 +923,13 @@ EXPORT_SYMBOL(pci_enable_msi_block);
 int pci_enable_msi_block_auto(struct pci_dev *dev, int *maxvec)
 {
 	int ret, nvec;
-	u16 msgctl;
 
-	if (!dev->msi_cap || dev->current_state != PCI_D0)
+	if (dev->current_state != PCI_D0)
 		return -EINVAL;
 
-	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &msgctl);
-	ret = 1 << ((msgctl & PCI_MSI_FLAGS_QMASK) >> 1);
+	ret = pci_msi_vec_count(dev);
+	if (ret < 0)
+		return ret;
 
 	if (maxvec)
 		*maxvec = ret;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4b7cf4c..0322af1 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1166,6 +1166,11 @@ struct msix_entry {
 
 
 #ifndef CONFIG_PCI_MSI
+static inline int pci_msi_vec_count(struct pci_dev *dev)
+{
+	return -ENOSYS;
+}
+
 static inline int pci_enable_msi_block(struct pci_dev *dev, int nvec)
 {
 	return -ENOSYS;
@@ -1207,6 +1212,7 @@ static inline int pci_msi_enabled(void)
 	return 0;
 }
 #else
+int pci_msi_vec_count(struct pci_dev *dev);
 int pci_enable_msi_block(struct pci_dev *dev, int nvec);
 int pci_enable_msi_block_auto(struct pci_dev *dev, int *maxvec);
 void pci_msi_shutdown(struct pci_dev *dev);
-- 
1.7.7.6

[PATCH v5 2/4] PCI/MSI: Get rid of pci_enable_msi_block_auto() interface

[Alexander Gordeev]

As result of introduction of pci_msi_vec_count() interface
pci_enable_msi_block_auto() function became superflous.

To enable maximum possible number of MSIs drivers will first
obtain that number from pci_msi_vec_count() function and then
call pci_enable_msi_block() interface.

Function pci_enable_msi_block_auto() has been introduced
recently and its only user is AHCI driver, which is also
updated by this change.

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Acked-by: Tejun Heo <tj@kernel.org>
---
 Documentation/PCI/MSI-HOWTO.txt |   39 ++++-----------------------
 drivers/ata/ahci.c              |   56 ++++++++++++++++++++++++--------------
 drivers/pci/msi.c               |   25 -----------------
 include/linux/pci.h             |    7 -----
 4 files changed, 41 insertions(+), 86 deletions(-)

diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
index a8b4178..aa4ad98 100644
--- a/Documentation/PCI/MSI-HOWTO.txt
+++ b/Documentation/PCI/MSI-HOWTO.txt
@@ -127,49 +127,22 @@ on the number of vectors that can be allocated; pci_enable_msi_block()
 returns as soon as it finds any constraint that doesn't allow the
 call to succeed.
 
-4.2.3 pci_enable_msi_block_auto
-
-int pci_enable_msi_block_auto(struct pci_dev *dev, int *count)
-
-This variation on pci_enable_msi() call allows a device driver to request
-the maximum possible number of MSIs.  The MSI specification only allows
-interrupts to be allocated in powers of two, up to a maximum of 2^5 (32).
-
-If this function returns a positive number, it indicates that it has
-succeeded and the returned value is the number of allocated interrupts. In
-this case, the function enables MSI on this device and updates dev->irq to
-be the lowest of the new interrupts assigned to it.  The other interrupts
-assigned to the device are in the range dev->irq to dev->irq + returned
-value - 1.
-
-If this function returns a negative number, it indicates an error and
-the driver should not attempt to request any more MSI interrupts for
-this device.
-
-If the device driver needs to know the number of interrupts the device
-supports it can pass the pointer count where that number is stored. The
-device driver must decide what action to take if pci_enable_msi_block_auto()
-succeeds, but returns a value less than the number of interrupts supported.
-If the device driver does not need to know the number of interrupts
-supported, it can set the pointer count to NULL.
-
-4.2.4 pci_disable_msi
+4.2.3 pci_disable_msi
 
 void pci_disable_msi(struct pci_dev *dev)
 
 This function should be used to undo the effect of pci_enable_msi() or
-pci_enable_msi_block() or pci_enable_msi_block_auto().  Calling it restores
-dev->irq to the pin-based interrupt number and frees the previously
-allocated message signaled interrupt(s).  The interrupt may subsequently be
-assigned to another device, so drivers should not cache the value of
-dev->irq.
+pci_enable_msi_block().  Calling it restores dev->irq to the pin-based
+interrupt number and frees the previously allocated message signaled
+interrupt(s).  The interrupt may subsequently be assigned to another
+device, so drivers should not cache the value of dev->irq.
 
 Before calling this function, a device driver must always call free_irq()
 on any interrupt for which it previously called request_irq().
 Failure to do so results in a BUG_ON(), leaving the device with
 MSI enabled and thus leaking its vector.
 
-4.2.5 pci_msi_vec_count
+4.2.4 pci_msi_vec_count
 
 int pci_msi_vec_count(struct pci_dev *dev)
 
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index e2903d0..8516f4d 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1095,26 +1095,40 @@ static inline void ahci_gtf_filter_workaround(struct ata_host *host)
 {}
 #endif
 
-int ahci_init_interrupts(struct pci_dev *pdev, struct ahci_host_priv *hpriv)
+int ahci_init_interrupts(struct pci_dev *pdev, unsigned int n_ports,
+			 struct ahci_host_priv *hpriv)
 {
-	int rc;
-	unsigned int maxvec;
+	int rc, nvec;
 
-	if (!(hpriv->flags & AHCI_HFLAG_NO_MSI)) {
-		rc = pci_enable_msi_block_auto(pdev, &maxvec);
-		if (rc > 0) {
-			if ((rc == maxvec) || (rc == 1))
-				return rc;
-			/*
-			 * Assume that advantage of multipe MSIs is negated,
-			 * so fallback to single MSI mode to save resources
-			 */
-			pci_disable_msi(pdev);
-			if (!pci_enable_msi(pdev))
-				return 1;
-		}
-	}
+	if (hpriv->flags & AHCI_HFLAG_NO_MSI)
+		goto intx;
+
+	rc = pci_msi_vec_count(pdev);
+	if (rc < 0)
+		goto intx;
+
+	/*
+	 * If number of MSIs is less than number of ports then Sharing Last
+	 * Message mode could be enforced. In this case assume that advantage
+	 * of multipe MSIs is negated and use single MSI mode instead.
+	 */
+	if (rc < n_ports)
+		goto single_msi;
+
+	nvec = rc;
+	rc = pci_enable_msi_block(pdev, nvec);
+	if (rc)
+		goto intx;
 
+	return nvec;
+
+single_msi:
+	rc = pci_enable_msi(pdev);
+	if (rc)
+		goto intx;
+	return 1;
+
+intx:
 	pci_intx(pdev, 1);
 	return 0;
 }
@@ -1281,10 +1295,6 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	hpriv->mmio = pcim_iomap_table(pdev)[ahci_pci_bar];
 
-	n_msis = ahci_init_interrupts(pdev, hpriv);
-	if (n_msis > 1)
-		hpriv->flags |= AHCI_HFLAG_MULTI_MSI;
-
 	/* save initial config */
 	ahci_pci_save_initial_config(pdev, hpriv);
 
@@ -1339,6 +1349,10 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 */
 	n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));
 
+	n_msis = ahci_init_interrupts(pdev, n_ports, hpriv);
+	if (n_msis > 1)
+		hpriv->flags |= AHCI_HFLAG_MULTI_MSI;
+
 	host = ata_host_alloc_pinfo(&pdev->dev, ppi, n_ports);
 	if (!host)
 		return -ENOMEM;
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 5233e28..ff7d4f0 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -920,31 +920,6 @@ int pci_enable_msi_block(struct pci_dev *dev, int nvec)
 }
 EXPORT_SYMBOL(pci_enable_msi_block);
 
-int pci_enable_msi_block_auto(struct pci_dev *dev, int *maxvec)
-{
-	int ret, nvec;
-
-	if (dev->current_state != PCI_D0)
-		return -EINVAL;
-
-	ret = pci_msi_vec_count(dev);
-	if (ret < 0)
-		return ret;
-
-	if (maxvec)
-		*maxvec = ret;
-
-	do {
-		nvec = ret;
-		ret = pci_enable_msi_block(dev, nvec);
-	} while (ret > 0);
-
-	if (ret < 0)
-		return ret;
-	return nvec;
-}
-EXPORT_SYMBOL(pci_enable_msi_block_auto);
-
 void pci_msi_shutdown(struct pci_dev *dev)
 {
 	struct msi_desc *desc;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 0322af1..206fa07 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1176,12 +1176,6 @@ static inline int pci_enable_msi_block(struct pci_dev *dev, int nvec)
 	return -ENOSYS;
 }
 
-static inline int
-pci_enable_msi_block_auto(struct pci_dev *dev, int *maxvec)
-{
-	return -ENOSYS;
-}
-
 static inline void pci_msi_shutdown(struct pci_dev *dev)
 { }
 static inline void pci_disable_msi(struct pci_dev *dev)
@@ -1214,7 +1208,6 @@ static inline int pci_msi_enabled(void)
 #else
 int pci_msi_vec_count(struct pci_dev *dev);
 int pci_enable_msi_block(struct pci_dev *dev, int nvec);
-int pci_enable_msi_block_auto(struct pci_dev *dev, int *maxvec);
 void pci_msi_shutdown(struct pci_dev *dev);
 void pci_disable_msi(struct pci_dev *dev);
 int pci_msix_table_size(struct pci_dev *dev);
-- 
1.7.7.6

[PATCH v5 3/4] PCI/MSI: Introduce pci_msix_vec_count() interface

[Alexander Gordeev]

This update is needed to create a consistent MSI-X counterpart
for pci_msi_vec_count() MSI interface. Device drivers can use
this function to obtain maximum number of MSI-X interrupts the
device supports and i.e. use that number in a following call
to pci_enable_msix() interface.

Interface pci_msix_vec_count() supersedes pci_msix_table_size()
function and returns a negative errno if device does not support
MSI-X interrupts. After this update callers must always check the
returned value.

The only user of pci_msix_table_size() function was PCI-Express
port driver, which is also updated by this change.

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Reviewed-by: Tejun Heo <tj@kernel.org>
---
 Documentation/PCI/MSI-HOWTO.txt |   13 +++++++++++++
 drivers/pci/msi.c               |   18 +++++++++++++-----
 drivers/pci/pcie/portdrv_core.c |    7 ++++---
 include/linux/pci.h             |    6 +++---
 4 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
index aa4ad98..b58f4a4 100644
--- a/Documentation/PCI/MSI-HOWTO.txt
+++ b/Documentation/PCI/MSI-HOWTO.txt
@@ -243,6 +243,19 @@ MSI-X Table.  This address is mapped by the PCI subsystem, and should not
 be accessed directly by the device driver.  If the driver wishes to
 mask or unmask an interrupt, it should call disable_irq() / enable_irq().
 
+4.3.4 pci_msix_vec_count
+
+int pci_msix_vec_count(struct pci_dev *dev)
+
+This function could be used to retrieve number of entries in the device
+MSI-X table.
+
+If this function returns a negative number, it indicates the device is
+not capable of sending MSI-Xs.
+
+If this function returns a positive number, it indicates the maximum
+number of MSI-X interrupt vectors that could be allocated.
+
 4.4 Handling devices implementing both MSI and MSI-X capabilities
 
 If a device implements both MSI and MSI-X capabilities, it can
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index ff7d4f0..32dca35 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -957,19 +957,25 @@ void pci_disable_msi(struct pci_dev *dev)
 EXPORT_SYMBOL(pci_disable_msi);
 
 /**
- * pci_msix_table_size - return the number of device's MSI-X table entries
+ * pci_msix_vec_count - return the number of device's MSI-X table entries
  * @dev: pointer to the pci_dev data structure of MSI-X device function
- */
-int pci_msix_table_size(struct pci_dev *dev)
+
+ * This function returns the number of device's MSI-X table entries and
+ * therefore the number of MSI-X vectors device is capable to send.
+ * It returns a negative errno if the device is not capable sending MSI-X
+ * interrupts.
+ **/
+int pci_msix_vec_count(struct pci_dev *dev)
 {
 	u16 control;
 
 	if (!dev->msix_cap)
-		return 0;
+		return -EINVAL;
 
 	pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &control);
 	return msix_table_size(control);
 }
+EXPORT_SYMBOL(pci_msix_vec_count);
 
 /**
  * pci_enable_msix - configure device's MSI-X capability structure
@@ -998,7 +1004,9 @@ int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec)
 	if (status)
 		return status;
 
-	nr_entries = pci_msix_table_size(dev);
+	nr_entries = pci_msix_vec_count(dev);
+	if (nr_entries < 0)
+		return nr_entries;
 	if (nvec > nr_entries)
 		return nr_entries;
 
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 16a1ce4..986f8ea 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -79,9 +79,10 @@ static int pcie_port_enable_msix(struct pci_dev *dev, int *vectors, int mask)
 	u16 reg16;
 	u32 reg32;
 
-	nr_entries = pci_msix_table_size(dev);
-	if (!nr_entries)
-		return -EINVAL;
+	nr_entries = pci_msix_vec_count(dev);
+	if (nr_entries < 0)
+		return nr_entries;
+	BUG_ON(!nr_entries);
 	if (nr_entries > PCIE_PORT_MAX_MSIX_ENTRIES)
 		nr_entries = PCIE_PORT_MAX_MSIX_ENTRIES;
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 206fa07..d031639 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1181,9 +1181,9 @@ static inline void pci_msi_shutdown(struct pci_dev *dev)
 static inline void pci_disable_msi(struct pci_dev *dev)
 { }
 
-static inline int pci_msix_table_size(struct pci_dev *dev)
+static inline int pci_msix_vec_count(struct pci_dev *dev)
 {
-	return 0;
+	return -ENOSYS;
 }
 static inline int pci_enable_msix(struct pci_dev *dev,
 				  struct msix_entry *entries, int nvec)
@@ -1210,7 +1210,7 @@ int pci_msi_vec_count(struct pci_dev *dev);
 int pci_enable_msi_block(struct pci_dev *dev, int nvec);
 void pci_msi_shutdown(struct pci_dev *dev);
 void pci_disable_msi(struct pci_dev *dev);
-int pci_msix_table_size(struct pci_dev *dev);
+int pci_msix_vec_count(struct pci_dev *dev);
 int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec);
 void pci_msix_shutdown(struct pci_dev *dev);
 void pci_disable_msix(struct pci_dev *dev);
-- 
1.7.7.6

[PATCH v5 4/4] PCI/MSI: Introduce pci_enable_msi/msix_range() interfaces

[Alexander Gordeev]

This update introduces pci_enable_msi_range() function which
supersedes pci_enable_msi() and pci_enable_msi_block()
interfaces for MSI and pci_enable_msi_range() function which
supersedes pci_enable_msix() interface for MSI-X interrupts.

The newly introduced functions return either a negative error
code or a number of successfully allocated MSI/MSI-X interrupts.

Unlike the old interfaces where the "third" state indicates
a possible number of MSI interrupts that could have been
allocated, the new interfaces allow device drivers do not deal
with tri-state return values and hide the fallback logic behind
the implementation.

As result, device drivers are expected to have more clearer
and straight code.

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Suggested-by: Ben Hutchings <bhutchings@solarflare.com>
Reviewed-by: Tejun Heo <tj@kernel.org>
---
 Documentation/PCI/MSI-HOWTO.txt |  261 ++++++++++++++++++++++++++++-----------
 drivers/pci/msi.c               |   74 +++++++++++
 include/linux/pci.h             |   14 ++
 3 files changed, 274 insertions(+), 75 deletions(-)

diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
index b58f4a4..ea6cd7a 100644
--- a/Documentation/PCI/MSI-HOWTO.txt
+++ b/Documentation/PCI/MSI-HOWTO.txt
@@ -82,67 +82,97 @@ Most of the hard work is done for the driver in the PCI layer.  It simply
 has to request that the PCI layer set up the MSI capability for this
 device.
 
-4.2.1 pci_enable_msi
+4.2.1 pci_enable_msi_range
 
-int pci_enable_msi(struct pci_dev *dev)
+int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec)
 
-A successful call allocates ONE interrupt to the device, regardless
-of how many MSIs the device supports.  The device is switched from
-pin-based interrupt mode to MSI mode.  The dev->irq number is changed
-to a new number which represents the message signaled interrupt;
-consequently, this function should be called before the driver calls
-request_irq(), because an MSI is delivered via a vector that is
-different from the vector of a pin-based interrupt.
+This function allows a device driver to request any number of MSI
+interrupts within specified range from 'minvec' to 'maxvec'.
 
-4.2.2 pci_enable_msi_block
+If this function returns a positive number it indicates the number of
+MSI interrupts that have been successfully allocated. In this case
+the device is switched from pin-based interrupt mode to MSI mode and
+updates dev->irq to be the lowest of the new interrupts assigned to it.
+The other interrupts assigned to the device are in the range dev->irq
+to dev->irq + returned value - 1. Device driver can use the returned
+number of successfully allocated MSI interrupts to further allocate
+and initialize device resources.
 
-int pci_enable_msi_block(struct pci_dev *dev, int count)
+If this function returns a negative number, it indicates an error and
+the driver should not attempt to request any more MSI interrupts for
+this device.
 
-This variation on the above call allows a device driver to request multiple
-MSIs.  The MSI specification only allows interrupts to be allocated in
-powers of two, up to a maximum of 2^5 (32).
+This function should be called before the driver calls request_irq(),
+because MSI interrupts are delivered via vectors that are different
+from the vector of a pin-based interrupt.
 
-If this function returns 0, it has succeeded in allocating at least as many
-interrupts as the driver requested (it may have allocated more in order
-to satisfy the power-of-two requirement).  In this case, the function
-enables MSI on this device and updates dev->irq to be the lowest of
-the new interrupts assigned to it.  The other interrupts assigned to
-the device are in the range dev->irq to dev->irq + count - 1.
+It is ideal if drivers can cope with a variable number of MSI interrupts;
+there are many reasons why the platform may not be able to provide the
+exact number that a driver asks for.
 
-If this function returns a negative number, it indicates an error and
-the driver should not attempt to request any more MSI interrupts for
-this device.  If this function returns a positive number, it is
-less than 'count' and indicates the number of interrupts that could have
-been allocated.  In neither case is the irq value updated or the device
-switched into MSI mode.
-
-The device driver must decide what action to take if
-pci_enable_msi_block() returns a value less than the number requested.
-For instance, the driver could still make use of fewer interrupts;
-in this case the driver should call pci_enable_msi_block()
-again.  Note that it is not guaranteed to succeed, even when the
-'count' has been reduced to the value returned from a previous call to
-pci_enable_msi_block().  This is because there are multiple constraints
-on the number of vectors that can be allocated; pci_enable_msi_block()
-returns as soon as it finds any constraint that doesn't allow the
-call to succeed.
-
-4.2.3 pci_disable_msi
+There could be devices that can not operate with just any number of MSI
+interrupts within a range. See chapter 4.3.1.3 to get the idea how to
+handle such devices for MSI-X - the same logic applies to MSI.
+
+4.2.1.1 Maximum possible number of MSI interrupts
+
+The typical usage of MSI interrupts is to allocate as many vectors as
+possible, likely up to the limit returned by pci_msi_vec_count() function:
+
+static int foo_driver_enable_msi(struct pci_dev *pdev, int nvec)
+{
+	return pci_enable_msi_range(pdev, 1, nvec);
+}
+
+Note the value of 'minvec' parameter is 1. As 'minvec' is inclusive,
+the value of 0 would be meaningless and could result in error.
+
+Some devices have a minimal limit on number of MSI interrupts.
+In this case the function could look like this:
+
+static int foo_driver_enable_msi(struct pci_dev *pdev, int nvec)
+{
+	return pci_enable_msi_range(pdev, FOO_DRIVER_MINIMUM_NVEC, nvec);
+}
+
+4.2.1.2 Exact number of MSI interrupts
+
+If a driver is unable or unwilling to deal with a variable number of MSI
+interrupts it could request a particular number of interrupts by passing
+that number to pci_enable_msi_range() function as both 'minvec' and 'maxvec'
+parameters:
+
+static int foo_driver_enable_msi(struct pci_dev *pdev, int nvec)
+{
+	return pci_enable_msi_range(pdev, nvec, nvec);
+}
+
+4.2.1.3 Single MSI mode
+
+The most notorious example of the request type described above is
+enabling the single MSI mode for a device. It could be done by passing
+two 1s as 'minvec' and 'maxvec':
+
+static int foo_driver_enable_single_msi(struct pci_dev *pdev)
+{
+	return pci_enable_msi_range(pdev, 1, 1);
+}
+
+4.2.2 pci_disable_msi
 
 void pci_disable_msi(struct pci_dev *dev)
 
-This function should be used to undo the effect of pci_enable_msi() or
-pci_enable_msi_block().  Calling it restores dev->irq to the pin-based
-interrupt number and frees the previously allocated message signaled
-interrupt(s).  The interrupt may subsequently be assigned to another
-device, so drivers should not cache the value of dev->irq.
+This function should be used to undo the effect of pci_enable_msi_range().
+Calling it restores dev->irq to the pin-based interrupt number and frees
+the previously allocated MSIs. The interrupts may subsequently be assigned
+to another device, so drivers should not cache the value of dev->irq.
 
 Before calling this function, a device driver must always call free_irq()
 on any interrupt for which it previously called request_irq().
 Failure to do so results in a BUG_ON(), leaving the device with
 MSI enabled and thus leaking its vector.
 
-4.2.4 pci_msi_vec_count
+4.2.3 pci_msi_vec_count
 
 int pci_msi_vec_count(struct pci_dev *dev)
 
@@ -176,26 +206,31 @@ in each element of the array to indicate for which entries the kernel
 should assign interrupts; it is invalid to fill in two entries with the
 same number.
 
-4.3.1 pci_enable_msix
+4.3.1 pci_enable_msix_range
 
-int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec)
+int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
+			  int minvec, int maxvec)
 
-Calling this function asks the PCI subsystem to allocate 'nvec' MSIs.
+Calling this function asks the PCI subsystem to allocate any number of
+MSI-X interrupts within specified range from 'minvec' to 'maxvec'.
 The 'entries' argument is a pointer to an array of msix_entry structs
-which should be at least 'nvec' entries in size.  On success, the
-device is switched into MSI-X mode and the function returns 0.
-The 'vector' member in each entry is populated with the interrupt number;
+which should be at least 'maxvec' entries in size.
+
+On success, the device is switched into MSI-X mode and the function
+returns the number of MSI-X interrupts that have been successfully
+allocated. In this case the 'vector' member in entries numbered from
+0 to the returned value - 1 is populated with the interrupt number;
 the driver should then call request_irq() for each 'vector' that it
 decides to use.  The device driver is responsible for keeping track of the
 interrupts assigned to the MSI-X vectors so it can free them again later.
+Device driver can use the returned number of successfully allocated MSI-X
+interrupts to further allocate and initialize device resources.
 
 If this function returns a negative number, it indicates an error and
 the driver should not attempt to allocate any more MSI-X interrupts for
-this device.  If it returns a positive number, it indicates the maximum
-number of interrupt vectors that could have been allocated. See example
-below.
+this device.
 
-This function, in contrast with pci_enable_msi(), does not adjust
+This function, in contrast with pci_enable_msi_range(), does not adjust
 dev->irq.  The device will not generate interrupts for this interrupt
 number once MSI-X is enabled.
 
@@ -206,28 +241,103 @@ It is ideal if drivers can cope with a variable number of MSI-X interrupts;
 there are many reasons why the platform may not be able to provide the
 exact number that a driver asks for.
 
-A request loop to achieve that might look like:
+There could be devices that can not operate with just any number of MSI-X
+interrupts within a range. I.e. an network adapter might need let's say
+four vectors per each queue it provides. Therefore, a number of MSI-X
+interrupts allocated should be a multiple of four. In this case interface
+pci_enable_msix_range() can not be used alone to request MSI-X interrupts
+(since it can allocate any number within the range, without any notion of
+the multiple of four) and the device driver should master a custom logic
+to request the required number of MSI-X interrupts.
+
+4.3.1.1 Maximum possible number of MSI-X interrupts
+
+The typical usage of MSI-X interrupts is to allocate as many vectors as
+possible, likely up to the limit returned by pci_msix_vec_count() function:
+
+static int foo_driver_enable_msix(struct foo_adapter *adapter, int nvec)
+{
+	return pci_enable_msi_range(adapter->pdev, adapter->msix_entries,
+				    1, nvec);
+}
+
+Note the value of 'minvec' parameter is 1. As 'minvec' is inclusive,
+the value of 0 would be meaningless and could result in error.
+
+Some devices have a minimal limit on number of MSI-X interrupts.
+In this case the function could look like this:
 
 static int foo_driver_enable_msix(struct foo_adapter *adapter, int nvec)
 {
-	while (nvec >= FOO_DRIVER_MINIMUM_NVEC) {
-		rc = pci_enable_msix(adapter->pdev,
-				     adapter->msix_entries, nvec);
-		if (rc > 0)
-			nvec = rc;
-		else
-			return rc;
+	return pci_enable_msi_range(adapter->pdev, adapter->msix_entries,
+				    FOO_DRIVER_MINIMUM_NVEC, nvec);
+}
+
+4.3.1.2 Exact number of MSI-X interrupts
+
+If a driver is unable or unwilling to deal with a variable number of MSI-X
+interrupts it could request a particular number of interrupts by passing
+that number to pci_enable_msix_range() function as both 'minvec' and 'maxvec'
+parameters:
+
+static int foo_driver_enable_msix(struct foo_adapter *adapter, int nvec)
+{
+	return pci_enable_msi_range(adapter->pdev, adapter->msix_entries,
+				    nvec, nvec);
+}
+
+4.3.1.3 Specific requirements to the number of MSI-X interrupts
+
+As noted above, there could be devices that can not operate with just any
+number of MSI-X interrupts within a range. I.e. let's assume a device that
+is only capable sending the number of MSI-X interrupts which is a power of
+two. A routine that enables MSI-X mode for such device might look like this:
+
+/*
+ * Assume 'minvec' and 'maxvec' are non-zero
+ */
+static int foo_driver_enable_msix(struct foo_adapter *adapter,
+				  int minvec, int maxvec)
+{
+	int rc;
+
+	minvec = roundup_pow_of_two(minvec);
+	maxvec = rounddown_pow_of_two(maxvec);
+
+	if (minvec > maxvec)
+		return -ERANGE;
+
+retry:
+	rc = pci_enable_msix_range(adapter->pdev, adapter->msix_entries,
+				   maxvec, maxvec);
+	/*
+	 * -ENOSPC is the only error code allowed to be analized
+	 */
+	if (rc == -ENOSPC) {
+		if (maxvec == 1)
+			return -ENOSPC;
+
+		maxvec /= 2;
+
+		if (minvec > maxvec)
+			return -ENOSPC;
+
+		goto retry;
 	}
 
-	return -ENOSPC;
+	return rc;
 }
 
+Note how pci_enable_msix_range() return value is analized for a fallback -
+any error code other than -ENOSPC indicates a fatal error and should not
+be retried.
+
 4.3.2 pci_disable_msix
 
 void pci_disable_msix(struct pci_dev *dev)
 
-This function should be used to undo the effect of pci_enable_msix().  It frees
-the previously allocated message signaled interrupts.  The interrupts may
+This function should be used to undo the effect of pci_enable_msix_range().
+It frees the previously allocated MSI-X interrupts. The interrupts may
 subsequently be assigned to another device, so drivers should not cache
 the value of the 'vector' elements over a call to pci_disable_msix().
 
@@ -261,13 +371,14 @@ number of MSI-X interrupt vectors that could be allocated.
 If a device implements both MSI and MSI-X capabilities, it can
 run in either MSI mode or MSI-X mode, but not both simultaneously.
 This is a requirement of the PCI spec, and it is enforced by the
-PCI layer.  Calling pci_enable_msi() when MSI-X is already enabled or
-pci_enable_msix() when MSI is already enabled results in an error.
-If a device driver wishes to switch between MSI and MSI-X at runtime,
-it must first quiesce the device, then switch it back to pin-interrupt
-mode, before calling pci_enable_msi() or pci_enable_msix() and resuming
-operation.  This is not expected to be a common operation but may be
-useful for debugging or testing during development.
+PCI layer. Calling pci_enable_msi_range() when MSI-X is already
+enabled or pci_enable_msix_range() when MSI is already enabled
+results in an error. If a device driver wishes to switch between MSI
+and MSI-X at runtime, it must first quiesce the device, then switch
+it back to pin-interrupt mode, before calling pci_enable_msi_range()
+or pci_enable_msix_range() and resuming operation. This is not expected
+to be a common operation but may be useful for debugging or testing
+during development.
 
 4.5 Considerations when using MSIs
 
@@ -382,5 +493,5 @@ or disabled (0).  If 0 is found in any of the msi_bus files belonging
 to bridges between the PCI root and the device, MSIs are disabled.
 
 It is also worth checking the device driver to see whether it supports MSIs.
-For example, it may contain calls to pci_enable_msi(), pci_enable_msix() or
-pci_enable_msi_block().
+For example, it may contain calls to pci_enable_msi_range() or
+pci_enable_msix_range().
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 32dca35..6329a9b 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1111,3 +1111,77 @@ void pci_msi_init_pci_dev(struct pci_dev *dev)
 	if (dev->msix_cap)
 		msix_set_enable(dev, 0);
 }
+
+/**
+ * pci_enable_msi_range - configure device's MSI capability structure
+ * @dev: device to configure
+ * @minvec: minimal number of interrupts to configure
+ * @maxvec: maximum number of interrupts to configure
+ *
+ * This function tries to allocate a maximum possible number of interrupts in a
+ * range between @minvec and @maxvec. It returns a negative errno if an error
+ * occurs. If it succeeds, it returns the actual number of interrupts allocated
+ * and updates the @dev's irq member to the lowest new interrupt number;
+ * the other interrupt numbers allocated to this device are consecutive.
+ **/
+int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec)
+{
+	int nvec = maxvec;
+	int rc;
+
+	if (maxvec < minvec)
+		return -ERANGE;
+
+	do {
+		rc = pci_enable_msi_block(dev, nvec);
+		if (rc < 0) {
+			return rc;
+		} else if (rc > 0) {
+			if (rc < minvec)
+				return -ENOSPC;
+			nvec = rc;
+		}
+	} while (rc);
+
+	return nvec;
+}
+EXPORT_SYMBOL(pci_enable_msi_range);
+
+/**
+ * pci_enable_msix_range - configure device's MSI-X capability structure
+ * @dev: pointer to the pci_dev data structure of MSI-X device function
+ * @entries: pointer to an array of MSI-X entries
+ * @minvec: minimum number of MSI-X irqs requested
+ * @maxvec: maximum number of MSI-X irqs requested
+ *
+ * Setup the MSI-X capability structure of device function with a maximum
+ * possible number of interrupts in the range between @minvec and @maxvec
+ * upon its software driver call to request for MSI-X mode enabled on its
+ * hardware device function. It returns a negative errno if an error occurs.
+ * If it succeeds, it returns the actual number of interrupts allocated and
+ * indicates the successful configuration of MSI-X capability structure
+ * with new allocated MSI-X interrupts.
+ **/
+int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
+			       int minvec, int maxvec)
+{
+	int nvec = maxvec;
+	int rc;
+
+	if (maxvec < minvec)
+		return -ERANGE;
+
+	do {
+		rc = pci_enable_msix(dev, entries, nvec);
+		if (rc < 0) {
+			return rc;
+		} else if (rc > 0) {
+			if (rc < minvec)
+				return -ENOSPC;
+			nvec = rc;
+		}
+	} while (rc);
+
+	return nvec;
+}
+EXPORT_SYMBOL(pci_enable_msix_range);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d031639..be45bae 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1205,6 +1205,17 @@ static inline int pci_msi_enabled(void)
 {
 	return 0;
 }
+
+int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec)
+{
+	return -ENOSYS;
+}
+static inline int
+pci_enable_msix_range(struct pci_dev *dev,
+		      struct msix_entry *entries, int minvec, int maxvec)
+{
+	return -ENOSYS;
+}
 #else
 int pci_msi_vec_count(struct pci_dev *dev);
 int pci_enable_msi_block(struct pci_dev *dev, int nvec);
@@ -1217,6 +1228,9 @@ void pci_disable_msix(struct pci_dev *dev);
 void msi_remove_pci_irq_vectors(struct pci_dev *dev);
 void pci_restore_msi_state(struct pci_dev *dev);
 int pci_msi_enabled(void);
+int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec);
+int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
+			  int minvec, int maxvec);
 #endif
 
 #ifdef CONFIG_PCIEPORTBUS
-- 
1.7.7.6

Re: [PATCH v5 0/4] Introduce pci_enable_msi/msix_range() interfaces

[Bjorn Helgaas]

On Mon, Dec 30, 2013 at 08:28:12AM +0100, Alexander Gordeev wrote:
> This series is against "next" branch in Bjorn's repo:
> git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git
> 
> Changes from v4 to v5:
>   - pci_auto_enable_msi* functions renamed to pci_enable_msi* ones;
>   - pci_enable_msi_range() and pci_enable_msix_range() are the only
>     basic interfaces left to enable MSI/MSI-X operation;
>   - pci_enable_msi(), pci_enable_msi_block() and pci_enable_msix() old
>     interfaces deprecated in favour of pci_enable_msi/msix_range() ones;
>     description of the old interfaces is removed from the documentation;
>   - pci_get_msi_vec_count()  renamed to pci_msi_vec_count() and
>     pci_get_msix_vec_count() renamed to pci_msix_vec_count();
> 
> Changes from v3 to v4:
>   - pcim_enable_msi* functions renamed to pci_auto_enable_msi* ones;
>   - PowerPC patches dropped;
>   - pci_get_msi_cap()     renamed to pci_get_msi_vec_count() and
>     pci_msix_table_size() renamed to pci_get_msix_vec_count();
> 
> Changes from v2 to v3:
>   - new public interfaces commented in drivers/pci/msi.c;
>   - patch "Make quota traversing and requesting race-safe" explained;
>   - pci_enable_msi/msix() 'nvec' arg type changed from 'unsigned int' to 'int';
>   - pcim_enable_msi*() arg 'nvec' renamed to 'maxvec' when upper limit passed;
>   - pcim_enable_msi*(..., maxvec, minvec) arg order swapped to minvec, maxvec;
>   - "PCI: Fail MSI/MSI-X initialization if device is not in PCI_D0" commit
>     869a161 and "PCI/MSI: Factor out pci_get_msi_cap() interface" patch
>     conflicts resolved;
> 
> This update introduces pci_enable_msi_range() function which
> supersedes pci_enable_msi() and pci_enable_msi_block()
> interfaces for MSI and pci_enable_msi_range() function which
> supersedes pci_enable_msix() interface for MSI-X interrupts.
> 
> The plan is to convert all drivers to the new interfaces and
> then remove existing pci_enable_msi(), pci_enable_msi_block()
> and pci_enable_msix() functions.
> 
> The newly introduced functions return either a negative error
> code or a number of successfully allocated MSI/MSI-X interrupts.
> 
> Unlike the old interfaces where the "third" state indicates
> a possible number of MSI interrupts that could have been
> allocated, the new interfaces allow device drivers not to deal
> with tri-state return values and hide fallback logic behind
> the implementation.
> 
> As result, device drivers are expected to have more clearer
> and straight code.
> 
> The tree could be found in "pci-next-msi-v5" branch in repo:
> https://github.com/a-gordeev/linux.git
> 
> Alexander Gordeev (4):
>   PCI/MSI: Factor out pci_msi_vec_count() interface
>   PCI/MSI: Get rid of pci_enable_msi_block_auto() interface
>   PCI/MSI: Introduce pci_msix_vec_count() interface
>   PCI/MSI: Introduce pci_enable_msi/msix_range() interfaces

I applied all four of these to my pci/msi branch for v3.14.  Thanks
a lot for all your work on this!  I'm looking forward to the driver
conversions and bug fixes :)

Bjorn

>  Documentation/PCI/MSI-HOWTO.txt |  308 ++++++++++++++++++++++++++-------------
>  drivers/ata/ahci.c              |   56 +++++---
>  drivers/pci/msi.c               |  150 +++++++++++++++-----
>  drivers/pci/pcie/portdrv_core.c |    7 +-
>  include/linux/pci.h             |   27 +++-
>  5 files changed, 385 insertions(+), 163 deletions(-)
> 
> -- 
> 1.7.7.6
>