From: Atanas Vladimirov <vlado@bsdbg.net>
Subject: Fwd: Re: OpenBSD and Supermicro BMC/IPMI keyboard
To: tech@openbsd.org
Date: Sat, 29 Nov 2025 01:25:35 +0200

xhci(4): move root hub status change handling to a task to allow port 
debouncing

This change addresses a race condition observed with SuperMicro/ATEN BMC 
iKVM devices.

Any comments are welcome - please note that I'm not an USB expert and 
sorry if I wrote something stupid.

Index: dev/usb/xhci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/xhci.c,v
diff -u -p -r1.136 xhci.c
--- dev/usb/xhci.c	1 Mar 2025 14:43:03 -0000	1.136
+++ dev/usb/xhci.c	26 Nov 2025 09:56:52 -0000
@@ -54,6 +54,72 @@ int xhcidebug = 3;
  #define TRBOFF(r, trb)	((char *)(trb) - (char *)((r)->trbs))
  #define DEQPTR(r)	((r).dma.paddr + (sizeof(struct xhci_trb) * 
(r).index))

+#define XHCI_PORT_DEBOUNCE_MS	300
+
+void
+xhci_rh_task(void *arg)
+{
+	struct xhci_softc *sc = arg;
+	struct usbd_xfer *xfer = sc->sc_intrxfer;
+	uint8_t *p;
+	int i, s;
+	uint32_t portsc;
+	int delay_needed = 0;
+
+	if (xfer == NULL)
+		return;
+
+	/* * Scan ALL ports for new connections.
+	 * If we find any port with a Connect Status Change (CSC)
+	 * that is currently Connected (CCS), we need to debounce.
+	 */
+	for (i = 1; i <= sc->sc_noport; i++) {
+		portsc = XOREAD4(sc, XHCI_PORTSC(i));
+
+		if ((portsc & XHCI_PS_CSC) && (portsc & XHCI_PS_CCS)) {
+			delay_needed = 1;
+			/* One event is enough to trigger the global wait */
+			break;
+		}
+	}
+
+	if (delay_needed) {
+#ifdef XHCI_DEBUG
+		printf("%s: New connection detected - debouncing for %dms\n",
+		    DEVNAME(sc), XHCI_PORT_DEBOUNCE_MS);
+#endif
+		usb_delay_ms(&sc->sc_bus, XHCI_PORT_DEBOUNCE_MS);
+	}
+
+	s = splusb();
+	p = KERNADDR(&xfer->dmabuf, 0);
+	memset(p, 0, xfer->length);
+
+	int has_events = 0;
+	for (i = 1; i <= sc->sc_noport; i++) {
+		portsc = XOREAD4(sc, XHCI_PORTSC(i));
+
+		/* Check for any status change bits */
+		if (portsc & (XHCI_PS_CSC | XHCI_PS_PEC | XHCI_PS_OCC |
+		              XHCI_PS_WRC | XHCI_PS_PRC | XHCI_PS_PLC | XHCI_PS_CEC)) 
{
+
+			/* Mark the bit in the bitmap */
+			p[i/8] |= 1 << (i%8);
+			has_events = 1;
+
+			DPRINTF(("%s: port %d status change: 0x%08x\n",
+			    DEVNAME(sc), i, portsc));
+		}
+	}
+
+	if (has_events) {
+		xfer->actlen = xfer->length;
+		xfer->status = USBD_NORMAL_COMPLETION;
+		usb_transfer_complete(xfer);
+	}
+	splx(s);
+}
+
  struct pool *xhcixfer;

  struct xhci_pipe {
@@ -295,6 +361,9 @@ xhci_init(struct xhci_softc *sc)
  	uint32_t hcr;
  	int npage, error;

+	/* Initialize the generic Root Hub task */
+	usb_init_task(&sc->sc_rh_task, xhci_rh_task, sc, 
USB_TASK_TYPE_GENERIC);
+
  	sc->sc_bus.usbrev = USBREV_3_0;
  	sc->sc_bus.methods = &xhci_bus_methods;
  	sc->sc_bus.pipe_size = sizeof(struct xhci_pipe);
@@ -1180,8 +1249,6 @@ void
  xhci_event_port_change(struct xhci_softc *sc, uint64_t paddr, uint32_t 
status)
  {
  	struct usbd_xfer *xfer = sc->sc_intrxfer;
-	uint32_t port = XHCI_TRB_PORTID(paddr);
-	uint8_t *p;

  	if (XHCI_TRB_GET_CODE(status) != XHCI_CODE_SUCCESS) {
  		DPRINTF(("%s: failed port status event\n", DEVNAME(sc)));
@@ -1191,16 +1258,7 @@ xhci_event_port_change(struct xhci_softc
  	if (xfer == NULL)
  		return;

-	p = KERNADDR(&xfer->dmabuf, 0);
-	memset(p, 0, xfer->length);
-
-	p[port/8] |= 1 << (port%8);
-	DPRINTF(("%s: port=%d change=0x%02x\n", DEVNAME(sc), port, *p));
-
-	xfer->actlen = xfer->length;
-	xfer->status = USBD_NORMAL_COMPLETION;
-
-	usb_transfer_complete(xfer);
+	usb_add_task(xfer->device, &sc->sc_rh_task);
  }

  void
Index: dev/usb/xhcivar.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/xhcivar.h,v
diff -u -p -r1.17 xhcivar.h
--- dev/usb/xhcivar.h	1 Feb 2025 22:46:34 -0000	1.17
+++ dev/usb/xhcivar.h	26 Nov 2025 09:56:52 -0000
@@ -124,6 +124,8 @@ struct xhci_softc {

  	int			 sc_flags;
  #define XHCI_NOCSS		 0x01
+
+	struct usb_task		 sc_rh_task;
  };

  int	xhci_init(struct xhci_softc *);