* [PATCH net 0/2] bnx2x: bug fixes
@ 2019-12-10 21:56 Manish Chopra
2019-12-10 21:56 ` [PATCH net 1/2] bnx2x: Do not handle requests from VFs after parity Manish Chopra
2019-12-10 21:56 ` [PATCH net 2/2] bnx2x: Fix logic to get total no. of PFs per engine Manish Chopra
0 siblings, 2 replies; 4+ messages in thread
From: Manish Chopra @ 2019-12-10 21:56 UTC (permalink / raw)
To: davem; +Cc: netdev, aelior, skalluru
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH net 1/2] bnx2x: Do not handle requests from VFs after parity
2019-12-10 21:56 [PATCH net 0/2] bnx2x: bug fixes Manish Chopra
@ 2019-12-10 21:56 ` Manish Chopra
2019-12-11 11:54 ` kbuild test robot
2019-12-10 21:56 ` [PATCH net 2/2] bnx2x: Fix logic to get total no. of PFs per engine Manish Chopra
1 sibling, 1 reply; 4+ messages in thread
From: Manish Chopra @ 2019-12-10 21:56 UTC (permalink / raw)
To: davem; +Cc: netdev, aelior, skalluru
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH net 2/2] bnx2x: Fix logic to get total no. of PFs per engine
2019-12-10 21:56 [PATCH net 0/2] bnx2x: bug fixes Manish Chopra
2019-12-10 21:56 ` [PATCH net 1/2] bnx2x: Do not handle requests from VFs after parity Manish Chopra
@ 2019-12-10 21:56 ` Manish Chopra
1 sibling, 0 replies; 4+ messages in thread
From: Manish Chopra @ 2019-12-10 21:56 UTC (permalink / raw)
To: davem; +Cc: netdev, aelior, skalluru
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net 1/2] bnx2x: Do not handle requests from VFs after parity
2019-12-10 21:56 ` [PATCH net 1/2] bnx2x: Do not handle requests from VFs after parity Manish Chopra
@ 2019-12-11 11:54 ` kbuild test robot
0 siblings, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2019-12-11 11:54 UTC (permalink / raw)
To: Manish Chopra; +Cc: kbuild-all, davem, netdev, aelior, skalluru
[-- 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 --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-12-11 11:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-10 21:56 [PATCH net 0/2] bnx2x: bug fixes Manish Chopra
2019-12-10 21:56 ` [PATCH net 1/2] bnx2x: Do not handle requests from VFs after parity Manish Chopra
2019-12-11 11:54 ` kbuild test robot
2019-12-10 21:56 ` [PATCH net 2/2] bnx2x: Fix logic to get total no. of PFs per engine Manish Chopra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).