[PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Manish Chopra]

Commit 0050dcf3e848 ("bnx2x: Add FW 7.13.20.0") added a new .bin
firmware file to linux-firmware.git tree. This new firmware addresses
few important issues and enhancements as mentioned below -

- Support direct invalidation of FP HSI Ver per function ID, required for
  invalidating FP HSI Ver prior to each VF start, as there is no VF start
- BRB hardware block parity error detection support for the driver
- Fix the FCOE underrun flow
- Fix PSOD during FCoE BFS over the NIC ports after preboot driver

This patch incorporates this new firmware 7.13.20.0 in bnx2x driver.

Signed-off-by: Manish Chopra <manishc@marvell.com>
Signed-off-by: Shai Malin <smalin@marvell.com>
Signed-off-by: Omkar Kulkarni <okulkarni@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 .../ethernet/broadcom/bnx2x/bnx2x_fw_defs.h   | 148 ++++++++++--------
 .../net/ethernet/broadcom/bnx2x/bnx2x_hsi.h   |  13 +-
 2 files changed, 91 insertions(+), 70 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_fw_defs.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_fw_defs.h
index 3f8435208bf4..5ee46c4234fb 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_fw_defs.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_fw_defs.h
@@ -32,31 +32,34 @@
 	* IRO[142].m2) + ((sbId) * IRO[142].m3))
 #define CSTORM_IGU_MODE_OFFSET (IRO[161].base)
 #define CSTORM_ISCSI_CQ_SIZE_OFFSET(pfId) \
-	(IRO[324].base + ((pfId) * IRO[324].m1))
-#define CSTORM_ISCSI_CQ_SQN_SIZE_OFFSET(pfId) \
 	(IRO[325].base + ((pfId) * IRO[325].m1))
+#define CSTORM_ISCSI_CQ_SQN_SIZE_OFFSET(pfId) \
+	(IRO[326].base + ((pfId) * IRO[326].m1))
 #define CSTORM_ISCSI_EQ_CONS_OFFSET(pfId, iscsiEqId) \
-	(IRO[317].base + ((pfId) * IRO[317].m1) + ((iscsiEqId) * IRO[317].m2))
+	(IRO[318].base + ((pfId) * IRO[318].m1) + ((iscsiEqId) * IRO[318].m2))
 #define CSTORM_ISCSI_EQ_NEXT_EQE_ADDR_OFFSET(pfId, iscsiEqId) \
-	(IRO[319].base + ((pfId) * IRO[319].m1) + ((iscsiEqId) * IRO[319].m2))
+	(IRO[320].base + ((pfId) * IRO[320].m1) + ((iscsiEqId) * IRO[320].m2))
 #define CSTORM_ISCSI_EQ_NEXT_PAGE_ADDR_OFFSET(pfId, iscsiEqId) \
-	(IRO[318].base + ((pfId) * IRO[318].m1) + ((iscsiEqId) * IRO[318].m2))
+	(IRO[319].base + ((pfId) * IRO[319].m1) + ((iscsiEqId) * IRO[319].m2))
+
 #define CSTORM_ISCSI_EQ_NEXT_PAGE_ADDR_VALID_OFFSET(pfId, iscsiEqId) \
-	(IRO[320].base + ((pfId) * IRO[320].m1) + ((iscsiEqId) * IRO[320].m2))
+	(IRO[321].base + ((pfId) * IRO[321].m1) + ((iscsiEqId) * IRO[321].m2))
 #define CSTORM_ISCSI_EQ_PROD_OFFSET(pfId, iscsiEqId) \
-	(IRO[316].base + ((pfId) * IRO[316].m1) + ((iscsiEqId) * IRO[316].m2))
+	(IRO[317].base + ((pfId) * IRO[317].m1) + ((iscsiEqId) * IRO[317].m2))
 #define CSTORM_ISCSI_EQ_SB_INDEX_OFFSET(pfId, iscsiEqId) \
-	(IRO[322].base + ((pfId) * IRO[322].m1) + ((iscsiEqId) * IRO[322].m2))
+	(IRO[323].base + ((pfId) * IRO[323].m1) + ((iscsiEqId) * IRO[323].m2))
 #define CSTORM_ISCSI_EQ_SB_NUM_OFFSET(pfId, iscsiEqId) \
-	(IRO[321].base + ((pfId) * IRO[321].m1) + ((iscsiEqId) * IRO[321].m2))
+	(IRO[322].base + ((pfId) * IRO[322].m1) + ((iscsiEqId) * IRO[322].m2))
+
 #define CSTORM_ISCSI_HQ_SIZE_OFFSET(pfId) \
-	(IRO[323].base + ((pfId) * IRO[323].m1))
+	(IRO[324].base + ((pfId) * IRO[324].m1))
 #define CSTORM_ISCSI_NUM_OF_TASKS_OFFSET(pfId) \
-	(IRO[315].base + ((pfId) * IRO[315].m1))
+	(IRO[316].base + ((pfId) * IRO[316].m1))
 #define CSTORM_ISCSI_PAGE_SIZE_LOG_OFFSET(pfId) \
-	(IRO[314].base + ((pfId) * IRO[314].m1))
+	(IRO[315].base + ((pfId) * IRO[315].m1))
 #define CSTORM_ISCSI_PAGE_SIZE_OFFSET(pfId) \
-	(IRO[313].base + ((pfId) * IRO[313].m1))
+	(IRO[314].base + ((pfId) * IRO[314].m1))
+
 #define CSTORM_RECORD_SLOW_PATH_OFFSET(funcId) \
 	(IRO[155].base + ((funcId) * IRO[155].m1))
 #define CSTORM_SP_STATUS_BLOCK_DATA_OFFSET(pfId) \
@@ -90,90 +93,92 @@
 #define CSTORM_VF_TO_PF_OFFSET(funcId) \
 	(IRO[154].base + ((funcId) * IRO[154].m1))
 #define TSTORM_APPROXIMATE_MATCH_MULTICAST_FILTERING_OFFSET(pfId) \
-	(IRO[207].base + ((pfId) * IRO[207].m1))
+	(IRO[208].base + ((pfId) * IRO[208].m1))
 #define TSTORM_ASSERT_LIST_INDEX_OFFSET	(IRO[102].base)
 #define TSTORM_ASSERT_LIST_OFFSET(assertListEntry) \
 	(IRO[101].base + ((assertListEntry) * IRO[101].m1))
 #define TSTORM_FUNCTION_COMMON_CONFIG_OFFSET(pfId) \
-	(IRO[205].base + ((pfId) * IRO[205].m1))
+	(IRO[206].base + ((pfId) * IRO[206].m1))
 #define TSTORM_FUNC_EN_OFFSET(funcId) \
 	(IRO[107].base + ((funcId) * IRO[107].m1))
 #define TSTORM_ISCSI_ERROR_BITMAP_OFFSET(pfId) \
-	(IRO[279].base + ((pfId) * IRO[279].m1))
-#define TSTORM_ISCSI_L2_ISCSI_OOO_CID_TABLE_OFFSET(pfId) \
 	(IRO[280].base + ((pfId) * IRO[280].m1))
-#define TSTORM_ISCSI_L2_ISCSI_OOO_CLIENT_ID_TABLE_OFFSET(pfId) \
+#define TSTORM_ISCSI_L2_ISCSI_OOO_CID_TABLE_OFFSET(pfId) \
 	(IRO[281].base + ((pfId) * IRO[281].m1))
-#define TSTORM_ISCSI_L2_ISCSI_OOO_PROD_OFFSET(pfId) \
+#define TSTORM_ISCSI_L2_ISCSI_OOO_CLIENT_ID_TABLE_OFFSET(pfId) \
 	(IRO[282].base + ((pfId) * IRO[282].m1))
+#define TSTORM_ISCSI_L2_ISCSI_OOO_PROD_OFFSET(pfId) \
+	(IRO[283].base + ((pfId) * IRO[283].m1))
 #define TSTORM_ISCSI_NUM_OF_TASKS_OFFSET(pfId) \
-	(IRO[278].base + ((pfId) * IRO[278].m1))
+	(IRO[279].base + ((pfId) * IRO[279].m1))
 #define TSTORM_ISCSI_PAGE_SIZE_LOG_OFFSET(pfId) \
-	(IRO[277].base + ((pfId) * IRO[277].m1))
+	(IRO[278].base + ((pfId) * IRO[278].m1))
 #define TSTORM_ISCSI_PAGE_SIZE_OFFSET(pfId) \
-	(IRO[276].base + ((pfId) * IRO[276].m1))
+	(IRO[277].base + ((pfId) * IRO[277].m1))
 #define TSTORM_ISCSI_RQ_SIZE_OFFSET(pfId) \
-	(IRO[275].base + ((pfId) * IRO[275].m1))
+	(IRO[276].base + ((pfId) * IRO[276].m1))
+
 #define TSTORM_ISCSI_TCP_LOCAL_ADV_WND_OFFSET(pfId) \
-	(IRO[285].base + ((pfId) * IRO[285].m1))
+	(IRO[286].base + ((pfId) * IRO[286].m1))
 #define TSTORM_ISCSI_TCP_VARS_FLAGS_OFFSET(pfId) \
-	(IRO[271].base + ((pfId) * IRO[271].m1))
-#define TSTORM_ISCSI_TCP_VARS_LSB_LOCAL_MAC_ADDR_OFFSET(pfId) \
 	(IRO[272].base + ((pfId) * IRO[272].m1))
-#define TSTORM_ISCSI_TCP_VARS_MID_LOCAL_MAC_ADDR_OFFSET(pfId) \
+#define TSTORM_ISCSI_TCP_VARS_LSB_LOCAL_MAC_ADDR_OFFSET(pfId) \
 	(IRO[273].base + ((pfId) * IRO[273].m1))
-#define TSTORM_ISCSI_TCP_VARS_MSB_LOCAL_MAC_ADDR_OFFSET(pfId) \
+#define TSTORM_ISCSI_TCP_VARS_MID_LOCAL_MAC_ADDR_OFFSET(pfId) \
 	(IRO[274].base + ((pfId) * IRO[274].m1))
+#define TSTORM_ISCSI_TCP_VARS_MSB_LOCAL_MAC_ADDR_OFFSET(pfId) \
+	(IRO[275].base + ((pfId) * IRO[275].m1))
 #define TSTORM_MAC_FILTER_CONFIG_OFFSET(pfId) \
-	(IRO[206].base + ((pfId) * IRO[206].m1))
+	(IRO[207].base + ((pfId) * IRO[207].m1))
 #define TSTORM_RECORD_SLOW_PATH_OFFSET(funcId) \
 	(IRO[109].base + ((funcId) * IRO[109].m1))
 #define TSTORM_TCP_MAX_CWND_OFFSET(pfId) \
-	(IRO[224].base + ((pfId) * IRO[224].m1))
+	(IRO[225].base + ((pfId) * IRO[225].m1))
 #define TSTORM_VF_TO_PF_OFFSET(funcId) \
 	(IRO[108].base + ((funcId) * IRO[108].m1))
-#define USTORM_AGG_DATA_OFFSET (IRO[213].base)
-#define USTORM_AGG_DATA_SIZE (IRO[213].size)
+#define USTORM_AGG_DATA_OFFSET (IRO[214].base)
+#define USTORM_AGG_DATA_SIZE (IRO[214].size)
 #define USTORM_ASSERT_LIST_INDEX_OFFSET	(IRO[181].base)
 #define USTORM_ASSERT_LIST_OFFSET(assertListEntry) \
 	(IRO[180].base + ((assertListEntry) * IRO[180].m1))
 #define USTORM_ETH_PAUSE_ENABLED_OFFSET(portId) \
 	(IRO[187].base + ((portId) * IRO[187].m1))
 #define USTORM_FCOE_EQ_PROD_OFFSET(pfId) \
-	(IRO[326].base + ((pfId) * IRO[326].m1))
+	(IRO[327].base + ((pfId) * IRO[327].m1))
 #define USTORM_FUNC_EN_OFFSET(funcId) \
 	(IRO[182].base + ((funcId) * IRO[182].m1))
 #define USTORM_ISCSI_CQ_SIZE_OFFSET(pfId) \
-	(IRO[290].base + ((pfId) * IRO[290].m1))
-#define USTORM_ISCSI_CQ_SQN_SIZE_OFFSET(pfId) \
 	(IRO[291].base + ((pfId) * IRO[291].m1))
+#define USTORM_ISCSI_CQ_SQN_SIZE_OFFSET(pfId) \
+	(IRO[292].base + ((pfId) * IRO[292].m1))
 #define USTORM_ISCSI_ERROR_BITMAP_OFFSET(pfId) \
-	(IRO[295].base + ((pfId) * IRO[295].m1))
+	(IRO[296].base + ((pfId) * IRO[296].m1))
 #define USTORM_ISCSI_GLOBAL_BUF_PHYS_ADDR_OFFSET(pfId) \
-	(IRO[292].base + ((pfId) * IRO[292].m1))
+	(IRO[293].base + ((pfId) * IRO[293].m1))
 #define USTORM_ISCSI_NUM_OF_TASKS_OFFSET(pfId) \
-	(IRO[288].base + ((pfId) * IRO[288].m1))
+	(IRO[289].base + ((pfId) * IRO[289].m1))
 #define USTORM_ISCSI_PAGE_SIZE_LOG_OFFSET(pfId) \
-	(IRO[287].base + ((pfId) * IRO[287].m1))
+	(IRO[288].base + ((pfId) * IRO[288].m1))
 #define USTORM_ISCSI_PAGE_SIZE_OFFSET(pfId) \
-	(IRO[286].base + ((pfId) * IRO[286].m1))
+	(IRO[287].base + ((pfId) * IRO[287].m1))
+
 #define USTORM_ISCSI_R2TQ_SIZE_OFFSET(pfId) \
-	(IRO[289].base + ((pfId) * IRO[289].m1))
+	(IRO[290].base + ((pfId) * IRO[290].m1))
 #define USTORM_ISCSI_RQ_BUFFER_SIZE_OFFSET(pfId) \
-	(IRO[293].base + ((pfId) * IRO[293].m1))
-#define USTORM_ISCSI_RQ_SIZE_OFFSET(pfId) \
 	(IRO[294].base + ((pfId) * IRO[294].m1))
+#define USTORM_ISCSI_RQ_SIZE_OFFSET(pfId) \
+	(IRO[295].base + ((pfId) * IRO[295].m1))
 #define USTORM_MEM_WORKAROUND_ADDRESS_OFFSET(pfId) \
 	(IRO[186].base + ((pfId) * IRO[186].m1))
 #define USTORM_RECORD_SLOW_PATH_OFFSET(funcId) \
 	(IRO[184].base + ((funcId) * IRO[184].m1))
 #define USTORM_RX_PRODS_E1X_OFFSET(portId, clientId) \
-	(IRO[216].base + ((portId) * IRO[216].m1) + ((clientId) * \
-	IRO[216].m2))
+	(IRO[217].base + ((portId) * IRO[217].m1) + ((clientId) * \
+	IRO[217].m2))
 #define USTORM_RX_PRODS_E2_OFFSET(qzoneId) \
-	(IRO[217].base + ((qzoneId) * IRO[217].m1))
-#define USTORM_TPA_BTR_OFFSET (IRO[214].base)
-#define USTORM_TPA_BTR_SIZE (IRO[214].size)
+	(IRO[218].base + ((qzoneId) * IRO[218].m1))
+#define USTORM_TPA_BTR_OFFSET (IRO[215].base)
+#define USTORM_TPA_BTR_SIZE (IRO[215].size)
 #define USTORM_VF_TO_PF_OFFSET(funcId) \
 	(IRO[183].base + ((funcId) * IRO[183].m1))
 #define XSTORM_AGG_INT_FINAL_CLEANUP_COMP_TYPE (IRO[67].base)
@@ -183,44 +188,49 @@
 	(IRO[50].base + ((assertListEntry) * IRO[50].m1))
 #define XSTORM_CMNG_PER_PORT_VARS_OFFSET(portId) \
 	(IRO[43].base + ((portId) * IRO[43].m1))
+#define XSTORM_ETH_FUNCTION_INFO_FP_HSI_VALID_E2_OFFSET(functionId) \
+	(IRO[205].base + ((functionId) * IRO[205].m1))
 #define XSTORM_FAIRNESS_PER_VN_VARS_OFFSET(pfId) \
 	(IRO[45].base + ((pfId) * IRO[45].m1))
 #define XSTORM_FUNC_EN_OFFSET(funcId) \
 	(IRO[47].base + ((funcId) * IRO[47].m1))
 #define XSTORM_ISCSI_HQ_SIZE_OFFSET(pfId) \
-	(IRO[303].base + ((pfId) * IRO[303].m1))
+	(IRO[304].base + ((pfId) * IRO[304].m1))
 #define XSTORM_ISCSI_LOCAL_MAC_ADDR0_OFFSET(pfId) \
-	(IRO[306].base + ((pfId) * IRO[306].m1))
-#define XSTORM_ISCSI_LOCAL_MAC_ADDR1_OFFSET(pfId) \
 	(IRO[307].base + ((pfId) * IRO[307].m1))
-#define XSTORM_ISCSI_LOCAL_MAC_ADDR2_OFFSET(pfId) \
+#define XSTORM_ISCSI_LOCAL_MAC_ADDR1_OFFSET(pfId) \
 	(IRO[308].base + ((pfId) * IRO[308].m1))
-#define XSTORM_ISCSI_LOCAL_MAC_ADDR3_OFFSET(pfId) \
+#define XSTORM_ISCSI_LOCAL_MAC_ADDR2_OFFSET(pfId) \
 	(IRO[309].base + ((pfId) * IRO[309].m1))
-#define XSTORM_ISCSI_LOCAL_MAC_ADDR4_OFFSET(pfId) \
+#define XSTORM_ISCSI_LOCAL_MAC_ADDR3_OFFSET(pfId) \
 	(IRO[310].base + ((pfId) * IRO[310].m1))
-#define XSTORM_ISCSI_LOCAL_MAC_ADDR5_OFFSET(pfId) \
+#define XSTORM_ISCSI_LOCAL_MAC_ADDR4_OFFSET(pfId) \
 	(IRO[311].base + ((pfId) * IRO[311].m1))
-#define XSTORM_ISCSI_LOCAL_VLAN_OFFSET(pfId) \
+#define XSTORM_ISCSI_LOCAL_MAC_ADDR5_OFFSET(pfId) \
 	(IRO[312].base + ((pfId) * IRO[312].m1))
+#define XSTORM_ISCSI_LOCAL_VLAN_OFFSET(pfId) \
+	(IRO[313].base + ((pfId) * IRO[313].m1))
 #define XSTORM_ISCSI_NUM_OF_TASKS_OFFSET(pfId) \
-	(IRO[302].base + ((pfId) * IRO[302].m1))
+	(IRO[303].base + ((pfId) * IRO[303].m1))
 #define XSTORM_ISCSI_PAGE_SIZE_LOG_OFFSET(pfId) \
-	(IRO[301].base + ((pfId) * IRO[301].m1))
+	(IRO[302].base + ((pfId) * IRO[302].m1))
 #define XSTORM_ISCSI_PAGE_SIZE_OFFSET(pfId) \
-	(IRO[300].base + ((pfId) * IRO[300].m1))
+	(IRO[301].base + ((pfId) * IRO[301].m1))
+
 #define XSTORM_ISCSI_R2TQ_SIZE_OFFSET(pfId) \
-	(IRO[305].base + ((pfId) * IRO[305].m1))
+	(IRO[306].base + ((pfId) * IRO[306].m1))
 #define XSTORM_ISCSI_SQ_SIZE_OFFSET(pfId) \
-	(IRO[304].base + ((pfId) * IRO[304].m1))
+	(IRO[305].base + ((pfId) * IRO[305].m1))
+
 #define XSTORM_ISCSI_TCP_VARS_ADV_WND_SCL_OFFSET(pfId) \
-	(IRO[299].base + ((pfId) * IRO[299].m1))
+	(IRO[300].base + ((pfId) * IRO[300].m1))
 #define XSTORM_ISCSI_TCP_VARS_FLAGS_OFFSET(pfId) \
-	(IRO[298].base + ((pfId) * IRO[298].m1))
+	(IRO[299].base + ((pfId) * IRO[299].m1))
 #define XSTORM_ISCSI_TCP_VARS_TOS_OFFSET(pfId) \
-	(IRO[297].base + ((pfId) * IRO[297].m1))
+	(IRO[298].base + ((pfId) * IRO[298].m1))
 #define XSTORM_ISCSI_TCP_VARS_TTL_OFFSET(pfId) \
-	(IRO[296].base + ((pfId) * IRO[296].m1))
+	(IRO[297].base + ((pfId) * IRO[297].m1))
+
 #define XSTORM_RATE_SHAPING_PER_VN_VARS_OFFSET(pfId) \
 	(IRO[44].base + ((pfId) * IRO[44].m1))
 #define XSTORM_RECORD_SLOW_PATH_OFFSET(funcId) \
@@ -233,12 +243,12 @@
 #define XSTORM_SPQ_PROD_OFFSET(funcId) \
 	(IRO[31].base + ((funcId) * IRO[31].m1))
 #define XSTORM_TCP_GLOBAL_DEL_ACK_COUNTER_ENABLED_OFFSET(portId) \
-	(IRO[218].base + ((portId) * IRO[218].m1))
-#define XSTORM_TCP_GLOBAL_DEL_ACK_COUNTER_MAX_COUNT_OFFSET(portId) \
 	(IRO[219].base + ((portId) * IRO[219].m1))
+#define XSTORM_TCP_GLOBAL_DEL_ACK_COUNTER_MAX_COUNT_OFFSET(portId) \
+	(IRO[220].base + ((portId) * IRO[220].m1))
 #define XSTORM_TCP_TX_SWS_TIMER_VAL_OFFSET(pfId) \
-	(IRO[221].base + (((pfId)>>1) * IRO[221].m1) + (((pfId)&1) * \
-	IRO[221].m2))
+	(IRO[222].base + (((pfId) >> 1) * IRO[222].m1) + (((pfId) & 1) * \
+	IRO[222].m2))
 #define XSTORM_VF_TO_PF_OFFSET(funcId) \
 	(IRO[48].base + ((funcId) * IRO[48].m1))
 #define COMMON_ASM_INVALID_ASSERT_OPCODE 0x0
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h
index 622fadc50316..bcc3fefca7ab 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h
@@ -2393,6 +2393,17 @@ struct shmem2_region {
 
 	/* mini dump driver info */
 	struct mdump_driver_info drv_info;			/* 0x218 */
+
+	/* written by mfw, read by driver, eg. feature capability support */
+	u32 mfw_flags;						/* 0x22c */
+	#define DISABLE_EMBEDDED_LLDP_SUPPORT	0x00000001
+	/* DEBUG DATA (EDDC) is enabled */
+	#define DEBUG_DATA_SUPPORT		0x00000002
+
+	/* Internal mask similar to the drv_status in the drv_mb
+	 * which used by MFW to sync message
+	 */
+	u32 drv_status_do_not_ack[E2_FUNC_MAX];			/* 0x230 */
 };
 
 
@@ -3024,7 +3035,7 @@ struct afex_stats {
 
 #define BCM_5710_FW_MAJOR_VERSION			7
 #define BCM_5710_FW_MINOR_VERSION			13
-#define BCM_5710_FW_REVISION_VERSION		15
+#define BCM_5710_FW_REVISION_VERSION		20
 #define BCM_5710_FW_ENGINEERING_VERSION		0
 #define BCM_5710_FW_COMPILE_FLAGS			1
 
-- 
2.27.0

[PATCH net-next 2/2] bnx2x: Invalidate fastpath HSI version for VFs

[Manish Chopra]

Commit 0a6890b9b4df ("bnx2x: Utilize FW 7.13.15.0.")
added validation for fastpath HSI versions for different
client init which was not meant for SR-IOV VF clients, which
resulted in firmware asserts when running VF clients with
different fastpath HSI version.

This patch along with the new firmware support in patch #1
fixes this behavior in order to not validate fastpath HSI
version for the VFs.

Fixes: 0a6890b9b4df ("bnx2x: Utilize FW 7.13.15.0.")
Signed-off-by: Manish Chopra <manishc@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
Signed-off-by: Omkar Kulkarni <okulkarni@marvell.com>
Signed-off-by: Shai Malin <smalin@marvell.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
index 9c2f51f23035..b1817f5e6179 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
@@ -758,9 +758,17 @@ static void bnx2x_vf_igu_reset(struct bnx2x *bp, struct bnx2x_virtf *vf)
 
 void bnx2x_vf_enable_access(struct bnx2x *bp, u8 abs_vfid)
 {
+	u16 abs_fid;
+
+	abs_fid = FW_VF_HANDLE(abs_vfid);
+
 	/* set the VF-PF association in the FW */
-	storm_memset_vf_to_pf(bp, FW_VF_HANDLE(abs_vfid), BP_FUNC(bp));
-	storm_memset_func_en(bp, FW_VF_HANDLE(abs_vfid), 1);
+	storm_memset_vf_to_pf(bp, abs_fid, BP_FUNC(bp));
+	storm_memset_func_en(bp, abs_fid, 1);
+
+	/* Invalidate fp_hsi version for vfs */
+	REG_WR8(bp,
+		BAR_XSTRORM_INTMEM + XSTORM_ETH_FUNCTION_INFO_FP_HSI_VALID_E2_OFFSET(abs_fid), 0);
 
 	/* clear vf errors*/
 	bnx2x_vf_semi_clear_err(bp, abs_vfid);
-- 
2.27.0

Re: [PATCH net-next 2/2] bnx2x: Invalidate fastpath HSI version for VFs

[Greg KH]

On Tue, Oct 26, 2021 at 12:37:17PM -0700, Manish Chopra wrote:
> Commit 0a6890b9b4df ("bnx2x: Utilize FW 7.13.15.0.")
> added validation for fastpath HSI versions for different
> client init which was not meant for SR-IOV VF clients, which
> resulted in firmware asserts when running VF clients with
> different fastpath HSI version.
> 
> This patch along with the new firmware support in patch #1
> fixes this behavior in order to not validate fastpath HSI
> version for the VFs.
> 
> Fixes: 0a6890b9b4df ("bnx2x: Utilize FW 7.13.15.0.")
> Signed-off-by: Manish Chopra <manishc@marvell.com>
> Signed-off-by: Ariel Elior <aelior@marvell.com>
> Signed-off-by: Omkar Kulkarni <okulkarni@marvell.com>
> Signed-off-by: Shai Malin <smalin@marvell.com>


<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Greg KH]

On Tue, Oct 26, 2021 at 12:37:16PM -0700, Manish Chopra wrote:
> Commit 0050dcf3e848 ("bnx2x: Add FW 7.13.20.0") added a new .bin
> firmware file to linux-firmware.git tree. This new firmware addresses
> few important issues and enhancements as mentioned below -
> 
> - Support direct invalidation of FP HSI Ver per function ID, required for
>   invalidating FP HSI Ver prior to each VF start, as there is no VF start
> - BRB hardware block parity error detection support for the driver
> - Fix the FCOE underrun flow
> - Fix PSOD during FCoE BFS over the NIC ports after preboot driver
> 
> This patch incorporates this new firmware 7.13.20.0 in bnx2x driver.
> 
> Signed-off-by: Manish Chopra <manishc@marvell.com>
> Signed-off-by: Shai Malin <smalin@marvell.com>
> Signed-off-by: Omkar Kulkarni <okulkarni@marvell.com>
> Signed-off-by: Ariel Elior <aelior@marvell.com>


<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

RE: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Manish Chopra]

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Wednesday, October 27, 2021 1:35 AM
> To: Manish Chopra <manishc@marvell.com>
> Cc: kuba@kernel.org; netdev@vger.kernel.org; stable@vger.kernel.org; Ariel
> Elior <aelior@marvell.com>; Sudarsana Reddy Kalluru <skalluru@marvell.com>;
> malin1024@gmail.com; Shai Malin <smalin@marvell.com>; Omkar Kulkarni
> <okulkarni@marvell.com>; Nilesh Javali <njavali@marvell.com>; GR-everest-
> linux-l2@marvell.com
> Subject: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Tue, Oct 26, 2021 at 12:37:16PM -0700, Manish Chopra wrote:
> > Commit 0050dcf3e848 ("bnx2x: Add FW 7.13.20.0") added a new .bin
> > firmware file to linux-firmware.git tree. This new firmware addresses
> > few important issues and enhancements as mentioned below -
> >
> > - Support direct invalidation of FP HSI Ver per function ID, required for
> >   invalidating FP HSI Ver prior to each VF start, as there is no VF
> > start
> > - BRB hardware block parity error detection support for the driver
> > - Fix the FCOE underrun flow
> > - Fix PSOD during FCoE BFS over the NIC ports after preboot driver
> >
> > This patch incorporates this new firmware 7.13.20.0 in bnx2x driver.
> >
> > Signed-off-by: Manish Chopra <manishc@marvell.com>
> > Signed-off-by: Shai Malin <smalin@marvell.com>
> > Signed-off-by: Omkar Kulkarni <okulkarni@marvell.com>
> > Signed-off-by: Ariel Elior <aelior@marvell.com>
> 
> 
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the stable kernel
> tree.  Please read:
>     https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__www.kernel.org_doc_html_latest_process_stable-2Dkernel-
> 2Drules.html&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=bMTgx2X48QVXyXO
> EL8ALyI4dsWoR-m74c5n3d-
> ruJI8&m=ty09KAp_t8LlTicBDOtEO7pxmxWrH0D0JgMAieGU5RA&s=2fheQ69qq4l
> tmmFzYYyaQXTj7naqE87MTdo3bL9sJYY&e=
> for how to do this properly.
> 
> </formletter>

Hello Greg,

This patch set is mainly meant for net-next.git, can you please tell about the issue more specifically ?
Do you mean that I should not have Cced stable here ? is that the problem ?

Thanks,
Manish

Re: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Greg KH]

On Tue, Oct 26, 2021 at 08:30:34PM +0000, Manish Chopra wrote:
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Wednesday, October 27, 2021 1:35 AM
> > To: Manish Chopra <manishc@marvell.com>
> > Cc: kuba@kernel.org; netdev@vger.kernel.org; stable@vger.kernel.org; Ariel
> > Elior <aelior@marvell.com>; Sudarsana Reddy Kalluru <skalluru@marvell.com>;
> > malin1024@gmail.com; Shai Malin <smalin@marvell.com>; Omkar Kulkarni
> > <okulkarni@marvell.com>; Nilesh Javali <njavali@marvell.com>; GR-everest-
> > linux-l2@marvell.com
> > Subject: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0
> > 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > On Tue, Oct 26, 2021 at 12:37:16PM -0700, Manish Chopra wrote:
> > > Commit 0050dcf3e848 ("bnx2x: Add FW 7.13.20.0") added a new .bin
> > > firmware file to linux-firmware.git tree. This new firmware addresses
> > > few important issues and enhancements as mentioned below -
> > >
> > > - Support direct invalidation of FP HSI Ver per function ID, required for
> > >   invalidating FP HSI Ver prior to each VF start, as there is no VF
> > > start
> > > - BRB hardware block parity error detection support for the driver
> > > - Fix the FCOE underrun flow
> > > - Fix PSOD during FCoE BFS over the NIC ports after preboot driver
> > >
> > > This patch incorporates this new firmware 7.13.20.0 in bnx2x driver.
> > >
> > > Signed-off-by: Manish Chopra <manishc@marvell.com>
> > > Signed-off-by: Shai Malin <smalin@marvell.com>
> > > Signed-off-by: Omkar Kulkarni <okulkarni@marvell.com>
> > > Signed-off-by: Ariel Elior <aelior@marvell.com>
> > 
> > 
> > <formletter>
> > 
> > This is not the correct way to submit patches for inclusion in the stable kernel
> > tree.  Please read:
> >     https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__www.kernel.org_doc_html_latest_process_stable-2Dkernel-
> > 2Drules.html&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=bMTgx2X48QVXyXO
> > EL8ALyI4dsWoR-m74c5n3d-
> > ruJI8&m=ty09KAp_t8LlTicBDOtEO7pxmxWrH0D0JgMAieGU5RA&s=2fheQ69qq4l
> > tmmFzYYyaQXTj7naqE87MTdo3bL9sJYY&e=
> > for how to do this properly.
> > 
> > </formletter>
> 
> Hello Greg,
> 
> This patch set is mainly meant for net-next.git, can you please tell about the issue more specifically ?
> Do you mean that I should not have Cced stable here ? is that the problem ?

Please read the link I sent you for how to properly get patches accepted
into the stable kernel trees, it should answer this question for you.

thanks,

greg k-h

Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Jakub Kicinski]

On Tue, 26 Oct 2021 12:37:16 -0700 Manish Chopra wrote:
> Commit 0050dcf3e848 ("bnx2x: Add FW 7.13.20.0") added a new .bin
> firmware file to linux-firmware.git tree. This new firmware addresses
> few important issues and enhancements as mentioned below -
> 
> - Support direct invalidation of FP HSI Ver per function ID, required for
>   invalidating FP HSI Ver prior to each VF start, as there is no VF start
> - BRB hardware block parity error detection support for the driver
> - Fix the FCOE underrun flow
> - Fix PSOD during FCoE BFS over the NIC ports after preboot driver
> 
> This patch incorporates this new firmware 7.13.20.0 in bnx2x driver.

How is this expected to work? Your drivers seems to select a very
specific FW version:

	/* Check FW version */
	offset = be32_to_cpu(fw_hdr->fw_version.offset);
	fw_ver = firmware->data + offset;
	if ((fw_ver[0] != BCM_5710_FW_MAJOR_VERSION) ||
	    (fw_ver[1] != BCM_5710_FW_MINOR_VERSION) ||
	    (fw_ver[2] != BCM_5710_FW_REVISION_VERSION) ||
	    (fw_ver[3] != BCM_5710_FW_ENGINEERING_VERSION)) {
		BNX2X_ERR("Bad FW version:%d.%d.%d.%d. Should be %d.%d.%d.%d\n",
		       fw_ver[0], fw_ver[1], fw_ver[2], fw_ver[3],
		       BCM_5710_FW_MAJOR_VERSION,
		       BCM_5710_FW_MINOR_VERSION,
		       BCM_5710_FW_REVISION_VERSION,
		       BCM_5710_FW_ENGINEERING_VERSION);
		return -EINVAL;
	}

so this change has a dependency on user updating their /lib/firmware.

Is it really okay to break the systems for people who do not have that
FW version with a stable backport?

Greg, do you have general guidance for this or is it subsystem by subsystem?

RE: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Ariel Elior]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, October 27, 2021 12:08 AM
> To: Manish Chopra <manishc@marvell.com>; Greg KH
> <gregkh@linuxfoundation.org>
> Cc: netdev@vger.kernel.org; stable@vger.kernel.org; Ariel Elior
> <aelior@marvell.com>; Sudarsana Reddy Kalluru <skalluru@marvell.com>;
> malin1024@gmail.com; Shai Malin <smalin@marvell.com>; Omkar Kulkarni
> <okulkarni@marvell.com>; Nilesh Javali <njavali@marvell.com>; GR-everest-
> linux-l2@marvell.com; Andrew Lunn <andrew@lunn.ch>
> Subject: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Tue, 26 Oct 2021 12:37:16 -0700 Manish Chopra wrote:
> > Commit 0050dcf3e848 ("bnx2x: Add FW 7.13.20.0") added a new .bin
> > firmware file to linux-firmware.git tree. This new firmware addresses
> > few important issues and enhancements as mentioned below -
> >
> > - Support direct invalidation of FP HSI Ver per function ID, required for
> >   invalidating FP HSI Ver prior to each VF start, as there is no VF
> > start
> > - BRB hardware block parity error detection support for the driver
> > - Fix the FCOE underrun flow
> > - Fix PSOD during FCoE BFS over the NIC ports after preboot driver
> >
> > This patch incorporates this new firmware 7.13.20.0 in bnx2x driver.
> 
> How is this expected to work? Your drivers seems to select a very specific FW
> version:
> 
> 	/* Check FW version */
> 	offset = be32_to_cpu(fw_hdr->fw_version.offset);
> 	fw_ver = firmware->data + offset;
> 	if ((fw_ver[0] != BCM_5710_FW_MAJOR_VERSION) ||
> 	    (fw_ver[1] != BCM_5710_FW_MINOR_VERSION) ||
> 	    (fw_ver[2] != BCM_5710_FW_REVISION_VERSION) ||
> 	    (fw_ver[3] != BCM_5710_FW_ENGINEERING_VERSION)) {
> 		BNX2X_ERR("Bad FW version:%d.%d.%d.%d. Should be
> %d.%d.%d.%d\n",
> 		       fw_ver[0], fw_ver[1], fw_ver[2], fw_ver[3],
> 		       BCM_5710_FW_MAJOR_VERSION,
> 		       BCM_5710_FW_MINOR_VERSION,
> 		       BCM_5710_FW_REVISION_VERSION,
> 		       BCM_5710_FW_ENGINEERING_VERSION);
> 		return -EINVAL;
> 	}
> 
> so this change has a dependency on user updating their /lib/firmware.
> 
> Is it really okay to break the systems for people who do not have that FW
> version with a stable backport?
> 
> Greg, do you have general guidance for this or is it subsystem by subsystem?
Hi Jacub,
You may recall we had a discussion on this during our last FW upgrade too.
Please note this is not FW which resides in flash, which may or may not be
updated during the life cycle of a specific board deployment, but rather an
initialization sequence recipe which happens to contain FW content (as well as
many other register and memory initializations) which is activated when driver
loads. We do have Flash based FW as well, with which we are fully backwards and
forwards compatible. There is no method to build the init sequence in a
backwards compatible mode for these devices - it would basically mean
duplicating most of the device interaction logic (control plane and data plane).
To support these products we need to be able to update this from time to time.
Please note these devices are EOLing, and therefore this may well be the last
update to this FW. The only theoretical way we can think of getting around this
if we had to is adding the entire thing as a huge header file and have the
driver compile with it. This would detach the dependency on the FW file being
present on disk, but has big disadvantages of making the compiled driver huge,
and bloating the kernel with redundant headers filled with what is essentially a
binary blob. We do make sure to add the FW files to the FW sub tree in advance
of modifying the drivers to use them.

Re: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Andrew Lunn]

> > ----------------------------------------------------------------------
> > On Tue, 26 Oct 2021 12:37:16 -0700 Manish Chopra wrote:
> > > Commit 0050dcf3e848 ("bnx2x: Add FW 7.13.20.0") added a new .bin
> > > firmware file to linux-firmware.git tree. This new firmware addresses
> > > few important issues and enhancements as mentioned below -
> > >
> > > - Support direct invalidation of FP HSI Ver per function ID, required for
> > >   invalidating FP HSI Ver prior to each VF start, as there is no VF
> > > start
> > > - BRB hardware block parity error detection support for the driver
> > > - Fix the FCOE underrun flow
> > > - Fix PSOD during FCoE BFS over the NIC ports after preboot driver
> > >
> > > This patch incorporates this new firmware 7.13.20.0 in bnx2x driver.
> > 
> > How is this expected to work? Your drivers seems to select a very specific FW
> > version:
> > 
> > 	/* Check FW version */
> > 	offset = be32_to_cpu(fw_hdr->fw_version.offset);
> > 	fw_ver = firmware->data + offset;
> > 	if ((fw_ver[0] != BCM_5710_FW_MAJOR_VERSION) ||
> > 	    (fw_ver[1] != BCM_5710_FW_MINOR_VERSION) ||
> > 	    (fw_ver[2] != BCM_5710_FW_REVISION_VERSION) ||
> > 	    (fw_ver[3] != BCM_5710_FW_ENGINEERING_VERSION)) {
> > 		BNX2X_ERR("Bad FW version:%d.%d.%d.%d. Should be
> > %d.%d.%d.%d\n",
> > 		       fw_ver[0], fw_ver[1], fw_ver[2], fw_ver[3],
> > 		       BCM_5710_FW_MAJOR_VERSION,
> > 		       BCM_5710_FW_MINOR_VERSION,
> > 		       BCM_5710_FW_REVISION_VERSION,
> > 		       BCM_5710_FW_ENGINEERING_VERSION);
> > 		return -EINVAL;
> > 	}
> > 
> > so this change has a dependency on user updating their /lib/firmware.
> > 
> > Is it really okay to break the systems for people who do not have that FW
> > version with a stable backport?
> > 
> > Greg, do you have general guidance for this or is it subsystem by subsystem?

I have been pushing back on a similar change for the Marvell Prestera
driver, which also loads the firmware from /lib/firmware and they are
proposing to break the ABI to the firmware, and not support older
version.

I don't like this. As Jakub points out, you are going to break systems
which don't update the firmware and the kernel at the same time. I
really would prefer you support two versions of the firmware, and
detect what features it supports to runtime.

	Andrew

Re: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Jakub Kicinski]

On Wed, 27 Oct 2021 05:17:43 +0000 Ariel Elior wrote:
> You may recall we had a discussion on this during our last FW upgrade too.

"During our last FW upgrade" is pretty misleading here. The discussion
seems to have been after user reported that you broke their systems:

https://lore.kernel.org/netdev/ffbcf99c-8274-eca1-5166-efc0828ca05b@molgen.mpg.de/

Now you want to make your users' lives even more miserable by pushing
your changes into stable.

> Please note this is not FW which resides in flash, which may or may not be
> updated during the life cycle of a specific board deployment, but rather an
> initialization sequence recipe which happens to contain FW content (as well as
> many other register and memory initializations) which is activated when driver
> loads. We do have Flash based FW as well, with which we are fully backwards and
> forwards compatible. There is no method to build the init sequence in a
> backwards compatible mode for these devices - it would basically mean
> duplicating most of the device interaction logic (control plane and data plane).
> To support these products we need to be able to update this from time to time.

And the driver can't support two versions of init sequence because...?

> Please note these devices are EOLing, and therefore this may well be the last
> update to this FW.

Solid argument.

> The only theoretical way we can think of getting around this if we
> had to is adding the entire thing as a huge header file and have the
> driver compile with it. This would detach the dependency on the FW
> file being present on disk, but has big disadvantages of making the
> compiled driver huge, and bloating the kernel with redundant headers
> filled with what is essentially a binary blob. We do make sure to add
> the FW files to the FW sub tree in advance of modifying the drivers
> to use them.

All the patch is doing is changing some offsets. Why can't you just
make the offset the driver uses dependent on the FW version?

Would be great if the engineer who wrote the code could answer that.

Re: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Andrew Lunn]

> All the patch is doing is changing some offsets. Why can't you just
> make the offset the driver uses dependent on the FW version?
> 
> Would be great if the engineer who wrote the code could answer that.

It is also not clear why the offsets need to change. Why not add the
new facility at the end, so the offsets don't change?

    Andrew

RE: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Ariel Elior]

> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, October 27, 2021 5:04 PM
> To: Ariel Elior <aelior@marvell.com>
> Cc: Manish Chopra <manishc@marvell.com>; Greg KH
> <gregkh@linuxfoundation.org>; netdev@vger.kernel.org;
> stable@vger.kernel.org; Sudarsana Reddy Kalluru <skalluru@marvell.com>;
> malin1024@gmail.com; Shai Malin <smalin@marvell.com>; Omkar Kulkarni
> <okulkarni@marvell.com>; Nilesh Javali <njavali@marvell.com>; GR-everest-
> linux-l2@marvell.com; Andrew Lunn <andrew@lunn.ch>
> Subject: Re: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0
> 
> On Wed, 27 Oct 2021 05:17:43 +0000 Ariel Elior wrote:
> > You may recall we had a discussion on this during our last FW upgrade too.
> 
> "During our last FW upgrade" is pretty misleading here. The discussion
> seems to have been after user reported that you broke their systems:
> 
> https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__lore.kernel.org_netdev_ffbcf99c-2D8274-2Deca1-2D5166-
> 2Defc0828ca05b-
> 40molgen.mpg.de_&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=cWBgNIF
> UifZRx2xhypdcaYrfIsMGt93NxP1r8GQtC0s&m=7SlExiNTnqs5yykfN6rmIWZvB
> wQtSHCEW1ZmdHhe3S4&s=5OfVYQjTBOqb9GeN7GmGw70U_7MZLYz9YDyl
> o1iqdtQ&e=
> 
> Now you want to make your users' lives even more miserable by pushing
> your changes into stable.
> 
> > Please note this is not FW which resides in flash, which may or may not be
> > updated during the life cycle of a specific board deployment, but rather an
> > initialization sequence recipe which happens to contain FW content (as well
> as
> > many other register and memory initializations) which is activated when
> driver
> > loads. We do have Flash based FW as well, with which we are fully
> backwards and
> > forwards compatible. There is no method to build the init sequence in a
> > backwards compatible mode for these devices - it would basically mean
> > duplicating most of the device interaction logic (control plane and data
> plane).
> > To support these products we need to be able to update this from time to
> time.
> 
> And the driver can't support two versions of init sequence because...?
Well. Generally speaking on the architecture of these devices, the init sequence
includes programing the PRAM of the processors on the fastpath, so two different
init sequences would mean two different FW versions, and supporting
two different data planes. It would also mean drastic changes in control plane
as well, for the same reason: the fastpath FW expects completely
different structures and messaging from one version to the next. For these
two versions, however, changes are admittedly not that big.

> 
> > Please note these devices are EOLing, and therefore this may well be the
> last
> > update to this FW.
> 
> Solid argument.
At this time we don't have any plans on further replacing the FW, so hopefully
we won't find ourselves here again. Another important point: the major fix we
are pushing here is a breakage in the SR-IOV virtual function compatibility
implementation in the FW (introduced in the previous FW version). If we
would maintain the ability to support that older FW in the PF driver, we
would also be maintaining the presence of the breakage. In other words you
may be better off with the driver failing to load with a clear message of "need
new FW file" rather than having it load, but then having all your virtual
functions which are passed through to virtual machines with distro kernel fail
in weird and unpredictable ways. For this reason we also ask to expedite
the acceptance of this change, as it is desirable to limit the exposure of this
problem. Back to the EOLing point: the set of people still familiar with this
 >13 years old architecture is severely reduced, so would rather not to try 
and invent new ways of doing things.

> 
> > The only theoretical way we can think of getting around this if we
> > had to is adding the entire thing as a huge header file and have the
> > driver compile with it. This would detach the dependency on the FW
> > file being present on disk, but has big disadvantages of making the
> > compiled driver huge, and bloating the kernel with redundant headers
> > filled with what is essentially a binary blob. We do make sure to add
> > the FW files to the FW sub tree in advance of modifying the drivers
> > to use them.
> 
> All the patch is doing is changing some offsets. Why can't you just
> make the offset the driver uses dependent on the FW version?
> 
> Would be great if the engineer who wrote the code could answer that.
My original answer was more for the general design/architecture of these
products, and a general design of supporting multiple FW versions on them.
Specifically for this FW change, as relatively little has changed, we
*could* maintain two sets of offsets based on the FW version. This might
impact driver performance, however, as some of the offsets are used in
data plane (e.g. updating the L2 ring producers), although I suppose we
could also work around that by preparing a new "generic" array of offsets,
populate it at FW load time and use that. However, as I stated above, I don't
think it is desirable in this case, as the previous version contains some nasty
behavior, and it may take us some considerable time to build that design,
allowing the problematic FW to spread to further Linux distributions.

Thanks,
Ariel

RE: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Manish Chopra]

> -----Original Message-----
> From: Ariel Elior <aelior@marvell.com>
> Sent: Thursday, October 28, 2021 2:02 PM
> To: Jakub Kicinski <kuba@kernel.org>
> Cc: Manish Chopra <manishc@marvell.com>; Greg KH
> <gregkh@linuxfoundation.org>; netdev@vger.kernel.org;
> stable@vger.kernel.org; Sudarsana Reddy Kalluru <skalluru@marvell.com>;
> malin1024@gmail.com; Shai Malin <smalin@marvell.com>; Omkar Kulkarni
> <okulkarni@marvell.com>; Nilesh Javali <njavali@marvell.com>; GR-everest-
> linux-l2@marvell.com; Andrew Lunn <andrew@lunn.ch>
> Subject: RE: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0
> 
> > From: Jakub Kicinski <kuba@kernel.org>
> > Sent: Wednesday, October 27, 2021 5:04 PM
> > To: Ariel Elior <aelior@marvell.com>
> > Cc: Manish Chopra <manishc@marvell.com>; Greg KH
> > <gregkh@linuxfoundation.org>; netdev@vger.kernel.org;
> > stable@vger.kernel.org; Sudarsana Reddy Kalluru
> > <skalluru@marvell.com>; malin1024@gmail.com; Shai Malin
> > <smalin@marvell.com>; Omkar Kulkarni <okulkarni@marvell.com>; Nilesh
> > Javali <njavali@marvell.com>; GR-everest- linux-l2@marvell.com; Andrew
> > Lunn <andrew@lunn.ch>
> > Subject: Re: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware
> > 7.13.20.0
> >
> > On Wed, 27 Oct 2021 05:17:43 +0000 Ariel Elior wrote:
> > > You may recall we had a discussion on this during our last FW upgrade too.
> >
> > "During our last FW upgrade" is pretty misleading here. The discussion
> > seems to have been after user reported that you broke their systems:
> >
> > https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__lore.kernel.org_netdev_ffbcf99c-2D8274-2Deca1-2D5166-
> > 2Defc0828ca05b-
> > 40molgen.mpg.de_&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=cWBgNIF
> > UifZRx2xhypdcaYrfIsMGt93NxP1r8GQtC0s&m=7SlExiNTnqs5yykfN6rmIWZvB
> > wQtSHCEW1ZmdHhe3S4&s=5OfVYQjTBOqb9GeN7GmGw70U_7MZLYz9YDyl
> > o1iqdtQ&e=
> >
> > Now you want to make your users' lives even more miserable by pushing
> > your changes into stable.
> >
> > > Please note this is not FW which resides in flash, which may or may
> > > not be updated during the life cycle of a specific board deployment,
> > > but rather an initialization sequence recipe which happens to
> > > contain FW content (as well
> > as
> > > many other register and memory initializations) which is activated
> > > when
> > driver
> > > loads. We do have Flash based FW as well, with which we are fully
> > backwards and
> > > forwards compatible. There is no method to build the init sequence
> > > in a backwards compatible mode for these devices - it would
> > > basically mean duplicating most of the device interaction logic
> > > (control plane and data
> > plane).
> > > To support these products we need to be able to update this from
> > > time to
> > time.
> >
> > And the driver can't support two versions of init sequence because...?
> Well. Generally speaking on the architecture of these devices, the init sequence
> includes programing the PRAM of the processors on the fastpath, so two
> different init sequences would mean two different FW versions, and supporting
> two different data planes. It would also mean drastic changes in control plane as
> well, for the same reason: the fastpath FW expects completely different
> structures and messaging from one version to the next. For these two versions,
> however, changes are admittedly not that big.
> 
> >
> > > Please note these devices are EOLing, and therefore this may well be
> > > the
> > last
> > > update to this FW.
> >
> > Solid argument.
> At this time we don't have any plans on further replacing the FW, so hopefully
> we won't find ourselves here again. Another important point: the major fix we
> are pushing here is a breakage in the SR-IOV virtual function compatibility
> implementation in the FW (introduced in the previous FW version). If we would
> maintain the ability to support that older FW in the PF driver, we would also be
> maintaining the presence of the breakage. In other words you may be better off
> with the driver failing to load with a clear message of "need new FW file" rather
> than having it load, but then having all your virtual functions which are passed
> through to virtual machines with distro kernel fail in weird and unpredictable
> ways. For this reason we also ask to expedite the acceptance of this change, as
> it is desirable to limit the exposure of this problem. Back to the EOLing point: the
> set of people still familiar with this
>  >13 years old architecture is severely reduced, so would rather not to try and
> invent new ways of doing things.
> 
> >
> > > The only theoretical way we can think of getting around this if we
> > > had to is adding the entire thing as a huge header file and have the
> > > driver compile with it. This would detach the dependency on the FW
> > > file being present on disk, but has big disadvantages of making the
> > > compiled driver huge, and bloating the kernel with redundant headers
> > > filled with what is essentially a binary blob. We do make sure to
> > > add the FW files to the FW sub tree in advance of modifying the
> > > drivers to use them.
> >
> > All the patch is doing is changing some offsets. Why can't you just
> > make the offset the driver uses dependent on the FW version?
> >
> > Would be great if the engineer who wrote the code could answer that.
> My original answer was more for the general design/architecture of these
> products, and a general design of supporting multiple FW versions on them.
> Specifically for this FW change, as relatively little has changed, we
> *could* maintain two sets of offsets based on the FW version. This might impact
> driver performance, however, as some of the offsets are used in data plane (e.g.
> updating the L2 ring producers), although I suppose we could also work around
> that by preparing a new "generic" array of offsets, populate it at FW load time
> and use that. However, as I stated above, I don't think it is desirable in this case,
> as the previous version contains some nasty behavior, and it may take us some
> considerable time to build that design, allowing the problematic FW to spread to
> further Linux distributions.
> 
> Thanks,
> Ariel

Hello Jakub et al,

Just following up based on the comments put by Ariel a week back. The earlier firmware has caused some important regression w.r.t SR-IOV compatibility, so it's critical to have these new
FW patches to be accepted sooner (thinking of the impact on various Linux distributions/kernels where that bug/regression will be carried over with earlier firmware), as Ariel pointed out
the complexities, in general making the FW backwards compatible on these devices architecture meaning supporting different data/control path (which is not good from performance perspective),
However these two particular versions are not changing that much (from data/control path perspective) so we could have made them backward compatible for these two particular versions but
given the time criticality, regression/bug introduced by the earlier FW, bnx2x devices being almost EOL, this would be our last FW submission hopefully so we don't want to re-invent something
which has been continued for many years now for these bnx2* devices.

PS: this series was not meant for stable (I have Cced stable mistakenly), please let me know if I can send v2 with stable removed from recipients.

Thanks,
Manish

Re: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Andrew Lunn]

> Hello Jakub et al,
> 
> Just following up based on the comments put by Ariel a week
> back. The earlier firmware has caused some important regression
> w.r.t SR-IOV compatibility, so it's critical to have these new FW
> patches to be accepted sooner (thinking of the impact on various
> Linux distributions/kernels where that bug/regression will be
> carried over with earlier firmware), as Ariel pointed out the
> complexities, in general making the FW backwards compatible on these
> devices architecture meaning supporting different data/control path
> (which is not good from performance perspective), However these two
> particular versions are not changing that much (from data/control
> path perspective) so we could have made them backward compatible for
> these two particular versions but given the time criticality,
> regression/bug introduced by the earlier FW, bnx2x devices being
> almost EOL, this would be our last FW submission hopefully so we
> don't want to re-invent something which has been continued for many
> years now for these bnx2* devices.

> PS: this series was not meant for stable (I have Cced stable
> mistakenly), please let me know if I can send v2 with stable removed
> from recipients.

I'm i right in says, the bad firmware was introduced with:

commit 0a6890b9b4df89a83678eba0bee3541bcca8753c
Author: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Date:   Mon Nov 4 21:51:09 2019 -0800

    bnx2x: Utilize FW 7.13.15.0.
    
    Commit 97a27d6d6e8d "bnx2x: Add FW 7.13.15.0" added said .bin FW to
    linux-firmware tree. This FW addresses few important issues in the earlier
    FW release.
    This patch incorporates FW 7.13.15.0 in the bnx2x driver.

And that means v5.5 through to at least 5.16 will be broken? It has
been broken for a little under 2 years? And both 5.10 and 5.15 are
LTS. And you don't care. You will leave them broken, even knowing that
distribution kernels are going to use these LTS kernel?

And you could of avoided this by not breaking the firmware ABI. Which
you now say is actually possible. And after being broken for 2 years
it is now time critical?


    Andrew

RE: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Ariel Elior]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Monday, November 8, 2021 3:45 PM
> To: Manish Chopra <manishc@marvell.com>
> Cc: Ariel Elior <aelior@marvell.com>; Jakub Kicinski <kuba@kernel.org>; Greg
> KH <gregkh@linuxfoundation.org>; netdev@vger.kernel.org;
> stable@vger.kernel.org; Sudarsana Reddy Kalluru <skalluru@marvell.com>;
> malin1024@gmail.com; Shai Malin <smalin@marvell.com>; Omkar Kulkarni
> <okulkarni@marvell.com>; Nilesh Javali <njavali@marvell.com>; GR-everest-
> linux-l2@marvell.com
> Subject: Re: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0
> 
> I'm i right in says, the bad firmware was introduced with:
Correct.

> commit 0a6890b9b4df89a83678eba0bee3541bcca8753c
> Author: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Date:   Mon Nov 4 21:51:09 2019 -0800
> 
>     bnx2x: Utilize FW 7.13.15.0.
> 
>     Commit 97a27d6d6e8d "bnx2x: Add FW 7.13.15.0" added said .bin FW to
>     linux-firmware tree. This FW addresses few important issues in the earlier
>     FW release.
>     This patch incorporates FW 7.13.15.0 in the bnx2x driver.
> 
> And that means v5.5 through to at least 5.16 will be broken? It has
> been broken for a little under 2 years? And both 5.10 and 5.15 are
> LTS. And you don't care.You will leave them broken, even knowing that
> distribution kernels are going to use these LTS kernel?
Not Correct. We would like to solve the problem here too. But what we plan
is to push these fixes upstream and to any other distro which will take them.
We did not face difficulties in the past to have the distros include the newer
FW files which the driver required.

> 
> And you could of avoided this by not breaking the firmware ABI. Which
> you now say is actually possible. And after being broken for 2 years
> it is now time critical?
It is not correct that this would have been avoided by not Breaking the ABI.
The breakage was a bug introduced in the FW for SR-IOV. Having
backwards/forwards compatible ABI would not change the fact that the bug
would be there. The bug is only exposed with old VM running on new
Hypervisor, so it is not correct to say "bug was there for 2 years".
Although problem was introduced 2 years ago, it was exposed now, and now
we want to fix it. Whether the fix is done in a manner by which driver
can work with old FW file on disk or not is not related to the problem itself.

I stand by that *generally* this HW architecture is not designed for
backward/forward compatibility with regard to this FW. But it is true that in
this case it can be done. Numerous FW versions of this device which were already
accepted and all were non backwards compatible and all had this same issue
(updating driver mandates syncing up to latest FW tree, otherwise driver load
gracefully fails). Since this is the last FW we are pushing for this EOLing
device it seems a bit meticulous to insist on this for this (hopefully) last
version of the device FW. If community insists, we will provide it in backwards
compatible manner (since it so happens that in this version it is possible), but
it may take us some time to prepare that. That may increase the exposure of the
SR-IOV bug to further distros which may pick up the older FW/Driver combination
in that time.
> 
> 
>     Andrew

Re: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

[Andrew Lunn]

> > I'm i right in says, the bad firmware was introduced with:
> Correct.
> 
> > commit 0a6890b9b4df89a83678eba0bee3541bcca8753c
> > Author: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> > Date:   Mon Nov 4 21:51:09 2019 -0800
> > 
> >     bnx2x: Utilize FW 7.13.15.0.
> > 
> >     Commit 97a27d6d6e8d "bnx2x: Add FW 7.13.15.0" added said .bin FW to
> >     linux-firmware tree. This FW addresses few important issues in the earlier
> >     FW release.
> >     This patch incorporates FW 7.13.15.0 in the bnx2x driver.
> > 
> > And that means v5.5 through to at least 5.16 will be broken? It has
> > been broken for a little under 2 years? And both 5.10 and 5.15 are
> > LTS. And you don't care.You will leave them broken, even knowing that
> > distribution kernels are going to use these LTS kernel?
> Not Correct. We would like to solve the problem here too. But what we plan
> is to push these fixes upstream

Isn't mainline the top of upstream? You cannot get any further up. Yet
you plan to drop stable? Please could you explain some more.

Are you thinking of releasing a 7.13.15.1 which only fixes the
problem, keeping ABI compatibility, so it can be added to stable?
And then submit 7.13.20.0 for net-next?

> It is not correct that this would have been avoided by not Breaking the ABI.
> The breakage was a bug introduced in the FW for SR-IOV. Having
> backwards/forwards compatible ABI would not change the fact that the bug
> would be there. The bug is only exposed with old VM running on new
> Hypervisor, so it is not correct to say "bug was there for 2 years".
> Although problem was introduced 2 years ago, it was exposed now, and now
> we want to fix it. Whether the fix is done in a manner by which driver
> can work with old FW file on disk or not is not related to the problem itself.
> 
> I stand by that *generally* this HW architecture is not designed for
> backward/forward compatibility with regard to this FW. But it is true that in
> this case it can be done. Numerous FW versions of this device which were already
> accepted and all were non backwards compatible and all had this same issue
> (updating driver mandates syncing up to latest FW tree, otherwise driver load
> gracefully fails). Since this is the last FW we are pushing for this EOLing
> device it seems a bit meticulous to insist on this for this (hopefully) last
> version of the device FW.

Part of the problem is the Marvell keeps doing this for its
products. See the discussion with Prestera. It is like there is a
Marvell policy to not even bother to try to keep ABI compatibility
with the firmware.

If the community wants Marvell to get better in this respect, we need
to push back and say ABI compatibility is important. I hope Prestera
has learned its lesion, they say they will never break ABI
compatibility again, but i think we need to wait a few years before we
can actually trust that statement.

What about other NIC drivers. I hope you don't have any other ABI
breaks planned.

       Andrew