Skip to contentSkip to conversation

bnx2x: fixes

PATCH Superseded Dmitry Kravkov 2 participants 9 messages Jul 17, 2013 – Jul 18, 2013

Subsystems (3)
Networking Network drivers Broadcom BNX2X 10 gigabit Ethernet driver

v3 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Dmitry Kravkov

author
#0 lore raw

v3 cover bnx2x: fixes

Hello Dave

Please consider applying the series of bnx2x fixes to net:
* statistics may cause FW assert
* missing fairness configuration in DCB flow
* memory leak in sriov releated part
* Illegal PTE access
* Pagefault crash in shutdown flow with cnic

v1->v2
* fixed sparse error pointed by Joe Perches
* added missing signed-off from Sergei Shtylyov
v2->v3
* added missing signed-off from Sergei Shtylyov
* fixed formatting from Sergei Shtylyov
Thanks
Dmitry

author
#1 lore raw

PATCH v3 1/6 bnx2x: properly initialize statistic counters

This prevent second statistics query be sent before first one is complete. This is required since two outstanding queries may cause FW assert.

Signed-off-by Dmitry Kravkov <[email protected]>
Signed-off-by Ariel Elior <[email protected]>
Signed-off-by Eilon Greenstein <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c +5 -0
index 98366ab..7b39424 100644
@@ -1591,10 +1591,15 @@ void bnx2x_stats_init(struct bnx2x *bp)
 {
 	int /*abs*/port = BP_PORT(bp);
 	int mb_idx = BP_FW_MB_IDX(bp);
+	struct stats_counter *counters = &bp->fw_stats_data->storm_counters;
 
 	bp->stats_pending = 0;
 	bp->executer_idx = 0;
 	bp->stats_counter = 0;
+	counters->xstats_counter = cpu_to_le16(0xFFFF);
+	counters->tstats_counter = cpu_to_le16(0xFFFF);
+	counters->ustats_counter = cpu_to_le16(0xFFFF);
+	counters->cstats_counter = cpu_to_le16(0xFFFF);
 
 	/* port and func stats for management */
 	if (!BP_NOMCP(bp)) {
Signature · 1 line
1.8.1.4
maintainer
#2 lore raw

Re: [PATCH net v3 1/6] bnx2x: properly initialize statistic counters

From: "Dmitry Kravkov" <[email protected]>
Date: Thu, 18 Jul 2013 01:00:35 +0300

drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c void bnx2x_stats_init(struct bnx2x *bp) · line 1594

Quoted 4 lines

{

drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c +1 -0
 	int /*abs*/port = BP_PORT(bp);
 	int mb_idx = BP_FW_MB_IDX(bp);
+	struct stats_counter *counters = &bp->fw_stats_data->storm_counters;

Please order local variable declarations from longest to shortest line.

Thank you.

author
#3 lore raw

PATCH v3 2/6 bnx2x: prevent statistics update flow to act before statistics are started

In reply to Dmitry Kravkov

This in order to serialize statistics requests sent to FW, otherwise two outstanding queries may cause FW assert.

Signed-off-by Dmitry Kravkov <[email protected]>
Signed-off-by Ariel Elior <[email protected]>
Signed-off-by Eilon Greenstein <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x.h       | 1 +
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c  | 1 +
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c | 6 +++++-
 3 files changed, 7 insertions(+), 1 deletion(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h +1 -0
index dedbd76..bdbd87e 100644
@@ -1830,6 +1830,7 @@ struct bnx2x {
 
 	int fp_array_size;
 	u32 dump_preset_idx;
+	atomic_t				stats_started;
 };
 
 /* Tx queues may be less or equal to Rx queues */
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +1 -0
index e5da078..7e9681f 100644
@@ -11524,6 +11524,7 @@ static int bnx2x_init_bp(struct bnx2x *bp)
 	mutex_init(&bp->port.phy_mutex);
 	mutex_init(&bp->fw_mb_mutex);
 	spin_lock_init(&bp->stats_lock);
+	atomic_set(&bp->stats_started, 0);
 
 	INIT_DELAYED_WORK(&bp->sp_task, bnx2x_sp_task);
 	INIT_DELAYED_WORK(&bp->sp_rtnl_task, bnx2x_sp_rtnl_task);
drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c +5 -1
index 7b39424..b117afe 100644
@@ -534,6 +534,8 @@ static void bnx2x_stats_start(struct bnx2x *bp)
 
 	bnx2x_hw_stats_post(bp);
 	bnx2x_storm_stats_post(bp);
+
+	atomic_set(&bp->stats_started, 1);
 }
 
 static void bnx2x_stats_pmf_start(struct bnx2x *bp)

@@ -1227,7 +1229,7 @@ static void bnx2x_stats_update(struct bnx2x *bp)
 {
 	u32 *stats_comp = bnx2x_sp(bp, stats_comp);
 
-	if (bnx2x_edebug_stats_stopped(bp))
+	if (bnx2x_edebug_stats_stopped(bp) || !atomic_read(&bp->stats_started))
 		return;
 
 	if (IS_PF(bp)) {

@@ -1332,6 +1334,8 @@ static void bnx2x_stats_stop(struct bnx2x *bp)
 {
 	int update = 0;
 
+	atomic_set(&bp->stats_started, 0);
+
 	bnx2x_stats_comp(bp);
 
 	if (bp->port.pmf)
Signature · 1 line
1.8.1.4
maintainer
#4 lore raw

Re: [PATCH net v3 2/6] bnx2x: prevent statistics update flow to act before statistics are started

From: "Dmitry Kravkov" <[email protected]>
Date: Thu, 18 Jul 2013 01:00:36 +0300

drivers/net/ethernet/broadcom/bnx2x/bnx2x.h struct bnx2x { · line 1833

Quoted 5 lines
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h +1 -0
@@ -1830,6 +1830,7 @@ struct bnx2x {
 
 	int fp_array_size;
 	u32 dump_preset_idx;
+	atomic_t				stats_started;

The only uses of those stats_started member are with atomic_set() and atomic_read(), neither of which are atomic operations.

If there is existing locking which protects this value, then a simple "bool stats_started;" would work.

Otherwise, you'll need to use an "unsigned long" set of atomic bits and operations such as test_and_set_bit().

author
#5 lore raw

PATCH v3 3/6 bnx2x: update fairness parameters following DCB negotiation

In reply to Dmitry Kravkov

ETS can be enabled as a result of DCB negotiation, then fairness must be recalculated after each negotiation.

Signed-off-by Dmitry Kravkov <[email protected]>
Signed-off-by Ariel Elior <[email protected]>
Signed-off-by Eilon Greenstein <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x.h      |  2 ++
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c  |  4 ++++
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 30 ++++++++++++++----------
 3 files changed, 24 insertions(+), 12 deletions(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h +2 -0
index bdbd87e..02f914d 100644
@@ -2452,4 +2452,6 @@ enum bnx2x_pci_bus_speed {
 	BNX2X_PCI_LINK_SPEED_5000 = 5000,
 	BNX2X_PCI_LINK_SPEED_8000 = 8000
 };
+
+void bnx2x_set_local_cmng(struct bnx2x *bp);
 #endif /* bnx2x.h */
drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c +4 -0
index 0c94df4..f9122f2 100644
@@ -753,6 +753,10 @@ void bnx2x_dcbx_set_params(struct bnx2x *bp, u32 state)
 		bnx2x_pfc_set_pfc(bp);
 
 		bnx2x_dcbx_update_ets_params(bp);
+
+		/* ets may affect cmng configuration: reinit it in hw */
+		bnx2x_set_local_cmng(bp);
+
 		bnx2x_dcbx_resume_hw_tx(bp);
 
 		return;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +18 -12
index 7e9681f..0f3de02 100644
@@ -2476,7 +2476,7 @@ static void bnx2x_cmng_fns_init(struct bnx2x *bp, u8 read_cfg, u8 cmng_type)
 
 	input.port_rate = bp->link_vars.line_speed;
 
-	if (cmng_type == CMNG_FNS_MINMAX) {
+	if (cmng_type == CMNG_FNS_MINMAX && input.port_rate) {
 		int vn;
 
 		/* read mf conf from shmem */

@@ -2533,6 +2533,21 @@ static void storm_memset_cmng(struct bnx2x *bp,
 	}
 }
 
+/* init cmng mode in HW according to local configuration */
+void bnx2x_set_local_cmng(struct bnx2x *bp)
+{
+	int cmng_fns = bnx2x_get_cmng_fns_mode(bp);
+
+	if (cmng_fns != CMNG_FNS_NONE) {
+		bnx2x_cmng_fns_init(bp, false, cmng_fns);
+		storm_memset_cmng(bp, &bp->cmng, BP_PORT(bp));
+	} else {
+		/* rate shaping and fairness are disabled */
+		DP(NETIF_MSG_IFUP,
+		   "single function mode without fairness\n");
+	}
+}
+
 /* This function is called upon link interrupt */
 static void bnx2x_link_attn(struct bnx2x *bp)
 {

@@ -2568,17 +2583,8 @@ static void bnx2x_link_attn(struct bnx2x *bp)
 			bnx2x_stats_handle(bp, STATS_EVENT_LINK_UP);
 	}
 
-	if (bp->link_vars.link_up && bp->link_vars.line_speed) {
-		int cmng_fns = bnx2x_get_cmng_fns_mode(bp);
-
-		if (cmng_fns != CMNG_FNS_NONE) {
-			bnx2x_cmng_fns_init(bp, false, cmng_fns);
-			storm_memset_cmng(bp, &bp->cmng, BP_PORT(bp));
-		} else
-			/* rate shaping and fairness are disabled */
-			DP(NETIF_MSG_IFUP,
-			   "single function mode without fairness\n");
-	}
+	if (bp->link_vars.link_up && bp->link_vars.line_speed)
+		bnx2x_set_local_cmng(bp);
 
 	__bnx2x_link_report(bp);
Signature · 1 line
1.8.1.4
author
#6 lore raw

PATCH v3 4/6 bnx2x: fix memory leak in VF

In reply to Dmitry Kravkov

From: Ariel Elior <[email protected]>

Signed-off-by Ariel Elior <[email protected]>
Signed-off-by Dmitry Kravkov <[email protected]>
Signed-off-by Eilon Greenstein <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c +1 -1
index 95861ef..44104fb 100644
@@ -3463,7 +3463,7 @@ int bnx2x_vf_pci_alloc(struct bnx2x *bp)
 alloc_mem_err:
 	BNX2X_PCI_FREE(bp->vf2pf_mbox, bp->vf2pf_mbox_mapping,
 		       sizeof(struct bnx2x_vf_mbx_msg));
-	BNX2X_PCI_FREE(bp->vf2pf_mbox, bp->vf2pf_mbox_mapping,
+	BNX2X_PCI_FREE(bp->vf2pf_mbox, bp->pf2vf_bulletin_mapping,
 		       sizeof(union pf_vf_bulletin));
 	return -ENOMEM;
 }
Signature · 1 line
1.8.1.4
author
#7 lore raw

PATCH v3 5/6 bnx2x: fix PTE write access error

In reply to Dmitry Kravkov

From: Barak Witkowsky <[email protected]>

PTE write access error might occur in MF_ALLOWED mode when IOMMU is active. The patch adds rmmod HSI indicating to MFW to stop running queries which might trigger this failure.

Signed-off-by Barak Witkowsky <[email protected]>
Signed-off-by Dmitry Kravkov <[email protected]>
Signed-off-by Ariel Elior <[email protected]>
Signed-off-by Eilon Greenstein <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x.h      | 1 +
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h  | 5 +++++
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 9 +++++++++
 3 files changed, 15 insertions(+)
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h +1 -0
index 02f914d..906c7d9 100644
@@ -1502,6 +1502,7 @@ struct bnx2x {
 #define BC_SUPPORTS_DCBX_MSG_NON_PMF	(1 << 21)
 #define IS_VF_FLAG			(1 << 22)
 #define INTERRUPTS_ENABLED_FLAG		(1 << 23)
+#define BC_SUPPORTS_RMMOD_CMD		(1 << 24)
 
 #define BP_NOMCP(bp)			((bp)->flags & NO_MCP_FLAG)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h +5 -0
index 5018e52..32767f6 100644
@@ -1300,6 +1300,9 @@ struct drv_func_mb {
 
 	#define DRV_MSG_CODE_EEE_RESULTS_ACK            0xda000000
 
+	#define DRV_MSG_CODE_RMMOD                      0xdb000000
+	#define REQ_BC_VER_4_RMMOD_CMD                  0x0007080f
+
 	#define DRV_MSG_CODE_SET_MF_BW                  0xe0000000
 	#define REQ_BC_VER_4_SET_MF_BW                  0x00060202
 	#define DRV_MSG_CODE_SET_MF_BW_ACK              0xe1000000

@@ -1372,6 +1375,8 @@ struct drv_func_mb {
 
 	#define FW_MSG_CODE_EEE_RESULS_ACK              0xda100000
 
+	#define FW_MSG_CODE_RMMOD_ACK                   0xdb100000
+
 	#define FW_MSG_CODE_SET_MF_BW_SENT              0xe0000000
 	#define FW_MSG_CODE_SET_MF_BW_DONE              0xe1000000
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +9 -0
index 0f3de02..9394b7d 100644
@@ -10368,6 +10368,10 @@ static void bnx2x_get_common_hwinfo(struct bnx2x *bp)
 
 	bp->flags |= (val >= REQ_BC_VER_4_DCBX_ADMIN_MSG_NON_PMF) ?
 			BC_SUPPORTS_DCBX_MSG_NON_PMF : 0;
+
+	bp->flags |= (val >= REQ_BC_VER_4_RMMOD_CMD) ?
+			BC_SUPPORTS_RMMOD_CMD : 0;
+
 	boot_mode = SHMEM_RD(bp,
 			dev_info.port_feature_config[BP_PORT(bp)].mba_config) &
 			PORT_FEATURE_MBA_BOOT_AGENT_TYPE_MASK;

@@ -12824,6 +12828,11 @@ static void __bnx2x_remove(struct pci_dev *pdev,
 	bnx2x_dcbnl_update_applist(bp, true);
 #endif
 
+	if (IS_PF(bp) &&
+	    !BP_NOMCP(bp) &&
+	    (bp->flags & BC_SUPPORTS_RMMOD_CMD))
+		bnx2x_fw_command(bp, DRV_MSG_CODE_RMMOD, 0);
+
 	/* Close the interface - either directly or implicitly */
 	if (remove_netdev) {
 		unregister_netdev(dev);
Signature · 1 line
1.8.1.4
author
#8 lore raw

PATCH v3 6/6 bnx2x: prevent crash in shutdown flow with CNIC

In reply to Dmitry Kravkov

From: Yuval Mintz <[email protected]>

There might be a crash as during shutdown flow CNIC might try to access resources already freed by bnx2x.
Change bnx2x_close() into dev_close() in __bnx2x_remove (shutdown flow) to guarantee CNIC is notified of the device's change of status.

Signed-off-by Yuval Mintz <[email protected]>
Signed-off-by Dmitry Kravkov <[email protected]>
Signed-off-by Ariel Elior <[email protected]>
Signed-off-by Eilon Greenstein <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +1 -2
index 9394b7d..2fdc8e6 100644
@@ -12838,8 +12838,7 @@ static void __bnx2x_remove(struct pci_dev *pdev,
 		unregister_netdev(dev);
 	} else {
 		rtnl_lock();
-		if (netif_running(dev))
-			bnx2x_close(dev);
+		dev_close(dev);
 		rtnl_unlock();
 	}
Signature · 1 line
1.8.1.4