diff --git a/usr.sbin/bhyve/pci_xhci.c b/usr.sbin/bhyve/pci_xhci.c index d8d471a34fc8..59719caa4647 100644 --- a/usr.sbin/bhyve/pci_xhci.c +++ b/usr.sbin/bhyve/pci_xhci.c @@ -38,6 +38,7 @@ #include #include +#include #include #include #include @@ -210,7 +211,16 @@ struct pci_xhci_portregs { /* xHC operational registers */ struct pci_xhci_opregs { uint32_t usbcmd; /* usb command */ - uint32_t usbsts; /* usb status */ + _Atomic uint32_t usbsts; /* usb status: mutated from both the + * MMIO/vCPU thread and a backend's + * event thread (usb_mouse/usb_passthru + * hci_intr(), usb_passthru hci_event()). + * Every update here is an unconditional + * set/clear of independent bits, so + * this is atomic rather than mutex + * protected -- see port_mtx for why a + * mutex is unsafe to take from those + * callbacks. */ uint32_t pgsz; /* page size */ uint32_t dnctrl; /* device notification control */ uint64_t crcr; /* command ring control */ @@ -280,6 +290,31 @@ struct pci_xhci_softc { * which also needs this lock (under sc->mtx). * Lock order: sc->mtx -> [xfer_lock ->] * event_mtx */ + pthread_mutex_t port_mtx; /* serialises all portsc mutations + * against each other across threads: + * portregs_write/reset_port/ + * usbcmd_write (MMIO/vCPU thread) vs. + * dev_event/dev_intr, called back into + * from usb_mouse/usb_passthru's own + * threads. This may NOT be sc->mtx: + * usb_passthru's synchronous control + * transfers can re-enter hci_intr() on + * the vCPU thread *while it already + * holds sc->mtx*, so a callee-side lock + * on sc->mtx here would self-deadlock a + * non-recursive mutex, independent of + * any cross-thread ordering concern. + * Consequently port_mtx must never be + * held across pci_xhci_insert_event/ + * pci_xhci_assert_interrupt, nor while + * calling into a backend's + * ue_request/ue_data -- otherwise a + * reentrant hci_intr()/hci_event() call + * could deadlock trying to reacquire + * it. opregs.usbsts is handled via + * atomics instead (see its declaration) + * for the same reason, since it is also + * touched from these callbacks. */ uint32_t caplength; /* caplen & hciversion */ uint32_t hcsparams1; /* structural parameters 1 */ @@ -309,7 +344,9 @@ struct pci_xhci_softc { #define XHCI_DEVINST_PTR(x,n) ((x)->devices[(n) - 1]) #define XHCI_SLOTDEV_PTR(x,n) ((x)->slots[(n) - 1]) -#define XHCI_HALTED(sc) ((sc)->opregs.usbsts & XHCI_STS_HCH) +#define XHCI_HALTED(sc) \ + (atomic_load_explicit(&(sc)->opregs.usbsts, memory_order_seq_cst) & \ + XHCI_STS_HCH) #define XHCI_GADDR_SIZE(a) (XHCI_PADDR_SZ - \ (((uint64_t) (a)) & (XHCI_PADDR_SZ - 1))) @@ -393,6 +430,25 @@ pci_xhci_reset(struct pci_xhci_softc *sc) } } +/* + * Atomically clear one set of usbsts bits and set another as a single + * transaction, so a concurrent reader (guest MMIO read, or another + * backend's hci_intr()/hci_event()) never observes a state where only one + * side of the pair has taken effect. + */ +static void +pci_xhci_usbsts_swap(struct pci_xhci_softc *sc, uint32_t clear_bits, + uint32_t set_bits) +{ + uint32_t old, new; + + old = atomic_load_explicit(&sc->opregs.usbsts, memory_order_relaxed); + do { + new = (old & ~clear_bits) | set_bits; + } while (!atomic_compare_exchange_weak_explicit(&sc->opregs.usbsts, + &old, new, memory_order_seq_cst, memory_order_relaxed)); +} + static uint32_t pci_xhci_usbcmd_write(struct pci_xhci_softc *sc, uint32_t cmd) { @@ -403,8 +459,7 @@ pci_xhci_usbcmd_write(struct pci_xhci_softc *sc, uint32_t cmd) do_intr = (sc->opregs.usbcmd & XHCI_CMD_RS) == 0; sc->opregs.usbcmd |= XHCI_CMD_RS; - sc->opregs.usbsts &= ~XHCI_STS_HCH; - sc->opregs.usbsts |= XHCI_STS_PCD; + pci_xhci_usbsts_swap(sc, XHCI_STS_HCH, XHCI_STS_PCD); /* Queue port change event on controller run from stop */ if (do_intr) @@ -412,24 +467,25 @@ pci_xhci_usbcmd_write(struct pci_xhci_softc *sc, uint32_t cmd) struct pci_xhci_dev_emu *dev; struct pci_xhci_portregs *port; struct xhci_trb evtrb; + uint32_t pls; if ((dev = XHCI_DEVINST_PTR(sc, i)) == NULL) continue; port = XHCI_PORTREG_PTR(sc, i); - port->portsc |= XHCI_PS_CSC | XHCI_PS_CCS; - port->portsc &= ~XHCI_PS_PLS_MASK; /* * XHCI 4.19.3 USB2 RxDetect->Polling, * USB3 Polling->U0 */ - if (dev->hci.hci_usbver <= 2) - port->portsc |= - XHCI_PS_PLS_SET(UPS_PORT_LS_POLL); - else - port->portsc |= - XHCI_PS_PLS_SET(UPS_PORT_LS_U0); + pls = (dev->hci.hci_usbver <= 2) ? + UPS_PORT_LS_POLL : UPS_PORT_LS_U0; + + pthread_mutex_lock(&sc->port_mtx); + port->portsc |= XHCI_PS_CSC | XHCI_PS_CCS; + port->portsc &= ~XHCI_PS_PLS_MASK; + port->portsc |= XHCI_PS_PLS_SET(pls); + pthread_mutex_unlock(&sc->port_mtx); pci_xhci_set_evtrb(&evtrb, i, XHCI_TRB_ERROR_SUCCESS, @@ -441,8 +497,7 @@ pci_xhci_usbcmd_write(struct pci_xhci_softc *sc, uint32_t cmd) } } else { sc->opregs.usbcmd &= ~XHCI_CMD_RS; - sc->opregs.usbsts |= XHCI_STS_HCH; - sc->opregs.usbsts &= ~XHCI_STS_PCD; + pci_xhci_usbsts_swap(sc, XHCI_STS_PCD, XHCI_STS_HCH); } /* start execution of schedule; stop when set to 0 */ @@ -495,25 +550,32 @@ pci_xhci_portregs_write(struct pci_xhci_softc *sc, uint64_t offset, p = XHCI_PORTREG_PTR(sc, port); switch (offset) { - case 0: + case 0: { + bool need_event = false; + /* port reset or warm reset */ if (value & (XHCI_PS_PR | XHCI_PS_WPR)) { pci_xhci_reset_port(sc, port, value & XHCI_PS_WPR); break; } + pthread_mutex_lock(&sc->port_mtx); + if ((p->portsc & XHCI_PS_PP) == 0) { if (value & XHCI_PS_PP) { pci_xhci_init_port(sc, port); p->portsc |= XHCI_PS_CSC; + need_event = true; + } else { + WPRINTF(("pci_xhci: portregs_write to unpowered " + "port %d", port)); + } + pthread_mutex_unlock(&sc->port_mtx); + if (need_event) { pci_xhci_set_evtrb(&evtrb, port, XHCI_TRB_ERROR_SUCCESS, XHCI_TRB_EVENT_PORT_STS_CHANGE); - pci_xhci_insert_event(sc, &evtrb, 1); - } else { - WPRINTF(("pci_xhci: portregs_write to unpowered " - "port %d", port)); } break; } @@ -552,34 +614,37 @@ pci_xhci_portregs_write(struct pci_xhci_softc *sc, uint64_t offset, if (value & XHCI_PS_PED) DPRINTF(("Disable port %d request", port)); - if (!(value & XHCI_PS_LWS)) - break; - - DPRINTF(("Port new PLS: %d", newpls)); - switch (newpls) { - case 0: /* U0 */ - case 3: /* U3 */ - if (oldpls != newpls) { - p->portsc &= ~XHCI_PS_PLS_MASK; - p->portsc |= XHCI_PS_PLS_SET(newpls) | - XHCI_PS_PLC; - - if (oldpls != 0 && newpls == 0) { - pci_xhci_set_evtrb(&evtrb, port, - XHCI_TRB_ERROR_SUCCESS, - XHCI_TRB_EVENT_PORT_STS_CHANGE); - - pci_xhci_insert_event(sc, &evtrb, 1); + if (value & XHCI_PS_LWS) { + DPRINTF(("Port new PLS: %d", newpls)); + switch (newpls) { + case 0: /* U0 */ + case 3: /* U3 */ + if (oldpls != newpls) { + p->portsc &= ~XHCI_PS_PLS_MASK; + p->portsc |= XHCI_PS_PLS_SET(newpls) | + XHCI_PS_PLC; + need_event = (oldpls != 0 && + newpls == 0); } + break; + + default: + DPRINTF(("Unhandled change port %d PLS %u", + port, newpls)); + break; } - break; + } - default: - DPRINTF(("Unhandled change port %d PLS %u", - port, newpls)); - break; + pthread_mutex_unlock(&sc->port_mtx); + + if (need_event) { + pci_xhci_set_evtrb(&evtrb, port, + XHCI_TRB_ERROR_SUCCESS, + XHCI_TRB_EVENT_PORT_STS_CHANGE); + pci_xhci_insert_event(sc, &evtrb, 1); } break; + } case 4: /* Port power management status and control register */ p->portpmsc = value; @@ -657,7 +722,8 @@ pci_xhci_assert_interrupt(struct pci_xhci_softc *sc) sc->rtsregs.intrreg.erdp |= XHCI_ERDP_LO_BUSY; sc->rtsregs.intrreg.iman |= XHCI_IMAN_INTR_PEND; - sc->opregs.usbsts |= XHCI_STS_EINT; + atomic_fetch_or_explicit(&sc->opregs.usbsts, XHCI_STS_EINT, + memory_order_seq_cst); /* only trigger interrupt if permitted */ if ((sc->opregs.usbcmd & XHCI_CMD_INTE) && @@ -2324,9 +2390,11 @@ pci_xhci_hostop_write(struct pci_xhci_softc *sc, uint64_t offset, case XHCI_USBSTS: /* clear bits on write */ - sc->opregs.usbsts &= ~(value & + atomic_fetch_and_explicit(&sc->opregs.usbsts, + (uint32_t)~(value & (XHCI_STS_HSE|XHCI_STS_EINT|XHCI_STS_PCD|XHCI_STS_SSS| - XHCI_STS_RSS|XHCI_STS_SRE|XHCI_STS_CNR)); + XHCI_STS_RSS|XHCI_STS_SRE|XHCI_STS_CNR)), + memory_order_seq_cst); break; case XHCI_PAGESIZE: @@ -2481,7 +2549,8 @@ pci_xhci_hostop_read(struct pci_xhci_softc *sc, uint64_t offset) break; case XHCI_USBSTS: /* 0x04 */ - value = sc->opregs.usbsts; + value = atomic_load_explicit(&sc->opregs.usbsts, + memory_order_seq_cst); break; case XHCI_PAGESIZE: /* 0x08 */ @@ -2665,6 +2734,7 @@ pci_xhci_reset_port(struct pci_xhci_softc *sc, int portn, int warm) struct pci_xhci_dev_emu *dev; struct xhci_trb evtrb; int error; + bool need_event = false; assert(portn <= XHCI_MAX_DEVS); @@ -2672,6 +2742,7 @@ pci_xhci_reset_port(struct pci_xhci_softc *sc, int portn, int warm) port = XHCI_PORTREG_PTR(sc, portn); dev = XHCI_DEVINST_PTR(sc, portn); + pthread_mutex_lock(&sc->port_mtx); if (dev) { port->portsc &= ~(XHCI_PS_PLS_MASK | XHCI_PS_PR | XHCI_PS_PRC); port->portsc |= XHCI_PS_PED | @@ -2683,16 +2754,19 @@ pci_xhci_reset_port(struct pci_xhci_softc *sc, int portn, int warm) if ((port->portsc & XHCI_PS_PRC) == 0) { port->portsc |= XHCI_PS_PRC; - - pci_xhci_set_evtrb(&evtrb, portn, - XHCI_TRB_ERROR_SUCCESS, - XHCI_TRB_EVENT_PORT_STS_CHANGE); - error = pci_xhci_insert_event(sc, &evtrb, 1); - if (error != XHCI_TRB_ERROR_SUCCESS) - DPRINTF(("xhci reset port insert event " - "failed")); + need_event = true; } } + pthread_mutex_unlock(&sc->port_mtx); + + if (need_event) { + pci_xhci_set_evtrb(&evtrb, portn, + XHCI_TRB_ERROR_SUCCESS, + XHCI_TRB_EVENT_PORT_STS_CHANGE); + error = pci_xhci_insert_event(sc, &evtrb, 1); + if (error != XHCI_TRB_ERROR_SUCCESS) + DPRINTF(("xhci reset port insert event failed")); + } } static void @@ -2780,19 +2854,25 @@ pci_xhci_dev_intr(struct usb_hci *hci, int epctx) p = XHCI_PORTREG_PTR(sc, hci->hci_port); /* raise event if link U3 (suspended) state */ + pthread_mutex_lock(&sc->port_mtx); if (XHCI_PS_PLS_GET(p->portsc) == 3) { p->portsc &= ~XHCI_PS_PLS_MASK; p->portsc |= XHCI_PS_PLS_SET(UPS_PORT_LS_RESUME); - if ((p->portsc & XHCI_PS_PLC) != 0) + if ((p->portsc & XHCI_PS_PLC) != 0) { + pthread_mutex_unlock(&sc->port_mtx); return (0); + } p->portsc |= XHCI_PS_PLC; + pthread_mutex_unlock(&sc->port_mtx); pci_xhci_set_evtrb(&evtrb, hci->hci_port, XHCI_TRB_ERROR_SUCCESS, XHCI_TRB_EVENT_PORT_STS_CHANGE); error = pci_xhci_insert_event(sc, &evtrb, 0); if (error != XHCI_TRB_ERROR_SUCCESS) goto done; + } else { + pthread_mutex_unlock(&sc->port_mtx); } dev_ctx = dev->dev_ctx; @@ -2826,25 +2906,31 @@ pci_xhci_dev_event(struct usb_hci *hci, enum hci_usbev evid, switch (evid) { case USBDEV_ATTACH: + pthread_mutex_lock(&xsc->port_mtx); pci_xhci_init_port(xsc, hci->hci_port); port->portsc |= XHCI_PS_CSC; + pthread_mutex_unlock(&xsc->port_mtx); pci_xhci_set_evtrb(&evtrb, hci->hci_port, XHCI_TRB_ERROR_SUCCESS, XHCI_TRB_EVENT_PORT_STS_CHANGE); if ((err = pci_xhci_insert_event(xsc, &evtrb, 1)) != XHCI_TRB_ERROR_SUCCESS) return (err); - xsc->opregs.usbsts |= XHCI_STS_PCD; + atomic_fetch_or_explicit(&xsc->opregs.usbsts, XHCI_STS_PCD, + memory_order_seq_cst); pci_xhci_assert_interrupt(xsc); return (0); case USBDEV_REMOVE: + pthread_mutex_lock(&xsc->port_mtx); pci_xhci_deinit_port(xsc, hci->hci_port); port->portsc |= XHCI_PS_CSC; + pthread_mutex_unlock(&xsc->port_mtx); pci_xhci_set_evtrb(&evtrb, hci->hci_port, XHCI_TRB_ERROR_SUCCESS, XHCI_TRB_EVENT_PORT_STS_CHANGE); if ((err = pci_xhci_insert_event(xsc, &evtrb, 1)) != XHCI_TRB_ERROR_SUCCESS) return (err); - xsc->opregs.usbsts |= XHCI_STS_PCD; + atomic_fetch_or_explicit(&xsc->opregs.usbsts, XHCI_STS_PCD, + memory_order_seq_cst); pci_xhci_assert_interrupt(xsc); return (0); default: @@ -3033,7 +3119,9 @@ pci_xhci_parse_devices(struct pci_xhci_softc *sc, nvlist_t *nvl) if (ndevices > 0) { for (i = 1; i <= XHCI_MAX_DEVS; i++) { + pthread_mutex_lock(&sc->port_mtx); pci_xhci_init_port(sc, i); + pthread_mutex_unlock(&sc->port_mtx); } } else { WPRINTF(("pci_xhci no USB devices configured")); @@ -3075,10 +3163,20 @@ pci_xhci_init(struct pci_devinst *pi, nvlist_t *nvl) pthread_mutex_init(&sc->mtx, NULL); pthread_mutex_init(&sc->event_mtx, NULL); + pthread_mutex_init(&sc->port_mtx, NULL); sc->usb2_port_start = (XHCI_MAX_DEVS/2) + 1; sc->usb3_port_start = 1; + /* + * Must be set before pci_xhci_parse_devices(), which runs each + * backend's ue_init() -- for a dynamic backend (e.g. usb_passthru) + * that starts its hotplug/event thread, which can call back into + * pci_xhci_dev_event()/pci_xhci_dev_intr() and touch opregs.usbsts + * before this function otherwise would have initialized it. + */ + sc->opregs.usbsts = XHCI_STS_HCH; + /* discover devices */ error = pci_xhci_parse_devices(sc, nvl); if (error < 0) @@ -3115,7 +3213,6 @@ pci_xhci_init(struct pci_devinst *pi, nvlist_t *nvl) DPRINTF(("pci_xhci dboff: 0x%x, rtsoff: 0x%x", sc->dboff, sc->rtsoff)); - sc->opregs.usbsts = XHCI_STS_HCH; sc->opregs.pgsz = XHCI_PAGESIZE_4K; pci_xhci_reset(sc); @@ -3146,6 +3243,7 @@ pci_xhci_init(struct pci_devinst *pi, nvlist_t *nvl) done: if (error) { + pthread_mutex_destroy(&sc->port_mtx); pthread_mutex_destroy(&sc->event_mtx); pthread_mutex_destroy(&sc->mtx); free(sc);