Re: [PATCH] bnx2: Enable MSI-X even if the amount of vectors is smaller than BNX2_MAX_MSIX_VEC

Re: [PATCH] bnx2: Enable MSI-X even if the amount of vectors is smaller than BNX2_MAX_MSIX_VEC

[Michael Chan]

On Tue, 2010-07-13 at 13:42 -0700, leitao@linux.vnet.ibm.com wrote:
> First of all, the function bnx2_enable_msix() has the msix_vecs parameter,
> but it's not used to request the amount of vectors. Currently the drivers is
> always requesting BNX2_MAX_MSIX_VEC vectors, even when the msix_vecs is
> different from it.

The chip was originally designed to work in MSI-X mode only if all
vectors have been configured, that's why it was coded this way.
However, there is a register (BNX2_HC_MSIX_BIT_VECTOR) that we program
to make the chip think that all vectors are configured.  We always
program 0x1ff instead of the actual number of vectors, so it should work
with any number of vectors.  I'd like to do more testing first.

We need to at least allocate one more vector for the cnic driver.  I'll
fix that and send a revised and tested patch.

Thanks.

> 
> Also, if the amount of available vectors are smaller than BNX2_MAX_MSIX_VEC,
> the MSI-X is disabled, which is something bad.
> 
> So, this patch tries to fix both issues. Now if the available amount of vectors
> is smaller than the requested amount, the driver re-request the current available
> amount and proceed with these vectors.
> 
> Signed-off-by: Breno Leitão <leitao@linux.vnet.ibm.com>
> ---
>  drivers/net/bnx2.c |   16 +++++++++-------
>  1 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> index 1174322..70ee664 100644
> --- a/drivers/net/bnx2.c
> +++ b/drivers/net/bnx2.c
> @@ -860,7 +860,7 @@ bnx2_alloc_mem(struct bnx2 *bp)
>  	bnapi->hw_rx_cons_ptr =
>  		&bnapi->status_blk.msi->status_rx_quick_consumer_index0;
>  	if (bp->flags & BNX2_FLAG_MSIX_CAP) {
> -		for (i = 1; i < BNX2_MAX_MSIX_VEC; i++) {
> +		for (i = 1; i < bp->irq_nvecs; i++) {
>  			struct status_block_msix *sblk;
>  
>  			bnapi = &bp->bnx2_napi[i];
> @@ -4886,7 +4886,7 @@ bnx2_init_chip(struct bnx2 *bp)
>  	bnx2_reg_wr_ind(bp, BNX2_RBUF_CONFIG3, BNX2_RBUF_CONFIG3_VAL(mtu));
>  
>  	memset(bp->bnx2_napi[0].status_blk.msi, 0, bp->status_stats_size);
> -	for (i = 0; i < BNX2_MAX_MSIX_VEC; i++)
> +	for (i = 0; i < bp->irq_nvecs; i++)
>  		bp->bnx2_napi[i].last_status_idx = 0;
>  
>  	bp->idle_chk_status_idx = 0xffff;
> @@ -5007,7 +5007,7 @@ bnx2_clear_ring_states(struct bnx2 *bp)
>  	struct bnx2_rx_ring_info *rxr;
>  	int i;
>  
> -	for (i = 0; i < BNX2_MAX_MSIX_VEC; i++) {
> +	for (i = 0; i < bp->irq_nvecs; i++) {
>  		bnapi = &bp->bnx2_napi[i];
>  		txr = &bnapi->tx_ring;
>  		rxr = &bnapi->rx_ring;
> @@ -6148,13 +6148,15 @@ bnx2_enable_msix(struct bnx2 *bp, int msix_vecs)
>  		msix_ent[i].vector = 0;
>  	}
>  
> -	rc = pci_enable_msix(bp->pdev, msix_ent, BNX2_MAX_MSIX_VEC);
> -	if (rc != 0)
> -		return;
> +	do {
> +		rc = pci_enable_msix(bp->pdev, msix_ent, msix_vecs);
> +		if (rc > 0)
> +			msix_vecs = rc;
> +	} while (rc > 0);
>  
>  	bp->irq_nvecs = msix_vecs;
>  	bp->flags |= BNX2_FLAG_USING_MSIX | BNX2_FLAG_ONE_SHOT_MSI;
> -	for (i = 0; i < BNX2_MAX_MSIX_VEC; i++) {
> +	for (i = 0; i < msix_vecs; i++) {
>  		bp->irq_tbl[i].vector = msix_ent[i].vector;
>  		snprintf(bp->irq_tbl[i].name, len, "%s-%d", dev->name, i);
>  		bp->irq_tbl[i].handler = bnx2_msi_1shot;

[PATCH] bnx2: Enable MSI-X even if the amount of vectors is smaller than BNX2_MAX_MSIX_VEC

[leitao]

First of all, the function bnx2_enable_msix() has the msix_vecs parameter,
but it's not used to request the amount of vectors. Currently the drivers is
always requesting BNX2_MAX_MSIX_VEC vectors, even when the msix_vecs is
different from it.

Also, if the amount of available vectors are smaller than BNX2_MAX_MSIX_VEC,
the MSI-X is disabled, which is something bad.

So, this patch tries to fix both issues. Now if the available amount of vectors
is smaller than the requested amount, the driver re-request the current available
amount and proceed with these vectors.

Signed-off-by: Breno Leitão <leitao@linux.vnet.ibm.com>
---
 drivers/net/bnx2.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 1174322..70ee664 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -860,7 +860,7 @@ bnx2_alloc_mem(struct bnx2 *bp)
 	bnapi->hw_rx_cons_ptr =
 		&bnapi->status_blk.msi->status_rx_quick_consumer_index0;
 	if (bp->flags & BNX2_FLAG_MSIX_CAP) {
-		for (i = 1; i < BNX2_MAX_MSIX_VEC; i++) {
+		for (i = 1; i < bp->irq_nvecs; i++) {
 			struct status_block_msix *sblk;
 
 			bnapi = &bp->bnx2_napi[i];
@@ -4886,7 +4886,7 @@ bnx2_init_chip(struct bnx2 *bp)
 	bnx2_reg_wr_ind(bp, BNX2_RBUF_CONFIG3, BNX2_RBUF_CONFIG3_VAL(mtu));
 
 	memset(bp->bnx2_napi[0].status_blk.msi, 0, bp->status_stats_size);
-	for (i = 0; i < BNX2_MAX_MSIX_VEC; i++)
+	for (i = 0; i < bp->irq_nvecs; i++)
 		bp->bnx2_napi[i].last_status_idx = 0;
 
 	bp->idle_chk_status_idx = 0xffff;
@@ -5007,7 +5007,7 @@ bnx2_clear_ring_states(struct bnx2 *bp)
 	struct bnx2_rx_ring_info *rxr;
 	int i;
 
-	for (i = 0; i < BNX2_MAX_MSIX_VEC; i++) {
+	for (i = 0; i < bp->irq_nvecs; i++) {
 		bnapi = &bp->bnx2_napi[i];
 		txr = &bnapi->tx_ring;
 		rxr = &bnapi->rx_ring;
@@ -6148,13 +6148,15 @@ bnx2_enable_msix(struct bnx2 *bp, int msix_vecs)
 		msix_ent[i].vector = 0;
 	}
 
-	rc = pci_enable_msix(bp->pdev, msix_ent, BNX2_MAX_MSIX_VEC);
-	if (rc != 0)
-		return;
+	do {
+		rc = pci_enable_msix(bp->pdev, msix_ent, msix_vecs);
+		if (rc > 0)
+			msix_vecs = rc;
+	} while (rc > 0);
 
 	bp->irq_nvecs = msix_vecs;
 	bp->flags |= BNX2_FLAG_USING_MSIX | BNX2_FLAG_ONE_SHOT_MSI;
-	for (i = 0; i < BNX2_MAX_MSIX_VEC; i++) {
+	for (i = 0; i < msix_vecs; i++) {
 		bp->irq_tbl[i].vector = msix_ent[i].vector;
 		snprintf(bp->irq_tbl[i].name, len, "%s-%d", dev->name, i);
 		bp->irq_tbl[i].handler = bnx2_msi_1shot;
-- 
1.6.0.2