Re: kern/51202: XHCI driver sends stale TRBs from a pipe previously opened

[Sprow <webpages%sprow.co.uk@localhost>]

The following reply was made to PR kern/51202; it has been noted by GNATS.

From: Sprow <webpages%sprow.co.uk@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: kern/51202: XHCI driver sends stale TRBs from a pipe previously opened
Date: Tue, 31 May 2016 13:55:49 +0100

 For completeness, xhci_ring_init sharing code for the host side dequeue too.
 
 
 --- xhci_1-46.c	2016-05-30 22:44:14 +0100
 +++ xhci.c	2016-05-31 12:52:30 +0100
 @@ -142,6 +142,7 @@
  static usbd_status xhci_reset_endpoint(struct usbd_pipe *);
  static usbd_status xhci_stop_endpoint(struct usbd_pipe *);
  
 +static void xhci_host_dequeue(struct xhci_ring * const);
  static usbd_status xhci_set_dequeue(struct usbd_pipe *);
  
  static usbd_status xhci_do_command(struct xhci_softc * const,
 @@ -1508,6 +1509,9 @@
  	cp = xhci_slot_get_icv(sc, xs, xhci_dci_to_ici(dci));
  	xhci_setup_endp_ctx(pipe, cp);
  
 +	/* configure ep context performs an implicit dequeue */
 +	xhci_host_dequeue(&xs->xs_ep[dci].xe_tr);
 +
  	/* sync input contexts before they are read from memory */
  	usb_syncmem(&xs->xs_ic_dma, 0, sc->sc_pgsz, BUS_DMASYNC_PREWRITE);
  	hexdump("input control context", xhci_slot_get_icv(sc, xs, 0),
 @@ -1600,6 +1604,19 @@
  	return err;
  }
  
 +static void
 +xhci_host_dequeue(struct xhci_ring * const xr)
 +{
 +	/* When dequeueing the controller, update our struct copy too */
 +	memset(xr->xr_trb, 0, xr->xr_ntrb * XHCI_TRB_SIZE);
 +	usb_syncmem(&xr->xr_dma, 0, xr->xr_ntrb * XHCI_TRB_SIZE,
 +	    BUS_DMASYNC_PREWRITE);
 +	memset(xr->xr_cookies, 0, xr->xr_ntrb * sizeof(*xr->xr_cookies));
 +
 +	xr->xr_ep = 0;
 +	xr->xr_cs = 1;
 +}
 +
  /*
   * Set TR Dequeue Pointer.
   * xCHI 1.1  4.6.10  6.4.3.9
 @@ -1619,13 +1636,7 @@
  	XHCIHIST_FUNC(); XHCIHIST_CALLED();
  	DPRINTFN(4, "slot %u dci %u", xs->xs_idx, dci, 0, 0);
  
 -	memset(xr->xr_trb, 0, xr->xr_ntrb * XHCI_TRB_SIZE);
 -	usb_syncmem(&xr->xr_dma, 0, xr->xr_ntrb * XHCI_TRB_SIZE,
 -	    BUS_DMASYNC_PREWRITE);
 -	memset(xr->xr_cookies, 0, xr->xr_ntrb * sizeof(*xr->xr_cookies));
 -
 -	xr->xr_ep = 0;
 -	xr->xr_cs = 1;
 +	xhci_host_dequeue(xr);
  
  	/* set DCS */
  	trb.trb_0 = xhci_ring_trbp(xr, 0) | 1; /* XXX */
 @@ -2449,13 +2460,10 @@
  	if (err)
  		return err;
  	mutex_init(&xr->xr_lock, MUTEX_DEFAULT, IPL_SOFTUSB);
 -	xr->xr_cookies = kmem_zalloc(sizeof(*xr->xr_cookies) * ntrb, KM_SLEEP);
 +	xr->xr_cookies = kmem_alloc(sizeof(*xr->xr_cookies) * ntrb, KM_SLEEP);
  	xr->xr_trb = xhci_ring_trbv(xr, 0);
  	xr->xr_ntrb = ntrb;
 -	xr->xr_ep = 0;
 -	xr->xr_cs = 1;
 -	memset(xr->xr_trb, 0, size);
 -	usb_syncmem(&xr->xr_dma, 0, size, BUS_DMASYNC_PREWRITE);
 +	xhci_host_dequeue(xr);
  	xr->is_halted = false;
  
  	return USBD_NORMAL_COMPLETION;
 @@ -2898,6 +2906,10 @@
  	*(uint64_t *)(&cp[2]) = htole64(
  	    xhci_ring_trbp(&xs->xs_ep[XHCI_DCI_EP_CONTROL].xe_tr, 0) |
  	    XHCI_EPCTX_2_DCS_SET(1));
 +	/* this performs an implicit dequeue of the ep transfer ring,
 +	 * but the call to xhci_ring_init above has dequeued the host
 +	 * view of the structures already. 
 +	 */
  	cp[4] = htole32(
  		XHCI_EPCTX_4_AVG_TRB_LEN_SET(8)
  		);