Message-ID: <58ede701-f93b-d9f9-2775-4593a7756a30@leemhuis.info>
Date:   Thu, 10 Mar 2022 11:13:31 +0100
From:   Thorsten Leemhuis <regressions@...mhuis.info>
To:     regressions@...ts.linux.dev
Cc:     netdev@...r.kernel.org, stable@...r.kernel.org,
        Paul Menzel <pmenzel@...gen.mpg.de>
Subject: Re: [PATCH v2 net-next 1/2] bnx2x: Utilize firmware 7.13.21.0

[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@...vell.com>
>>> Signed-off-by: Prabhakar Kushwaha <pkushwaha@...vell.com>
>>> Signed-off-by: Alok Prasad <palok@...vell.com>
>>> Signed-off-by: Ariel Elior <aelior@...vell.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?
> 
> 

Powered by blists - more mailing lists