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

Powered by blists - more mailing lists