Skip to contentSkip to conversation

patch "usb: host: xhci: handle COMP_STOP from SETUP phase too" added to usb-linus

PATCH [email protected] 1 participant 1 message Jan 3, 2017

Subsystems (3)
Stable branch USB USB xHCI driver
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

usb: host: xhci: handle COMP_STOP from SETUP phase too

to my usb git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-linus branch.

The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)

The patch will hopefully also be merged in Linus's tree for the next -rc kernel release.

If you have any questions about this process, please let me know.

Quoted 1 line

From 29fc1aa454d0603493b47a8e2410ae6e9ab20258 Mon Sep 17 00:00:00 2001

From: Felipe Balbi <[email protected]>
Date: Tue, 3 Jan 2017 18:28:53 +0200
Subject: usb: host: xhci: handle COMP_STOP from SETUP phase too

Stop Endpoint command can come at any point and we have no control of that. We should make sure to handle COMP_STOP on SETUP phase as well, otherwise urb->actual_length might be set to negative values in some occasions such as below:

urb->length = 4;
build_control_transfer_td_for(urb, ep);

stop_endpoint(ep);

COMP_STOP:

	[...]
	urb->actual_length = urb->length - trb->length;

trb->length is 8 for SETUP stage (8 control request bytes), so actual_length would be set to -4 in this case.

While doing that, also make sure to use TRB_TYPE field of the actual TRB instead of matching pointers to figure out in which stage of the control transfer we got our completion event.

Cc <[email protected]>
Signed-off-by Felipe Balbi <[email protected]>
Signed-off-by Mathias Nyman <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/usb/host/xhci-ring.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)
drivers/usb/host/xhci-ring.c +21 -12
index 7a14e9ad664d..25f522b09dd9 100644
@@ -1971,8 +1971,9 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td,
 	struct xhci_ep_ctx *ep_ctx;
 	u32 trb_comp_code;
 	u32 remaining, requested;
-	bool on_data_stage;
+	u32 trb_type;
 
+	trb_type = TRB_FIELD_TO_TYPE(le32_to_cpu(ep_trb->generic.field[3]));
 	slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
 	xdev = xhci->devs[slot_id];
 	ep_index = TRB_TO_EP_ID(le32_to_cpu(event->flags)) - 1;

@@ -1982,14 +1983,11 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td,
 	requested = td->urb->transfer_buffer_length;
 	remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
 
-	/* not setup (dequeue), or status stage means we are at data stage */
-	on_data_stage = (ep_trb != ep_ring->dequeue && ep_trb != td->last_trb);
-
 	switch (trb_comp_code) {
 	case COMP_SUCCESS:
-		if (ep_trb != td->last_trb) {
+		if (trb_type != TRB_STATUS) {
 			xhci_warn(xhci, "WARN: Success on ctrl %s TRB without IOC set?\n",
-				  on_data_stage ? "data" : "setup");
+				  (trb_type == TRB_DATA) ? "data" : "setup");
 			*status = -ESHUTDOWN;
 			break;
 		}

@@ -1999,15 +1997,25 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td,
 		*status = 0;
 		break;
 	case COMP_STOP_SHORT:
-		if (on_data_stage)
+		if (trb_type == TRB_DATA || trb_type == TRB_NORMAL)
 			td->urb->actual_length = remaining;
 		else
 			xhci_warn(xhci, "WARN: Stopped Short Packet on ctrl setup or status TRB\n");
 		goto finish_td;
 	case COMP_STOP:
-		if (on_data_stage)
+		switch (trb_type) {
+		case TRB_SETUP:
+			td->urb->actual_length = 0;
+			goto finish_td;
+		case TRB_DATA:
+		case TRB_NORMAL:
 			td->urb->actual_length = requested - remaining;
-		goto finish_td;
+			goto finish_td;
+		default:
+			xhci_warn(xhci, "WARN: unexpected TRB Type %d\n",
+				  trb_type);
+			goto finish_td;
+		}
 	case COMP_STOP_INVAL:
 		goto finish_td;
 	default:

@@ -2019,7 +2027,7 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td,
 		/* else fall through */
 	case COMP_STALL:
 		/* Did we transfer part of the data (middle) phase? */
-		if (on_data_stage)
+		if (trb_type == TRB_DATA || trb_type == TRB_NORMAL)
 			td->urb->actual_length = requested - remaining;
 		else if (!td->urb_length_set)
 			td->urb->actual_length = 0;

@@ -2027,14 +2035,15 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td,
 	}
 
 	/* stopped at setup stage, no data transferred */
-	if (ep_trb == ep_ring->dequeue)
+	if (trb_type == TRB_SETUP)
 		goto finish_td;
 
 	/*
 	 * if on data stage then update the actual_length of the URB and flag it
 	 * as set, so it won't be overwritten in the event for the last TRB.
 	 */
-	if (on_data_stage) {
+	if (trb_type == TRB_DATA ||
+		trb_type == TRB_NORMAL) {
 		td->urb_length_set = true;
 		td->urb->actual_length = requested - remaining;
 		xhci_dbg(xhci, "Waiting for status stage event\n");
Signature · 1 line
2.11.0