From owner-freebsd-arm@freebsd.org Sat Sep 19 08:09:33 2020 Return-Path: Delivered-To: freebsd-arm@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id DAE553FE1CA for ; Sat, 19 Sep 2020 08:09:33 +0000 (UTC) (envelope-from hps@selasky.org) Received: from mail.turbocat.net (turbocat.net [88.99.82.50]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4BtjzJ6WL3z4kSR for ; Sat, 19 Sep 2020 08:09:32 +0000 (UTC) (envelope-from hps@selasky.org) Received: from hps2020.home.selasky.org (unknown [178.17.145.105]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.turbocat.net (Postfix) with ESMTPSA id 9416D26010A; Sat, 19 Sep 2020 10:09:25 +0200 (CEST) Subject: Re: Comment #135 for bugzilla 237666 : a USB3-handling problem with a investigatory fix for a cortex-a72 context To: Mark Millard , freebsd-arm References: <6E618C3D-12DF-429E-A249-5BAB90FC6B15.ref@yahoo.com> <6E618C3D-12DF-429E-A249-5BAB90FC6B15@yahoo.com> From: Hans Petter Selasky Message-ID: <866bd652-5a8b-7500-b1e0-7528c035048b@selasky.org> Date: Sat, 19 Sep 2020 10:08:54 +0200 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <6E618C3D-12DF-429E-A249-5BAB90FC6B15@yahoo.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 4BtjzJ6WL3z4kSR X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of hps@selasky.org designates 88.99.82.50 as permitted sender) smtp.mailfrom=hps@selasky.org X-Spamd-Result: default: False [-2.38 / 15.00]; RCVD_TLS_ALL(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; FROM_HAS_DN(0.00)[]; R_SPF_ALLOW(-0.20)[+a:mail.turbocat.net]; NEURAL_HAM_LONG(-1.01)[-1.014]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[selasky.org]; ARC_NA(0.00)[]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; NEURAL_HAM_SHORT(-0.11)[-0.110]; RCPT_COUNT_TWO(0.00)[2]; NEURAL_HAM_MEDIUM(-0.96)[-0.959]; FREEMAIL_TO(0.00)[yahoo.com,freebsd.org]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:24940, ipnet:88.99.0.0/16, country:DE]; RCVD_COUNT_TWO(0.00)[2]; MAILMAN_DEST(0.00)[freebsd-arm] X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 19 Sep 2020 08:09:33 -0000 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