Fix a problem related to isochronous transfers appearing in certain

constellations resulting in bogus frame sizes being returned by xhci(4).
E.g. for uvideo(4) devices erroneous video streams were reported.

The problem occurs when multi-trb TDs are queued and being processed
as a zero-length or short transfer.  Those cases were not handled in
the current isochronous code path, which this patch is adding.

Feedback and testing done by many on tech@.  Thanks!

ok mpi@

1 files changed, 32 insertions, 9 deletions

diff --git a/sys/dev/usb/xhci.c b/sys/dev/usb/xhci.c
index 9f138b4d0e3..1513c085aae 100644
--- a/sys/dev/usb/xhci.c
+++ b/sys/dev/usb/xhci.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: xhci.c,v 1.116 2020/06/30 10:21:59 gerhard Exp $ */
+/* $OpenBSD: xhci.c,v 1.117 2020/07/28 15:58:45 mglocker Exp $ */
 
 /*
  * Copyright (c) 2014-2015 Martin Pieuchot
@@ -73,6 +73,12 @@ struct xhci_pipe {
 	int			 halted;
 	size_t			 free_trbs;
 	int			 skip;
+enum {
+	TRB_PROCESSED_NO,
+	TRB_PROCESSED_YES,
+	TRB_PROCESSED_SHORT
+};
+	uint8_t			 trb_processed[XHCI_MAX_XFER];
 };
 
 int	xhci_reset(struct xhci_softc *);
@@ -82,7 +88,7 @@ void	xhci_event_xfer(struct xhci_softc *, uint64_t, uint32_t, uint32_t);
 int	xhci_event_xfer_generic(struct xhci_softc *, struct usbd_xfer *,
 	    struct xhci_pipe *, uint32_t, int, uint8_t, uint8_t, uint8_t);
 int	xhci_event_xfer_isoc(struct usbd_xfer *, struct xhci_pipe *,
-	    uint32_t, int);
+	    uint32_t, int, uint8_t);
 void	xhci_event_command(struct xhci_softc *, uint64_t);
 void	xhci_event_port_change(struct xhci_softc *, uint64_t, uint32_t);
 int	xhci_pipe_init(struct xhci_softc *, struct usbd_pipe *);
@@ -800,7 +806,7 @@ xhci_event_xfer(struct xhci_softc *sc, uint64_t paddr, uint32_t status,
 			return;
 		break;
 	case UE_ISOCHRONOUS:
-		if (xhci_event_xfer_isoc(xfer, xp, remain, trb_idx))
+		if (xhci_event_xfer_isoc(xfer, xp, remain, trb_idx, code))
 			return;
 		break;
 	default:
@@ -927,13 +933,23 @@ xhci_event_xfer_generic(struct xhci_softc *sc, struct usbd_xfer *xfer,
 
 int
 xhci_event_xfer_isoc(struct usbd_xfer *xfer, struct xhci_pipe *xp,
-    uint32_t remain, int trb_idx)
+    uint32_t remain, int trb_idx, uint8_t code)
 {
 	struct usbd_xfer *skipxfer;
 	struct xhci_xfer *xx = (struct xhci_xfer *)xfer;
-	int trb0_idx, frame_idx = 0;
+	int trb0_idx, frame_idx = 0, skip_trb = 0;
 
 	KASSERT(xx->index >= 0);
+
+	switch (code) {
+	case XHCI_CODE_SHORT_XFER:
+		xp->trb_processed[trb_idx] = TRB_PROCESSED_SHORT;
+		break;
+	default:
+		xp->trb_processed[trb_idx] = TRB_PROCESSED_YES;
+		break;
+	}
+
 	trb0_idx =
 	    ((xx->index + xp->ring.ntrb) - xx->ntrb) % (xp->ring.ntrb - 1);
 
@@ -958,15 +974,20 @@ xhci_event_xfer_isoc(struct usbd_xfer *xfer, struct xhci_pipe *xp,
 			trb0_idx = xp->ring.ntrb - 2;
 		else
 			trb0_idx = trb_idx - 1;
-		if (xfer->frlengths[frame_idx] == 0) {
+		if (xp->trb_processed[trb0_idx] == TRB_PROCESSED_NO) {
 			xfer->frlengths[frame_idx] = XHCI_TRB_LEN(letoh32(
 			    xp->ring.trbs[trb0_idx].trb_status));
+		} else if (xp->trb_processed[trb0_idx] == TRB_PROCESSED_SHORT) {
+			skip_trb = 1;
 		}
 	}
 
-	xfer->frlengths[frame_idx] +=
-	    XHCI_TRB_LEN(letoh32(xp->ring.trbs[trb_idx].trb_status)) - remain;
-	xfer->actlen += xfer->frlengths[frame_idx];
+	if (!skip_trb) {
+		xfer->frlengths[frame_idx] +=
+		    XHCI_TRB_LEN(letoh32(xp->ring.trbs[trb_idx].trb_status)) -
+		    remain;
+		xfer->actlen += xfer->frlengths[frame_idx];
+	}
 
 	if (xx->index != trb_idx)
 		return (1);
@@ -1836,6 +1857,8 @@ xhci_xfer_get_trb(struct xhci_softc *sc, struct usbd_xfer *xfer,
 		break;
 	}
 
+	xp->trb_processed[xp->ring.index] = TRB_PROCESSED_NO;
+
 	return (xhci_ring_produce(sc, &xp->ring));
 }