[PATCH v2 0/2] usb: xhci: bandwidth and endpoints accounting fixes

[PATCH v2 0/2] usb: xhci: bandwidth and endpoints accounting fixes

[Niklas Neronin]

Fix two xhci accounting issues: one related to bandwidth state after
hibernation resume, and another causing total active endpoints accounting
to be left incorrect after virtual device allocation failures.

v2 changes:
 * [PATCH 1]: Fix commit message format
 * [PATCH 2]: Reword commit message
 * [PATCH 3-5]: Dropped

Niklas Neronin (2):
  usb: xhci: clear stale bandwidth data after hibernation
  usb: xhci: fix 'num_active_eps' accounting on allocation failure

 drivers/usb/host/xhci-mem.c | 16 +++++++++++++++-
 drivers/usb/host/xhci.c     |  6 ++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

-- 
2.50.1

[PATCH v2 1/2] usb: xhci: clear stale bandwidth data after hibernation

[Niklas Neronin]

Controllers using software managed bandwidth accounting maintain
periodic endpoint bandwidth information in struct 'xhci_interval_bw_table'.

Each root hub port owns a bandwidth table and a list of TT bandwidth
domains, where each TT has its own bandwidth table. Virtual devices
reference the bandwidth table of their current bandwidth domain through
'bw_table' or 'tt_info'.

Historically, resume from S4 reinitialized the entire xHCI driver state,
which implicitly cleared all bandwidth accounting data. After the
hibernation resume path was optimized to preserve parts of the driver
state, virtual devices and TT bandwidth information were freed and
recreated, but the root hub bandwidth tables were left intact.

As a result, stale bandwidth accounting data could remain in the root
hub bandwidth tables across hibernation resume, leading to incorrect
bandwidth calculations after devices were rediscovered.

Fix this by resetting all bandwidth accounting data in xhci_rh_bw_cleanup()
so that resume starts with a clean bandwidth state, and resetting the total
active endpoints count.

Fixes: 2a70e5dc0301 ("usb: xhci: optimize resuming from S4 (suspend-to-disk)")
Reported-by: Harald Judt <h.judt@gmx.at>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=222071
Suggested-by: Lovekesh Solanki <lovekeshsolanki00@gmail.com>
Tested-by: Harald Judt <h.judt@gmx.at>
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
v2:
 * Fix commit message format

 drivers/usb/host/xhci-mem.c | 16 +++++++++++++++-
 drivers/usb/host/xhci.c     |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index af8d4b74c4ba..75577f441cdd 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1903,24 +1903,38 @@ EXPORT_SYMBOL_GPL(xhci_remove_secondary_interrupter);
 void xhci_rh_bw_cleanup(struct xhci_hcd *xhci)
 {
 	struct xhci_root_port_bw_info *rh_bw;
+	struct xhci_interval_bw_table *bw_table;
+	struct xhci_interval_bw *interval_bw;
 	struct xhci_tt_bw_info *tt_info, *tt_next;
 	struct list_head *eps, *ep, *ep_next;
 
 	for (int i = 0; i < xhci->max_ports; i++) {
 		rh_bw = &xhci->rh_bw[i];
 
+		rh_bw->num_active_tts = 0;
 		/* Clear and free all TT bandwidth entries */
 		list_for_each_entry_safe(tt_info, tt_next, &rh_bw->tts, tt_list) {
 			list_del(&tt_info->tt_list);
 			kfree(tt_info);
 		}
 
+		bw_table = &rh_bw->bw_table;
+		bw_table->interval0_esit_payload = 0;
+		bw_table->bw_used = 0;
+		bw_table->ss_bw_in = 0;
+		bw_table->ss_bw_out = 0;
+
 		/* Clear per-interval endpoint lists */
 		for (int j = 0; j < XHCI_MAX_INTERVAL; j++) {
-			eps = &rh_bw->bw_table.interval_bw[j].endpoints;
+			interval_bw = &bw_table->interval_bw[j];
+			eps = &interval_bw->endpoints;
 
+			interval_bw->num_packets = 0;
 			list_for_each_safe(ep, ep_next, eps)
 				list_del_init(ep);
+			interval_bw->overhead[LS_OVERHEAD_TYPE] = 0;
+			interval_bw->overhead[FS_OVERHEAD_TYPE] = 0;
+			interval_bw->overhead[HS_OVERHEAD_TYPE] = 0;
 		}
 	}
 }
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 30ac1bc4559b..0361dfaecdd7 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1185,6 +1185,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
 		for (int i = xhci->max_slots; i > 0; i--)
 			xhci_free_virt_devices_depth_first(xhci, i);
 
+		xhci->num_active_eps = 0;
 		xhci_rh_bw_cleanup(xhci);
 
 		xhci->cmd_ring_reserved_trbs = 0;
-- 
2.50.1

[PATCH v2 2/2] usb: xhci: fix 'num_active_eps' accounting on allocation failure

[Niklas Neronin]

For controllers affected by XHCI_EP_LIMIT_QUIRK, endpoint resources for
the default control endpoint are accounted before the virtual device is
allocated.

If xhci_alloc_virt_device() fails, the slot is disabled and cleanup is
handled through the Disable Slot completion path. However, because no
virtual device is installed in 'xhci->devs', the endpoint accounting
is never released and 'num_active_eps' remains incremented.

As a result, each allocation failure leaks one endpoint from the
driver's accounting.

Fix this by undoing the endpoint reservation when virtual device
allocation fails.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
v2:
 * Rework commit message

 drivers/usb/host/xhci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 0361dfaecdd7..394f69b3cfbf 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4279,6 +4279,11 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
 	 */
 	if (!xhci_alloc_virt_device(xhci, slot_id, udev, GFP_NOIO)) {
 		xhci_warn(xhci, "Could not allocate xHCI USB device data structures\n");
+		if (xhci->quirks & XHCI_EP_LIMIT_QUIRK) {
+			spin_lock_irqsave(&xhci->lock, flags);
+			xhci->num_active_eps -= 1;
+			spin_unlock_irqrestore(&xhci->lock, flags);
+		}
 		goto disable_slot;
 	}
 	vdev = xhci->devs[slot_id];
-- 
2.50.1

Re: [PATCH v2 1/2] usb: xhci: clear stale bandwidth data after hibernation

[sashiko-bot]

> Controllers using software managed bandwidth accounting maintain
> periodic endpoint bandwidth information in struct 'xhci_interval_bw_table'.
> 
> Each root hub port owns a bandwidth table and a list of TT bandwidth
> domains, where each TT has its own bandwidth table. Virtual devices
> reference the bandwidth table of their current bandwidth domain through
> 'bw_table' or 'tt_info'.
> 
> Historically, resume from S4 reinitialized the entire xHCI driver state,
> which implicitly cleared all bandwidth accounting data. After the
> hibernation resume path was optimized to preserve parts of the driver
> state, virtual devices and TT bandwidth information were freed and
> recreated, but the root hub bandwidth tables were left intact.
> 
> As a result, stale bandwidth accounting data could remain in the root
> hub bandwidth tables across hibernation resume, leading to incorrect
> bandwidth calculations after devices were rediscovered.
> 
> Fix this by resetting all bandwidth accounting data in xhci_rh_bw_cleanup()
> so that resume starts with a clean bandwidth state, and resetting the total
> active endpoints count.
> 
> Fixes: 2a70e5dc0301 ("usb: xhci: optimize resuming from S4 (suspend-to-disk)")
> Reported-by: Harald Judt <h.judt@gmx.at>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=222071
> Suggested-by: Lovekesh Solanki <lovekeshsolanki00@gmail.com>
> Tested-by: Harald Judt <h.judt@gmx.at>
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102134228.584032-1-niklas.neronin@linux.intel.com?part=1

Re: [PATCH v2 2/2] usb: xhci: fix 'num_active_eps' accounting on allocation failure

[sashiko-bot]

> For controllers affected by XHCI_EP_LIMIT_QUIRK, endpoint resources for
> the default control endpoint are accounted before the virtual device is
> allocated.
> 
> If xhci_alloc_virt_device() fails, the slot is disabled and cleanup is
> handled through the Disable Slot completion path. However, because no
> virtual device is installed in 'xhci->devs', the endpoint accounting
> is never released and 'num_active_eps' remains incremented.
> 
> As a result, each allocation failure leaks one endpoint from the
> driver's accounting.
> 
> Fix this by undoing the endpoint reservation when virtual device
> allocation fails.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102134228.584032-1-niklas.neronin@linux.intel.com?part=2