From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: wsmouse(4) and wstpad: mp-safe filterops
To: Alexander Bluhm <bluhm@openbsd.org>, "Kirill A. Korinsky" <kirill@openbsd.org>, tech@openbsd.org
Date: Thu, 23 Jan 2025 13:14:31 +0300

The last one.

As in wskbd(4), we don't share `sc_refcnt' with interrupts, so we don't
need to protect it. May be `sc_refcnt' should be replaced with refcnt
structure, may be completely removed - it's not yet clean to me.

Please note, filt_wseventread() is the only thread that runs
simultaneously with the rest of the wscons(4). This thread only compare
`ws_get' with `ws_put'. wscons(4) drivers only increment `ws_put' index
so at driver side we only need to protect this increment to ensure
`ws_put' is consistent with WSEVENT_QSIZE. I wrapped `ws_put' and
`ws_get' access with `ws_mtx' mutex(9) but this is not required.

Index: sys/dev/wscons/wsmouse.c
===================================================================
RCS file: /cvs/src/sys/dev/wscons/wsmouse.c,v
diff -u -p -r1.74 wsmouse.c
--- sys/dev/wscons/wsmouse.c	30 Dec 2024 02:46:00 -0000	1.74
+++ sys/dev/wscons/wsmouse.c	23 Jan 2025 09:43:25 -0000
@@ -249,7 +249,6 @@ wsmouse_detach(struct device *self, int 
 	struct wsmouse_softc *sc = (struct wsmouse_softc *)self;
 	struct wseventvar *evar;
 	int maj, mn;
-	int s;
 
 #if NWSMUX > 0
 	/* Tell parent mux we're leaving. */
@@ -262,18 +261,18 @@ wsmouse_detach(struct device *self, int 
 	/* If we're open ... */
 	evar = sc->sc_base.me_evp;
 	if (evar != NULL) {
-		s = spltty();
 		if (--sc->sc_refcnt >= 0) {
+			mtx_enter(&evar->ws_mtx);
 			/* Wake everyone by generating a dummy event. */
 			if (++evar->ws_put >= WSEVENT_QSIZE)
 				evar->ws_put = 0;
-			WSEVENT_WAKEUP(evar);
+			mtx_leave(&evar->ws_mtx);
+			wsevent_wakeup(evar);
 			/* Wait for processes to go away. */
 			if (tsleep_nsec(sc, PZERO, "wsmdet", SEC_TO_NSEC(60)))
 				printf("wsmouse_detach: %s didn't detach\n",
 				       sc->sc_base.me_dv.dv_xname);
 		}
-		splx(s);
 	}
 
 	/* locate the major number */
@@ -326,7 +325,7 @@ wsmouseopen(dev_t dev, int flags, int mo
 		return (EBUSY);
 
 	evar = &sc->sc_base.me_evar;
-	if (wsevent_init(evar))
+	if (wsevent_init_flags(evar, WSEVENT_MPSAFE))
 		return (EBUSY);
 
 	error = wsmousedoopen(sc, evar);
@@ -495,7 +494,9 @@ wsmouse_do_ioctl(struct wsmouse_softc *s
 	case FIOASYNC:
 		if (sc->sc_base.me_evp == NULL)
 			return (EINVAL);
+		mtx_enter(&sc->sc_base.me_evp->ws_mtx);
 		sc->sc_base.me_evp->ws_async = *(int *)data != 0;
+		mtx_leave(&sc->sc_base.me_evp->ws_mtx);
 		return (0);
 
 	case FIOGETOWN:
@@ -955,7 +956,10 @@ wsmouse_evq_put(struct evq_access *evq, 
 	struct wscons_event *ev;
 	int space;
 
+	mtx_enter(&evq->evar->ws_mtx);
 	space = evq->evar->ws_get - evq->put;
+	mtx_leave(&evq->evar->ws_mtx);
+
 	if (space != 1 && space != 1 - WSEVENT_QSIZE) {
 		ev = &evq->evar->ws_q[evq->put++];
 		evq->put %= WSEVENT_QSIZE;
@@ -1099,7 +1103,11 @@ void
 wsmouse_log_events(struct wsmouseinput *input, struct evq_access *evq)
 {
 	struct wscons_event *ev;
-	int n = evq->evar->ws_put;
+	int n;
+
+	mtx_enter(&evq->evar->ws_mtx);
+	n = evq->evar->ws_put;
+	mtx_leave(&evq->evar->ws_mtx);
 
 	if (n != evq->put) {
 		printf("[%s-ev][%04d]", DEVNAME(input), LOGTIME(&evq->ts));
@@ -1138,7 +1146,9 @@ wsmouse_input_sync(struct device *sc)
 	evq.evar = *input->evar;
 	if (evq.evar == NULL)
 		return;
+	mtx_enter(&evq.evar->ws_mtx);
 	evq.put = evq.evar->ws_put;
+	mtx_leave(&evq.evar->ws_mtx);
 	evq.result = EVQ_RESULT_NONE;
 	getnanotime(&evq.ts);
 
@@ -1181,8 +1191,10 @@ wsmouse_input_sync(struct device *sc)
 			if (input->flags & LOG_EVENTS) {
 				wsmouse_log_events(input, &evq);
 			}
+			mtx_enter(&evq.evar->ws_mtx);
 			evq.evar->ws_put = evq.put;
-			WSEVENT_WAKEUP(evq.evar);
+			mtx_leave(&evq.evar->ws_mtx);
+			wsevent_wakeup(evq.evar);
 		}
 	}
 
Index: sys/dev/wscons/wstpad.c
===================================================================
RCS file: /cvs/src/sys/dev/wscons/wstpad.c,v
diff -u -p -r1.34 wstpad.c
--- sys/dev/wscons/wstpad.c	25 Mar 2024 13:01:49 -0000	1.34
+++ sys/dev/wscons/wstpad.c	23 Jan 2025 09:43:25 -0000
@@ -931,7 +931,9 @@ wstpad_tap_timeout(void *p)
 			tp->tap.state = TAP_DETECT;
 		}
 		if (ev) {
+			mtx_enter(&evq.evar->ws_mtx);
 			evq.put = evq.evar->ws_put;
+			mtx_leave(&evq.evar->ws_mtx);
 			evq.result = EVQ_RESULT_NONE;
 			getnanotime(&evq.ts);
 			wsmouse_evq_put(&evq, ev, btn);
@@ -940,8 +942,10 @@ wstpad_tap_timeout(void *p)
 				if (input->flags & LOG_EVENTS) {
 					wsmouse_log_events(input, &evq);
 				}
+				mtx_enter(&evq.evar->ws_mtx);
 				evq.evar->ws_put = evq.put;
-				WSEVENT_WAKEUP(evq.evar);
+				mtx_leave(&evq.evar->ws_mtx);
+				wsevent_wakeup(evq.evar);
 			} else {
 				input->sbtn.sync |= tp->tap.button;
 			}