[PATCH net] tg3: Invert restart quirk logic to restrict SNP power-down bypass

[PATCH net] tg3: Invert restart quirk logic to restrict SNP power-down bypass

[Noam D. Eliyahu]

Commit 9fc3bc764334 ("tg3: power down device only on SYSTEM_POWER_OFF")
bypassed `tg3_power_down()` on system restarts to prevent reboot hangs and
NMIs on servers where NICs were initialized via SNP (such as Dell PowerEdge
R650 and R750).

However, skipping the power-down flow globally causes the ACPI _PTS method
to trigger fatal PCIe AER events and hangs on other models. Commit
e0efe83ed325 ("tg3: Disable tg3 PCIe AER on system reboot") attempted to
mitigate this by adding an AER-disable DMI quirk table. Maintaining a
growing list of models for a secondary symptom misses the root cause,
leaving systems like the PowerEdge T440 (sharing the BCM5720 NIC) exposed.

Fix this by inverting the quirk logic:
1. Restrict SNP power-down avoidance strictly to
   `tg3_restart_snp_quirk_table` for the specific systems requiring it
   (Dell PowerEdge R650 and R750).
2. Restore standard power-down behavior for all other systems, eliminating
   the separate AER-disable quirk table.

For systems affected by the original SNP restart issue, their shutdown flow
remains completely untouched.

Link: https://lore.kernel.org/netdev/20251221175050.14089-1-noam.d.eliyahu@gmail.com/T/#u
Fixes: 9fc3bc764334 ("tg3: power down device only on SYSTEM_POWER_OFF")
Fixes: e0efe83ed325 ("tg3: Disable tg3 PCIe AER on system reboot")
Cc: stable@vger.kernel.org
Signed-off-by: Noam D. Eliyahu <noam.d.eliyahu@gmail.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 48 +++++++----------------------
 1 file changed, 11 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index ce108705bbaf..95827df4cf6a 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -18256,53 +18256,23 @@ static int tg3_resume(struct device *device)
 
 static SIMPLE_DEV_PM_OPS(tg3_pm_ops, tg3_suspend, tg3_resume);
 
-/* Systems where ACPI _PTS (Prepare To Sleep) S5 will result in a fatal
- * PCIe AER event on the tg3 device if the tg3 device is not, or cannot
- * be, powered down.
+/* Systems where powering down a tg3 device on restarts triggers
+ * NMIs and fatal hangs during reboot if the network adapters were
+ * initialized under SNP.
  */
-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"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "PowerEdge R540"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "PowerEdge R640"),
-		},
-	},
+static const struct dmi_system_id tg3_restart_snp_quirk_table[] = {
 	{
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
 			DMI_MATCH(DMI_PRODUCT_NAME, "PowerEdge R650"),
 		},
 	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "PowerEdge R740"),
-		},
-	},
 	{
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
 			DMI_MATCH(DMI_PRODUCT_NAME, "PowerEdge R750"),
 		},
 	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "datto"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "SIRIS 5 1U Server"),
-		},
-	},
 	{}
 };
 
@@ -18320,14 +18290,18 @@ static void tg3_shutdown(struct pci_dev *pdev)
 	if (netif_running(dev))
 		dev_close(dev);
 
-	if (system_state == SYSTEM_POWER_OFF)
+	if (!dmi_first_match(tg3_restart_snp_quirk_table) ||
+	    system_state == SYSTEM_POWER_OFF) {
 		tg3_power_down(tp);
-	else if (system_state == SYSTEM_RESTART &&
-		 dmi_first_match(tg3_restart_aer_quirk_table) &&
+	} else if (system_state == SYSTEM_RESTART &&
 		 pdev->current_state != PCI_D3cold &&
 		 pdev->current_state != PCI_UNKNOWN) {
 		/* Disable PCIe AER on the tg3 to avoid a fatal
 		 * error during this system restart.
+		 * Needed since on restarts for systems in
+		 * tg3_restart_snp_quirk_table skipping on powering down the
+		 * tg3 device causes a fatal AER error during ACPI _PTS
+		 * (Prepare To Sleep).
 		 */
 		pcie_capability_clear_word(pdev, PCI_EXP_DEVCTL,
 					   PCI_EXP_DEVCTL_CERE |
-- 
2.39.5

Re: [PATCH net] tg3: Invert restart quirk logic to restrict SNP power-down bypass

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] tg3: Invert restart quirk logic to restrict SNP power-down bypass

[Noam D. Eliyahu]

On Tue Oct 27, 2026 at 12:15 AM IST,  wrote:
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
It's mentioned in the link to the mail discussion earlier, I had
discovered it on my own Dell PowerEdge T440 in production it caused a
reboot hang 100pc of the time due to the known AER issue that my model
wasn't listed in it's growing quirk table
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
Yes it was triggered the AER logs are identical to those in 2ca1c94ce0b6
Fatal AER event regarding ACPI _PTS and the main symptom is that iDRAC
regards this as fatal and hangs to restart until manually interrupted
>
>  - What hardware the change was tested on. For driver fixes please
>    mention the device (and if relevant firmware version) used for
>    testing, or say that the change was not tested on real hardware.
This change was tested on said Dell PowerEdge T440 which shares the same
Dell Firmware as the R-series and has the same physical NIC mentioned in
all other relevant commits (BCM5720)
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Re: [PATCH net] tg3: Invert restart quirk logic to restrict SNP power-down bypass

[Pavan Chebbi]

[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]

On Tue, Oct 27, 2026 at 3:41 AM Noam D. Eliyahu
<noam.d.eliyahu@gmail.com> wrote:
>
> Commit 9fc3bc764334 ("tg3: power down device only on SYSTEM_POWER_OFF")
> bypassed `tg3_power_down()` on system restarts to prevent reboot hangs and
> NMIs on servers where NICs were initialized via SNP (such as Dell PowerEdge
> R650 and R750).
>
> However, skipping the power-down flow globally causes the ACPI _PTS method
> to trigger fatal PCIe AER events and hangs on other models. Commit
> e0efe83ed325 ("tg3: Disable tg3 PCIe AER on system reboot") attempted to
> mitigate this by adding an AER-disable DMI quirk table. Maintaining a
> growing list of models for a secondary symptom misses the root cause,
> leaving systems like the PowerEdge T440 (sharing the BCM5720 NIC) exposed.
>
> Fix this by inverting the quirk logic:
> 1. Restrict SNP power-down avoidance strictly to
>    `tg3_restart_snp_quirk_table` for the specific systems requiring it
>    (Dell PowerEdge R650 and R750).
> 2. Restore standard power-down behavior for all other systems, eliminating
>    the separate AER-disable quirk table.
>
> For systems affected by the original SNP restart issue, their shutdown flow
> remains completely untouched.
>
> Link: https://lore.kernel.org/netdev/20251221175050.14089-1-noam.d.eliyahu@gmail.com/T/#u
> Fixes: 9fc3bc764334 ("tg3: power down device only on SYSTEM_POWER_OFF")
> Fixes: e0efe83ed325 ("tg3: Disable tg3 PCIe AER on system reboot")
> Cc: stable@vger.kernel.org
> Signed-off-by: Noam D. Eliyahu <noam.d.eliyahu@gmail.com>
> ---
>  drivers/net/ethernet/broadcom/tg3.c | 48 +++++++----------------------
>  1 file changed, 11 insertions(+), 37 deletions(-)
>

Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

Re: [PATCH net] tg3: Invert restart quirk logic to restrict SNP power-down bypass

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 27 Oct 2026 00:09:56 +0200 you wrote:
> Commit 9fc3bc764334 ("tg3: power down device only on SYSTEM_POWER_OFF")
> bypassed `tg3_power_down()` on system restarts to prevent reboot hangs and
> NMIs on servers where NICs were initialized via SNP (such as Dell PowerEdge
> R650 and R750).
> 
> However, skipping the power-down flow globally causes the ACPI _PTS method
> to trigger fatal PCIe AER events and hangs on other models. Commit
> e0efe83ed325 ("tg3: Disable tg3 PCIe AER on system reboot") attempted to
> mitigate this by adding an AER-disable DMI quirk table. Maintaining a
> growing list of models for a secondary symptom misses the root cause,
> leaving systems like the PowerEdge T440 (sharing the BCM5720 NIC) exposed.
> 
> [...]

Here is the summary with links:
  - [net] tg3: Invert restart quirk logic to restrict SNP power-down bypass
    https://git.kernel.org/netdev/net/c/d7314b51341c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html