public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Pavan Kondeti <quic_pkondeti@quicinc.com>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>,
	youling 257 <youling257@gmail.com>,
	gregkh@linuxfoundation.org, linux-usb@vger.kernel.org
Subject: Re: [PATCH 4/5] xhci: Fix command ring pointer corruption while aborting a command
Date: Fri, 29 Oct 2021 15:48:23 +0300	[thread overview]
Message-ID: <e96e2a96-00c4-6e6b-fc5a-e4a881325dc0@linux.intel.com> (raw)
In-Reply-To: <20211028080302.GA14180@hu-pkondeti-hyd.qualcomm.com>

> Thanks Mathias for the detailed explanation. Agree that your patch is good
> enough for the current situation. May be a TODO comment here would help. 
> 

Changed the patch to write the _next_ command instead of the current to CRCR.

Pavan, youling 257, I'd appreciate if you could give this one more patch a try.
Your cases are hard to reprodudce with my setup.

And if it works can I add your "Tested-by:" tags to it?

Thanks, 
-Mathias

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 311597bba80e..eaa49aef2935 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -366,7 +366,9 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci,
 /* Must be called with xhci->lock held, releases and aquires lock back */
 static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, unsigned long flags)
 {
-	u32 temp_32;
+	struct xhci_segment *new_seg    = xhci->cmd_ring->deq_seg;
+	union xhci_trb *new_deq         = xhci->cmd_ring->dequeue;
+	u64 crcr;
	int ret;

	xhci_dbg(xhci, "Abort command ring\n");
@@ -375,13 +377,18 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, unsigned long flags)

	/*
	 * The control bits like command stop, abort are located in lower
-	 * dword of the command ring control register. Limit the write
-	 * to the lower dword to avoid corrupting the command ring pointer
-	 * in case if the command ring is stopped by the time upper dword
-	 * is written.
+	 * dword of the command ring control register.
+	 * Some controllers require all 64 bits to be written to abort the ring.
+	 * Make sure the upper dword is valid, pointing to the next command,
+	 * avoiding corrupting the command ring pointer in case the command ring
+	 * is stopped by the time the upper dword is written.
	 */
-	temp_32 = readl(&xhci->op_regs->cmd_ring);
-	writel(temp_32 | CMD_RING_ABORT, &xhci->op_regs->cmd_ring);
+	next_trb(xhci, NULL, &new_seg, &new_deq);
+	if (trb_is_link(new_deq))
+		next_trb(xhci, NULL, &new_seg, &new_deq);
+
+	crcr = xhci_trb_virt_to_dma(new_seg, new_deq);
+	xhci_write_64(xhci, crcr | CMD_RING_ABORT, &xhci->op_regs->cmd_ring);

	/* Section 4.6.1.2 of xHCI 1.0 spec says software should also time the
	 * completion of the Command Abort operation. If CRR is not negated in 5

  reply	other threads:[~2021-10-29 12:47 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-08  9:25 [PATCH 0/5] xhci fixes for usb-linus Mathias Nyman
2021-10-08  9:25 ` [PATCH 1/5] xhci: guard accesses to ep_state in xhci_endpoint_reset() Mathias Nyman
2021-10-08  9:25 ` [PATCH 2/5] xhci: add quirk for host controllers that don't update endpoint DCS Mathias Nyman
2021-10-08  9:25 ` [PATCH 3/5] USB: xhci: dbc: fix tty registration race Mathias Nyman
2021-10-08  9:25 ` [PATCH 4/5] xhci: Fix command ring pointer corruption while aborting a command Mathias Nyman
2021-10-22 10:59   ` youling257
2021-10-22 11:00     ` youling 257
2021-10-25 11:21       ` Mathias Nyman
2021-10-25 15:01         ` youling 257
2021-10-25 15:21           ` Mathias Nyman
2021-10-26 10:49             ` Pavan Kondeti
2021-10-27 13:59               ` Mathias Nyman
2021-10-28  8:03                 ` Pavan Kondeti
2021-10-29 12:48                   ` Mathias Nyman [this message]
2021-10-29 12:51                     ` [RFT PATCH] xhci: Fix commad ring abort, write all 64 bits to CRCR register Mathias Nyman
2021-10-29 15:35                       ` youling 257
2021-11-01  8:37                         ` Mathias Nyman
2021-11-01  3:31                       ` Pavan Kondeti
2021-11-01  8:36                         ` Mathias Nyman
2021-10-08  9:25 ` [PATCH 5/5] xhci: Enable trust tx length quirk for Fresco FL11 USB controller Mathias Nyman
2021-10-08 16:13 ` [PATCH 0/5] xhci fixes for usb-linus Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e96e2a96-00c4-6e6b-fc5a-e4a881325dc0@linux.intel.com \
    --to=mathias.nyman@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=pkondeti@codeaurora.org \
    --cc=quic_pkondeti@quicinc.com \
    --cc=youling257@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox