On 09/10/20(Fri) 12:37, Jonathon Fletcher wrote:
> In xhci_xfer_get_trb, the count of transfer buffers in the pipe
> (xp->free_trbs) is always decremented but the count of transfer buffers used
> in the transfer (xx->ntrb) is not incremented for zero-length transfers. The
> result of this is that, at the end of a zero length transfer, xp->free_trbs
> has 'lost' one.
>
> Over time, this mismatch of unconditional decrement (xp->free_trbs) vs
> conditional increment (xx->ntrb) results in xhci_device_*_start returning
> USBD_NOMEM.
>
> The patch below works around this by only decrementing xp->free_trbs in the
> cases when xx->ntrb is incremented.
Did you consider incrementing xx->ntrb instead?
With the diff below the produced TRB isn't accounted which might lead to
and off-by-one.
> Index: xhci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/xhci.c,v
> retrieving revision 1.119
> diff -u -p -u -r1.119 xhci.c
> --- xhci.c 31 Jul 2020 19:27:57 -0000 1.119
> +++ xhci.c 9 Oct 2020 19:11:45 -0000
> @@ -1836,7 +1836,6 @@ xhci_xfer_get_trb(struct xhci_softc *sc,
> struct xhci_xfer *xx = (struct xhci_xfer *)xfer;
>
> KASSERT(xp->free_trbs >= 1);
> - xp->free_trbs--;
> *togglep = xp->ring.toggle;
>
> switch (last) {
> @@ -1847,11 +1846,13 @@ xhci_xfer_get_trb(struct xhci_softc *sc,
> xp->pending_xfers[xp->ring.index] = xfer;
> xx->index = -2;
> xx->ntrb += 1;
> + xp->free_trbs--;
> break;
> case 1: /* This will terminate a chain. */
> xp->pending_xfers[xp->ring.index] = xfer;
> xx->index = xp->ring.index;
> xx->ntrb += 1;
> + xp->free_trbs--;
> break;
> }
>
>
