Date: Sat, 19 Sep 2020 10:08:54 +0200 From: Hans Petter Selasky <hps@selasky.org> To: Mark Millard <marklmi@yahoo.com>, freebsd-arm <freebsd-arm@freebsd.org> Subject: Re: Comment #135 for bugzilla 237666 : a USB3-handling problem with a investigatory fix for a cortex-a72 context Message-ID: <866bd652-5a8b-7500-b1e0-7528c035048b@selasky.org> In-Reply-To: <6E618C3D-12DF-429E-A249-5BAB90FC6B15@yahoo.com> References: <6E618C3D-12DF-429E-A249-5BAB90FC6B15.ref@yahoo.com> <6E618C3D-12DF-429E-A249-5BAB90FC6B15@yahoo.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2020-09-19 05:41, Mark Millard via freebsd-arm wrote: > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=237666 is about USB3 > problems that folks have been having for over a year. Bjoern A. Zeeb > comment #125 that looked like something I'd seen on a cortex-a72 when > I tried a kernel that had been built with -mcpu=cortex-a72 : an > indefinite looping that involved "Resetting controller" for xhci0. > (This prevented booting from the USB3 device.) > > Well, I got the A72 to boot and comment #135 indicates how but I'll > paste the information below. All that was involved was adding > examples of "dsb st" and one "dsb ld". > > > Comment 135's content: > > A cortex-a72 success! (In the form of investigatory code, not > code to check-in as is.) > > Just by adding some "dsb st" commands and a "dsb ld" command in > places in the xhci code (just for __arach64__), I've gotten the > cortext-A72 to boot from the USB3 SSD via a -mcpu=cortex-a72 > based kernel build. No more indefinitely repeating "Resetting > controller" problem. > > I do not claim the additions are the minimal ones. I know one place > is required for sure: the last one added enabled the boot (given > the others were already present). Prior to that I still had the > indefinitely repeating "Resetting controller" problem. > > There could be more places that should have dsb st or dsb ld for > overall correctness. > > This evidence may suggest somewhat analogous problems for other > platforms than aarch64 that are seeing problems. > > For the aarch64 context, the evidence is (the changes are) > as follows. The first "dsb st" textually is the last one I > added. > > # svnlite diff /usr/src/sys/dev/usb/controller/xhci.c > Index: /usr/src/sys/dev/usb/controller/xhci.c > =================================================================== > --- /usr/src/sys/dev/usb/controller/xhci.c (revision 363590) > +++ /usr/src/sys/dev/usb/controller/xhci.c (working copy) > @@ -431,6 +431,9 @@ > > phwr->hwr_ring_seg[0].qwEvrsTablePtr = htole64(addr); > phwr->hwr_ring_seg[0].dwEvrsTableSize = htole32(XHCI_MAX_EVENTS); > +#ifdef __aarch64__ > +__asm __volatile("dsb st" : : : "memory"); > +#endif > > DPRINTF("ERDP(0)=0x%016llx\n", (unsigned long long)addr); > > @@ -1098,6 +1101,9 @@ > > while (1) { > > +#ifdef __aarch64__ > +__asm __volatile("dsb ld" : : : "memory"); > +#endif > temp = le32toh(phwr->hwr_events[i].dwTrb3); > > k = (temp & XHCI_TRB_3_CYCLE_BIT) ? 1 : 0; > @@ -1107,6 +1113,9 @@ > > event = XHCI_TRB_3_TYPE_GET(temp); > > +#ifdef __aarch64__ > +__asm __volatile("dsb ld" : : : "memory"); > +#endif > DPRINTFN(10, "event[%u] = %u (0x%016llx 0x%08lx 0x%08lx)\n", > i, event, (long long)le64toh(phwr->hwr_events[i].qwTrb0), > (long)le32toh(phwr->hwr_events[i].dwTrb2), > @@ -1239,6 +1248,9 @@ > sc->sc_command_idx = i; > sc->sc_command_ccs = j; > > +#ifdef __aarch64__ > +__asm __volatile("dsb st" : : : "memory"); > +#endif > XWRITE4(sc, door, XHCI_DOORBELL(0), 0); > > err = cv_timedwait(&sc->sc_cmd_cv, &sc->sc_bus.bus_mtx, > @@ -2855,6 +2867,9 @@ > index = xfer->xroot->udev->controller_slot_id; > > if (xfer->xroot->udev->flags.self_suspended == 0) { > +#ifdef __aarch64__ > +__asm __volatile("dsb st" : : : "memory"); > +#endif > XWRITE4(sc, door, XHCI_DOORBELL(index), > epno | XHCI_DB_SID_SET(xfer->stream_id)); > } > @@ -4180,6 +4195,9 @@ > > for (n = 1; n != XHCI_MAX_ENDPOINTS; n++) { > for (p = 0; p != XHCI_MAX_STREAMS; p++) { > +#ifdef __aarch64__ > +__asm __volatile("dsb st" : : : "memory"); > +#endif > XWRITE4(sc, door, XHCI_DOORBELL(index), > n | XHCI_DB_SID_SET(p)); > } > > > I'm clearly just "evidence hacking" here. I've no clue what a > good design for allowing aarch64 build to supply having any of > those "dsb st"s or the "dsb ld". > > Hopefully someone that knows what they are doing can figure out > if any of the other examples on other platforms are analogous > --and ,if they are, provide some hook for platform to contribute > such code to the xhci code. > Hi Mark, Your finding indicate a problem in usb_pc_cpu_flush() and bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_PREWRITE); Try to put the dsb only after dmamap_sync. void usb_pc_cpu_flush(struct usb_page_cache *pc) { if (pc->page_offset_end == pc->page_offset_buf) { /* nothing has been loaded into this page cache! */ return; } bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_PREWRITE); } The PCI I/O memory should be coherent and should not need any DSB's. --HPS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?866bd652-5a8b-7500-b1e0-7528c035048b>