linux-kernel - [PATCH 2/4] usb: xhci: remove CRR polling in xhci_abort_cmd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 23 Dec 2016 14:52:56 +0800
From:   Lu Baolu <baolu.lu@...ux.intel.com>
To:     Mathias Nyman <mathias.nyman@...ux.intel.com>
Cc:     linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        Lu Baolu <baolu.lu@...ux.intel.com>
Subject: [PATCH 2/4] usb: xhci: remove CRR polling in xhci_abort_cmd_ring()

xHCI driver aborts the command ring by setting CA (Command
Abort) bit in the command register. With this setting, host
should abort all command executions and generate a command
completion event with the completion code set to command
ring stopped. Software should time the completion of command
abort by checking the CRR (Command Ring Running) and waiting
for the command ring stopped event.

Current xhci_abort_cmd_ring() does this in the following
way: setting CA bit; busy polling CRR bit until it negates;
sleep and wait for the command ring stopped event. This is
not an efficient way since cpu cycles are wasted on polling
registers. This patch removes polling for CRR (Command Ring
Running). Wait for completion, and check CRR if completion
times out is enough.

Suggested-by: Mathias Nyman <mathias.nyman@...ux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@...ux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 49 ++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index e3bcf6d..5935dce 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -323,7 +323,6 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, unsigned long *flags)
 {
 	unsigned long timeleft;
 	u64 temp_64;
-	int ret;
 
 	xhci_dbg(xhci, "Abort command ring\n");
 
@@ -333,34 +332,34 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, unsigned long *flags)
 	xhci_write_64(xhci, temp_64 | CMD_RING_ABORT,
 			&xhci->op_regs->cmd_ring);
 
-	/* Section 4.6.1.2 of xHCI 1.0 spec says software should
-	 * time the completion od all xHCI commands, including
-	 * the Command Abort operation. If software doesn't see
-	 * CRR negated in a timely manner (e.g. longer than 5
-	 * seconds), then it should assume that the there are
-	 * larger problems with the xHC and assert HCRST.
-	 */
-	ret = xhci_handshake(&xhci->op_regs->cmd_ring,
-			CMD_RING_RUNNING, 0, 5 * 1000 * 1000);
-	if (ret < 0) {
-		xhci_err(xhci,
-			 "Stop command ring failed, maybe the host is dead\n");
-		xhci->xhc_state |= XHCI_STATE_DYING;
-		xhci_halt(xhci);
-		return -ESHUTDOWN;
-	}
-	/*
-	 * Writing the CMD_RING_ABORT bit should cause a cmd completion event,
-	 * however on some host hw the CMD_RING_RUNNING bit is correctly cleared
-	 * but the completion event in never sent. Wait 2 secs (arbitrary
-	 * number) to handle those cases after negation of CMD_RING_RUNNING.
-	 */
 	spin_unlock_irqrestore(&xhci->lock, *flags);
 	timeleft = wait_for_completion_timeout(&xhci->cmd_ring_stop_completion,
-					       2 * HZ);
+					       XHCI_CMD_DEFAULT_TIMEOUT);
 	spin_lock_irqsave(&xhci->lock, *flags);
 	if (!timeleft) {
-		xhci_dbg(xhci, "No stop event for abort, ring start fail?\n");
+		/* Section 4.6.1.2 of xHCI 1.0 spec says software should
+		 * time the completion of all xHCI commands, including
+		 * the Command Abort operation. If software doesn't see
+		 * CRR negated in a timely manner (e.g. longer than 5
+		 * seconds), then it should assume that the there are
+		 * larger problems with the xHC and assert HCRST.
+		 */
+		temp_64 = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
+		if (temp_64 & CMD_RING_RUNNING) {
+			xhci_err(xhci,
+				 "Stop command ring failed, maybe the host is dead\n");
+			xhci->xhc_state |= XHCI_STATE_DYING;
+			xhci_halt(xhci);
+			return -ESHUTDOWN;
+		}
+
+		/*
+		 * Writing the CMD_RING_ABORT bit should cause a cmd
+		 * completion event, however on some hosts the bit is
+		 * correctly cleared but the completion event is never
+		 * sent.
+		 */
+		xhci_warn(xhci, "No stop event for abort, ring start fail?\n");
 		xhci_cleanup_command_queue(xhci);
 	} else {
 		xhci_handle_stopped_cmd_ring(xhci, xhci_next_queued_cmd(xhci));
-- 
2.1.4