* Re: [RFC][PATCH] thunderbolt: Quiesce AMD NHI during suspend
2026-07-08 15:34 ` [RFC][PATCH] thunderbolt: Quiesce AMD NHI during suspend Mario Limonciello
@ 2026-07-08 18:37 ` Mario Limonciello
0 siblings, 0 replies; 2+ messages in thread
From: Mario Limonciello @ 2026-07-08 18:37 UTC (permalink / raw)
To: Chia-Lin Kao (AceLan), Andreas Noever, Mika Westerberg,
Yehezkel Bernat
Cc: Yo-Jung Leo Lin, linux-usb, linux-kernel, S, Sanath,
Natikar, Basavaraj, open list:PCI SUBSYSTEM
Some comments about your changes too.
On 7/8/26 10:34, Mario Limonciello wrote:
> ++ linux-pci
> ++ Sanath && Basavaraj
>
> On 7/5/26 21:34, Chia-Lin Kao (AceLan) wrote:
>> On some AMD USB4 systems, resuming from system suspend with a
>> Thunderbolt dock in the picture hard-hangs the machine very early in the
>> resume path, before the Thunderbolt driver gets a chance to rescan and
>> re-establish the topology. It is a PCIe bus wedge: the system never
>> reaches the point where the thunderbolt module could recover the dock,
>> and the kernel journal does not persist past "PM: suspend entry
>> (s2idle)".
>>
>> Reproduce:
>>
>> 1. Affected platform: an AMD USB4 host with the thunderbolt driver
>> bound to its NHI PCI functions. Observed on a Dell Pro 14 Plus
>> (PB14255, AMD Phoenix) which exposes two USB4 NHI functions
>> (0000:c6:00.5 and 0000:c6:00.6).
>>
>> 2. Put the system into s2idle (Low Power S0 Idle / Modern Standby)
>> with no dock attached.
>>
>> 3. While the system is suspended, plug in a Dell SD25TB4 Thunderbolt 4
>> dock (Goshen Ridge). The plug event wakes the system, and the
>> dock's
>> downstream PCIe bridges are enumerated behind the AMD USB4 PCIe
>> tunnel during the resume.
>>
>> 4. The machine hard-hangs during the noirq resume phase, before the
>> thunderbolt driver re-establishes the topology. The Dell power
>> indicator flashes the "3A5W" code (3 amber + 5 white lights).
>>
>> Removing the trigger makes resume reliable: unbinding the two NHI PCI
>> functions, or unloading the thunderbolt module, before suspend both
>> avoid the hang completely. The difference in those cases is the PCI
>> state the core restores for the driver-bound NHI on resume.
>>
>> EC team findings (from the platform EC log):
>>
>> The EC/PD/dock side is healthy across the failing cycle, which points
>> the finger at the host resume path rather than the dock:
>>
>> - Modern Standby entry and resume are seen on the EC side; the dock
>> plug interrupt during MS is handled and the full power sequence
>> recovers (SLP_S3# okay, PLTRST# received, U8-U20 sequence
>> completed).
>>
>> - The EC-to-dock DDM command exchange fully succeeds: all CMsd/CMgd
>> transactions return success (device type, TBT cable type, dock
>> module config for USB/TBT/Audio/LAN).
>>
>> - PD/Type-C renegotiation is normal (power contracts up to 95W/100W);
>> no persistent protocol error appears in the log.
>>
>> - The only suspicious symptom is host-side: the EC repeatedly logs
>> PmSt_Timeout_Count while waiting for the BIOS/host to consume the
>> wake (PWRB pressed) event. That indicates the host resume path is
>> late or stuck, not an EC-to-dock transport failure. The Dell "3A5W"
>> diagnostic LED code (3 amber + 5 white) maps in the EC code to an
>> SLP_S3#/PLTRST#/RUNPWROK failure indication.
>>
>> Root cause:
>>
>> With the driver bound, the PCI core restores the saved NHI config space
>> during the noirq resume phase, including PCI_COMMAND with memory space
>> and bus mastering enabled. This is the same phase in which the core
>> restores the config space of the tunneled PCI bridges and whatever sits
>> behind them (the dock). If the AMD host router is made active (memory +
>> bus mastering) while the downstream USB4 PCIe tunnel to the dock is still
>> being restored and is not yet ready, the host interface starts bus
>> activity toward a not-yet-ready endpoint and the PCIe hierarchy wedges.
>>
>> This matches the workarounds: unbinding the NHI or unloading the module
>> means the core does not restore an active NHI over the tunnel during
>> noirq, and the hang disappears. Several Thunderbolt-side attempts did not
>> help (returning early from nhi_resume_noirq(), disabling host-router wake
>> flags, adding an unconditional delay before tunnel activation), which is
>> why the actionable boundary is the PCI PM state of the driver-bound AMD
>> NHI rather than a later Thunderbolt protocol step. We do not have
>> visibility into what the host router does internally in that window; that
>> uncertainty is the main reason for sending this as an RFC (see open
>> questions below).
>>
>> What this patch does:
>>
>> Add a QUIRK_QUIESCE_ON_SUSPEND quirk, currently set for all AMD USB4 host
>> routers (matched by vendor AMD + USB4 PCI class), and:
>>
>> - clear PCI_COMMAND_MEMORY and PCI_COMMAND_MASTER before the PCI core
>> saves the NHI config space in runtime and system suspend, so the NHI
>> is restored inactive on resume;
>>
>> - keep the NHI quiescent across nhi_resume_noirq() (return early
>> without
>> touching the controller), so it stays inactive while the core
>> restores the tunneled bridges and the dock;
>>
>> - once the whole system has resumed, re-enable memory space and bus
>> mastering and bring the domain back up from the PM complete callback
>> via tb_domain_resume_noirq(), and from nhi_runtime_resume() for the
>> runtime PM path.
>>
>> The result is that Thunderbolt/USB4 stays functional after resume instead
>> of the machine hanging or the tunnels staying down until the module is
>> reloaded.
>>
>> Test results:
>>
>> Tested on the Dell Pro 14 Plus (PB14255, AMD Phoenix, two USB4 host
>> routers) with the Dell SD25TB4 dock. Without the patch, the sequence
>> above hard-hangs during resume. With the patch, repeated suspend/resume
>> cycles with the dock connected complete cleanly with no hang. After
>> resume the NHI PCI_COMMAND reads back with MEMORY|MASTER set again, and
>> the dock's downstream USB devices re-enumerate, confirming the USB tunnel
>> is restored and the domain is functional.
>>
>> Caveats observed while the NHI is quiescent across noirq: config space
>> reads to the host router time out and TMU configuration restore fails
>> during that window; the controller and domain are fully brought back in
>> nhi_complete(). The DP tunnel restore path was not exercised in this
>> test, as no external display was driven through the dock during the
>> tested cycle.
>>
>> Open questions for RFC discussion:
>>
>> 1. Root cause: what does the AMD host router actually do when it is
>> restored with memory + bus mastering enabled during noirq while the
>> downstream tunnel is not ready? Is this an expected ordering
>> constraint, a firmware behavior, or a hardware erratum?
>>
>> 2. Scope: the quirk currently matches all AMD USB4 host routers by
>> class. Which parts are actually affected? Should this be narrowed
>> (e.g. by DMI or specific device IDs) instead of applied to all AMD
>> USB4?
>>
>> 3. Mechanism: is clearing both PCI_COMMAND_MEMORY and
>> PCI_COMMAND_MASTER
>> the right lever, or would clearing bus mastering alone suffice
>> and be
>> less invasive? Is poking PCI_COMMAND directly acceptable in the NHI
>> driver, or should this move into a PCI backend op / PCI quirk?
>>
>> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
>
> I have no idea why I didn't receive this patch in my mailbox - I'm
> obviously CC'ed. But I checked my spam and it's not there either.
>
> But anyway, I am wondering if the real issue here is that we're
> accessing the hardware before it's ready. If you can readily reproduce
> the issue, can you try this series to see if it helps?
>
> https://lore.kernel.org/linux-pci/20260708152650.536604-1-
> mario.limonciello@amd.com/
>
>> ---
>> drivers/thunderbolt/nhi.c | 78 +++++++++++++++++++++++++++++++++++++--
>> drivers/thunderbolt/nhi.h | 5 ++-
>> drivers/thunderbolt/pci.c | 8 ++++
>> 3 files changed, 85 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
>> index 698fb124d529..787185c745ae 100644
>> --- a/drivers/thunderbolt/nhi.c
>> +++ b/drivers/thunderbolt/nhi.c
>> @@ -16,6 +16,7 @@
>> #include <linux/interrupt.h>
>> #include <linux/iommu.h>
>> #include <linux/module.h>
>> +#include <linux/pci.h>
>> #include <linux/delay.h>
>> #include <linux/property.h>
>> #include <linux/string_choices.h>
>> @@ -40,6 +41,31 @@ static bool host_reset = true;
>> module_param(host_reset, bool, 0444);
>> MODULE_PARM_DESC(host_reset, "reset USB4 host router (default: true)");
>> +static bool nhi_quirk_quiesce_on_suspend(const struct tb_nhi *nhi)
>> +{
>> + return nhi->quirks & QUIRK_QUIESCE_ON_SUSPEND;
>> +}
>> +
>> +static void nhi_disable_pci_mem_master(struct tb_nhi *nhi)
>> +{
>> + struct pci_dev *pdev = to_pci_dev(nhi->dev);
>> + u16 cmd;
>> +
>> + pci_read_config_word(pdev, PCI_COMMAND, &cmd);
>> + cmd &= ~(PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
>> + pci_write_config_word(pdev, PCI_COMMAND, cmd);
>> +}
>> +
>> +static void nhi_enable_pci_mem_master(struct tb_nhi *nhi)
>> +{
>> + struct pci_dev *pdev = to_pci_dev(nhi->dev);
>> + u16 cmd;
>> +
>> + pci_read_config_word(pdev, PCI_COMMAND, &cmd);
>> + cmd |= PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER;
>> + pci_write_config_word(pdev, PCI_COMMAND, cmd);
>> +}
>> +
Is this basically an open coded version of pci_set_master() and
pci_clear_master()?
Or is the PCI_COMMAND_MEMORY actually important for this issue?
>> static int ring_interrupt_index(const struct tb_ring *ring)
>> {
>> int bit = ring->hop;
>> @@ -988,6 +1014,9 @@ static int __nhi_suspend_noirq(struct device
>> *dev, bool wakeup)
>> return ret;
>> }
>> + if (nhi_quirk_quiesce_on_suspend(nhi))
>> + nhi_disable_pci_mem_master(nhi);
>> +
Isn't this going to break wakeup interrupts?
>> return 0;
>> }
>> @@ -1038,6 +1067,18 @@ static int nhi_resume_noirq(struct device *dev)
>> struct tb_nhi *nhi = tb->nhi;
>> int ret;
>> + /*
>> + * If the NHI was quiesced on suspend keep it quiescent (bus
>> + * mastering and memory access disabled) across the noirq phase.
>> + * The PCI core restores the config space of the tunneled PCI
>> + * bridges and anything behind them (e.g. a dock) here; letting the
>> + * AMD host router become active now can wedge the PCIe bus before
>> + * the dock side is ready. The controller and domain are brought
>> + * back up in nhi_complete() once the whole system has resumed.
>> + */
>> + if (nhi_quirk_quiesce_on_suspend(nhi))
>> + return 0;
>> +
>> /*
>> * Check that the device is still there. It may be that the user
>> * unplugged last device which causes the host controller to go
>> @@ -1064,16 +1105,33 @@ static int nhi_suspend(struct device *dev)
>> static void nhi_complete(struct device *dev)
>> {
>> struct tb *tb = dev_get_drvdata(dev);
>> + struct tb_nhi *nhi = tb->nhi;
>> /*
>> * If we were runtime suspended when system suspend started,
>> * schedule runtime resume now. It should bring the domain back
>> - * to functional state.
>> + * to functional state (and undo the quiesce, if any, via
>> + * nhi_runtime_resume()).
>> */
>> - if (pm_runtime_suspended(dev))
>> + if (pm_runtime_suspended(dev)) {
>> pm_runtime_resume(dev);
>> - else
>> - tb_domain_complete(tb);
>> + return;
>> + }
>> +
>> + /*
>> + * A quiesced NHI (see __nhi_suspend_noirq()) was intentionally
>> + * left with bus mastering and memory access disabled and its
>> + * domain was not resumed in nhi_resume_noirq(). Now that the whole
>> + * system has resumed and the PCI core has finished restoring the
>> + * tunneled bridges, re-enable the controller and bring the domain
>> + * back up so Thunderbolt/USB4 keeps working after resume.
>> + */
>> + if (nhi_quirk_quiesce_on_suspend(nhi)) {
>> + nhi_enable_pci_mem_master(nhi);
>> + tb_domain_resume_noirq(tb);
>> + }
>> +
>> + tb_domain_complete(tb);
>> }
>> static int nhi_runtime_suspend(struct device *dev)
>> @@ -1091,6 +1149,10 @@ static int nhi_runtime_suspend(struct device *dev)
>> if (ret)
>> return ret;
>> }
>> +
>> + if (nhi_quirk_quiesce_on_suspend(nhi))
>> + nhi_disable_pci_mem_master(nhi);
>> +
>> return 0;
>> }
>> @@ -1100,6 +1162,14 @@ static int nhi_runtime_resume(struct device *dev)
>> struct tb_nhi *nhi = tb->nhi;
>> int ret;
>> + /*
>> + * The NHI is quiesced (bus mastering and memory access disabled)
>> + * across runtime suspend as well; re-enable it before resuming the
>> + * domain.
>> + */
>> + if (nhi_quirk_quiesce_on_suspend(nhi))
>> + nhi_enable_pci_mem_master(nhi);
>> +
>> if (nhi->ops->runtime_resume) {
>> ret = nhi->ops->runtime_resume(nhi);
>> if (ret)
>> diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
>> index d488eadadfce..c8b973150434 100644
>> --- a/drivers/thunderbolt/nhi.h
>> +++ b/drivers/thunderbolt/nhi.h
>> @@ -119,8 +119,9 @@ struct tb_nhi_ops {
>> #define PCI_CLASS_SERIAL_USB_USB4 0x0c0340
>> /* Host interface quirks */
>> -#define QUIRK_AUTO_CLEAR_INT BIT(0)
>> -#define QUIRK_E2E BIT(1)
>> +#define QUIRK_AUTO_CLEAR_INT BIT(0)
>> +#define QUIRK_E2E BIT(1)
>> +#define QUIRK_QUIESCE_ON_SUSPEND BIT(2)
>> /*
>> * Minimal number of vectors when we use MSI-X. Two for control channel
>> diff --git a/drivers/thunderbolt/pci.c b/drivers/thunderbolt/pci.c
>> index dbb6badda867..4cf8107f1ce8 100644
>> --- a/drivers/thunderbolt/pci.c
>> +++ b/drivers/thunderbolt/pci.c
>> @@ -43,6 +43,14 @@ static void nhi_pci_check_quirks(struct tb_nhi_pci
>> *nhi_pci)
>> struct tb_nhi *nhi = &nhi_pci->nhi;
>> struct pci_dev *pdev = to_pci_dev(nhi->dev);
>> + /*
>> + * AMD USB4 host routers may otherwise become active too early
>> during
>> + * resume after the PCI core restores the saved COMMAND register.
>> + */
>> + if (pdev->vendor == PCI_VENDOR_ID_AMD &&
>> + pdev->class == PCI_CLASS_SERIAL_USB_USB4)
>> + nhi->quirks |= QUIRK_QUIESCE_ON_SUSPEND;
>> +
If quirks do end up being correct direction thunderbolt quirks are
applied via drivers/thunderbolt/quirks.c
>> if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
>> /*
>> * Intel hardware supports auto clear of the interrupt
>
^ permalink raw reply [flat|nested] 2+ messages in thread