[PATCH net 0/2] bnx2x: bug fixes

[PATCH net 0/2] bnx2x: bug fixes

[Manish Chopra]

Hi David,

This series has two driver changes, one to fix some unexpected
hardware behaviour casued during the parity error recovery in
presence of SR-IOV VFs and another one related for fixing resource
management in the driver among the PFs configured on an engine.

Please consider applying it to "net".

Thanks,
Manish

Manish Chopra (2):
  bnx2x: Do not handle requests from VFs after parity
  bnx2x: Fix logic to get total no. of PFs per engine

 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h   |  2 +-
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c  | 11 +++++++++--
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h |  1 +
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c  | 12 ++++++++++++
 4 files changed, 23 insertions(+), 3 deletions(-)

-- 
2.18.1

[PATCH net 1/2] bnx2x: Do not handle requests from VFs after parity

[Manish Chopra]

Parity error from the hardware will cause PF to lose the state
of their VFs due to PF's internal reload and hardware reset following
the parity error. Restrict any configuration request from the VFs after
the parity as it could cause unexpected hardware behavior, only way
for VFs to recover would be to trigger FLR on VFs and reload them.

Signed-off-by: Manish Chopra <manishc@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c  | 11 +++++++++--
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h |  1 +
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c  | 12 ++++++++++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 192ff8d5da32..119e5135f090 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -9976,9 +9976,16 @@ static void bnx2x_recovery_failed(struct bnx2x *bp)
  */
 static void bnx2x_parity_recover(struct bnx2x *bp)
 {
-	bool global = false;
 	u32 error_recovered, error_unrecovered;
-	bool is_parity;
+	bool is_parity, global = false;
+	int vf_idx;
+
+	for (vf_idx = 0; vf_idx < bp->requested_nr_virtfn; vf_idx++) {
+		struct bnx2x_virtf *vf = BP_VF(bp, vf_idx);
+
+		if (vf)
+			vf->state = VF_LOST;
+	}
 
 	DP(NETIF_MSG_HW, "Handling parity\n");
 	while (1) {
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h
index b6ebd92ec565..3a716c015415 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h
@@ -139,6 +139,7 @@ struct bnx2x_virtf {
 #define VF_ACQUIRED	1	/* VF acquired, but not initialized */
 #define VF_ENABLED	2	/* VF Enabled */
 #define VF_RESET	3	/* VF FLR'd, pending cleanup */
+#define VF_LOST		4	/* Recovery while VFs are loaded */
 
 	bool flr_clnup_stage;	/* true during flr cleanup */
 	bool malicious;		/* true if FW indicated so, until FLR */
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
index 0752b7fa4d9c..ea0e9394f898 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
@@ -2107,6 +2107,18 @@ static void bnx2x_vf_mbx_request(struct bnx2x *bp, struct bnx2x_virtf *vf,
 {
 	int i;
 
+	if (vf->state == VF_LOST) {
+		/* Just ack the FW and return if VFs are lost
+		 * in case of parity error. VFs are supposed to be timedout
+		 * on waiting for PF response.
+		 */
+		DP(BNX2X_MSG_IOV,
+		   "VF 0x%x lost, not handling the request\n", vf->abs_vfid);
+
+		storm_memset_vf_mbx_ack(bp, vf->abs_vfid);
+		return;
+	}
+
 	/* check if tlv type is known */
 	if (bnx2x_tlv_supported(mbx->first_tlv.tl.type)) {
 		/* Lock the per vf op mutex and note the locker's identity.
-- 
2.18.1

Re: [PATCH net 1/2] bnx2x: Do not handle requests from VFs after parity

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 8060 bytes --]

Hi Manish,

I love your patch! Yet something to improve:

[auto build test ERROR on net/master]
[cannot apply to net-next/master sparc-next/master v5.5-rc1 next-20191210]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Manish-Chopra/bnx2x-bug-fixes/20191211-112633
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 24dee0c7478d1a1e00abdf5625b7f921467325dc
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c: In function 'bnx2x_parity_recover':
>> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:9983:30: error: 'struct bnx2x' has no member named 'requested_nr_virtfn'
     for (vf_idx = 0; vf_idx < bp->requested_nr_virtfn; vf_idx++) {
                                 ^~
>> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:9984:28: error: implicit declaration of function 'BP_VF'; did you mean 'BP_VN'? [-Werror=implicit-function-declaration]
      struct bnx2x_virtf *vf = BP_VF(bp, vf_idx);
                               ^~~~~
                               BP_VN
>> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:9984:28: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
>> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:9987:6: error: dereferencing pointer to incomplete type 'struct bnx2x_virtf'
       vf->state = VF_LOST;
         ^~
>> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:9987:16: error: 'VF_LOST' undeclared (first use in this function); did you mean 'AF_ROSE'?
       vf->state = VF_LOST;
                   ^~~~~~~
                   AF_ROSE
   drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:9987:16: note: each undeclared identifier is reported only once for each function it appears in
   cc1: some warnings being treated as errors

vim +9983 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c

  9971	
  9972	/*
  9973	 * Assumption: runs under rtnl lock. This together with the fact
  9974	 * that it's called only from bnx2x_sp_rtnl() ensure that it
  9975	 * will never be called when netif_running(bp->dev) is false.
  9976	 */
  9977	static void bnx2x_parity_recover(struct bnx2x *bp)
  9978	{
  9979		u32 error_recovered, error_unrecovered;
  9980		bool is_parity, global = false;
  9981		int vf_idx;
  9982	
> 9983		for (vf_idx = 0; vf_idx < bp->requested_nr_virtfn; vf_idx++) {
> 9984			struct bnx2x_virtf *vf = BP_VF(bp, vf_idx);
  9985	
  9986			if (vf)
> 9987				vf->state = VF_LOST;
  9988		}
  9989	
  9990		DP(NETIF_MSG_HW, "Handling parity\n");
  9991		while (1) {
  9992			switch (bp->recovery_state) {
  9993			case BNX2X_RECOVERY_INIT:
  9994				DP(NETIF_MSG_HW, "State is BNX2X_RECOVERY_INIT\n");
  9995				is_parity = bnx2x_chk_parity_attn(bp, &global, false);
  9996				WARN_ON(!is_parity);
  9997	
  9998				/* Try to get a LEADER_LOCK HW lock */
  9999				if (bnx2x_trylock_leader_lock(bp)) {
 10000					bnx2x_set_reset_in_progress(bp);
 10001					/*
 10002					 * Check if there is a global attention and if
 10003					 * there was a global attention, set the global
 10004					 * reset bit.
 10005					 */
 10006	
 10007					if (global)
 10008						bnx2x_set_reset_global(bp);
 10009	
 10010					bp->is_leader = 1;
 10011				}
 10012	
 10013				/* Stop the driver */
 10014				/* If interface has been removed - break */
 10015				if (bnx2x_nic_unload(bp, UNLOAD_RECOVERY, false))
 10016					return;
 10017	
 10018				bp->recovery_state = BNX2X_RECOVERY_WAIT;
 10019	
 10020				/* Ensure "is_leader", MCP command sequence and
 10021				 * "recovery_state" update values are seen on other
 10022				 * CPUs.
 10023				 */
 10024				smp_mb();
 10025				break;
 10026	
 10027			case BNX2X_RECOVERY_WAIT:
 10028				DP(NETIF_MSG_HW, "State is BNX2X_RECOVERY_WAIT\n");
 10029				if (bp->is_leader) {
 10030					int other_engine = BP_PATH(bp) ? 0 : 1;
 10031					bool other_load_status =
 10032						bnx2x_get_load_status(bp, other_engine);
 10033					bool load_status =
 10034						bnx2x_get_load_status(bp, BP_PATH(bp));
 10035					global = bnx2x_reset_is_global(bp);
 10036	
 10037					/*
 10038					 * In case of a parity in a global block, let
 10039					 * the first leader that performs a
 10040					 * leader_reset() reset the global blocks in
 10041					 * order to clear global attentions. Otherwise
 10042					 * the gates will remain closed for that
 10043					 * engine.
 10044					 */
 10045					if (load_status ||
 10046					    (global && other_load_status)) {
 10047						/* Wait until all other functions get
 10048						 * down.
 10049						 */
 10050						schedule_delayed_work(&bp->sp_rtnl_task,
 10051									HZ/10);
 10052						return;
 10053					} else {
 10054						/* If all other functions got down -
 10055						 * try to bring the chip back to
 10056						 * normal. In any case it's an exit
 10057						 * point for a leader.
 10058						 */
 10059						if (bnx2x_leader_reset(bp)) {
 10060							bnx2x_recovery_failed(bp);
 10061							return;
 10062						}
 10063	
 10064						/* If we are here, means that the
 10065						 * leader has succeeded and doesn't
 10066						 * want to be a leader any more. Try
 10067						 * to continue as a none-leader.
 10068						 */
 10069						break;
 10070					}
 10071				} else { /* non-leader */
 10072					if (!bnx2x_reset_is_done(bp, BP_PATH(bp))) {
 10073						/* Try to get a LEADER_LOCK HW lock as
 10074						 * long as a former leader may have
 10075						 * been unloaded by the user or
 10076						 * released a leadership by another
 10077						 * reason.
 10078						 */
 10079						if (bnx2x_trylock_leader_lock(bp)) {
 10080							/* I'm a leader now! Restart a
 10081							 * switch case.
 10082							 */
 10083							bp->is_leader = 1;
 10084							break;
 10085						}
 10086	
 10087						schedule_delayed_work(&bp->sp_rtnl_task,
 10088									HZ/10);
 10089						return;
 10090	
 10091					} else {
 10092						/*
 10093						 * If there was a global attention, wait
 10094						 * for it to be cleared.
 10095						 */
 10096						if (bnx2x_reset_is_global(bp)) {
 10097							schedule_delayed_work(
 10098								&bp->sp_rtnl_task,
 10099								HZ/10);
 10100							return;
 10101						}
 10102	
 10103						error_recovered =
 10104						  bp->eth_stats.recoverable_error;
 10105						error_unrecovered =
 10106						  bp->eth_stats.unrecoverable_error;
 10107						bp->recovery_state =
 10108							BNX2X_RECOVERY_NIC_LOADING;
 10109						if (bnx2x_nic_load(bp, LOAD_NORMAL)) {
 10110							error_unrecovered++;
 10111							netdev_err(bp->dev,
 10112								   "Recovery failed. Power cycle needed\n");
 10113							/* Disconnect this device */
 10114							netif_device_detach(bp->dev);
 10115							/* Shut down the power */
 10116							bnx2x_set_power_state(
 10117								bp, PCI_D3hot);
 10118							smp_mb();
 10119						} else {
 10120							bp->recovery_state =
 10121								BNX2X_RECOVERY_DONE;
 10122							error_recovered++;
 10123							smp_mb();
 10124						}
 10125						bp->eth_stats.recoverable_error =
 10126							error_recovered;
 10127						bp->eth_stats.unrecoverable_error =
 10128							error_unrecovered;
 10129	
 10130						return;
 10131					}
 10132				}
 10133			default:
 10134				return;
 10135			}
 10136		}
 10137	}
 10138	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25662 bytes --]

[PATCH net 2/2] bnx2x: Fix logic to get total no. of PFs per engine

[Manish Chopra]

Driver doesn't calculate total number of PFs configured on a
given engine correctly which messed up resources in the PFs
loaded on that engine, leading driver to exceed configuration
of resources (like vlan filters etc.) beyond the limit per
engine, which ended up with asserts from the firmware.

Signed-off-by: Manish Chopra <manishc@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
index 8b08cb18e363..3f63ffd7561b 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
@@ -1109,7 +1109,7 @@ static inline u8 bnx2x_get_path_func_num(struct bnx2x *bp)
 		for (i = 0; i < E1H_FUNC_MAX / 2; i++) {
 			u32 func_config =
 				MF_CFG_RD(bp,
-					  func_mf_config[BP_PORT(bp) + 2 * i].
+					  func_mf_config[BP_PATH(bp) + 2 * i].
 					  config);
 			func_num +=
 				((func_config & FUNC_MF_CFG_FUNC_HIDE) ? 0 : 1);
-- 
2.18.1