Hello Tom,
On 28/08/18(Tue) 11:10, Tom Murphy wrote:
> On Tue, Aug 28, 2018 at 02:49:38PM +0900, Bryan Linton wrote:
> > On 2018-08-25 21:40:57, Tom Murphy <open...@pertho.net> wrote:
> > > On Thu, Aug 23, 2018 at 08:45:54PM +0900, Tom Murphy wrote:
> > > > I've narrowed it down.
> > > >
> > > >Last kernel where adb works: June 24 09:59:46 MDT 2018
> > > >1st Kernel where adb panics: June 25 13:10:32 MDT 2018
The real problem is in the xhci(4) driver. When a command with a
timeout is submitted we should ensure no other command is enqueued
before continuing. Sadly the driver did not include any mechanism
to serialize command submissions. Diff below does that and should
fix your problem.
Can you try it on top of -current? Make sure you have no diff
reverted.
Index: xhci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/xhci.c,v
retrieving revision 1.87
diff -u -p -r1.87 xhci.c
--- xhci.c 16 Jul 2018 07:48:17 -0000 1.87
+++ xhci.c 28 Aug 2018 19:12:11 -0000
@@ -25,6 +25,7 @@
#include <sys/timeout.h>
#include <sys/pool.h>
#include <sys/endian.h>
+#include <sys/rwlock.h>
#include <machine/bus.h>
@@ -344,6 +345,7 @@ xhci_init(struct xhci_softc *sc)
return (ENOMEM);
/* Setup command ring. */
+ rw_init(&sc->sc_cmd_lock, "xhcicmd");
error = xhci_ring_alloc(sc, &sc->sc_cmd_ring, XHCI_MAX_CMDS,
XHCI_CMDS_RING_ALIGN);
if (error) {
@@ -1663,7 +1665,7 @@ xhci_command_submit(struct xhci_softc *s
return (0);
}
- assertwaitok();
+ rw_assert_wrlock(&sc->sc_cmd_lock);
s = splusb();
sc->sc_cmd_trb = trb;
@@ -1729,6 +1731,7 @@ int
xhci_cmd_configure_ep(struct xhci_softc *sc, uint8_t slot, uint64_t addr)
{
struct xhci_trb trb;
+ int error;
DPRINTF(("%s: %s dev %u\n", DEVNAME(sc), __func__, slot));
@@ -1738,13 +1741,17 @@ xhci_cmd_configure_ep(struct xhci_softc
XHCI_TRB_SET_SLOT(slot) | XHCI_CMD_CONFIG_EP
);
- return (xhci_command_submit(sc, &trb, XHCI_CMD_TIMEOUT));
+ rw_enter_write(&sc->sc_cmd_lock);
+ error = xhci_command_submit(sc, &trb, XHCI_CMD_TIMEOUT);
+ rw_exit_write(&sc->sc_cmd_lock);
+ return (error);
}
int
xhci_cmd_stop_ep(struct xhci_softc *sc, uint8_t slot, uint8_t dci)
{
struct xhci_trb trb;
+ int error;
DPRINTF(("%s: %s dev %u dci %u\n", DEVNAME(sc), __func__, slot, dci));
@@ -1754,7 +1761,10 @@ xhci_cmd_stop_ep(struct xhci_softc *sc,
XHCI_TRB_SET_SLOT(slot) | XHCI_TRB_SET_EP(dci) | XHCI_CMD_STOP_EP
);
- return (xhci_command_submit(sc, &trb, XHCI_CMD_TIMEOUT));
+ rw_enter_write(&sc->sc_cmd_lock);
+ error = xhci_command_submit(sc, &trb, XHCI_CMD_TIMEOUT);
+ rw_exit_write(&sc->sc_cmd_lock);
+ return (error);
}
void
@@ -1794,6 +1804,7 @@ int
xhci_cmd_slot_control(struct xhci_softc *sc, uint8_t *slotp, int enable)
{
struct xhci_trb trb;
+ int error;
DPRINTF(("%s: %s\n", DEVNAME(sc), __func__));
@@ -1806,7 +1817,10 @@ xhci_cmd_slot_control(struct xhci_softc
XHCI_TRB_SET_SLOT(*slotp) | XHCI_CMD_DISABLE_SLOT
);
- if (xhci_command_submit(sc, &trb, XHCI_CMD_TIMEOUT))
+ rw_enter_write(&sc->sc_cmd_lock);
+ error = xhci_command_submit(sc, &trb, XHCI_CMD_TIMEOUT);
+ rw_exit_write(&sc->sc_cmd_lock);
+ if (error != 0)
return (EIO);
if (enable)
@@ -1820,6 +1834,7 @@ xhci_cmd_set_address(struct xhci_softc *
uint32_t bsr)
{
struct xhci_trb trb;
+ int error;
DPRINTF(("%s: %s BSR=%u\n", DEVNAME(sc), __func__, bsr ? 1 : 0));
@@ -1829,13 +1844,17 @@ xhci_cmd_set_address(struct xhci_softc *
XHCI_TRB_SET_SLOT(slot) | XHCI_CMD_ADDRESS_DEVICE | bsr
);
- return (xhci_command_submit(sc, &trb, XHCI_CMD_TIMEOUT));
+ rw_enter_write(&sc->sc_cmd_lock);
+ error = xhci_command_submit(sc, &trb, XHCI_CMD_TIMEOUT);
+ rw_exit_write(&sc->sc_cmd_lock);
+ return (error);
}
int
xhci_cmd_evaluate_ctx(struct xhci_softc *sc, uint8_t slot, uint64_t addr)
{
struct xhci_trb trb;
+ int error;
DPRINTF(("%s: %s dev %u\n", DEVNAME(sc), __func__, slot));
@@ -1845,7 +1864,10 @@ xhci_cmd_evaluate_ctx(struct xhci_softc
XHCI_TRB_SET_SLOT(slot) | XHCI_CMD_EVAL_CTX
);
- return (xhci_command_submit(sc, &trb, XHCI_CMD_TIMEOUT));
+ rw_enter_write(&sc->sc_cmd_lock);
+ error = xhci_command_submit(sc, &trb, XHCI_CMD_TIMEOUT);
+ rw_exit_write(&sc->sc_cmd_lock);
+ return (error);
}
#ifdef XHCI_DEBUG
@@ -1853,6 +1875,7 @@ int
xhci_cmd_noop(struct xhci_softc *sc)
{
struct xhci_trb trb;
+ int error;
DPRINTF(("%s: %s\n", DEVNAME(sc), __func__));
@@ -1860,7 +1883,10 @@ xhci_cmd_noop(struct xhci_softc *sc)
trb.trb_status = 0;
trb.trb_flags = htole32(XHCI_CMD_NOOP);
- return (xhci_command_submit(sc, &trb, XHCI_CMD_TIMEOUT));
+ rw_enter_write(&sc->sc_cmd_lock);
+ error = xhci_command_submit(sc, &trb, XHCI_CMD_TIMEOUT);
+ rw_exit_write(&sc->sc_cmd_lock);
+ return (error);
}
#endif
Index: xhcivar.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/xhcivar.h,v
retrieving revision 1.9
diff -u -p -r1.9 xhcivar.h
--- xhcivar.h 8 May 2018 13:41:52 -0000 1.9
+++ xhcivar.h 28 Aug 2018 19:07:19 -0000
@@ -102,6 +102,7 @@ struct xhci_softc {
struct xhci_devctx sc_dcbaa; /* Device context base addr. */
struct xhci_ring sc_cmd_ring; /* Command ring */
+ struct rwlock sc_cmd_lock; /* Serialize commands */
struct xhci_erst sc_erst; /* Event ring segment table */
struct xhci_ring sc_evt_ring; /* Event ring */
