possible race condition with PM entering test mode

possible race condition with PM entering test mode

[Oliver Neukum]

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

Hi,

it looks to me like you need to disable runtime PM
earlier if you wish for an undisturbed entry into
test mode.

Does the attached patch look good to you?

    Regards
        Oliver

[-- Attachment #2: 0001-XHCI-race-conditions-entering-test-mode.patch --]
[-- Type: text/x-patch, Size: 2189 bytes --]

From da128eb84c1ca8195a9a28a0e7157c80075d1685 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Tue, 15 Feb 2022 11:09:22 +0100
Subject: [PATCH] XHCI: race conditions entering test mode

If you do not want runtime PM to mess with your
HC during test mode, you need to disable it before
entering the transition, even if that makes
error handling uglier.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/usb/host/xhci-hub.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index df3522dab31b..4bfc71b0ae43 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -709,6 +709,8 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
 {
 	int i, retval;
 
+	/* Disable runtime PM for test mode */
+	pm_runtime_forbid(xhci_to_hcd(xhci)->self.controller);
 	/* Disable all Device Slots */
 	xhci_dbg(xhci, "Disable all slots\n");
 	spin_unlock_irqrestore(&xhci->lock, *flags);
@@ -735,15 +737,17 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
 	xhci_dbg(xhci, "Stop controller\n");
 	retval = xhci_halt(xhci);
 	if (retval)
-		return retval;
-	/* Disable runtime PM for test mode */
-	pm_runtime_forbid(xhci_to_hcd(xhci)->self.controller);
+		goto err_out;
 	/* Set PORTPMSC.PTC field to enter selected test mode */
 	/* Port is selected by wIndex. port_id = wIndex + 1 */
 	xhci_dbg(xhci, "Enter Test Mode: %d, Port_id=%d\n",
 					test_mode, wIndex + 1);
 	xhci_port_set_test_mode(xhci, test_mode, wIndex);
 	return retval;
+err_out:
+	/* reset will not work if the HC has not halted */
+	pm_runtime_allow(xhci_to_hcd(xhci)->self.controller);
+	return retval;
 }
 
 static int xhci_exit_test_mode(struct xhci_hcd *xhci)
@@ -760,9 +764,10 @@ static int xhci_exit_test_mode(struct xhci_hcd *xhci)
 		if (retval)
 			return retval;
 	}
-	pm_runtime_allow(xhci_to_hcd(xhci)->self.controller);
+	retval = xhci_reset(xhci);
 	xhci->test_mode = 0;
-	return xhci_reset(xhci);
+	pm_runtime_allow(xhci_to_hcd(xhci)->self.controller);
+	return retval;
 }
 
 void xhci_set_link_state(struct xhci_hcd *xhci, struct xhci_port *port,
-- 
2.34.1