DM USB: fix an USB endpoint reset flow issue

Original code will reset the whole USB device when xHCI Reset
Endpoint command is received, this behavior is not right. This
patch is used to fix it.

And according to xhci spec 4.6.8, if the endpoint is not in the
halted state, xHC should reject to execute this command and the
Context State Error should be returned. This patch also add this
logic.

Change-Id: I55a5918148d82d103fb3eb27d582f9676f9f61d3
Tracked-On:
Signed-off-by: Xiaoguang Wu <xiaoguang.wu@intel.com>
Reviewed-by: Liang Yang <liang3.yang@intel.com>
Acked-by: Yu Wang <yu1.wang@intel.com>

Author: xiaoguangwu

devicemodel/hw/pci/xhci.c

@@ -1748,15 +1748,19 @@ pci_xhci_cmd_reset_ep(struct pci_xhci_vdev *xdev,
 		goto done;
 	}
 
-	devep = &dev->eps[epid];
-	if (devep->ep_xfer != NULL)
-		USB_DATA_XFER_RESET(devep->ep_xfer);
-
 	dev_ctx = dev->dev_ctx;
 	assert(dev_ctx != NULL);
-
 	ep_ctx = &dev_ctx->ctx_ep[epid];
 
+	if ((ep_ctx->dwEpCtx0 & 0x7) != XHCI_ST_EPCTX_HALTED) {
+		cmderr = XHCI_TRB_ERROR_CONTEXT_STATE;
+		goto done;
+	}
+
+	devep = &dev->eps[epid];
+	if (devep->ep_xfer != NULL)
+		USB_DATA_XFER_RESET(devep->ep_xfer);
+
 	ep_ctx->dwEpCtx0 = (ep_ctx->dwEpCtx0 & ~0x7) | XHCI_ST_EPCTX_STOPPED;
 
 	if (XHCI_EPCTX_0_MAXP_STREAMS_GET(ep_ctx->dwEpCtx0) == 0)
@@ -1766,13 +1770,6 @@ pci_xhci_cmd_reset_ep(struct pci_xhci_vdev *xdev,
 		epid, ep_ctx->dwEpCtx0, ep_ctx->dwEpCtx1, ep_ctx->qwEpCtx2,
 		ep_ctx->dwEpCtx4);
 
-	if (type == XHCI_TRB_TYPE_RESET_EP &&
-	    (dev->dev_ue->ue_reset == NULL ||
-	    dev->dev_ue->ue_reset(dev->dev_instance) < 0)) {
-		cmderr = XHCI_TRB_ERROR_ENDP_NOT_ON;
-		goto done;
-	}
-
 done:
 	return cmderr;
 }
@@ -2159,7 +2156,7 @@ pci_xhci_xfer_complete(struct pci_xhci_vdev *xdev,
 	uint32_t trbflags;
 	uint32_t edtla;
 	uint32_t i;
-	int  err;
+	int  err = XHCI_TRB_ERROR_SUCCESS;
 
 	dev = XHCI_SLOTDEV_PTR(xdev, slot);
 	devep = &dev->eps[epid];
@@ -2169,7 +2166,20 @@ pci_xhci_xfer_complete(struct pci_xhci_vdev *xdev,
 
 	ep_ctx = &dev_ctx->ctx_ep[epid];
 
-	err = XHCI_TRB_ERROR_SUCCESS;
+	/* err is used as completion code and sent to guest driver */
+	switch (xfer->status) {
+	case USB_ERR_STALLED:
+		ep_ctx->dwEpCtx0 = (ep_ctx->dwEpCtx0 & ~0x7) |
+			XHCI_ST_EPCTX_HALTED;
+		err = XHCI_TRB_ERROR_STALL;
+		break;
+	case USB_ERR_NORMAL_COMPLETION:
+		break;
+	default:
+		/* FIXME: should process other failures */
+		UPRINTF(LFTL, "unknown error %d\r\n", xfer->status);
+	}
+
 	*do_intr = 0;
 	edtla = 0;
 
@@ -2494,6 +2504,9 @@ pci_xhci_handle_transfer(struct pci_xhci_vdev *xdev,
 
 	UPRINTF(LDBG, "[%d]: xfer->ndata %u\r\n", __LINE__, xfer->ndata);
 
+	if (xfer->ndata <= 0)
+		goto errout;
+
 	if (epid == 1) {
 		err = USB_ERR_NOT_STARTED;
 		if (dev->dev_ue->ue_request != NULL)
@@ -2525,7 +2538,9 @@ pci_xhci_handle_transfer(struct pci_xhci_vdev *xdev,
 		pci_xhci_assert_interrupt(xdev);
 
 	if (do_retry) {
-		USB_DATA_XFER_RESET(xfer);
+		if (epid == 1)
+			USB_DATA_XFER_RESET(xfer);
+
 		UPRINTF(LDBG, "[%d]: retry:continuing with next TRBs\r\n",
 			 __LINE__);
 		goto retry;

devicemodel/hw/platform/usb_pmapper.c

@@ -18,6 +18,8 @@
 #define LOG_TAG "USBPM: "
 
 static struct usb_dev_sys_ctx_info g_ctx;
+static inline uint8_t usb_dev_get_ep_type(struct usb_dev *udev, int pid,
+		int epnum);
 
 static void
 usb_dev_comp_req(struct libusb_transfer *libusb_xfer)
@@ -30,12 +32,15 @@ usb_dev_comp_req(struct libusb_transfer *libusb_xfer)
 	int bstart, bcount;
 
 	assert(libusb_xfer);
-	assert(libusb_xfer->user_data);
 
+	/* async request */
 	req = libusb_xfer->user_data;
 	len = libusb_xfer->actual_length;
-	xfer = req->xfer;
+	assert(req);
+	assert(req->udev);
 
+	/* async transfer */
+	xfer = req->xfer;
 	assert(xfer);
 	assert(xfer->dev);
 
@@ -52,13 +57,39 @@ usb_dev_comp_req(struct libusb_transfer *libusb_xfer)
 	USB_DATA_XFER_LOCK(xfer);
 	xfer->status = USB_ERR_NORMAL_COMPLETION;
 
+	switch (libusb_xfer->status) {
+	case LIBUSB_TRANSFER_STALL:
+		xfer->status = USB_ERR_STALLED;
+		goto out;
+	case LIBUSB_TRANSFER_NO_DEVICE:
+	case LIBUSB_TRANSFER_ERROR:
+	case LIBUSB_TRANSFER_TIMED_OUT:
+	case LIBUSB_TRANSFER_CANCELLED:
+	case LIBUSB_TRANSFER_OVERFLOW:
+		/* FIXME: should treat every failure properly */
+		UPRINTF(LWRN, "failure: %x\r\n", libusb_xfer->status);
+		break;
+	case LIBUSB_TRANSFER_COMPLETED:
+		break;
+	default:
+		UPRINTF(LWRN, "unknown failure: %x\r\n", libusb_xfer->status);
+		break;
+	}
+
 	/* in case the xfer is reset by the USB_DATA_XFER_RESET */
-	if (xfer->reset == 1 ||
-			libusb_xfer->status != LIBUSB_TRANSFER_COMPLETED) {
+	if (xfer->reset == 1) {
 		UPRINTF(LDBG, "ep%d reset detected\r\n", xfer->epid);
 		xfer->reset = 0;
-		USB_DATA_XFER_UNLOCK(xfer);
-		goto reset_out;
+		/* ONLY interrupt transfer needs this.
+		 * The transfer here is an old one before endpoint reset, so it
+		 * should be discarded. But for bulk transfer, the transfer here
+		 * is a new one after reset, so it should be kept.
+		 */
+		if (usb_dev_get_ep_type(req->udev, xfer->pid & 1,
+					xfer->epid / 2) == USB_ENDPOINT_INT) {
+			UPRINTF(LDBG, "goto reset out\r\n");
+			goto reset_out;
+		}
 	}
 
 	/* post process the usb transfer data */
@@ -84,10 +115,9 @@ usb_dev_comp_req(struct libusb_transfer *libusb_xfer)
 		idx = (idx + 1) % USB_MAX_XFER_BLOCKS;
 	}
 
-reset_out:
 	if (short_data)
 		xfer->status = USB_ERR_SHORT_XFER;
-
+out:
 	/* notify the USB core this transfer is over */
 	if (g_ctx.notify_cb)
 		do_intr = g_ctx.notify_cb(xfer->dev, xfer);
@@ -96,6 +126,7 @@ usb_dev_comp_req(struct libusb_transfer *libusb_xfer)
 	if (do_intr && g_ctx.intr_cb)
 		g_ctx.intr_cb(xfer->dev, NULL);
 
+reset_out:
 	/* unlock and release memory */
 	USB_DATA_XFER_UNLOCK(xfer);
 	libusb_free_transfer(libusb_xfer);
@@ -649,11 +680,19 @@ usb_dev_request(void *pdata, struct usb_data_xfer *xfer)
 		usb_dev_set_if(udev, index, value, xfer);
 		goto out;
 	case UREQ(UR_CLEAR_FEATURE, UT_WRITE_ENDPOINT):
-		if (value == 0) {
-			UPRINTF(LDBG, "UR_CLEAR_HALT\n");
-			libusb_clear_halt(udev->handle, index);
-			goto out;
+		if (value) {
+			/* according to usb spec (ch9), this is impossible */
+			UPRINTF(LWRN, "Clear Feature request with non-zero "
+					"value %d\r\n", value);
+			break;
 		}
+
+		UPRINTF(LDBG, "UR_CLEAR_HALT\n");
+		rc = libusb_clear_halt(udev->handle, index);
+		if (rc)
+			UPRINTF(LWRN, "fail to clear halted ep, rc %d\r\n", rc);
+		goto out;
+
 	}
 
 	/* send it to physical device */