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

[Ariel Elior <aelior@marvell.com>]

> -----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.