Thread (23 messages) 23 messages, 8 authors, 2022-03-16

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

flat view

From: Manish Chopra <manishc@marvell.com>
Date: 2022-03-09 19:23:16
Also in: regressions, stable 

Hi Paul,

-----Original Message-----
From: Paul Menzel <redacted>
Sent: Wednesday, March 9, 2022 11:31 PM
To: Manish Chopra <manishc@marvell.com>
Cc: kuba@kernel.org; netdev@vger.kernel.org; Ariel Elior
[off-list ref]; Alok Prasad [off-list ref]; Prabhakar
Kushwaha [off-list ref]; David S. Miller
[off-list ref]; Greg KH [off-list ref];
stable@vger.kernel.org; it+netdev@molgen.mpg.de; Linus Torvalds
[off-list ref]; regressions@lists.linux.dev
Subject: Re: [EXT] Re: [PATCH v2 net-next 1/2] bnx2x: Utilize firmware
7.13.21.0

Dear Manish,


Am 09.03.22 um 17:46 schrieb Manish Chopra:
quoted
quoted
-----Original Message-----
From: Paul Menzel <redacted>
Sent: Wednesday, March 9, 2022 9:45 PM
[…]
quoted
quoted
Dear Manish,


Am 17.12.21 um 17:55 schrieb Manish Chopra:
quoted
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
- Maintains backward compatibility

This patch incorporates this new firmware 7.13.21.0 in bnx2x driver.

Signed-off-by: Manish Chopra <manishc@marvell.com>
Signed-off-by: Prabhakar Kushwaha <redacted>
Signed-off-by: Alok Prasad <redacted>
Signed-off-by: Ariel Elior <redacted>
---

v1->v2:
------------
* Modified the patch such that driver to be backward compatible
    with older firmware too (New fw v7.13.21.0 on linux-firmware.git
    enables driver to maintain backward compatibility)
---
   drivers/net/ethernet/broadcom/bnx2x/bnx2x.h        | 11 +++-
   drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c    |  6 +-
   .../net/ethernet/broadcom/bnx2x/bnx2x_fw_defs.h    |  2 +
   drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h    |  3 +-
   drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c   | 75
+++++++++++++++-------
quoted
quoted
quoted
   5 files changed, 69 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index 2b06d78b..a19dd67 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -1850,6 +1850,14 @@ struct bnx2x {

   	/* Vxlan/Geneve related information */
   	u16 udp_tunnel_ports[BNX2X_UDP_PORT_MAX];
+
+#define FW_CAP_INVALIDATE_VF_FP_HSI	BIT(0)
+	u32 fw_cap;
+
+	u32 fw_major;
+	u32 fw_minor;
+	u32 fw_rev;
+	u32 fw_eng;
   };

   /* Tx queues may be less or equal to Rx queues */ @@ -2525,5
+2533,6 @@ enum {
    * Meant for implicit re-load flows.
    */
   int bnx2x_vlan_reconfigure_vid(struct bnx2x *bp);
-
+int bnx2x_init_firmware(struct bnx2x *bp); void
+bnx2x_release_firmware(struct bnx2x *bp);
   #endif /* bnx2x.h */
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 54a2334..8d36ebb 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -2365,10 +2365,8 @@ int bnx2x_compare_fw_ver(struct bnx2x *bp,
u32 load_code, bool print_err)
quoted
   	if (load_code != FW_MSG_CODE_DRV_LOAD_COMMON_CHIP &&
   	    load_code != FW_MSG_CODE_DRV_LOAD_COMMON) {
   		/* build my FW version dword */
-		u32 my_fw = (BCM_5710_FW_MAJOR_VERSION) +
-			(BCM_5710_FW_MINOR_VERSION << 8) +
-			(BCM_5710_FW_REVISION_VERSION << 16) +
-			(BCM_5710_FW_ENGINEERING_VERSION << 24);
+		u32 my_fw = (bp->fw_major) + (bp->fw_minor << 8) +
+				(bp->fw_rev << 16) + (bp->fw_eng << 24);

   		/* read loaded FW from chip */
   		u32 loaded_fw = REG_RD(bp, XSEM_REG_PRAM); diff --git
a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_fw_defs.h
b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_fw_defs.h
index 3f84352..a84d015 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_fw_defs.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_fw_defs.h
@@ -241,6 +241,8 @@
   	IRO[221].m2))
   #define XSTORM_VF_TO_PF_OFFSET(funcId) \
   	(IRO[48].base + ((funcId) * IRO[48].m1))
+#define XSTORM_ETH_FUNCTION_INFO_FP_HSI_VALID_E2_OFFSET(fid)
	\
quoted
+	(IRO[386].base + ((fid) * IRO[386].m1))
   #define COMMON_ASM_INVALID_ASSERT_OPCODE 0x0

   /* eth hsi version */
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h
b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h
index 622fadc..611efee 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h
@@ -3024,7 +3024,8 @@ 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		21
+#define BCM_5710_FW_REVISION_VERSION_V15	15
   #define BCM_5710_FW_ENGINEERING_VERSION		0
   #define BCM_5710_FW_COMPILE_FLAGS			1

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index aec666e..125dafe 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -74,9 +74,19 @@
   	__stringify(BCM_5710_FW_MINOR_VERSION) "."	\
   	__stringify(BCM_5710_FW_REVISION_VERSION) "."	\
   	__stringify(BCM_5710_FW_ENGINEERING_VERSION)
+
+#define FW_FILE_VERSION_V15				\
+	__stringify(BCM_5710_FW_MAJOR_VERSION) "."      \
+	__stringify(BCM_5710_FW_MINOR_VERSION) "."	\
+	__stringify(BCM_5710_FW_REVISION_VERSION_V15) "."	\
+	__stringify(BCM_5710_FW_ENGINEERING_VERSION)
+
   #define FW_FILE_NAME_E1		"bnx2x/bnx2x-e1-"
FW_FILE_VERSION
quoted
quoted
".fw"
quoted
   #define FW_FILE_NAME_E1H	"bnx2x/bnx2x-e1h-"
FW_FILE_VERSION ".fw"
quoted
   #define FW_FILE_NAME_E2		"bnx2x/bnx2x-e2-"
FW_FILE_VERSION
quoted
quoted
".fw"
quoted
+#define FW_FILE_NAME_E1_V15	"bnx2x/bnx2x-e1-"
FW_FILE_VERSION_V15 ".fw"
quoted
+#define FW_FILE_NAME_E1H_V15	"bnx2x/bnx2x-e1h-"
FW_FILE_VERSION_V15 ".fw"
quoted
+#define FW_FILE_NAME_E2_V15	"bnx2x/bnx2x-e2-"
FW_FILE_VERSION_V15 ".fw"
quoted
   /* Time in jiffies before concluding the transmitter is hung */
   #define TX_TIMEOUT		(5*HZ)
@@ -747,9 +757,7 @@ static int bnx2x_mc_assert(struct bnx2x *bp)
   		  CHIP_IS_E1(bp) ? "everest1" :
   		  CHIP_IS_E1H(bp) ? "everest1h" :
   		  CHIP_IS_E2(bp) ? "everest2" : "everest3",
-		  BCM_5710_FW_MAJOR_VERSION,
-		  BCM_5710_FW_MINOR_VERSION,
-		  BCM_5710_FW_REVISION_VERSION);
+		  bp->fw_major, bp->fw_minor, bp->fw_rev);

   	return rc;
   }
@@ -12308,6 +12316,15 @@ static int bnx2x_init_bp(struct bnx2x *bp)

   	bnx2x_read_fwinfo(bp);

+	if (IS_PF(bp)) {
+		rc = bnx2x_init_firmware(bp);
+
+		if (rc) {
+			bnx2x_free_mem_bp(bp);
+			return rc;
+		}
+	}
+
   	func = BP_FUNC(bp);

   	/* need to reset chip if undi was active */ @@ -12320,6 +12337,7
@@ static int bnx2x_init_bp(struct bnx2x *bp)

   		rc = bnx2x_prev_unload(bp);
   		if (rc) {
+			bnx2x_release_firmware(bp);
   			bnx2x_free_mem_bp(bp);
   			return rc;
   		}
@@ -13317,16 +13335,11 @@ static int bnx2x_check_firmware(struct
bnx2x *bp)
quoted
   	/* 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)) {
+	if (fw_ver[0] != bp->fw_major || fw_ver[1] != bp->fw_minor ||
+	    fw_ver[2] != bp->fw_rev || fw_ver[3] != bp->fw_eng) {
   		BNX2X_ERR("Bad FW version:%d.%d.%d.%d. Should be
%d.%d.%d.%d\n",
quoted
-		       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);
+			  fw_ver[0], fw_ver[1], fw_ver[2], fw_ver[3],
+			  bp->fw_major, bp->fw_minor, bp->fw_rev, bp-
fw_eng);
   		return -EINVAL;
   	}

@@ -13404,34 +13417,51 @@ static void be16_to_cpu_n(const u8
*_source, u8 *_target, u32 n)
quoted
   	     (u8 *)bp->arr, len);					\
   } while (0)

-static int bnx2x_init_firmware(struct bnx2x *bp)
+int bnx2x_init_firmware(struct bnx2x *bp)
   {
-	const char *fw_file_name;
+	const char *fw_file_name, *fw_file_name_v15;
   	struct bnx2x_fw_file_hdr *fw_hdr;
   	int rc;

   	if (bp->firmware)
   		return 0;

-	if (CHIP_IS_E1(bp))
+	if (CHIP_IS_E1(bp)) {
   		fw_file_name = FW_FILE_NAME_E1;
-	else if (CHIP_IS_E1H(bp))
+		fw_file_name_v15 = FW_FILE_NAME_E1_V15;
+	} else if (CHIP_IS_E1H(bp)) {
   		fw_file_name = FW_FILE_NAME_E1H;
-	else if (!CHIP_IS_E1x(bp))
+		fw_file_name_v15 = FW_FILE_NAME_E1H_V15;
+	} else if (!CHIP_IS_E1x(bp)) {
   		fw_file_name = FW_FILE_NAME_E2;
-	else {
+		fw_file_name_v15 = FW_FILE_NAME_E2_V15;
+	} else {
   		BNX2X_ERR("Unsupported chip revision\n");
   		return -EINVAL;
   	}
+
   	BNX2X_DEV_INFO("Loading %s\n", fw_file_name);

   	rc = request_firmware(&bp->firmware, fw_file_name, &bp->pdev-
dev);
quoted
quoted
   	if (rc) {
-		BNX2X_ERR("Can't load firmware file %s\n",
-			  fw_file_name);
-		goto request_firmware_exit;
+		BNX2X_DEV_INFO("Trying to load older fw %s\n",
fw_file_name_v15);
quoted
quoted
quoted
+
+		/* try to load prev version */
+		rc = request_firmware(&bp->firmware, fw_file_name_v15,
++&bp->pdev->dev);
+
+		if (rc)
+			goto request_firmware_exit;
+
+		bp->fw_rev = BCM_5710_FW_REVISION_VERSION_V15;
+	} else {
+		bp->fw_cap |= FW_CAP_INVALIDATE_VF_FP_HSI;
+		bp->fw_rev = BCM_5710_FW_REVISION_VERSION;
   	}

+	bp->fw_major = BCM_5710_FW_MAJOR_VERSION;
+	bp->fw_minor = BCM_5710_FW_MINOR_VERSION;
+	bp->fw_eng = BCM_5710_FW_ENGINEERING_VERSION;
+
   	rc = bnx2x_check_firmware(bp);
   	if (rc) {
   		BNX2X_ERR("Corrupt firmware file %s\n", fw_file_name); @@
-13487,7
quoted
+13517,7 @@ static int bnx2x_init_firmware(struct bnx2x *bp)
   	return rc;
   }

-static void bnx2x_release_firmware(struct bnx2x *bp)
+void bnx2x_release_firmware(struct bnx2x *bp)
   {
   	kfree(bp->init_ops_offsets);
   	kfree(bp->init_ops);
@@ -14004,6 +14034,7 @@ static int bnx2x_init_one(struct pci_dev
*pdev,
quoted
quoted
quoted
   	return 0;

   init_one_freemem:
+	bnx2x_release_firmware(bp);
   	bnx2x_free_mem_bp(bp);

   init_one_exit:
This change was added to Linux in commit b7a49f73059f (bnx2x: Utilize
firmware 7.13.21.0) [1] to Linux v5.17-rc1, and backported to the
stable series, for example, Linux v5.10.95.

Due to CVE-2022-0847 (Dirty Pipe) [1] we updated systems from, for
example, Linux 5.10.24 to Linux v5.10.103 and noticed that the
Broadcom network devices failed to initialize.

[   20.477325] bnx2 0000:02:00.0 eth0: Broadcom NetXtreme II BCM5709

1000Base-T (C0) PCI Express found at mem c6000000, IRQ 41, node addr
c8:1f:66:cf:34:45
quoted
quoted
[   20.491782] bnx2 0000:02:00.1 eth1: Broadcom NetXtreme II BCM5709
1000Base-T (C0) PCI Express found at mem c8000000, IRQ 42, node addr
c8:1f:66:cf:34:47
quoted
quoted
[   20.506223] bnx2 0000:03:00.0 eth2: Broadcom NetXtreme II BCM5709
1000Base-T (C0) PCI Express found at mem ca000000, IRQ 43, node addr
c8:1f:66:cf:34:49
quoted
quoted
[   20.520644] bnx2 0000:03:00.1 eth3: Broadcom NetXtreme II BCM5709
1000Base-T (C0) PCI Express found at mem cc000000, IRQ 44, node addr
c8:1f:66:cf:34:4b
quoted
quoted
[   20.534985] bnx2x 0000:45:00.0: msix capability found
[   20.540342] bnx2x 0000:45:00.0: part number 394D4342-31373735-
31314131-473331
quoted
quoted
[   20.548605] bnx2x 0000:45:00.0: Direct firmware load for bnx2x/bnx2x-
e1h-7.13.21.0.fw failed with error -2
quoted
quoted
[   20.558373] bnx2x 0000:45:00.0: Direct firmware load for bnx2x/bnx2x-
e1h-7.13.15.0.fw failed with error -2
quoted
quoted
[   20.568319] bnx2x: probe of 0000:45:00.0 failed with error -2
[   20.574148] bnx2x 0000:45:00.1: msix capability found
[   20.579470] bnx2x 0000:45:00.1: part number 394D4342-31373735-
31314131-473331
quoted
quoted
[   20.587708] bnx2x 0000:45:00.1: Direct firmware load for bnx2x/bnx2x-
e1h-7.13.21.0.fw failed with error -2
quoted
quoted
[   20.597479] bnx2x 0000:45:00.1: Direct firmware load for bnx2x/bnx2x-
e1h-7.13.15.0.fw failed with error -2
quoted
quoted
[   20.607355] bnx2x: probe of 0000:45:00.1 failed with error -2
[   20.613179] bnx2x 0000:46:00.0: msix capability found
[   20.618501] bnx2x 0000:46:00.0: part number 394D4342-31373735-
31314131-473331
quoted
quoted
[   20.626805] bnx2x 0000:46:00.0: Direct firmware load for bnx2x/bnx2x-
e1h-7.13.21.0.fw failed with error -2
quoted
quoted
[   20.636580] bnx2x 0000:46:00.0: Direct firmware load for bnx2x/bnx2x-
e1h-7.13.15.0.fw failed with error -2
quoted
quoted
[   20.646453] bnx2x: probe of 0000:46:00.0 failed with error -2
[   20.652279] bnx2x 0000:46:00.1: msix capability found
[   20.657593] bnx2x 0000:46:00.1: part number 394D4342-31373735-
31314131-473331
quoted
quoted
[   20.665813] bnx2x 0000:46:00.1: Direct firmware load for bnx2x/bnx2x-
e1h-7.13.21.0.fw failed with error -2
quoted
quoted
[   20.675590] bnx2x 0000:46:00.1: Direct firmware load for bnx2x/bnx2x-
e1h-7.13.15.0.fw failed with error -2
quoted
quoted
[   20.685457] bnx2x: probe of 0000:46:00.1 failed with error -2
Due to reasons, we do not have firmware in the initrd, and the
firmware files are only the root partition in `/lib/firmware`.

Logging in using other means¹, and removing the PCI devices, and
rescanning brought the devices online.

      $ echo 1 | sudo tee

/sys/bus/pci/devices/0000\:{45,46}\:00.{0,1}/remove
quoted
quoted
      $ echo 1 | sudo tee /sys/bus/pci/rescan

Adding the firmware files to the initrd also fixes the issue.

I didn’t bisect the change, but the refactoring of the init methods
looks like the reason for the change of behavior. As it’s not
documented in the commit message, was that intentional? I think it
breaks Linux’ no regression rule, and the commit (and follow-ups)
should be reverted in Linux v5.17 and backed out from the stable series.
quoted
The whole motivation of sending this v2 was to make FW backward
compatible with older FW so that it doesn't break the system if someone
updates the kernel, however the initrd case was slipped which was fixed
recently with below commit and should already be added to affected stable
branches. You may want to apply this commit to resolve the problem.
quoted
commit e13ad1443684f7afaff24cf207e85e97885256bd
Author: Manish Chopra [off-list ref]
Date:   Wed Feb 23 00:57:20 2022 -0800

     bnx2x: fix driver load from initrd

     Commit b7a49f73059f ("bnx2x: Utilize firmware 7.13.21.0") added
     new firmware support in the driver with maintaining older firmware
     compatibility. However, older firmware was not added in
MODULE_FIRMWARE()
quoted
     which caused missing firmware files in initrd image leading to driver load
     failure from initrd. This patch adds MODULE_FIRMWARE() for older
firmware
quoted
     version to have firmware files included in initrd.

     Fixes: b7a49f73059f ("bnx2x: Utilize firmware 7.13.21.0")
     Link: https://urldefense.proofpoint.com/v2/url?u=https-
3A__bugzilla.kernel.org_show-5Fbug.cgi-3Fid-
3D215627&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=bMTgx2X48QVXyX
OEL8ALyI4dsWoR-m74c5n3d-
ruJI8&m=DWtPUJhj8ALDm3F8Z6yRzueXgOQYUZjabDdRRZgAxN107KS-
brGjhwlenYylwJfh&s=FOolAGCSdsIl3ADjsbNpyuvYJS9UFqNOdf1pNXO5Y90&e
=
quoted
     Signed-off-by: Manish Chopra [off-list ref]
     Signed-off-by: Alok Prasad [off-list ref]
     Signed-off-by: Ariel Elior [off-list ref]
     Link: https://urldefense.proofpoint.com/v2/url?u=https-
3A__lore.kernel.org_r_20220223085720.12021-2D1-2Dmanishc-
40marvell.com&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=bMTgx2X48QV
XyXOEL8ALyI4dsWoR-m74c5n3d-
ruJI8&m=DWtPUJhj8ALDm3F8Z6yRzueXgOQYUZjabDdRRZgAxN107KS-
brGjhwlenYylwJfh&s=kWJAQqsp93gdiS4He5uT7oprXMx3WHE3pYMFb9pSJW
4&e=
quoted
     Signed-off-by: Jakub Kicinski [off-list ref]
I am aware of this commit, and it’s in Linux 5.10.103, we built and used. It’s
not about firmware version 7.13.15.0 not being loaded, but as written, about
the code being changed, so the firmware files are required earlier now. For
unknown reasons to me `bnx2x_init_firmware()` is now also called in
`bnx2x_init_bp()`, which is probably an earlier code path. (I do not know the
driver, so please take everything I write with a grain of salt.)
This move was intentional, as follow up driver flow [bnx2x_compare_fw_ver()] needs to know which exact FW version (newer or older fw version which will be decided at run time now)
the function is supposed to be run with in order to compare against already loaded FW on the adapter to decide on function probe/init failure (as opposed to earlier where driver was
always stick to the one specific/fixed firmware version). So for that reason I chose the right place to invoke the bnx2x_init_firmware() during the probe early instead of later stage.

Thanks,
Manish










  



    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread

          
kprevious message in thread

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help