Skip to contentSkip to conversation

Don't make noise about disconnected USB4 devices

PATCH In review Mario Limonciello 4 participants 15 messages Jun 9, 2025

Subsystems (3)
USB PCI USB xHCI driver
lore
Share & tools
lore

Mario Limonciello

author
#0 lore raw

cover Don't make noise about disconnected USB4 devices

From: Mario Limonciello <[email protected]>

When a USB4 or TBT3 dock is disconnected a lot of warnings and errors are emitted related to the PCIe tunnels and XHCI controllers in th dock.

The messages are loud, but it's mostly because the functions that emit the messages don't check whether the device is actually alive. The PCIe hotplug services mark the device as perm dead, so that can be used to hide some of the messsages.

In the XHCI driver the device is marked as dying already, so that can also be used to hide messages.

Mario Limonciello (4):
PCI: Don't show errors on inaccessible PCI devices
PCI: Fix runtime PM usage count underflow
usb: xhci: Avoid showing errors during surprise removal
usb: xhci: Avoid showing warnings for dying controller

 drivers/pci/pci-driver.c     | 3 ++-
 drivers/pci/pci.c            | 5 +++--
 drivers/usb/host/xhci-ring.c | 7 +++++--
 drivers/usb/host/xhci.c      | 6 ++++--
 4 files changed, 14 insertions(+), 7 deletions(-)
Signature · 1 line
2.43.0
author
#1 lore raw

PATCH 1/4 PCI: Don't show errors on inaccessible PCI devices

From: Mario Limonciello <[email protected]>

When a USB4 dock is unplugged the PCIe bridge it's connected to will remove issue a "Link Down" and "Card not detected event". The PCI core will treat this as a surprise hotplug event and unconfigure all downstream devices. This involves setting the device error state to pci_channel_io_perm_failure.

As the device is already gone and the PCI core is cleaning up there isn't really any reason to show error messages to the user about failing to change power states. Detect the error state and skip the messaging.

Signed-off-by Mario Limonciello <[email protected]>
---
 drivers/pci/pci.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/pci/pci.c +3 -2
index e9448d55113bd..7b0b4087da4d3 100644
@@ -1376,8 +1376,9 @@ int pci_power_up(struct pci_dev *dev)
 
 	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
 	if (PCI_POSSIBLE_ERROR(pmcsr)) {
-		pci_err(dev, "Unable to change power state from %s to D0, device inaccessible\n",
-			pci_power_name(dev->current_state));
+		if (dev->error_state != pci_channel_io_perm_failure)
+			pci_err(dev, "Unable to change power state from %s to D0, device inaccessible\n",
+				pci_power_name(dev->current_state));
 		dev->current_state = PCI_D3cold;
 		return -EIO;
 	}
Signature · 1 line
2.43.0
#2 lore raw

Re: [PATCH 1/4] PCI: Don't show errors on inaccessible PCI devices

[cc += Rafael, Mika]

Quoted 4 lines · Mario Limonciello
drivers/pci/pci.c +0 -0
@@ -1376,8 +1376,9 @@ int pci_power_up(struct pci_dev *dev)

drivers/pci/pci.c int pci_power_up(struct pci_dev *dev) · line 1384

Quoted 10 lines
drivers/pci/pci.c +3 -2
 	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
 	if (PCI_POSSIBLE_ERROR(pmcsr)) {
-		pci_err(dev, "Unable to change power state from %s to D0, device inaccessible\n",
-			pci_power_name(dev->current_state));
+		if (dev->error_state != pci_channel_io_perm_failure)
+			pci_err(dev, "Unable to change power state from %s to D0, device inaccessible\n",
+				pci_power_name(dev->current_state));
 		dev->current_state = PCI_D3cold;
 		return -EIO;
 	}

Instead of merely silencing the error message, why not bail out early on in the function, i.e.

	if (pci_dev_is_disconnected(dev)) {
		dev->current_state = PCI_D3cold;
		return -EIO;
	}

Thanks,

Lukas

author
#3 lore raw

Re: [PATCH 1/4] PCI: Don't show errors on inaccessible PCI devices

Quoted 21 lines · Lukas Wunner

[cc += Rafael, Mika]

Quoted 4 lines · Mario Limonciello
drivers/pci/pci.c +0 -0
@@ -1376,8 +1376,9 @@ int pci_power_up(struct pci_dev *dev)

drivers/pci/pci.c int pci_power_up(struct pci_dev *dev) · line 1384

Quoted 10 lines
drivers/pci/pci.c +3 -2
  	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
  	if (PCI_POSSIBLE_ERROR(pmcsr)) {
-		pci_err(dev, "Unable to change power state from %s to D0, device inaccessible\n",
-			pci_power_name(dev->current_state));
+		if (dev->error_state != pci_channel_io_perm_failure)
+			pci_err(dev, "Unable to change power state from %s to D0, device inaccessible\n",
+				pci_power_name(dev->current_state));
  		dev->current_state = PCI_D3cold;
  		return -EIO;
  	}

Instead of merely silencing the error message, why not bail out early on in the function, i.e.

Quoted 5 lines
	if (pci_dev_is_disconnected(dev)) {
		dev->current_state = PCI_D3cold;
		return -EIO;
	}

Thanks for the suggestion. That sounds good to me, I'll have a try.

author
#4 lore raw

PATCH 2/4 PCI: Fix runtime PM usage count underflow

In reply to Mario Limonciello

From: Mario Limonciello <[email protected]>

When a USB4 dock is unplugged the PCIe bridge it's connected to will remove issue a "Link Down" and "Card not detected event". The PCI core will treat this as a surprise hotplug event and unconfigure all downstream devices. This involves setting the device error state to pci_channel_io_perm_failure.

When PCI core gets to the point that the device is removed using pci_device_remove() the runtime count has already been decremented and so calling pm_runtime_put_sync() will cause an underflow.

Detect the device is in the error state and skip the call for this cleanup path.

Signed-off-by Mario Limonciello <[email protected]>
---
 drivers/pci/pci-driver.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/pci/pci-driver.c +2 -1
index 9f6e145d93d62..ab4cfdfc8fbc0 100644
@@ -479,7 +479,8 @@ static void pci_device_remove(struct device *dev)
 	pci_iov_remove(pci_dev);
 
 	/* Undo the runtime PM settings in local_pci_probe() */
-	pm_runtime_put_sync(dev);
+	if (pci_dev->error_state != pci_channel_io_perm_failure)
+		pm_runtime_put_sync(dev);
 
 	/*
 	 * If the device is still on, set the power state as "unknown",
Signature · 1 line
2.43.0
#5 lore raw

Re: [PATCH 2/4] PCI: Fix runtime PM usage count underflow

[cc += Rafael, Mika]

Quoted 4 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

When a USB4 dock is unplugged the PCIe bridge it's connected to will remove issue a "Link Down" and "Card not detected event". The PCI core

Nit: s/remove//

Quoted 7 lines

will treat this as a surprise hotplug event and unconfigure all downstream devices. This involves setting the device error state to pci_channel_io_perm_failure.

When PCI core gets to the point that the device is removed using pci_device_remove() the runtime count has already been decremented and so calling pm_runtime_put_sync() will cause an underflow.

Where has it been decremented? I think this needs to be identified and a Fixes tag added.

drivers/pci/pci-driver.c static void pci_device_remove(struct device *dev) · line 483

Quoted 9 lines
drivers/pci/pci-driver.c +2 -1
@@ -479,7 +479,8 @@ static void pci_device_remove(struct device *dev)
 	pci_iov_remove(pci_dev);
 
 	/* Undo the runtime PM settings in local_pci_probe() */
-	pm_runtime_put_sync(dev);
+	if (pci_dev->error_state != pci_channel_io_perm_failure)
+		pm_runtime_put_sync(dev);

Usually pci_dev_is_disconnected() is used in lieu of checking for the error_state directly.

Thanks,

Lukas

author
#6 lore raw

PATCH 3/4 usb: xhci: Avoid showing errors during surprise removal

In reply to Mario Limonciello

From: Mario Limonciello <[email protected]>

When a USB4 dock is unplugged from a system it won't respond to ring events. The PCI core handles the surprise removal event and notifies all PCI drivers. The XHCI PCI driver sets a flag that the device is being removed as well.

When that flag is set don't show messages in the cleanup path for marking the controller dead.

Signed-off-by Mario Limonciello <[email protected]>
---
 drivers/usb/host/xhci-ring.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/usb/host/xhci-ring.c +5 -2
index e038ad3375dc9..997628f8d06e5 100644
@@ -1377,12 +1377,15 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci,
  */
 void xhci_hc_died(struct xhci_hcd *xhci)
 {
+	bool notify;
 	int i, j;
 
 	if (xhci->xhc_state & XHCI_STATE_DYING)
 		return;
 
-	xhci_err(xhci, "xHCI host controller not responding, assume dead\n");
+	notify = !(xhci->xhc_state & XHCI_STATE_REMOVING);
+	if (notify)
+		xhci_err(xhci, "xHCI host controller not responding, assume dead\n");
 	xhci->xhc_state |= XHCI_STATE_DYING;
 
 	xhci_cleanup_command_queue(xhci);

@@ -1396,7 +1399,7 @@ void xhci_hc_died(struct xhci_hcd *xhci)
 	}
 
 	/* inform usb core hc died if PCI remove isn't already handling it */
-	if (!(xhci->xhc_state & XHCI_STATE_REMOVING))
+	if (notify)
 		usb_hc_died(xhci_to_hcd(xhci));
 }
Signature · 1 line
2.43.0
Acked-by on v1 3/4
#7 lore raw

Re: [PATCH 3/4] usb: xhci: Avoid showing errors during surprise removal

Quoted 11 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

When a USB4 dock is unplugged from a system it won't respond to ring events. The PCI core handles the surprise removal event and notifies all PCI drivers. The XHCI PCI driver sets a flag that the device is being removed as well.

When that flag is set don't show messages in the cleanup path for marking the controller dead.

Signed-off-by Mario Limonciello <[email protected]>
Acked-by Mathias Nyman <[email protected]>
author
#8 lore raw

Re: [PATCH 3/4] usb: xhci: Avoid showing errors during surprise removal

Quoted 8 lines · Mathias Nyman
Quoted 7 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

When a USB4 dock is unplugged from a system it won't respond to ring events. The PCI core handles the surprise removal event and notifies all PCI drivers. The XHCI PCI driver sets a flag that the device is being removed as well.

Quoted 7 lines
Quoted 4 lines

When that flag is set don't show messages in the cleanup path for marking the controller dead.

Signed-off-by Mario Limonciello <[email protected]>
Acked-by Mathias Nyman <[email protected]>

Thanks for the acks Mathias!

All 4 patches were sent together because of the relation but they really don't have a dependency to need to be committed together.

Would you mind picking up the patches for 3 and 4 to USB tree and I'll keep discussing 1 and 2 with linux-pci M/L.

#9 lore raw

Re: [PATCH 3/4] usb: xhci: Avoid showing errors during surprise removal

Quoted 14 lines · Mario Limonciello
Quoted 13 lines · Mathias Nyman
Quoted 11 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

When a USB4 dock is unplugged from a system it won't respond to ring events. The PCI core handles the surprise removal event and notifies all PCI drivers. The XHCI PCI driver sets a flag that the device is being removed as well.

When that flag is set don't show messages in the cleanup path for marking the controller dead.

Signed-off-by Mario Limonciello <[email protected]>
Quoted 8 lines
Quoted 2 lines
Acked-by Mathias Nyman <[email protected]>

Thanks for the acks Mathias!

All 4 patches were sent together because of the relation but they really don't have a dependency to need to be committed together.

Would you mind picking up the patches for 3 and 4 to USB tree and I'll keep discussing 1 and 2 with linux-pci M/L.

I can do that yes.
Added patches 3/4 and 4/4 to my for-usb-next branch

Thanks
Mathias

author
#10 lore raw

PATCH 4/4 usb: xhci: Avoid showing warnings for dying controller

In reply to Mario Limonciello

From: Mario Limonciello <[email protected]>

When a USB4 dock is unplugged from a system it won't respond to ring events. The PCI core handles the surprise removal event and notifies all PCI drivers. The XHCI PCI driver sets a flag that the device is being removed, and when the device stops responding a flag is also added to indicate it's dying.

When that flag is set don't bother to show warnings about a missing controller.

Signed-off-by Mario Limonciello <[email protected]>
---
 drivers/usb/host/xhci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/usb/host/xhci.c +4 -2
index 4e6dbd2375c3f..86d4bcc5faaf0 100644
@@ -144,7 +144,8 @@ int xhci_halt(struct xhci_hcd *xhci)
 	ret = xhci_handshake(&xhci->op_regs->status,
 			STS_HALT, STS_HALT, XHCI_MAX_HALT_USEC);
 	if (ret) {
-		xhci_warn(xhci, "Host halt failed, %d\n", ret);
+		if (!(xhci->xhc_state & XHCI_STATE_DYING))
+			xhci_warn(xhci, "Host halt failed, %d\n", ret);
 		return ret;
 	}
 

@@ -203,7 +204,8 @@ int xhci_reset(struct xhci_hcd *xhci, u64 timeout_us)
 	state = readl(&xhci->op_regs->status);
 
 	if (state == ~(u32)0) {
-		xhci_warn(xhci, "Host not accessible, reset failed.\n");
+		if (!(xhci->xhc_state & XHCI_STATE_DYING))
+			xhci_warn(xhci, "Host not accessible, reset failed.\n");
 		return -ENODEV;
 	}
Signature · 1 line
2.43.0
Acked-by on v1 4/4
#11 lore raw

Re: [PATCH 4/4] usb: xhci: Avoid showing warnings for dying controller

Quoted 8 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

When a USB4 dock is unplugged from a system it won't respond to ring events. The PCI core handles the surprise removal event and notifies all PCI drivers. The XHCI PCI driver sets a flag that the device is being removed, and when the device stops responding a flag is also added to indicate it's dying.

Quoted 4 lines

When that flag is set don't bother to show warnings about a missing controller.

Signed-off-by Mario Limonciello <[email protected]>
Acked-by Mathias Nyman <[email protected]>
#12 lore raw
In reply to Mario Limonciello

Hi,

General remarks:
- broken threading on 1/2 and 2/2
- some Cc missing on individual patch emails

Quoted 3 lines · Mario Limonciello

When a USB4 or TBT3 dock is disconnected a lot of warnings and errors are emitted related to the PCIe tunnels and XHCI controllers in th dock.

These patches will probably also trigger on any loss of PCIe link for any reason: badly seated card, worn connector, EMI, etc.

Will there be any remaining message about dead PCIe links, or just a silent disappearence? Like dev_info("USB disconnect ...") in USB.

Quoted 7 lines

The messages are loud, but it's mostly because the functions that emit the messages don't check whether the device is actually alive. The PCIe hotplug services mark the device as perm dead, so that can be used to hide some of the messsages.

In the XHCI driver the device is marked as dying already, so that can also be used to hide messages.

Are PCI drivers expected to stay silent on sudden removal mid operation? Is there no "safe ejection" procedure for those Thunderbolt devices?

Quoted 5 lines

Mario Limonciello (4):
PCI: Don't show errors on inaccessible PCI devices
PCI: Fix runtime PM usage count underflow
usb: xhci: Avoid showing errors during surprise removal
usb: xhci: Avoid showing warnings for dying controller

Regards,
Michal

author
#13 lore raw
Quoted 5 lines · Michał Pecio

Hi,

General remarks:
- broken threading on 1/2 and 2/2
- some Cc missing on individual patch emails

Yeah; sorry about that. I got bit by https://github.com/kworkflow/kworkflow/issues/1207 once again. Once I realized that happened I figured unthreaded was better than missing so I ended off sending the missing ones to each of the lists that missed them.

If I send a v2 with them together again I'll just manually do to/cc for everything.

Quoted 6 lines · Mario Limonciello
Quoted 3 lines

When a USB4 or TBT3 dock is disconnected a lot of warnings and errors are emitted related to the PCIe tunnels and XHCI controllers in th dock.

Quoted 6 lines

These patches will probably also trigger on any loss of PCIe link for any reason: badly seated card, worn connector, EMI, etc.

Will there be any remaining message about dead PCIe links, or just a silent disappearence? Like dev_info("USB disconnect ...") in USB.

Good point on the PCIe patches with other failures. Those wouldn't have any "hotplug event" though would they? This all stems from the hotplug event, so would it be worth storing the state on the struct pci_dev to conditionally show these PCIe messages?

Quoted 11 lines
Quoted 7 lines

The messages are loud, but it's mostly because the functions that emit the messages don't check whether the device is actually alive. The PCIe hotplug services mark the device as perm dead, so that can be used to hide some of the messsages.

In the XHCI driver the device is marked as dying already, so that can also be used to hide messages.

Are PCI drivers expected to stay silent on sudden removal mid operation? Is there no "safe ejection" procedure for those Thunderbolt devices?

With docking surprise hot removal is a standard operation.
Userspace doesn't offer anything for a clean removal event of PCIe like USB storage does.

Quoted 8 lines
Quoted 5 lines

Mario Limonciello (4):
PCI: Don't show errors on inaccessible PCI devices
PCI: Fix runtime PM usage count underflow
usb: xhci: Avoid showing errors during surprise removal
usb: xhci: Avoid showing warnings for dying controller

Regards,
Michal

#14 lore raw
In reply to Mario Limonciello
Quoted 5 lines · Mario Limonciello

Mario Limonciello (4):
PCI: Don't show errors on inaccessible PCI devices
PCI: Fix runtime PM usage count underflow
usb: xhci: Avoid showing errors during surprise removal
usb: xhci: Avoid showing warnings for dying controller

Patches [3/4] and [4/4] (which touch xhci) were only cc'ed to Bjorn. You may want to resend these two to Mathias and Greg.
You might also want to split the series in two separate ones for PCI and xhci if/when respinning.

Thanks,

Lukas