Message-ID: <CALs4sv0EYR=bMSW6pF6W=W_mZHhQBpkeg=ugwTtpBc7_FyPDug@mail.gmail.com>
Date: Mon, 22 Dec 2025 11:23:46 +0530
From: Pavan Chebbi <pavan.chebbi@...adcom.com>
To: "Noam D. Eliyahu" <noam.d.eliyahu@...il.com>
Cc: netdev@...r.kernel.org, mchan@...adcom.com
Subject: Re: [DISCUSS] tg3 reboot handling on Dell T440 (BCM5720)

On Sun, Dec 21, 2025 at 11:20 PM Noam D. Eliyahu
<noam.d.eliyahu@...il.com> wrote:
>
> Hi all,
>
> I have not contributed to the Linux kernel before, but after following and learning from the work here for several years, I am now trying to make my first contribution.
>
> I would like to discuss a reboot-related issue I am seeing on a Dell PowerEdge T440 with a BCM5720 NIC. With recent stable kernels (6.17+) and up-to-date Dell firmware, the system hits AER fatal errors during ACPI _PTS. On older kernels (starting around 6.6.2), the behavior is different and appears related to the conditional tg3_power_down flow introduced by commit 9931c9d04f4d.
>
> Below is a short summary of how the driver logic evolved.
>
> ### Relevant driver evolution
>
> * **9931c9d04f4d – tg3: power down device only on SYSTEM_POWER_OFF**

I think this commit id is wrong. Anyway, I know the commit.

>   Added to avoid reboot hangs when the NIC was initialized via SNP (PXE):
>
> ```
> - tg3_power_down(tp);
> + if (system_state == SYSTEM_POWER_OFF)
> +     tg3_power_down(tp);
> ```
>
> * **2ca1c94ce0b6 – tg3: Disable tg3 device on system reboot to avoid triggering AER**
>   Addressed a separate issue and partially reverted earlier behavior:
>
> ```
> + tg3_reset_task_cancel(tp);
> +
>   rtnl_lock();
> +
>   netif_device_detach(dev);
>
>   if (netif_running(dev))
>       dev_close(dev);
>
> - if (system_state == SYSTEM_POWER_OFF)
> -     tg3_power_down(tp);
> + tg3_power_down(tp);  /* unconditional again */
>
>   rtnl_unlock();
> +
> + pci_disable_device(pdev);
> ```
>
> * **e0efe83ed3252 – tg3: Disable tg3 PCIe AER on system reboot**
>   Combined both approaches, resulting in:
>
>   * Conditional tg3_power_down based on SYSTEM_STATE
>   * A DMI-based whitelist for AER handling
>
> ```
> static const struct dmi_system_id tg3_restart_aer_quirk_table[] = {
>     {
>         .matches = {
>             DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>             DMI_MATCH(DMI_PRODUCT_NAME, "PowerEdge R440"),
>         },
>     },
>     /* other R-series entries omitted */
>     {}
> };
> ```
>
> ```
> else if (system_state == SYSTEM_RESTART &&
>          dmi_first_match(tg3_restart_aer_quirk_table) &&
>          pdev->current_state != PCI_D3cold &&
>          pdev->current_state != PCI_UNKNOWN) {
>     pcie_capability_clear_word(pdev, PCI_EXP_DEVCTL,
>                                PCI_EXP_DEVCTL_CERE |
>                                PCI_EXP_DEVCTL_NFERE |
>                                PCI_EXP_DEVCTL_FERE |
>                                PCI_EXP_DEVCTL_URRE);
> }
> ```
>
> On my T440, this combined design is what causes problems. Simply removing the DMI table does not help, since the conditional tg3_power_down logic itself also causes issues on this hardware. Adding “PowerEdge T440” to the DMI list avoids the AER error, but this does not scale well and requires constant maintenance.
>
> I also could not reproduce the original reboot hang that motivated 9931c9d04f4d when running current firmware, even when initializing the NICs via SNP (PXE). This makes it look like the original problem has been resolved in firmware, while the workaround logic now causes trouble on up to date systems.
>
> ### Possible ways forward (from cleanest to minimal)
>
> 1. **Cleanest (recent firmware only)**
>    Remove both the conditional tg3_power_down and the AER disablement, and always call tg3_power_down. This restores the pre-workaround behavior and works reliably on my system.

This is going to be a problem, please follow the discussion here:
https://lore.kernel.org/netdev/CALs4sv1-6mgQ2JfF9MYiRADxumJD7m7OGWhCB5aWj1tGP0OPJg@mail.gmail.com/
where regression risk is flagged and it came true in
https://lore.kernel.org/netdev/CALs4sv2_JZd5K-ZgBkjL=QpXVEXnoJrjuqwwKg0+jo2-4taHJw@mail.gmail.com/

>
> 2. **Flip the conditioning**
>    Keep the DMI list, but use it to guard the conditional tg3_power_down instead (only for models where the original issue was observed, e.g. R650xs). Drop the AER handling entirely. This limits risk to known systems while simplifying the flow.

But I am not sure how systems affected in the commit e0efe83ed3252
will react. Can't tell 100pc without testing.

>
> 3. **Minimal change**
>    Add “T” variants (e.g. T440) to the existing DMI table. This fixes my system but keeps the current complexity and maintenance burden.

I can understand the burden. But unless Dell gets involved I don't see
a better way. Intel i40e also has a similar work around.

>
> I wanted to start with a discussion and a detailed report before sending any patches, to get guidance on what approach makes sense upstream. I can provide logs, kernel versions, and test results if useful. My testing (down to 6.6.1) was done on my own hardware. I could not reproduce either the original SNP reboot hang (9931c9d04f4d) or the AER ACPI _PTS failure (e0efe83ed3252) on current firmware so currently only (2ca1c94ce0b6) seems required, which suggests both issues may now be firmware-resolved.
>
> Thanks for taking the time to read this.
>
> Best regards,
> Noam D. Eliyahu
>

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5469 bytes)

Powered by blists - more mailing lists