Patchwork [v3,10/11] PCI/MSI: Convert pci_msix_table_size() to a public interface

login
register
mail settings
Submitter Alexander Gordeev
Date Nov. 26, 2013, 9:09 a.m.
Message ID <4c879b0928ded8b4c9d2fab3fc413b98d34d8c7c.1385399393.git.agordeev@redhat.com>
Download mbox | patch
Permalink /patch/294411/
State Changes Requested
Headers show

Comments

Alexander Gordeev - Nov. 26, 2013, 9:09 a.m.
Make pci_msix_table_size() function to return a negative errno
if device does not support MSI-X interrupts. After this update
pci_msix_table_size() can fail and callers must always check
the returned value.

This update is needed to create a consistent MSI-X counterpart
for pci_get_msi_cap() 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.

The only user of pci_msix_table_size() function is 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               |   12 ++++++++++--
 drivers/pci/pcie/portdrv_core.c |    5 +++--
 include/linux/pci.h             |    2 +-
 4 files changed, 27 insertions(+), 5 deletions(-)
Bjorn Helgaas - Dec. 10, 2013, 11:08 p.m.
On Tue, Nov 26, 2013 at 10:09:59AM +0100, Alexander Gordeev wrote:
> Make pci_msix_table_size() function to return a negative errno
> if device does not support MSI-X interrupts. After this update
> pci_msix_table_size() can fail and callers must always check
> the returned value.
> 
> This update is needed to create a consistent MSI-X counterpart
> for pci_get_msi_cap() 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.

If pci_msix_table_size() is a counterpart to pci_get_msi_cap(), can we make
the names similar?

> The only user of pci_msix_table_size() function is 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               |   12 ++++++++++--
>  drivers/pci/pcie/portdrv_core.c |    5 +++--
>  include/linux/pci.h             |    2 +-
>  4 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
> index 1fe4900..5955389 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_table_size
> +
> +int pci_msix_table_size(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 e4b02ac..6fe0add 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -932,17 +932,23 @@ EXPORT_SYMBOL(pci_disable_msi);
>  /**
>   * pci_msix_table_size - return the number of device's MSI-X table entries
>   * @dev: pointer to the pci_dev data structure of MSI-X device function
> - */
> +
> + * 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_table_size(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_table_size);
>  
>  /**
>   * pci_enable_msix - configure device's MSI-X capability structure
> @@ -972,6 +978,8 @@ int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec)
>  		return status;
>  
>  	nr_entries = pci_msix_table_size(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 08d131f..5bebeec 100644
> --- a/drivers/pci/pcie/portdrv_core.c
> +++ b/drivers/pci/pcie/portdrv_core.c
> @@ -80,8 +80,9 @@ static int pcie_port_enable_msix(struct pci_dev *dev, int *vectors, int mask)
>  	u32 reg32;
>  
>  	nr_entries = pci_msix_table_size(dev);
> -	if (!nr_entries)
> -		return -EINVAL;
> +	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 9ab1692..8af1217 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1172,7 +1172,7 @@ static inline void pci_disable_msi(struct pci_dev *dev)
>  
>  static inline int pci_msix_table_size(struct pci_dev *dev)
>  {
> -	return 0;
> +	return -ENOSYS;
>  }
>  static inline int pci_enable_msix(struct pci_dev *dev,
>  				  struct msix_entry *entries, int nvec)
> -- 
> 1.7.7.6
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Gordeev - Dec. 12, 2013, 4:06 p.m.
On Tue, Dec 10, 2013 at 04:08:47PM -0700, Bjorn Helgaas wrote:
> If pci_msix_table_size() is a counterpart to pci_get_msi_cap(), can we make
> the names similar?

Tejun also pointed this out. What about pci_get_msi_vec_count() and
pci_get_msix_vec_count()?

Nevertheless, the existing names are more informative for me, since they
hint where the MSI/MSI-X vector count comes from. Up to you.
Bjorn Helgaas - Dec. 12, 2013, 9:16 p.m.
On Thu, Dec 12, 2013 at 9:06 AM, Alexander Gordeev <agordeev@redhat.com> wrote:
> On Tue, Dec 10, 2013 at 04:08:47PM -0700, Bjorn Helgaas wrote:
>> If pci_msix_table_size() is a counterpart to pci_get_msi_cap(), can we make
>> the names similar?
>
> Tejun also pointed this out. What about pci_get_msi_vec_count() and
> pci_get_msix_vec_count()?

Sounds perfect to me.  I think the fact that they return the same type
of information is more important than where the information came from.

> Nevertheless, the existing names are more informative for me, since they
> hint where the MSI/MSI-X vector count comes from. Up to you.
>
> --
> Regards,
> Alexander Gordeev
> agordeev@redhat.com
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
index 1fe4900..5955389 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_table_size
+
+int pci_msix_table_size(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 e4b02ac..6fe0add 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -932,17 +932,23 @@  EXPORT_SYMBOL(pci_disable_msi);
 /**
  * pci_msix_table_size - return the number of device's MSI-X table entries
  * @dev: pointer to the pci_dev data structure of MSI-X device function
- */
+
+ * 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_table_size(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_table_size);
 
 /**
  * pci_enable_msix - configure device's MSI-X capability structure
@@ -972,6 +978,8 @@  int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec)
 		return status;
 
 	nr_entries = pci_msix_table_size(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 08d131f..5bebeec 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -80,8 +80,9 @@  static int pcie_port_enable_msix(struct pci_dev *dev, int *vectors, int mask)
 	u32 reg32;
 
 	nr_entries = pci_msix_table_size(dev);
-	if (!nr_entries)
-		return -EINVAL;
+	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 9ab1692..8af1217 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1172,7 +1172,7 @@  static inline void pci_disable_msi(struct pci_dev *dev)
 
 static inline int pci_msix_table_size(struct pci_dev *dev)
 {
-	return 0;
+	return -ENOSYS;
 }
 static inline int pci_enable_msix(struct pci_dev *dev,
 				  struct msix_entry *entries, int nvec)