[PATCH net v2] nfp: correct number of MSI vectors requests returned

[PATCH net v2] nfp: correct number of MSI vectors requests returned

[Louis Peens]

From: Xiaoyu Li <xiaoyu.li@corigine.com>

Before the referenced commit, if fewer interrupts are supported by
hardware than requested, then pci_msix_vec_count() returned the
former. However, after the referenced commit, an error is returned
for this condition. This causes a regression in the NFP driver
preventing probe from completing.

This situation may occur because the firmware allows sharing of
more than one queue per interrupt vector. And, thus, it is valid for
the firmware to advertise the number of queues it does. However,
interrupt sharing is not currently implemented by the NFP driver as
it seems likely - though not tested - that any gains obtained by
having more queues would be mitigated by sharing of interrupts.

Address this problem by limiting the number of vectors requested to
the number supported by hardware.

Also, make correct the max/min_irq types. They were unsigned
previously but should be signed.

Fixes: bab65e48cb06 ("PCI/MSI: Sanitize MSI-X checks")
CC: stable@vger.kernel.org
Signed-off-by: Xiaoyu Li <xiaoyu.li@corigine.com>
Acked-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Louis Peens <louis.peens@corigine.com>
---
Changes: V1-->V2
* Updated the max/min_irq types to be signed instead of unsigned
* Fixed formatting of commit message to be better aligned at 72 chars
* Also updated the commit message to better explain why this is even
  possible to happen, in response to the question from V1.

 drivers/net/ethernet/netronome/nfp/nfp_net.h        |  4 ++--
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 12 +++++++++---
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c   |  9 +++++----
 drivers/net/ethernet/netronome/nfp/nfp_netvf_main.c |  8 ++++----
 4 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index 939cfce15830..960f69325287 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -971,9 +971,9 @@ int nfp_net_mbox_reconfig_and_unlock(struct nfp_net *nn, u32 mbox_cmd);
 void nfp_net_mbox_reconfig_post(struct nfp_net *nn, u32 update);
 int nfp_net_mbox_reconfig_wait_posted(struct nfp_net *nn);
 
-unsigned int
+int
 nfp_net_irqs_alloc(struct pci_dev *pdev, struct msix_entry *irq_entries,
-		   unsigned int min_irqs, unsigned int want_irqs);
+		   int min_irqs, int want_irqs);
 void nfp_net_irqs_disable(struct pci_dev *pdev);
 void
 nfp_net_irqs_assign(struct nfp_net *nn, struct msix_entry *irq_entries,
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 62f0bf91d1e1..ae309ea48356 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -362,14 +362,20 @@ int nfp_net_mbox_reconfig_and_unlock(struct nfp_net *nn, u32 mbox_cmd)
  * @min_irqs:    Minimal acceptable number of interrupts
  * @wanted_irqs: Target number of interrupts to allocate
  *
- * Return: Number of irqs obtained or 0 on error.
+ * Return: Number of irqs obtained or an errno.
  */
-unsigned int
+int
 nfp_net_irqs_alloc(struct pci_dev *pdev, struct msix_entry *irq_entries,
-		   unsigned int min_irqs, unsigned int wanted_irqs)
+		   int min_irqs, int wanted_irqs)
 {
 	unsigned int i;
 	int got_irqs;
+	int max_irqs;
+
+	max_irqs = pci_msix_vec_count(pdev);
+	if (max_irqs < 0)
+		return max_irqs;
+	wanted_irqs = min_t(int, max_irqs, wanted_irqs);
 
 	for (i = 0; i < wanted_irqs; i++)
 		irq_entries[i].entry = i;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index cbe4972ba104..c1ac380542b5 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -222,7 +222,8 @@ static void nfp_net_pf_clean_vnic(struct nfp_pf *pf, struct nfp_net *nn)
 
 static int nfp_net_pf_alloc_irqs(struct nfp_pf *pf)
 {
-	unsigned int wanted_irqs, num_irqs, vnics_left, irqs_left;
+	unsigned int vnics_left, irqs_left;
+	int wanted_irqs, num_irqs;
 	struct nfp_net *nn;
 
 	/* Get MSI-X vectors */
@@ -237,10 +238,10 @@ static int nfp_net_pf_alloc_irqs(struct nfp_pf *pf)
 	num_irqs = nfp_net_irqs_alloc(pf->pdev, pf->irq_entries,
 				      NFP_NET_MIN_VNIC_IRQS * pf->num_vnics,
 				      wanted_irqs);
-	if (!num_irqs) {
-		nfp_warn(pf->cpp, "Unable to allocate MSI-X vectors\n");
+	if (num_irqs < 0) {
+		nfp_warn(pf->cpp, "Unable to allocate MSI-X vectors (err=%d)\n", num_irqs);
 		kfree(pf->irq_entries);
-		return -ENOMEM;
+		return num_irqs;
 	}
 
 	/* Distribute IRQs to vNICs */
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_netvf_main.c b/drivers/net/ethernet/netronome/nfp/nfp_netvf_main.c
index e19bb0150cb5..5f89c7198606 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_netvf_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_netvf_main.c
@@ -84,7 +84,7 @@ static int nfp_netvf_pci_probe(struct pci_dev *pdev,
 	u32 tx_bar_sz, rx_bar_sz;
 	int tx_bar_no, rx_bar_no;
 	struct nfp_net_vf *vf;
-	unsigned int num_irqs;
+	int num_irqs;
 	u8 __iomem *ctrl_bar;
 	struct nfp_net *nn;
 	u32 startq;
@@ -255,9 +255,9 @@ static int nfp_netvf_pci_probe(struct pci_dev *pdev,
 				      NFP_NET_MIN_VNIC_IRQS,
 				      NFP_NET_NON_Q_VECTORS +
 				      nn->dp.num_r_vecs);
-	if (!num_irqs) {
-		nn_warn(nn, "Unable to allocate MSI-X Vectors. Exiting\n");
-		err = -EIO;
+	if (num_irqs < 0) {
+		nn_warn(nn, "Unable to allocate MSI-X Vectors. Exiting (err=%d)\n", num_irqs);
+		err = num_irqs;
 		goto err_unmap_rx;
 	}
 	nfp_net_irqs_assign(nn, vf->irq_entries, num_irqs);
-- 
2.34.1

RE: [PATCH net v2] nfp: correct number of MSI vectors requests returned

[David Laight]

From: Louis Peens
> Sent: 19 April 2023 09:15
> 
> From: Xiaoyu Li <xiaoyu.li@corigine.com>
> 
> Before the referenced commit, if fewer interrupts are supported by
> hardware than requested, then pci_msix_vec_count() returned the
> former. However, after the referenced commit, an error is returned
> for this condition. This causes a regression in the NFP driver
> preventing probe from completing.

I believe the relevant change to the msix vector allocation
function has been reverted.
(Or at least, the over-zealous check of nvec removed.)

So this change to bound the number of interrupts
isn't needed.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH net v2] nfp: correct number of MSI vectors requests returned

[Jakub Kicinski]

On Wed, 19 Apr 2023 15:37:57 +0000 David Laight wrote:
> > Before the referenced commit, if fewer interrupts are supported by
> > hardware than requested, then pci_msix_vec_count() returned the
> > former. However, after the referenced commit, an error is returned
> > for this condition. This causes a regression in the NFP driver
> > preventing probe from completing.  
> 
> I believe the relevant change to the msix vector allocation
> function has been reverted.
> (Or at least, the over-zealous check of nvec removed.)
> 
> So this change to bound the number of interrupts
> isn't needed.

Great, thanks, I was about to ask!
-- 
pw-bot: cr

Re: [PATCH net v2] nfp: correct number of MSI vectors requests returned

[Simon Horman]

On Wed, Apr 19, 2023 at 06:34:09PM -0700, Jakub Kicinski wrote:
> On Wed, 19 Apr 2023 15:37:57 +0000 David Laight wrote:
> > > Before the referenced commit, if fewer interrupts are supported by
> > > hardware than requested, then pci_msix_vec_count() returned the
> > > former. However, after the referenced commit, an error is returned
> > > for this condition. This causes a regression in the NFP driver
> > > preventing probe from completing.  
> > 
> > I believe the relevant change to the msix vector allocation
> > function has been reverted.
> > (Or at least, the over-zealous check of nvec removed.)
> > 
> > So this change to bound the number of interrupts
> > isn't needed.
> 
> Great, thanks, I was about to ask!

Likewise, thanks.
We'll look into this.