Comment #135 for bugzilla 237666 : a USB3-handling problem with a investigatory fix for a cortex-a72 context

[Mark Millard]

On 2020-Sep-19, at 13:13, Hans Petter Selasky <hps at selasky.org> wrote:

> On 2020-09-19 21:58, Mark Millard wrote:
>> In my context:
>> # 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,7 @@
>>    	phwr->hwr_ring_seg[0].qwEvrsTablePtr = htole64(addr);
>>  	phwr->hwr_ring_seg[0].dwEvrsTableSize = htole32(XHCI_MAX_EVENTS);
>> +	usb_bus_mem_flush_all(&sc->sc_bus, &xhci_iterate_hw_softc);
>>    	DPRINTF("ERDP(0)=0x%016llx\n", (unsigned long long)addr);
>>  The result booted just fine, no "Resetting controller" notices at
> 
> Could you add a comment describing the need for this flush? And then I think we can push that fix upstream!

I'll take a stab at it (not in this note) but 1.5 days ago or
so I had no clue about such things as xhci event rings,
mailbox/boorbell handling on aarch64, and so on. You may find
your own wording(s) more reasonable than whatever I come up with.

I'll note that if aarch64 requires the DSB ST and DSB LD additions
in the two dev/usb/usb_busdma.c routines, then powerpc64, powerpc,
and powerpcspe probably(?) have SYNC-related additions needed as
well. (I do not have access to any modern powerpc* hardware that
would have xhci involved.) There could be additions required for
other architectures (that I'm unlikely to have any familiarity
with or access to, other than amd64).

Also, my diffs are relative to back a ways: -r363590 . While things
have looked very close during this in the relevant files, I've
avoiding synchronizing to head during the recent large changes
and various problems I'd been monitoring the status of for some
time. (And before that other parts of life had priority.)

As stands, in my head -r363590 based context, the patch set for this
overall currently looks like (up to E-mail variability in spaces):

# svnlite diff /usr/src/sys/dev/usb/usb_busdma.c /usr/src/sys/dev/usb/controller/xhci.c
Index: /usr/src/sys/dev/usb/usb_busdma.c
===================================================================
--- /usr/src/sys/dev/usb/usb_busdma.c	(revision 363590)
+++ /usr/src/sys/dev/usb/usb_busdma.c	(working copy)
@@ -737,6 +737,9 @@
 	 */
 	bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_POSTREAD);
 	bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_PREREAD);
+#ifdef __aarch64__
+__asm __volatile("dsb ld" : : : "memory");
+#endif
 }
 
 /*------------------------------------------------------------------------*
@@ -750,6 +753,9 @@
 		return;
 	}
 	bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_PREWRITE);
+#ifdef __aarch64__
+__asm __volatile("dsb st" : : : "memory");
+#endif
 }
 
 /*------------------------------------------------------------------------*
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,7 @@
 
 	phwr->hwr_ring_seg[0].qwEvrsTablePtr = htole64(addr);
 	phwr->hwr_ring_seg[0].dwEvrsTableSize = htole32(XHCI_MAX_EVENTS);
+	usb_bus_mem_flush_all(&sc->sc_bus, &xhci_iterate_hw_softc);
 
 	DPRINTF("ERDP(0)=0x%016llx\n", (unsigned long long)addr);
 


===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)