[PATCH v2 0/4] ntb: Use pci_enable_msix_range() instead of pci_enable_msix()

[PATCH v2 0/4] ntb: Use pci_enable_msix_range() instead of pci_enable_msix()

[Alexander Gordeev]

Hi Jon,

This is an attempt to make ntb_setup_msix() more readable ( to me :) )

The idea is to get rid of confusing branching between SNB and BWD
and split MSI-X ininialization into ntb_setup_bwd_msix() and
ntb_setup_snb_msix() - both small and straight.

Cc: Jon Mason <jon.mason@intel.com>
Cc: linux-pci@vger.kernel.org

Alexander Gordeev (4):
  ntb: Fix leakage of ntb_device::msix_entries[] array
  ntb: Use pci_msix_vec_count() to obtain number of MSI-Xs
  ntb: Split ntb_setup_msix() into separate BWD/SNB routines
  ntb: Use pci_enable_msix_range() instead of pci_enable_msix()

 drivers/ntb/ntb_hw.c |  174 +++++++++++++++++++++++++++++---------------------
 drivers/ntb/ntb_hw.h |    2 -
 2 files changed, 100 insertions(+), 76 deletions(-)

-- 
1.7.7.6

[PATCH v2 1/4] ntb: Fix leakage of ntb_device::msix_entries[] array

[Alexander Gordeev]

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Cc: Jon Mason <jon.mason@intel.com>
Cc: linux-pci@vger.kernel.org
---
 drivers/ntb/ntb_hw.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 170e8e6..487169dd 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1281,6 +1281,7 @@ static void ntb_free_interrupts(struct ntb_device *ndev)
 				free_irq(msix->vector, &ndev->db_cb[i]);
 		}
 		pci_disable_msix(pdev);
+		kfree(ndev->msix_entries);
 	} else {
 		free_irq(pdev->irq, ndev);
 
-- 
1.7.7.6

[PATCH v2 2/4] ntb: Use pci_msix_vec_count() to obtain number of MSI-Xs

[Alexander Gordeev]

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Cc: Jon Mason <jon.mason@intel.com>
Cc: linux-pci@vger.kernel.org
---
 drivers/ntb/ntb_hw.c |   15 ++++-----------
 drivers/ntb/ntb_hw.h |    2 --
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 487169dd..53468f4 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1085,19 +1085,12 @@ static int ntb_setup_msix(struct ntb_device *ndev)
 	struct msix_entry *msix;
 	int msix_entries;
 	int rc, i;
-	u16 val;
 
-	if (!pdev->msix_cap) {
-		rc = -EIO;
-		goto err;
-	}
-
-	rc = pci_read_config_word(pdev, pdev->msix_cap + PCI_MSIX_FLAGS, &val);
-	if (rc)
+	msix_entries = pci_msix_vec_count(pdev);
+	if (msix_entries < 0) {
+		rc = msix_entries;
 		goto err;
-
-	msix_entries = msix_table_size(val);
-	if (msix_entries > ndev->limits.msix_cnt) {
+	} else if (msix_entries > ndev->limits.msix_cnt) {
 		rc = -EINVAL;
 		goto err;
 	}
diff --git a/drivers/ntb/ntb_hw.h b/drivers/ntb/ntb_hw.h
index bbdb7ed..d307107 100644
--- a/drivers/ntb/ntb_hw.h
+++ b/drivers/ntb/ntb_hw.h
@@ -60,8 +60,6 @@
 #define PCI_DEVICE_ID_INTEL_NTB_SS_HSX		0x2F0F
 #define PCI_DEVICE_ID_INTEL_NTB_B2B_BWD		0x0C4E
 
-#define msix_table_size(control)	((control & PCI_MSIX_FLAGS_QSIZE)+1)
-
 #ifndef readq
 static inline u64 readq(void __iomem *addr)
 {
-- 
1.7.7.6

[PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines

[Alexander Gordeev]

This is an cleanup effort to make ntb_setup_msix() more
readable - use ntb_setup_bwd_msix() to init MSI-Xs on
BWD hardware and ntb_setup_snb_msix() - on SNB hardware.

Function ntb_setup_snb_msix() also initializes MSI-Xs the
way it should has been done - looping pci_enable_msix()
until success or failure.

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Cc: Jon Mason <jon.mason@intel.com>
Cc: linux-pci@vger.kernel.org
---
 drivers/ntb/ntb_hw.c |  170 +++++++++++++++++++++++++++++++-------------------
 1 files changed, 106 insertions(+), 64 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 53468f4..0472045 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1079,6 +1079,107 @@ static irqreturn_t ntb_interrupt(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
+{
+	struct pci_dev *pdev = ndev->pdev;
+	struct msix_entry *msix;
+	int rc, i;
+
+	if (msix_entries < SNB_MSIX_CNT)
+		return -ENOSPC;
+
+	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
+	if (rc < 0)
+		return rc;
+	else if (rc > 0)
+		return -ENOSPC;
+
+	for (i = 0; i < msix_entries; i++) {
+		msix = &ndev->msix_entries[i];
+		WARN_ON(!msix->vector);
+
+		if (i == msix_entries - 1) {
+			rc = request_irq(msix->vector,
+					 xeon_event_msix_irq, 0,
+					 "ntb-event-msix", ndev);
+			if (rc)
+				goto err;
+		} else {
+			rc = request_irq(msix->vector,
+					 xeon_callback_msix_irq, 0,
+					 "ntb-callback-msix",
+					 &ndev->db_cb[i]);
+			if (rc)
+				goto err;
+		}
+	}
+
+	ndev->num_msix = msix_entries;
+	ndev->max_cbs = msix_entries - 1;
+
+	return 0;
+
+err:
+	while (--i >= 0) {
+		msix = &ndev->msix_entries[i];
+		WARN_ON(i == ndev->num_msix - 1);
+
+		if (i == ndev->num_msix - 1)
+			free_irq(msix->vector, ndev);
+		else
+			free_irq(msix->vector, &ndev->db_cb[i]);
+	}
+
+	pci_disable_msix(pdev);
+	ndev->num_msix = 0;
+
+	return rc;
+}
+
+static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
+{
+	struct pci_dev *pdev = ndev->pdev;
+	struct msix_entry *msix;
+	int rc, i;
+
+retry:
+	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
+	if (rc < 0)
+		return rc;
+	else if (rc > 0) {
+		dev_warn(&pdev->dev,
+			 "Only %d MSI-X vectors. "
+			 "Limiting the number of queues to that number.\n",
+			 rc);
+		msix_entries = rc;
+		goto retry;
+	}
+
+	for (i = 0; i < msix_entries; i++) {
+		msix = &ndev->msix_entries[i];
+		WARN_ON(!msix->vector);
+
+		rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
+				 "ntb-callback-msix", &ndev->db_cb[i]);
+		if (rc)
+			goto err;
+	}
+
+	ndev->num_msix = msix_entries;
+	ndev->max_cbs = msix_entries;
+
+	return 0;
+
+err:
+	while (--i >= 0)
+		free_irq(msix->vector, &ndev->db_cb[i]);
+
+	pci_disable_msix(pdev);
+	ndev->num_msix = 0;
+
+	return rc;
+}
+
 static int ntb_setup_msix(struct ntb_device *ndev)
 {
 	struct pci_dev *pdev = ndev->pdev;
@@ -1105,78 +1206,19 @@ static int ntb_setup_msix(struct ntb_device *ndev)
 	for (i = 0; i < msix_entries; i++)
 		ndev->msix_entries[i].entry = i;
 
-	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
-	if (rc < 0)
-		goto err1;
-	if (rc > 0) {
-		/* On SNB, the link interrupt is always tied to 4th vector.  If
-		 * we can't get all 4, then we can't use MSI-X.
-		 */
-		if (ndev->hw_type != BWD_HW) {
-			rc = -EIO;
-			goto err1;
-		}
-
-		dev_warn(&pdev->dev,
-			 "Only %d MSI-X vectors.  Limiting the number of queues to that number.\n",
-			 rc);
-		msix_entries = rc;
-
-		rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
-		if (rc)
-			goto err1;
-	}
-
-	for (i = 0; i < msix_entries; i++) {
-		msix = &ndev->msix_entries[i];
-		WARN_ON(!msix->vector);
-
-		/* Use the last MSI-X vector for Link status */
-		if (ndev->hw_type == BWD_HW) {
-			rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
-					 "ntb-callback-msix", &ndev->db_cb[i]);
-			if (rc)
-				goto err2;
-		} else {
-			if (i == msix_entries - 1) {
-				rc = request_irq(msix->vector,
-						 xeon_event_msix_irq, 0,
-						 "ntb-event-msix", ndev);
-				if (rc)
-					goto err2;
-			} else {
-				rc = request_irq(msix->vector,
-						 xeon_callback_msix_irq, 0,
-						 "ntb-callback-msix",
-						 &ndev->db_cb[i]);
-				if (rc)
-					goto err2;
-			}
-		}
-	}
-
-	ndev->num_msix = msix_entries;
 	if (ndev->hw_type == BWD_HW)
-		ndev->max_cbs = msix_entries;
+		rc = ntb_setup_bwd_msix(ndev, msix_entries);
 	else
-		ndev->max_cbs = msix_entries - 1;
+		rc = ntb_setup_snb_msix(ndev, msix_entries);
+	if (rc)
+		goto err1;
 
 	return 0;
 
-err2:
-	while (--i >= 0) {
-		msix = &ndev->msix_entries[i];
-		if (ndev->hw_type != BWD_HW && i == ndev->num_msix - 1)
-			free_irq(msix->vector, ndev);
-		else
-			free_irq(msix->vector, &ndev->db_cb[i]);
-	}
-	pci_disable_msix(pdev);
 err1:
 	kfree(ndev->msix_entries);
-	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
 err:
-	ndev->num_msix = 0;
+	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
 	return rc;
 }
 
-- 
1.7.7.6

[PATCH v2 4/4] ntb: Use pci_enable_msix_range() instead of pci_enable_msix()

[Alexander Gordeev]

As result of deprecation of MSI-X/MSI enablement functions
pci_enable_msix() and pci_enable_msi_block() all drivers
using these two interfaces need to be updated to use the
new pci_enable_msi_range()  or pci_enable_msi_exact()
and pci_enable_msix_range() or pci_enable_msix_exact()
interfaces.

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Cc: Jon Mason <jon.mason@intel.com>
Cc: linux-pci@vger.kernel.org
---
 drivers/ntb/ntb_hw.c |   20 +++++---------------
 1 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 0472045..9f2005d 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1088,11 +1088,9 @@ static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
 	if (msix_entries < SNB_MSIX_CNT)
 		return -ENOSPC;
 
-	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
+	rc = pci_enable_msix_exact(pdev, ndev->msix_entries, msix_entries);
 	if (rc < 0)
 		return rc;
-	else if (rc > 0)
-		return -ENOSPC;
 
 	for (i = 0; i < msix_entries; i++) {
 		msix = &ndev->msix_entries[i];
@@ -1142,18 +1140,10 @@ static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
 	struct msix_entry *msix;
 	int rc, i;
 
-retry:
-	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
-	if (rc < 0)
-		return rc;
-	else if (rc > 0) {
-		dev_warn(&pdev->dev,
-			 "Only %d MSI-X vectors. "
-			 "Limiting the number of queues to that number.\n",
-			 rc);
-		msix_entries = rc;
-		goto retry;
-	}
+	msix_entries = pci_enable_msix_range(pdev, ndev->msix_entries,
+					     1, msix_entries);
+	if (msix_entries < 0)
+		return msix_entries;
 
 	for (i = 0; i < msix_entries; i++) {
 		msix = &ndev->msix_entries[i];
-- 
1.7.7.6

Re: [PATCH v2 0/4] ntb: Use pci_enable_msix_range() instead of pci_enable_msix()

[Alexander Gordeev]

On Fri, Feb 21, 2014 at 04:49:28PM +0100, Alexander Gordeev wrote:
> Hi Jon,
> 
> This is an attempt to make ntb_setup_msix() more readable ( to me :) )
> 
> The idea is to get rid of confusing branching between SNB and BWD
> and split MSI-X ininialization into ntb_setup_bwd_msix() and
> ntb_setup_snb_msix() - both small and straight.

Ouch, I was too fast. Patch #3 introduces warning "unused variable ‘msix’"
I can re-post if you want, but the rest is still is up to review ;)

Thanks!

-- 
Regards,
Alexander Gordeev
agordeev@redhat.com

Re: [PATCH v2 1/4] ntb: Fix leakage of ntb_device::msix_entries[] array

[Jon Mason]

On Fri, Feb 21, 2014 at 04:49:29PM +0100, Alexander Gordeev wrote:
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> Cc: Jon Mason <jon.mason@intel.com>
> Cc: linux-pci@vger.kernel.org

Good catch.  Applied.

> ---
>  drivers/ntb/ntb_hw.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index 170e8e6..487169dd 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -1281,6 +1281,7 @@ static void ntb_free_interrupts(struct ntb_device *ndev)
>  				free_irq(msix->vector, &ndev->db_cb[i]);
>  		}
>  		pci_disable_msix(pdev);
> +		kfree(ndev->msix_entries);
>  	} else {
>  		free_irq(pdev->irq, ndev);
>  
> -- 
> 1.7.7.6
> 

[PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines

[Alexander Gordeev]

This is an cleanup effort to make ntb_setup_msix() more
readable - use ntb_setup_bwd_msix() to init MSI-Xs on
BWD hardware and ntb_setup_snb_msix() - on SNB hardware.

Function ntb_setup_snb_msix() also initializes MSI-Xs the
way it should has been done - looping pci_enable_msix()
until success or failure.

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Cc: Jon Mason <jon.mason@intel.com>
Cc: linux-pci@vger.kernel.org
---
 drivers/ntb/ntb_hw.c |  171 +++++++++++++++++++++++++++++++-------------------
 1 files changed, 106 insertions(+), 65 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 53468f4..679d4d0 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1079,10 +1079,110 @@ static irqreturn_t ntb_interrupt(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
-static int ntb_setup_msix(struct ntb_device *ndev)
+static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
+{
+	struct pci_dev *pdev = ndev->pdev;
+	struct msix_entry *msix;
+	int rc, i;
+
+	if (msix_entries < SNB_MSIX_CNT)
+		return -ENOSPC;
+
+	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
+	if (rc < 0)
+		return rc;
+	else if (rc > 0)
+		return -ENOSPC;
+
+	for (i = 0; i < msix_entries; i++) {
+		msix = &ndev->msix_entries[i];
+		WARN_ON(!msix->vector);
+
+		if (i == msix_entries - 1) {
+			rc = request_irq(msix->vector,
+					 xeon_event_msix_irq, 0,
+					 "ntb-event-msix", ndev);
+			if (rc)
+				goto err;
+		} else {
+			rc = request_irq(msix->vector,
+					 xeon_callback_msix_irq, 0,
+					 "ntb-callback-msix",
+					 &ndev->db_cb[i]);
+			if (rc)
+				goto err;
+		}
+	}
+
+	ndev->num_msix = msix_entries;
+	ndev->max_cbs = msix_entries - 1;
+
+	return 0;
+
+err:
+	while (--i >= 0) {
+		msix = &ndev->msix_entries[i];
+		WARN_ON(i == ndev->num_msix - 1);
+
+		if (i == ndev->num_msix - 1)
+			free_irq(msix->vector, ndev);
+		else
+			free_irq(msix->vector, &ndev->db_cb[i]);
+	}
+
+	pci_disable_msix(pdev);
+	ndev->num_msix = 0;
+
+	return rc;
+}
+
+static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
 {
 	struct pci_dev *pdev = ndev->pdev;
 	struct msix_entry *msix;
+	int rc, i;
+
+retry:
+	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
+	if (rc < 0)
+		return rc;
+	else if (rc > 0) {
+		dev_warn(&pdev->dev,
+			 "Only %d MSI-X vectors. "
+			 "Limiting the number of queues to that number.\n",
+			 rc);
+		msix_entries = rc;
+		goto retry;
+	}
+
+	for (i = 0; i < msix_entries; i++) {
+		msix = &ndev->msix_entries[i];
+		WARN_ON(!msix->vector);
+
+		rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
+				 "ntb-callback-msix", &ndev->db_cb[i]);
+		if (rc)
+			goto err;
+	}
+
+	ndev->num_msix = msix_entries;
+	ndev->max_cbs = msix_entries;
+
+	return 0;
+
+err:
+	while (--i >= 0)
+		free_irq(msix->vector, &ndev->db_cb[i]);
+
+	pci_disable_msix(pdev);
+	ndev->num_msix = 0;
+
+	return rc;
+}
+
+static int ntb_setup_msix(struct ntb_device *ndev)
+{
+	struct pci_dev *pdev = ndev->pdev;
 	int msix_entries;
 	int rc, i;
 
@@ -1105,78 +1205,19 @@ static int ntb_setup_msix(struct ntb_device *ndev)
 	for (i = 0; i < msix_entries; i++)
 		ndev->msix_entries[i].entry = i;
 
-	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
-	if (rc < 0)
-		goto err1;
-	if (rc > 0) {
-		/* On SNB, the link interrupt is always tied to 4th vector.  If
-		 * we can't get all 4, then we can't use MSI-X.
-		 */
-		if (ndev->hw_type != BWD_HW) {
-			rc = -EIO;
-			goto err1;
-		}
-
-		dev_warn(&pdev->dev,
-			 "Only %d MSI-X vectors.  Limiting the number of queues to that number.\n",
-			 rc);
-		msix_entries = rc;
-
-		rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
-		if (rc)
-			goto err1;
-	}
-
-	for (i = 0; i < msix_entries; i++) {
-		msix = &ndev->msix_entries[i];
-		WARN_ON(!msix->vector);
-
-		/* Use the last MSI-X vector for Link status */
-		if (ndev->hw_type == BWD_HW) {
-			rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
-					 "ntb-callback-msix", &ndev->db_cb[i]);
-			if (rc)
-				goto err2;
-		} else {
-			if (i == msix_entries - 1) {
-				rc = request_irq(msix->vector,
-						 xeon_event_msix_irq, 0,
-						 "ntb-event-msix", ndev);
-				if (rc)
-					goto err2;
-			} else {
-				rc = request_irq(msix->vector,
-						 xeon_callback_msix_irq, 0,
-						 "ntb-callback-msix",
-						 &ndev->db_cb[i]);
-				if (rc)
-					goto err2;
-			}
-		}
-	}
-
-	ndev->num_msix = msix_entries;
 	if (ndev->hw_type == BWD_HW)
-		ndev->max_cbs = msix_entries;
+		rc = ntb_setup_bwd_msix(ndev, msix_entries);
 	else
-		ndev->max_cbs = msix_entries - 1;
+		rc = ntb_setup_snb_msix(ndev, msix_entries);
+	if (rc)
+		goto err1;
 
 	return 0;
 
-err2:
-	while (--i >= 0) {
-		msix = &ndev->msix_entries[i];
-		if (ndev->hw_type != BWD_HW && i == ndev->num_msix - 1)
-			free_irq(msix->vector, ndev);
-		else
-			free_irq(msix->vector, &ndev->db_cb[i]);
-	}
-	pci_disable_msix(pdev);
 err1:
 	kfree(ndev->msix_entries);
-	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
 err:
-	ndev->num_msix = 0;
+	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
 	return rc;
 }
 
-- 
1.7.7.6

-- 
Regards,
Alexander Gordeev
agordeev@redhat.com

Re: [PATCH v2 0/4] ntb: Use pci_enable_msix_range() instead of pci_enable_msix()

[Alexander Gordeev]

On Fri, Feb 21, 2014 at 05:09:37PM +0100, Alexander Gordeev wrote:
> Ouch, I was too fast. Patch #3 introduces warning "unused variable ‘msix’"
> I can re-post if you want, but the rest is still is up to review ;)

Resent.

-- 
Regards,
Alexander Gordeev
agordeev@redhat.com

Re: [PATCH v2 1/4] ntb: Fix leakage of ntb_device::msix_entries[] array

[Alexander Gordeev]

On Fri, Feb 21, 2014 at 04:33:25PM -0700, Jon Mason wrote:
> On Fri, Feb 21, 2014 at 04:49:29PM +0100, Alexander Gordeev wrote:
> > Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> > Cc: Jon Mason <jon.mason@intel.com>
> > Cc: linux-pci@vger.kernel.org
> 
> Good catch.  Applied.

Hi Jon,

If the other three patches are okay?

-- 
Regards,
Alexander Gordeev
agordeev@redhat.com

Re: [PATCH v2 2/4] ntb: Use pci_msix_vec_count() to obtain number of MSI-Xs

[Jon Mason]

On Fri, Feb 21, 2014 at 04:49:30PM +0100, Alexander Gordeev wrote:
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> Cc: Jon Mason <jon.mason@intel.com>

Looks good.  Pulling into my tree.

Thanks,
Jon

> Cc: linux-pci@vger.kernel.org
> ---
>  drivers/ntb/ntb_hw.c |   15 ++++-----------
>  drivers/ntb/ntb_hw.h |    2 --
>  2 files changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index 487169dd..53468f4 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -1085,19 +1085,12 @@ static int ntb_setup_msix(struct ntb_device *ndev)
>  	struct msix_entry *msix;
>  	int msix_entries;
>  	int rc, i;
> -	u16 val;
>  
> -	if (!pdev->msix_cap) {
> -		rc = -EIO;
> -		goto err;
> -	}
> -
> -	rc = pci_read_config_word(pdev, pdev->msix_cap + PCI_MSIX_FLAGS, &val);
> -	if (rc)
> +	msix_entries = pci_msix_vec_count(pdev);
> +	if (msix_entries < 0) {
> +		rc = msix_entries;
>  		goto err;
> -
> -	msix_entries = msix_table_size(val);
> -	if (msix_entries > ndev->limits.msix_cnt) {
> +	} else if (msix_entries > ndev->limits.msix_cnt) {
>  		rc = -EINVAL;
>  		goto err;
>  	}
> diff --git a/drivers/ntb/ntb_hw.h b/drivers/ntb/ntb_hw.h
> index bbdb7ed..d307107 100644
> --- a/drivers/ntb/ntb_hw.h
> +++ b/drivers/ntb/ntb_hw.h
> @@ -60,8 +60,6 @@
>  #define PCI_DEVICE_ID_INTEL_NTB_SS_HSX		0x2F0F
>  #define PCI_DEVICE_ID_INTEL_NTB_B2B_BWD		0x0C4E
>  
> -#define msix_table_size(control)	((control & PCI_MSIX_FLAGS_QSIZE)+1)
> -
>  #ifndef readq
>  static inline u64 readq(void __iomem *addr)
>  {
> -- 
> 1.7.7.6
> 

Re: [PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines

[Jon Mason]

On Fri, Feb 21, 2014 at 04:49:31PM +0100, Alexander Gordeev wrote:
> This is an cleanup effort to make ntb_setup_msix() more
> readable - use ntb_setup_bwd_msix() to init MSI-Xs on
> BWD hardware and ntb_setup_snb_msix() - on SNB hardware.
> 
> Function ntb_setup_snb_msix() also initializes MSI-Xs the
> way it should has been done - looping pci_enable_msix()
> until success or failure.
> 
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> Cc: Jon Mason <jon.mason@intel.com>
> Cc: linux-pci@vger.kernel.org
> ---
>  drivers/ntb/ntb_hw.c |  170 +++++++++++++++++++++++++++++++-------------------
>  1 files changed, 106 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index 53468f4..0472045 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -1079,6 +1079,107 @@ static irqreturn_t ntb_interrupt(int irq, void *dev)
>  	return IRQ_HANDLED;
>  }
>  
> +static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
> +{
> +	struct pci_dev *pdev = ndev->pdev;
> +	struct msix_entry *msix;
> +	int rc, i;
> +
> +	if (msix_entries < SNB_MSIX_CNT)

I would prefer ndev->limits.msix_cnt here to keep it generic.  I could
foresee this number changing on upcoming hardware with a similar
design.

> +		return -ENOSPC;
> +
> +	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> +	if (rc < 0)
> +		return rc;
> +	else if (rc > 0)
> +		return -ENOSPC;
> +
> +	for (i = 0; i < msix_entries; i++) {
> +		msix = &ndev->msix_entries[i];
> +		WARN_ON(!msix->vector);
> +
> +		if (i == msix_entries - 1) {
> +			rc = request_irq(msix->vector,
> +					 xeon_event_msix_irq, 0,
> +					 "ntb-event-msix", ndev);
> +			if (rc)
> +				goto err;
> +		} else {
> +			rc = request_irq(msix->vector,
> +					 xeon_callback_msix_irq, 0,
> +					 "ntb-callback-msix",
> +					 &ndev->db_cb[i]);
> +			if (rc)
> +				goto err;
> +		}
> +	}
> +
> +	ndev->num_msix = msix_entries;
> +	ndev->max_cbs = msix_entries - 1;
> +
> +	return 0;
> +
> +err:
> +	while (--i >= 0) {
> +		msix = &ndev->msix_entries[i];
> +		WARN_ON(i == ndev->num_msix - 1);
> +
> +		if (i == ndev->num_msix - 1)

Why have a WARN followed by a if statement on the same condition?  As
it stands right now, there is no way it could be hit.  So I'd say
remove both of them.

Aside from that it looks good.  Thanks for taking the time to cleanup
the code!

Thanks,
Jon

> +			free_irq(msix->vector, ndev);
> +		else
> +			free_irq(msix->vector, &ndev->db_cb[i]);
> +	}
> +
> +	pci_disable_msix(pdev);
> +	ndev->num_msix = 0;
> +
> +	return rc;
> +}
> +
> +static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
> +{
> +	struct pci_dev *pdev = ndev->pdev;
> +	struct msix_entry *msix;
> +	int rc, i;
> +
> +retry:
> +	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> +	if (rc < 0)
> +		return rc;
> +	else if (rc > 0) {
> +		dev_warn(&pdev->dev,
> +			 "Only %d MSI-X vectors. "
> +			 "Limiting the number of queues to that number.\n",
> +			 rc);
> +		msix_entries = rc;
> +		goto retry;
> +	}
> +
> +	for (i = 0; i < msix_entries; i++) {
> +		msix = &ndev->msix_entries[i];
> +		WARN_ON(!msix->vector);
> +
> +		rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> +				 "ntb-callback-msix", &ndev->db_cb[i]);
> +		if (rc)
> +			goto err;
> +	}
> +
> +	ndev->num_msix = msix_entries;
> +	ndev->max_cbs = msix_entries;
> +
> +	return 0;
> +
> +err:
> +	while (--i >= 0)
> +		free_irq(msix->vector, &ndev->db_cb[i]);
> +
> +	pci_disable_msix(pdev);
> +	ndev->num_msix = 0;
> +
> +	return rc;
> +}
> +
>  static int ntb_setup_msix(struct ntb_device *ndev)
>  {
>  	struct pci_dev *pdev = ndev->pdev;
> @@ -1105,78 +1206,19 @@ static int ntb_setup_msix(struct ntb_device *ndev)
>  	for (i = 0; i < msix_entries; i++)
>  		ndev->msix_entries[i].entry = i;
>  
> -	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> -	if (rc < 0)
> -		goto err1;
> -	if (rc > 0) {
> -		/* On SNB, the link interrupt is always tied to 4th vector.  If
> -		 * we can't get all 4, then we can't use MSI-X.
> -		 */
> -		if (ndev->hw_type != BWD_HW) {
> -			rc = -EIO;
> -			goto err1;
> -		}
> -
> -		dev_warn(&pdev->dev,
> -			 "Only %d MSI-X vectors.  Limiting the number of queues to that number.\n",
> -			 rc);
> -		msix_entries = rc;
> -
> -		rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> -		if (rc)
> -			goto err1;
> -	}
> -
> -	for (i = 0; i < msix_entries; i++) {
> -		msix = &ndev->msix_entries[i];
> -		WARN_ON(!msix->vector);
> -
> -		/* Use the last MSI-X vector for Link status */
> -		if (ndev->hw_type == BWD_HW) {
> -			rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> -					 "ntb-callback-msix", &ndev->db_cb[i]);
> -			if (rc)
> -				goto err2;
> -		} else {
> -			if (i == msix_entries - 1) {
> -				rc = request_irq(msix->vector,
> -						 xeon_event_msix_irq, 0,
> -						 "ntb-event-msix", ndev);
> -				if (rc)
> -					goto err2;
> -			} else {
> -				rc = request_irq(msix->vector,
> -						 xeon_callback_msix_irq, 0,
> -						 "ntb-callback-msix",
> -						 &ndev->db_cb[i]);
> -				if (rc)
> -					goto err2;
> -			}
> -		}
> -	}
> -
> -	ndev->num_msix = msix_entries;
>  	if (ndev->hw_type == BWD_HW)
> -		ndev->max_cbs = msix_entries;
> +		rc = ntb_setup_bwd_msix(ndev, msix_entries);
>  	else
> -		ndev->max_cbs = msix_entries - 1;
> +		rc = ntb_setup_snb_msix(ndev, msix_entries);
> +	if (rc)
> +		goto err1;
>  
>  	return 0;
>  
> -err2:
> -	while (--i >= 0) {
> -		msix = &ndev->msix_entries[i];
> -		if (ndev->hw_type != BWD_HW && i == ndev->num_msix - 1)
> -			free_irq(msix->vector, ndev);
> -		else
> -			free_irq(msix->vector, &ndev->db_cb[i]);
> -	}
> -	pci_disable_msix(pdev);
>  err1:
>  	kfree(ndev->msix_entries);
> -	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
>  err:
> -	ndev->num_msix = 0;
> +	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
>  	return rc;
>  }
>  
> -- 
> 1.7.7.6
> 

Re: [PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines

[Alexander Gordeev]

On Mon, Mar 03, 2014 at 05:12:25PM -0700, Jon Mason wrote:
> On Fri, Feb 21, 2014 at 04:49:31PM +0100, Alexander Gordeev wrote:
> > This is an cleanup effort to make ntb_setup_msix() more
> > readable - use ntb_setup_bwd_msix() to init MSI-Xs on
> > BWD hardware and ntb_setup_snb_msix() - on SNB hardware.
> > 
> > Function ntb_setup_snb_msix() also initializes MSI-Xs the
> > way it should has been done - looping pci_enable_msix()
> > until success or failure.
> > 
> > Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> > Cc: Jon Mason <jon.mason@intel.com>
> > Cc: linux-pci@vger.kernel.org
> > ---
> >  drivers/ntb/ntb_hw.c |  170 +++++++++++++++++++++++++++++++-------------------
> >  1 files changed, 106 insertions(+), 64 deletions(-)
> > 
> > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > index 53468f4..0472045 100644
> > --- a/drivers/ntb/ntb_hw.c
> > +++ b/drivers/ntb/ntb_hw.c
> > @@ -1079,6 +1079,107 @@ static irqreturn_t ntb_interrupt(int irq, void *dev)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > +static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
> > +{
> > +	struct pci_dev *pdev = ndev->pdev;
> > +	struct msix_entry *msix;
> > +	int rc, i;
> > +
> > +	if (msix_entries < SNB_MSIX_CNT)
> 
> I would prefer ndev->limits.msix_cnt here to keep it generic.  I could
> foresee this number changing on upcoming hardware with a similar
> design.
> 
> > +		return -ENOSPC;
> > +
> > +	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > +	if (rc < 0)
> > +		return rc;
> > +	else if (rc > 0)
> > +		return -ENOSPC;
> > +
> > +	for (i = 0; i < msix_entries; i++) {
> > +		msix = &ndev->msix_entries[i];
> > +		WARN_ON(!msix->vector);
> > +
> > +		if (i == msix_entries - 1) {
> > +			rc = request_irq(msix->vector,
> > +					 xeon_event_msix_irq, 0,
> > +					 "ntb-event-msix", ndev);
> > +			if (rc)
> > +				goto err;
> > +		} else {
> > +			rc = request_irq(msix->vector,
> > +					 xeon_callback_msix_irq, 0,
> > +					 "ntb-callback-msix",
> > +					 &ndev->db_cb[i]);
> > +			if (rc)
> > +				goto err;
> > +		}
> > +	}
> > +
> > +	ndev->num_msix = msix_entries;
> > +	ndev->max_cbs = msix_entries - 1;
> > +
> > +	return 0;
> > +
> > +err:
> > +	while (--i >= 0) {
> > +		msix = &ndev->msix_entries[i];
> > +		WARN_ON(i == ndev->num_msix - 1);
> > +
> > +		if (i == ndev->num_msix - 1)
> 
> Why have a WARN followed by a if statement on the same condition?  As
> it stands right now, there is no way it could be hit.  So I'd say
> remove both of them.

Right, WARN_ON() is stupid here. But we can not remove the condition,
because of 'dev_id' parameter for free_irq() is 'ndev' for the last
MSI-X, unlike &ndev->db_cb[i] for all other MSI-Xs, unless I am
missing something.

> 
> Aside from that it looks good.  Thanks for taking the time to cleanup
> the code!
> 
> Thanks,
> Jon
> 
> > +			free_irq(msix->vector, ndev);
> > +		else
> > +			free_irq(msix->vector, &ndev->db_cb[i]);
> > +	}
> > +
> > +	pci_disable_msix(pdev);
> > +	ndev->num_msix = 0;
> > +
> > +	return rc;
> > +}
> > +
> > +static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
> > +{
> > +	struct pci_dev *pdev = ndev->pdev;
> > +	struct msix_entry *msix;
> > +	int rc, i;
> > +
> > +retry:
> > +	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > +	if (rc < 0)
> > +		return rc;
> > +	else if (rc > 0) {
> > +		dev_warn(&pdev->dev,
> > +			 "Only %d MSI-X vectors. "
> > +			 "Limiting the number of queues to that number.\n",
> > +			 rc);
> > +		msix_entries = rc;
> > +		goto retry;
> > +	}
> > +
> > +	for (i = 0; i < msix_entries; i++) {
> > +		msix = &ndev->msix_entries[i];
> > +		WARN_ON(!msix->vector);
> > +
> > +		rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> > +				 "ntb-callback-msix", &ndev->db_cb[i]);
> > +		if (rc)
> > +			goto err;
> > +	}
> > +
> > +	ndev->num_msix = msix_entries;
> > +	ndev->max_cbs = msix_entries;
> > +
> > +	return 0;
> > +
> > +err:
> > +	while (--i >= 0)
> > +		free_irq(msix->vector, &ndev->db_cb[i]);
> > +
> > +	pci_disable_msix(pdev);
> > +	ndev->num_msix = 0;
> > +
> > +	return rc;
> > +}
> > +
> >  static int ntb_setup_msix(struct ntb_device *ndev)
> >  {
> >  	struct pci_dev *pdev = ndev->pdev;
> > @@ -1105,78 +1206,19 @@ static int ntb_setup_msix(struct ntb_device *ndev)
> >  	for (i = 0; i < msix_entries; i++)
> >  		ndev->msix_entries[i].entry = i;
> >  
> > -	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > -	if (rc < 0)
> > -		goto err1;
> > -	if (rc > 0) {
> > -		/* On SNB, the link interrupt is always tied to 4th vector.  If
> > -		 * we can't get all 4, then we can't use MSI-X.
> > -		 */
> > -		if (ndev->hw_type != BWD_HW) {
> > -			rc = -EIO;
> > -			goto err1;
> > -		}
> > -
> > -		dev_warn(&pdev->dev,
> > -			 "Only %d MSI-X vectors.  Limiting the number of queues to that number.\n",
> > -			 rc);
> > -		msix_entries = rc;
> > -
> > -		rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > -		if (rc)
> > -			goto err1;
> > -	}
> > -
> > -	for (i = 0; i < msix_entries; i++) {
> > -		msix = &ndev->msix_entries[i];
> > -		WARN_ON(!msix->vector);
> > -
> > -		/* Use the last MSI-X vector for Link status */
> > -		if (ndev->hw_type == BWD_HW) {
> > -			rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> > -					 "ntb-callback-msix", &ndev->db_cb[i]);
> > -			if (rc)
> > -				goto err2;
> > -		} else {
> > -			if (i == msix_entries - 1) {
> > -				rc = request_irq(msix->vector,
> > -						 xeon_event_msix_irq, 0,
> > -						 "ntb-event-msix", ndev);
> > -				if (rc)
> > -					goto err2;
> > -			} else {
> > -				rc = request_irq(msix->vector,
> > -						 xeon_callback_msix_irq, 0,
> > -						 "ntb-callback-msix",
> > -						 &ndev->db_cb[i]);
> > -				if (rc)
> > -					goto err2;
> > -			}
> > -		}
> > -	}
> > -
> > -	ndev->num_msix = msix_entries;
> >  	if (ndev->hw_type == BWD_HW)
> > -		ndev->max_cbs = msix_entries;
> > +		rc = ntb_setup_bwd_msix(ndev, msix_entries);
> >  	else
> > -		ndev->max_cbs = msix_entries - 1;
> > +		rc = ntb_setup_snb_msix(ndev, msix_entries);
> > +	if (rc)
> > +		goto err1;
> >  
> >  	return 0;
> >  
> > -err2:
> > -	while (--i >= 0) {
> > -		msix = &ndev->msix_entries[i];
> > -		if (ndev->hw_type != BWD_HW && i == ndev->num_msix - 1)
> > -			free_irq(msix->vector, ndev);
> > -		else
> > -			free_irq(msix->vector, &ndev->db_cb[i]);
> > -	}
> > -	pci_disable_msix(pdev);
> >  err1:
> >  	kfree(ndev->msix_entries);
> > -	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
> >  err:
> > -	ndev->num_msix = 0;
> > +	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
> >  	return rc;
> >  }
> >  
> > -- 
> > 1.7.7.6
> > 

-- 
Regards,
Alexander Gordeev
agordeev@redhat.com

Re: [PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines

[Jon Mason]

On Tue, Mar 04, 2014 at 05:41:28PM +0100, Alexander Gordeev wrote:
> On Mon, Mar 03, 2014 at 05:12:25PM -0700, Jon Mason wrote:
> > On Fri, Feb 21, 2014 at 04:49:31PM +0100, Alexander Gordeev wrote:
> > > This is an cleanup effort to make ntb_setup_msix() more
> > > readable - use ntb_setup_bwd_msix() to init MSI-Xs on
> > > BWD hardware and ntb_setup_snb_msix() - on SNB hardware.
> > > 
> > > Function ntb_setup_snb_msix() also initializes MSI-Xs the
> > > way it should has been done - looping pci_enable_msix()
> > > until success or failure.
> > > 
> > > Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> > > Cc: Jon Mason <jon.mason@intel.com>
> > > Cc: linux-pci@vger.kernel.org
> > > ---
> > >  drivers/ntb/ntb_hw.c |  170 +++++++++++++++++++++++++++++++-------------------
> > >  1 files changed, 106 insertions(+), 64 deletions(-)
> > > 
> > > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > > index 53468f4..0472045 100644
> > > --- a/drivers/ntb/ntb_hw.c
> > > +++ b/drivers/ntb/ntb_hw.c
> > > @@ -1079,6 +1079,107 @@ static irqreturn_t ntb_interrupt(int irq, void *dev)
> > >  	return IRQ_HANDLED;
> > >  }
> > >  
> > > +static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
> > > +{
> > > +	struct pci_dev *pdev = ndev->pdev;
> > > +	struct msix_entry *msix;
> > > +	int rc, i;
> > > +
> > > +	if (msix_entries < SNB_MSIX_CNT)
> > 
> > I would prefer ndev->limits.msix_cnt here to keep it generic.  I could
> > foresee this number changing on upcoming hardware with a similar
> > design.
> > 
> > > +		return -ENOSPC;
> > > +
> > > +	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > > +	if (rc < 0)
> > > +		return rc;
> > > +	else if (rc > 0)
> > > +		return -ENOSPC;
> > > +
> > > +	for (i = 0; i < msix_entries; i++) {
> > > +		msix = &ndev->msix_entries[i];
> > > +		WARN_ON(!msix->vector);
> > > +
> > > +		if (i == msix_entries - 1) {
> > > +			rc = request_irq(msix->vector,
> > > +					 xeon_event_msix_irq, 0,
> > > +					 "ntb-event-msix", ndev);
> > > +			if (rc)
> > > +				goto err;
> > > +		} else {
> > > +			rc = request_irq(msix->vector,
> > > +					 xeon_callback_msix_irq, 0,
> > > +					 "ntb-callback-msix",
> > > +					 &ndev->db_cb[i]);
> > > +			if (rc)
> > > +				goto err;
> > > +		}
> > > +	}
> > > +
> > > +	ndev->num_msix = msix_entries;
> > > +	ndev->max_cbs = msix_entries - 1;
> > > +
> > > +	return 0;
> > > +
> > > +err:
> > > +	while (--i >= 0) {
> > > +		msix = &ndev->msix_entries[i];
> > > +		WARN_ON(i == ndev->num_msix - 1);
> > > +
> > > +		if (i == ndev->num_msix - 1)
> > 
> > Why have a WARN followed by a if statement on the same condition?  As
> > it stands right now, there is no way it could be hit.  So I'd say
> > remove both of them.
> 
> Right, WARN_ON() is stupid here. But we can not remove the condition,
> because of 'dev_id' parameter for free_irq() is 'ndev' for the last
> MSI-X, unlike &ndev->db_cb[i] for all other MSI-Xs, unless I am
> missing something.

The "i == ndev->num_msix - 1" cannot be hit.  If a failure hits in
request_irq with i < msix_entries - 1, then there is no need to free
the irq for xeon_event_msix_irq allocation because it was never
allocated.  If i == msix_entries - 1 when it fails, there is no need
to free the irq because the allocation failed.  These 2 cases being
true, it's never possible to hit the case and the clean-up code is
unnecessary.

Thanks,
Jon

> 
> > 
> > Aside from that it looks good.  Thanks for taking the time to cleanup
> > the code!
> > 
> > Thanks,
> > Jon
> > 
> > > +			free_irq(msix->vector, ndev);
> > > +		else
> > > +			free_irq(msix->vector, &ndev->db_cb[i]);
> > > +	}
> > > +
> > > +	pci_disable_msix(pdev);
> > > +	ndev->num_msix = 0;
> > > +
> > > +	return rc;
> > > +}
> > > +
> > > +static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
> > > +{
> > > +	struct pci_dev *pdev = ndev->pdev;
> > > +	struct msix_entry *msix;
> > > +	int rc, i;
> > > +
> > > +retry:
> > > +	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > > +	if (rc < 0)
> > > +		return rc;
> > > +	else if (rc > 0) {
> > > +		dev_warn(&pdev->dev,
> > > +			 "Only %d MSI-X vectors. "
> > > +			 "Limiting the number of queues to that number.\n",
> > > +			 rc);
> > > +		msix_entries = rc;
> > > +		goto retry;
> > > +	}
> > > +
> > > +	for (i = 0; i < msix_entries; i++) {
> > > +		msix = &ndev->msix_entries[i];
> > > +		WARN_ON(!msix->vector);
> > > +
> > > +		rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> > > +				 "ntb-callback-msix", &ndev->db_cb[i]);
> > > +		if (rc)
> > > +			goto err;
> > > +	}
> > > +
> > > +	ndev->num_msix = msix_entries;
> > > +	ndev->max_cbs = msix_entries;
> > > +
> > > +	return 0;
> > > +
> > > +err:
> > > +	while (--i >= 0)
> > > +		free_irq(msix->vector, &ndev->db_cb[i]);
> > > +
> > > +	pci_disable_msix(pdev);
> > > +	ndev->num_msix = 0;
> > > +
> > > +	return rc;
> > > +}
> > > +
> > >  static int ntb_setup_msix(struct ntb_device *ndev)
> > >  {
> > >  	struct pci_dev *pdev = ndev->pdev;
> > > @@ -1105,78 +1206,19 @@ static int ntb_setup_msix(struct ntb_device *ndev)
> > >  	for (i = 0; i < msix_entries; i++)
> > >  		ndev->msix_entries[i].entry = i;
> > >  
> > > -	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > > -	if (rc < 0)
> > > -		goto err1;
> > > -	if (rc > 0) {
> > > -		/* On SNB, the link interrupt is always tied to 4th vector.  If
> > > -		 * we can't get all 4, then we can't use MSI-X.
> > > -		 */
> > > -		if (ndev->hw_type != BWD_HW) {
> > > -			rc = -EIO;
> > > -			goto err1;
> > > -		}
> > > -
> > > -		dev_warn(&pdev->dev,
> > > -			 "Only %d MSI-X vectors.  Limiting the number of queues to that number.\n",
> > > -			 rc);
> > > -		msix_entries = rc;
> > > -
> > > -		rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> > > -		if (rc)
> > > -			goto err1;
> > > -	}
> > > -
> > > -	for (i = 0; i < msix_entries; i++) {
> > > -		msix = &ndev->msix_entries[i];
> > > -		WARN_ON(!msix->vector);
> > > -
> > > -		/* Use the last MSI-X vector for Link status */
> > > -		if (ndev->hw_type == BWD_HW) {
> > > -			rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> > > -					 "ntb-callback-msix", &ndev->db_cb[i]);
> > > -			if (rc)
> > > -				goto err2;
> > > -		} else {
> > > -			if (i == msix_entries - 1) {
> > > -				rc = request_irq(msix->vector,
> > > -						 xeon_event_msix_irq, 0,
> > > -						 "ntb-event-msix", ndev);
> > > -				if (rc)
> > > -					goto err2;
> > > -			} else {
> > > -				rc = request_irq(msix->vector,
> > > -						 xeon_callback_msix_irq, 0,
> > > -						 "ntb-callback-msix",
> > > -						 &ndev->db_cb[i]);
> > > -				if (rc)
> > > -					goto err2;
> > > -			}
> > > -		}
> > > -	}
> > > -
> > > -	ndev->num_msix = msix_entries;
> > >  	if (ndev->hw_type == BWD_HW)
> > > -		ndev->max_cbs = msix_entries;
> > > +		rc = ntb_setup_bwd_msix(ndev, msix_entries);
> > >  	else
> > > -		ndev->max_cbs = msix_entries - 1;
> > > +		rc = ntb_setup_snb_msix(ndev, msix_entries);
> > > +	if (rc)
> > > +		goto err1;
> > >  
> > >  	return 0;
> > >  
> > > -err2:
> > > -	while (--i >= 0) {
> > > -		msix = &ndev->msix_entries[i];
> > > -		if (ndev->hw_type != BWD_HW && i == ndev->num_msix - 1)
> > > -			free_irq(msix->vector, ndev);
> > > -		else
> > > -			free_irq(msix->vector, &ndev->db_cb[i]);
> > > -	}
> > > -	pci_disable_msix(pdev);
> > >  err1:
> > >  	kfree(ndev->msix_entries);
> > > -	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
> > >  err:
> > > -	ndev->num_msix = 0;
> > > +	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
> > >  	return rc;
> > >  }
> > >  
> > > -- 
> > > 1.7.7.6
> > > 
> 
> -- 
> Regards,
> Alexander Gordeev
> agordeev@redhat.com

[PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines

[Alexander Gordeev]

This is an cleanup effort to make ntb_setup_msix() more
readable - use ntb_setup_bwd_msix() to init MSI-Xs on
BWD hardware and ntb_setup_snb_msix() - on SNB hardware.

Function ntb_setup_snb_msix() also initializes MSI-Xs the
way it should has been done - looping pci_enable_msix()
until success or failure.

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Cc: Jon Mason <jon.mason@intel.com>
Cc: linux-pci@vger.kernel.org
---
 drivers/ntb/ntb_hw.c |  167 ++++++++++++++++++++++++++++++-------------------
 1 files changed, 102 insertions(+), 65 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 53468f4..1f8decd 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1079,10 +1079,106 @@ static irqreturn_t ntb_interrupt(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
-static int ntb_setup_msix(struct ntb_device *ndev)
+static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
 {
 	struct pci_dev *pdev = ndev->pdev;
 	struct msix_entry *msix;
+	int rc, i;
+
+	if (msix_entries < ndev->limits.msix_cnt)
+		return -ENOSPC;
+
+	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
+	if (rc < 0)
+		return rc;
+	else if (rc > 0)
+		return -ENOSPC;
+
+	for (i = 0; i < msix_entries; i++) {
+		msix = &ndev->msix_entries[i];
+		WARN_ON(!msix->vector);
+
+		if (i == msix_entries - 1) {
+			rc = request_irq(msix->vector,
+					 xeon_event_msix_irq, 0,
+					 "ntb-event-msix", ndev);
+			if (rc)
+				goto err;
+		} else {
+			rc = request_irq(msix->vector,
+					 xeon_callback_msix_irq, 0,
+					 "ntb-callback-msix",
+					 &ndev->db_cb[i]);
+			if (rc)
+				goto err;
+		}
+	}
+
+	ndev->num_msix = msix_entries;
+	ndev->max_cbs = msix_entries - 1;
+
+	return 0;
+
+err:
+	while (--i >= 0) {
+		/* Code never reaches here for entry nr 'ndev->num_msix - 1' */
+		msix = &ndev->msix_entries[i];
+		free_irq(msix->vector, &ndev->db_cb[i]);
+	}
+
+	pci_disable_msix(pdev);
+	ndev->num_msix = 0;
+
+	return rc;
+}
+
+static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
+{
+	struct pci_dev *pdev = ndev->pdev;
+	struct msix_entry *msix;
+	int rc, i;
+
+retry:
+	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
+	if (rc < 0)
+		return rc;
+	else if (rc > 0) {
+		dev_warn(&pdev->dev,
+			 "Only %d MSI-X vectors. "
+			 "Limiting the number of queues to that number.\n",
+			 rc);
+		msix_entries = rc;
+		goto retry;
+	}
+
+	for (i = 0; i < msix_entries; i++) {
+		msix = &ndev->msix_entries[i];
+		WARN_ON(!msix->vector);
+
+		rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
+				 "ntb-callback-msix", &ndev->db_cb[i]);
+		if (rc)
+			goto err;
+	}
+
+	ndev->num_msix = msix_entries;
+	ndev->max_cbs = msix_entries;
+
+	return 0;
+
+err:
+	while (--i >= 0)
+		free_irq(msix->vector, &ndev->db_cb[i]);
+
+	pci_disable_msix(pdev);
+	ndev->num_msix = 0;
+
+	return rc;
+}
+
+static int ntb_setup_msix(struct ntb_device *ndev)
+{
+	struct pci_dev *pdev = ndev->pdev;
 	int msix_entries;
 	int rc, i;
 
@@ -1105,78 +1201,19 @@ static int ntb_setup_msix(struct ntb_device *ndev)
 	for (i = 0; i < msix_entries; i++)
 		ndev->msix_entries[i].entry = i;
 
-	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
-	if (rc < 0)
-		goto err1;
-	if (rc > 0) {
-		/* On SNB, the link interrupt is always tied to 4th vector.  If
-		 * we can't get all 4, then we can't use MSI-X.
-		 */
-		if (ndev->hw_type != BWD_HW) {
-			rc = -EIO;
-			goto err1;
-		}
-
-		dev_warn(&pdev->dev,
-			 "Only %d MSI-X vectors.  Limiting the number of queues to that number.\n",
-			 rc);
-		msix_entries = rc;
-
-		rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
-		if (rc)
-			goto err1;
-	}
-
-	for (i = 0; i < msix_entries; i++) {
-		msix = &ndev->msix_entries[i];
-		WARN_ON(!msix->vector);
-
-		/* Use the last MSI-X vector for Link status */
-		if (ndev->hw_type == BWD_HW) {
-			rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
-					 "ntb-callback-msix", &ndev->db_cb[i]);
-			if (rc)
-				goto err2;
-		} else {
-			if (i == msix_entries - 1) {
-				rc = request_irq(msix->vector,
-						 xeon_event_msix_irq, 0,
-						 "ntb-event-msix", ndev);
-				if (rc)
-					goto err2;
-			} else {
-				rc = request_irq(msix->vector,
-						 xeon_callback_msix_irq, 0,
-						 "ntb-callback-msix",
-						 &ndev->db_cb[i]);
-				if (rc)
-					goto err2;
-			}
-		}
-	}
-
-	ndev->num_msix = msix_entries;
 	if (ndev->hw_type == BWD_HW)
-		ndev->max_cbs = msix_entries;
+		rc = ntb_setup_bwd_msix(ndev, msix_entries);
 	else
-		ndev->max_cbs = msix_entries - 1;
+		rc = ntb_setup_snb_msix(ndev, msix_entries);
+	if (rc)
+		goto err1;
 
 	return 0;
 
-err2:
-	while (--i >= 0) {
-		msix = &ndev->msix_entries[i];
-		if (ndev->hw_type != BWD_HW && i == ndev->num_msix - 1)
-			free_irq(msix->vector, ndev);
-		else
-			free_irq(msix->vector, &ndev->db_cb[i]);
-	}
-	pci_disable_msix(pdev);
 err1:
 	kfree(ndev->msix_entries);
-	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
 err:
-	ndev->num_msix = 0;
+	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
 	return rc;
 }
 
-- 
1.7.7.6

-- 
Regards,
Alexander Gordeev
agordeev@redhat.com

[PATCH v2 4/4] ntb: Use pci_enable_msix_range() instead of pci_enable_msix()

[Alexander Gordeev]

As result of deprecation of MSI-X/MSI enablement functions
pci_enable_msix() and pci_enable_msi_block() all drivers
using these two interfaces need to be updated to use the
new pci_enable_msi_range()  or pci_enable_msi_exact()
and pci_enable_msix_range() or pci_enable_msix_exact()
interfaces.

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Cc: Jon Mason <jon.mason@intel.com>
Cc: linux-pci@vger.kernel.org
---
 drivers/ntb/ntb_hw.c |   20 +++++---------------
 1 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 1f8decd..ff5c0a3 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1088,11 +1088,9 @@ static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
 	if (msix_entries < ndev->limits.msix_cnt)
 		return -ENOSPC;
 
-	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
+	rc = pci_enable_msix_exact(pdev, ndev->msix_entries, msix_entries);
 	if (rc < 0)
 		return rc;
-	else if (rc > 0)
-		return -ENOSPC;
 
 	for (i = 0; i < msix_entries; i++) {
 		msix = &ndev->msix_entries[i];
@@ -1138,18 +1136,10 @@ static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
 	struct msix_entry *msix;
 	int rc, i;
 
-retry:
-	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
-	if (rc < 0)
-		return rc;
-	else if (rc > 0) {
-		dev_warn(&pdev->dev,
-			 "Only %d MSI-X vectors. "
-			 "Limiting the number of queues to that number.\n",
-			 rc);
-		msix_entries = rc;
-		goto retry;
-	}
+	msix_entries = pci_enable_msix_range(pdev, ndev->msix_entries,
+					     1, msix_entries);
+	if (msix_entries < 0)
+		return msix_entries;
 
 	for (i = 0; i < msix_entries; i++) {
 		msix = &ndev->msix_entries[i];
-- 
1.7.7.6

-- 
Regards,
Alexander Gordeev
agordeev@redhat.com

Re: [PATCH v2 3/4] ntb: Split ntb_setup_msix() into separate BWD/SNB routines

[Jon Mason]

On Tue, Mar 11, 2014 at 05:00:22PM +0100, Alexander Gordeev wrote:
> This is an cleanup effort to make ntb_setup_msix() more
> readable - use ntb_setup_bwd_msix() to init MSI-Xs on
> BWD hardware and ntb_setup_snb_msix() - on SNB hardware.
> 
> Function ntb_setup_snb_msix() also initializes MSI-Xs the
> way it should has been done - looping pci_enable_msix()
> until success or failure.
> 
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> Cc: Jon Mason <jon.mason@intel.com>

Looks good.  I will include it in my next release of the NTB driver
(e.g. 3.15).

Thanks,
Jon

> Cc: linux-pci@vger.kernel.org
> ---
>  drivers/ntb/ntb_hw.c |  167 ++++++++++++++++++++++++++++++-------------------
>  1 files changed, 102 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index 53468f4..1f8decd 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -1079,10 +1079,106 @@ static irqreturn_t ntb_interrupt(int irq, void *dev)
>  	return IRQ_HANDLED;
>  }
>  
> -static int ntb_setup_msix(struct ntb_device *ndev)
> +static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
>  {
>  	struct pci_dev *pdev = ndev->pdev;
>  	struct msix_entry *msix;
> +	int rc, i;
> +
> +	if (msix_entries < ndev->limits.msix_cnt)
> +		return -ENOSPC;
> +
> +	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> +	if (rc < 0)
> +		return rc;
> +	else if (rc > 0)
> +		return -ENOSPC;
> +
> +	for (i = 0; i < msix_entries; i++) {
> +		msix = &ndev->msix_entries[i];
> +		WARN_ON(!msix->vector);
> +
> +		if (i == msix_entries - 1) {
> +			rc = request_irq(msix->vector,
> +					 xeon_event_msix_irq, 0,
> +					 "ntb-event-msix", ndev);
> +			if (rc)
> +				goto err;
> +		} else {
> +			rc = request_irq(msix->vector,
> +					 xeon_callback_msix_irq, 0,
> +					 "ntb-callback-msix",
> +					 &ndev->db_cb[i]);
> +			if (rc)
> +				goto err;
> +		}
> +	}
> +
> +	ndev->num_msix = msix_entries;
> +	ndev->max_cbs = msix_entries - 1;
> +
> +	return 0;
> +
> +err:
> +	while (--i >= 0) {
> +		/* Code never reaches here for entry nr 'ndev->num_msix - 1' */
> +		msix = &ndev->msix_entries[i];
> +		free_irq(msix->vector, &ndev->db_cb[i]);
> +	}
> +
> +	pci_disable_msix(pdev);
> +	ndev->num_msix = 0;
> +
> +	return rc;
> +}
> +
> +static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
> +{
> +	struct pci_dev *pdev = ndev->pdev;
> +	struct msix_entry *msix;
> +	int rc, i;
> +
> +retry:
> +	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> +	if (rc < 0)
> +		return rc;
> +	else if (rc > 0) {
> +		dev_warn(&pdev->dev,
> +			 "Only %d MSI-X vectors. "
> +			 "Limiting the number of queues to that number.\n",
> +			 rc);
> +		msix_entries = rc;
> +		goto retry;
> +	}
> +
> +	for (i = 0; i < msix_entries; i++) {
> +		msix = &ndev->msix_entries[i];
> +		WARN_ON(!msix->vector);
> +
> +		rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> +				 "ntb-callback-msix", &ndev->db_cb[i]);
> +		if (rc)
> +			goto err;
> +	}
> +
> +	ndev->num_msix = msix_entries;
> +	ndev->max_cbs = msix_entries;
> +
> +	return 0;
> +
> +err:
> +	while (--i >= 0)
> +		free_irq(msix->vector, &ndev->db_cb[i]);
> +
> +	pci_disable_msix(pdev);
> +	ndev->num_msix = 0;
> +
> +	return rc;
> +}
> +
> +static int ntb_setup_msix(struct ntb_device *ndev)
> +{
> +	struct pci_dev *pdev = ndev->pdev;
>  	int msix_entries;
>  	int rc, i;
>  
> @@ -1105,78 +1201,19 @@ static int ntb_setup_msix(struct ntb_device *ndev)
>  	for (i = 0; i < msix_entries; i++)
>  		ndev->msix_entries[i].entry = i;
>  
> -	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> -	if (rc < 0)
> -		goto err1;
> -	if (rc > 0) {
> -		/* On SNB, the link interrupt is always tied to 4th vector.  If
> -		 * we can't get all 4, then we can't use MSI-X.
> -		 */
> -		if (ndev->hw_type != BWD_HW) {
> -			rc = -EIO;
> -			goto err1;
> -		}
> -
> -		dev_warn(&pdev->dev,
> -			 "Only %d MSI-X vectors.  Limiting the number of queues to that number.\n",
> -			 rc);
> -		msix_entries = rc;
> -
> -		rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> -		if (rc)
> -			goto err1;
> -	}
> -
> -	for (i = 0; i < msix_entries; i++) {
> -		msix = &ndev->msix_entries[i];
> -		WARN_ON(!msix->vector);
> -
> -		/* Use the last MSI-X vector for Link status */
> -		if (ndev->hw_type == BWD_HW) {
> -			rc = request_irq(msix->vector, bwd_callback_msix_irq, 0,
> -					 "ntb-callback-msix", &ndev->db_cb[i]);
> -			if (rc)
> -				goto err2;
> -		} else {
> -			if (i == msix_entries - 1) {
> -				rc = request_irq(msix->vector,
> -						 xeon_event_msix_irq, 0,
> -						 "ntb-event-msix", ndev);
> -				if (rc)
> -					goto err2;
> -			} else {
> -				rc = request_irq(msix->vector,
> -						 xeon_callback_msix_irq, 0,
> -						 "ntb-callback-msix",
> -						 &ndev->db_cb[i]);
> -				if (rc)
> -					goto err2;
> -			}
> -		}
> -	}
> -
> -	ndev->num_msix = msix_entries;
>  	if (ndev->hw_type == BWD_HW)
> -		ndev->max_cbs = msix_entries;
> +		rc = ntb_setup_bwd_msix(ndev, msix_entries);
>  	else
> -		ndev->max_cbs = msix_entries - 1;
> +		rc = ntb_setup_snb_msix(ndev, msix_entries);
> +	if (rc)
> +		goto err1;
>  
>  	return 0;
>  
> -err2:
> -	while (--i >= 0) {
> -		msix = &ndev->msix_entries[i];
> -		if (ndev->hw_type != BWD_HW && i == ndev->num_msix - 1)
> -			free_irq(msix->vector, ndev);
> -		else
> -			free_irq(msix->vector, &ndev->db_cb[i]);
> -	}
> -	pci_disable_msix(pdev);
>  err1:
>  	kfree(ndev->msix_entries);
> -	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
>  err:
> -	ndev->num_msix = 0;
> +	dev_err(&pdev->dev, "Error allocating MSI-X interrupt\n");
>  	return rc;
>  }
>  
> -- 
> 1.7.7.6
> 
> -- 
> Regards,
> Alexander Gordeev
> agordeev@redhat.com

Re: [PATCH v2 4/4] ntb: Use pci_enable_msix_range() instead of pci_enable_msix()

[Jon Mason]

On Tue, Mar 11, 2014 at 05:00:35PM +0100, Alexander Gordeev wrote:
> As result of deprecation of MSI-X/MSI enablement functions
> pci_enable_msix() and pci_enable_msi_block() all drivers
> using these two interfaces need to be updated to use the
> new pci_enable_msi_range()  or pci_enable_msi_exact()
> and pci_enable_msix_range() or pci_enable_msix_exact()
> interfaces.
> 
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> Cc: Jon Mason <jon.mason@intel.com>

Looks good.  I will include it in my next release of the NTB driver
(e.g. 3.15).

Thanks,
Jon

> Cc: linux-pci@vger.kernel.org
> ---
>  drivers/ntb/ntb_hw.c |   20 +++++---------------
>  1 files changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index 1f8decd..ff5c0a3 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -1088,11 +1088,9 @@ static int ntb_setup_snb_msix(struct ntb_device *ndev, int msix_entries)
>  	if (msix_entries < ndev->limits.msix_cnt)
>  		return -ENOSPC;
>  
> -	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> +	rc = pci_enable_msix_exact(pdev, ndev->msix_entries, msix_entries);
>  	if (rc < 0)
>  		return rc;
> -	else if (rc > 0)
> -		return -ENOSPC;
>  
>  	for (i = 0; i < msix_entries; i++) {
>  		msix = &ndev->msix_entries[i];
> @@ -1138,18 +1136,10 @@ static int ntb_setup_bwd_msix(struct ntb_device *ndev, int msix_entries)
>  	struct msix_entry *msix;
>  	int rc, i;
>  
> -retry:
> -	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> -	if (rc < 0)
> -		return rc;
> -	else if (rc > 0) {
> -		dev_warn(&pdev->dev,
> -			 "Only %d MSI-X vectors. "
> -			 "Limiting the number of queues to that number.\n",
> -			 rc);
> -		msix_entries = rc;
> -		goto retry;
> -	}
> +	msix_entries = pci_enable_msix_range(pdev, ndev->msix_entries,
> +					     1, msix_entries);
> +	if (msix_entries < 0)
> +		return msix_entries;
>  
>  	for (i = 0; i < msix_entries; i++) {
>  		msix = &ndev->msix_entries[i];
> -- 
> 1.7.7.6
> 
> -- 
> Regards,
> Alexander Gordeev
> agordeev@redhat.com