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
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);
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>
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>
-----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
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
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?
-----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.
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
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.m...
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.
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
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
-----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
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
-----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
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
linux-stable-mirror@lists.linaro.org
-
Andrew Lunn -
Ariel Elior -
Greg KH -
Jakub Kicinski -
Manish Chopra