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

[Thorsten Leemhuis <regressions@leemhuis.info>]

TWIMC: this mail is primarily send for documentation purposes and for
regzbot, my Linux kernel regression tracking bot. These mails usually
contain '#forregzbot' in the subject, to make them easy to spot and filter.

Workaround for a slightly misplaced ^introduced:

#regzbot monitor:
https://lore.kernel.org/stable/ea05bcab-fe72-4bc2-3337-460888b2c44e@molgen.mpg.de/

On 10.03.22 11:13, Thorsten Leemhuis wrote:
> [TLDR: I'm adding the regression report below to regzbot, the Linux
> kernel regression tracking bot; all text you find below is compiled from
> a few templates paragraphs you might have encountered already already
> from similar mails.]
> 
> Hi, this is your Linux kernel regression tracker. Top-posting for once,
> to make this easily accessible to everyone.
> 
> To be sure below issue doesn't fall through the cracks unnoticed, I'm
> adding it to regzbot, my Linux kernel regression tracking bot:
> 
> #regzbot ^introduced b7a49f73059fe6147b6b7
> #regzbot title firmware now as to be in the initramfs
> #regzbot ignore-activity
> 
> If it turns out this isn't a regression, free free to remove it from the
> tracking by sending a reply to this thread containing a paragraph like
> "#regzbot invalid: reason why this is invalid" (without the quotes).
> 
> Reminder for developers: when fixing the issue, please add a 'Link:'
> tags pointing to the report (the mail quoted above) using
> lore.kernel.org/r/, as explained in
> 'Documentation/process/submitting-patches.rst' and
> 'Documentation/process/5.Posting.rst'. Regzbot needs them to
> automatically connect reports with fixes, but they are useful in
> general, too.
> 
> Sending this just to the lists, as this seems to be handled already.
> 
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> 
> P.S.: As the Linux kernel's regression tracker I'm getting a lot of
> reports on my table. I can only look briefly into most of them and lack
> knowledge about most of the areas they concern. I thus unfortunately
> will sometimes get things wrong or miss something important. I hope
> that's not the case here; if you think it is, don't hesitate to tell me
> in a public reply, it's in everyone's interest to set the public record
> straight.
> 
> 
> On 09.03.22 17:18, Paul Menzel wrote:
>> [Use correct regression address]
>>
>> Am 09.03.22 um 17:15 schrieb Paul Menzel:
>>> Dear Manish,
>>>
>>>
>>> Am 17.12.21 um 17:55 schrieb Manish Chopra:
>>>> 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 <pkushwaha@marvell.com>
>>>> Signed-off-by: Alok Prasad <palok@marvell.com>
>>>> Signed-off-by: Ariel Elior <aelior@marvell.com>
>>>> ---
>>>>
>>>> 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
>>>> +++++++++++++++-------
>>>>   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)
>>>>       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)    \
>>>> +    (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 ".fw"
>>>>   #define FW_FILE_NAME_E1H    "bnx2x/bnx2x-e1h-" FW_FILE_VERSION ".fw"
>>>>   #define FW_FILE_NAME_E2        "bnx2x/bnx2x-e2-" FW_FILE_VERSION ".fw"
>>>> +#define FW_FILE_NAME_E1_V15    "bnx2x/bnx2x-e1-" FW_FILE_VERSION_V15
>>>> ".fw"
>>>> +#define FW_FILE_NAME_E1H_V15    "bnx2x/bnx2x-e1h-"
>>>> FW_FILE_VERSION_V15 ".fw"
>>>> +#define FW_FILE_NAME_E2_V15    "bnx2x/bnx2x-e2-" FW_FILE_VERSION_V15
>>>> ".fw"
>>>>   /* 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)
>>>>       /* 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",
>>>> -               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)
>>>>            (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);
>>>>       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);
>>>> +
>>>> +        /* 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 +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,
>>>>       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
>>> [   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
>>> [   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
>>> [   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
>>> [   20.534985] bnx2x 0000:45:00.0: msix capability found
>>> [   20.540342] bnx2x 0000:45:00.0: part number
>>> 394D4342-31373735-31314131-473331
>>> [   20.548605] bnx2x 0000:45:00.0: Direct firmware load for
>>> bnx2x/bnx2x-e1h-7.13.21.0.fw failed with error -2
>>> [   20.558373] bnx2x 0000:45:00.0: Direct firmware load for
>>> bnx2x/bnx2x-e1h-7.13.15.0.fw failed with error -2
>>> [   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
>>> [   20.587708] bnx2x 0000:45:00.1: Direct firmware load for
>>> bnx2x/bnx2x-e1h-7.13.21.0.fw failed with error -2
>>> [   20.597479] bnx2x 0000:45:00.1: Direct firmware load for
>>> bnx2x/bnx2x-e1h-7.13.15.0.fw failed with error -2
>>> [   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
>>> [   20.626805] bnx2x 0000:46:00.0: Direct firmware load for
>>> bnx2x/bnx2x-e1h-7.13.21.0.fw failed with error -2
>>> [   20.636580] bnx2x 0000:46:00.0: Direct firmware load for
>>> bnx2x/bnx2x-e1h-7.13.15.0.fw failed with error -2
>>> [   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
>>> [   20.665813] bnx2x 0000:46:00.1: Direct firmware load for
>>> bnx2x/bnx2x-e1h-7.13.21.0.fw failed with error -2
>>> [   20.675590] bnx2x 0000:46:00.1: Direct firmware load for
>>> bnx2x/bnx2x-e1h-7.13.15.0.fw failed with error -2
>>> [   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
>>>      $ 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.
>>>
>>>
>>> Kind regards,
>>>
>>> Paul
>>>
>>>
>>> ¹ Why can’t even without proper firmware, the card at least initialize
>>> to have a network connection even if it’s degraded?
>>
>>