Fix a problem reported by Pierre Beyssac. Sometinmes when ndis_get_info()

calls MiniportQueryInformation(), it will return NDIS_STATUS_PENDING.
When this happens, ndis_get_info() will sleep waiting for a completion
event. If two threads call ndis_get_info() and both end up having to
sleep, they will both end up waiting on the same wait channel, which
can cause a panic in sleepq_add() if INVARIANTS are turned on.

Fix this by having ndis_get_info() use a common mutex rather than
using the process mutex with PROC_LOCK(). Also do the same for
ndis_set_info(). Note that Pierre's original patch also made ndis_thsuspend()
use the new mutex, but ndis_thsuspend() shouldn't need this since
it will make each thread that calls it sleep on a unique wait channel.

Also, it occured to me that we probably don't want to enter
MiniportQueryInformation() or MiniportSetInformation() from more
than one thread at any given time, so now we acquire a Windows
spinlock before calling either of them. The Microsoft documentation
says that MiniportQueryInformation() and MiniportSetInformation()
are called at DISPATCH_LEVEL, and previously we would call
KeRaiseIrql() to set the IRQL to DISPATCH_LEVEL before entering
either routine, but this only guarantees mutual exclusion on
uniprocessor machines. To make it SMP safe, we need to use a real
spinlock. For now, I'm abusing the spinlock embedded in the
NDIS_MINIPORT_BLOCK structure for this purpose. (This may need to be
applied to some of the other routines in kern_ndis.c at a later date.)

Export ntoskrnl_init_lock() (KeInitializeSpinlock()) from subr_ntoskrnl.c
since we need to use in in kern_ndis.c, and since it's technically part
of the Windows kernel DDK API along with the other spinlock routines. Use
it in subr_ndis.c too rather than frobbing the spinlock directly.

4 files changed, 23 insertions, 18 deletions

diff --git a/sys/compat/ndis/kern_ndis.c b/sys/compat/ndis/kern_ndis.c
index 5e44ab8..c0efdae 100644
--- a/sys/compat/ndis/kern_ndis.c
+++ b/sys/compat/ndis/kern_ndis.c
@@ -110,6 +110,7 @@ static void ndis_runq(void *);
 
 static uma_zone_t ndis_packet_zone, ndis_buffer_zone;
 struct mtx ndis_thr_mtx;
+struct mtx ndis_req_mtx;
 static STAILQ_HEAD(ndisqhead, ndis_req) ndis_ttodo;
 struct ndisqhead ndis_itodo;
 struct ndisqhead ndis_free;
@@ -259,6 +260,8 @@ ndis_create_kthreads()
 
 	mtx_init(&ndis_thr_mtx, "NDIS thread lock",
 	   MTX_NDIS_LOCK, MTX_DEF);
+	mtx_init(&ndis_req_mtx, "NDIS request lock",
+	   MTX_NDIS_LOCK, MTX_DEF);
 
 	STAILQ_INIT(&ndis_ttodo);
 	STAILQ_INIT(&ndis_itodo);
@@ -317,6 +320,7 @@ ndis_destroy_kthreads()
 		free(r, M_DEVBUF);
 	}
 
+	mtx_destroy(&ndis_req_mtx);
 	mtx_destroy(&ndis_thr_mtx);
 
 	return;
@@ -566,7 +570,7 @@ ndis_setdone_func(adapter, status)
 	block = adapter;
 
 	block->nmb_setstat = status;
-	wakeup(&block->nmb_wkupdpctimer);
+	wakeup(&block->nmb_setstat);
 	return;
 }
 
@@ -579,7 +583,7 @@ ndis_getdone_func(adapter, status)
 	block = adapter;
 
 	block->nmb_getstat = status;
-	wakeup(&block->nmb_wkupdpctimer);
+	wakeup(&block->nmb_getstat);
 	return;
 }
 
@@ -1132,15 +1136,15 @@ ndis_set_info(arg, oid, buf, buflen)
 	if (adapter == NULL || setfunc == NULL)
 		return(ENXIO);
 
-	irql = ntoskrnl_raise_irql(DISPATCH_LEVEL);
+	ntoskrnl_acquire_spinlock(&sc->ndis_block.nmb_lock, &irql);
 	rval = setfunc(adapter, oid, buf, *buflen,
 	    &byteswritten, &bytesneeded);
-	ntoskrnl_lower_irql(irql);
+	ntoskrnl_release_spinlock(&sc->ndis_block.nmb_lock, irql);
 
 	if (rval == NDIS_STATUS_PENDING) {
-		PROC_LOCK(curthread->td_proc);
-		error = msleep(&sc->ndis_block.nmb_wkupdpctimer,
-		    &curthread->td_proc->p_mtx,
+		mtx_lock(&ndis_req_mtx);
+		error = msleep(&sc->ndis_block.nmb_setstat,
+		    &ndis_req_mtx,
 		    curthread->td_priority|PDROP,
 		    "ndisset", 5 * hz);
 		rval = sc->ndis_block.nmb_setstat;
@@ -1322,8 +1326,8 @@ ndis_reset_nic(arg)
 	ntoskrnl_lower_irql(irql);
 
 	if (rval == NDIS_STATUS_PENDING) {
-		PROC_LOCK(curthread->td_proc);
-		msleep(sc, &curthread->td_proc->p_mtx,
+		mtx_lock(&ndis_req_mtx);
+		msleep(sc, &ndis_req_mtx,
 		    curthread->td_priority|PDROP, "ndisrst", 0);
 	}
 
@@ -1550,17 +1554,17 @@ ndis_get_info(arg, oid, buf, buflen)
 	if (adapter == NULL || queryfunc == NULL)
 		return(ENXIO);
 
-	irql = ntoskrnl_raise_irql(DISPATCH_LEVEL);
+	ntoskrnl_acquire_spinlock(&sc->ndis_block.nmb_lock, &irql);
 	rval = queryfunc(adapter, oid, buf, *buflen,
 	    &byteswritten, &bytesneeded);
-	ntoskrnl_lower_irql(irql);
+	ntoskrnl_release_spinlock(&sc->ndis_block.nmb_lock, irql);
 
 	/* Wait for requests that block. */
 
 	if (rval == NDIS_STATUS_PENDING) {
-		PROC_LOCK(curthread->td_proc);
-		error = msleep(&sc->ndis_block.nmb_wkupdpctimer,
-		    &curthread->td_proc->p_mtx,
+		mtx_lock(&ndis_req_mtx);
+		error = msleep(&sc->ndis_block.nmb_getstat,
+		    &ndis_req_mtx,
 		    curthread->td_priority|PDROP,
 		    "ndisget", 5 * hz);
 		rval = sc->ndis_block.nmb_getstat;
@@ -1707,6 +1711,7 @@ ndis_load_driver(img, arg)
 	ndis_enlarge_thrqueue(8);
 
 	TAILQ_INSERT_TAIL(&ndis_devhead, block, link);
+	ntoskrnl_init_lock(&block->nmb_lock);
 
 	return(0);
 }
diff --git a/sys/compat/ndis/ntoskrnl_var.h b/sys/compat/ndis/ntoskrnl_var.h
index 8681320..60eceaf 100644
--- a/sys/compat/ndis/ntoskrnl_var.h
+++ b/sys/compat/ndis/ntoskrnl_var.h
@@ -508,6 +508,7 @@ __stdcall extern void ntoskrnl_clear_event(nt_kevent *);
 __stdcall extern uint32_t ntoskrnl_read_event(nt_kevent *);
 __stdcall extern uint32_t ntoskrnl_set_event(nt_kevent *, uint32_t, uint8_t);
 __stdcall extern uint32_t ntoskrnl_reset_event(nt_kevent *);
+__stdcall extern void ntoskrnl_init_lock(kspin_lock *);
 __fastcall extern void ntoskrnl_lock_dpc(REGARGS1(kspin_lock *));
 __fastcall extern void ntoskrnl_unlock_dpc(REGARGS1(kspin_lock *));
 
diff --git a/sys/compat/ndis/subr_ndis.c b/sys/compat/ndis/subr_ndis.c
index ca57489..7953fe1 100644
--- a/sys/compat/ndis/subr_ndis.c
+++ b/sys/compat/ndis/subr_ndis.c
@@ -776,7 +776,7 @@ __stdcall static void
 ndis_create_lock(lock)
 	ndis_spin_lock		*lock;
 {
-	lock->nsl_spinlock = 0;
+	ntoskrnl_init_lock(&lock->nsl_spinlock);
 	lock->nsl_kirql = 0;
 
 	return;
@@ -795,7 +795,7 @@ ndis_destroy_lock(lock)
 	ndis_spin_lock		*lock;
 {
 #ifdef notdef
-	lock->nsl_spinlock = 0;
+	ntoskrnl_init_lock(&lock->nsl_spinlock);
 	lock->nsl_kirql = 0;
 #endif
 	return;
diff --git a/sys/compat/ndis/subr_ntoskrnl.c b/sys/compat/ndis/subr_ntoskrnl.c
index 0fc123c..5cfa724 100644
--- a/sys/compat/ndis/subr_ntoskrnl.c
+++ b/sys/compat/ndis/subr_ntoskrnl.c
@@ -142,7 +142,6 @@ __stdcall static void *ntoskrnl_mmaplockedpages(ndis_buffer *, uint8_t);
 __stdcall static void *ntoskrnl_mmaplockedpages_cache(ndis_buffer *,
 	uint8_t, uint32_t, void *, uint32_t, uint32_t);
 __stdcall static void ntoskrnl_munmaplockedpages(void *, ndis_buffer *);
-__stdcall static void ntoskrnl_init_lock(kspin_lock *);
 __stdcall static size_t ntoskrnl_memcmp(const void *, const void *, size_t);
 __stdcall static void ntoskrnl_init_ansi_string(ndis_ansi_string *, char *);
 __stdcall static void ntoskrnl_init_unicode_string(ndis_unicode_string *,
@@ -1137,7 +1136,7 @@ ntoskrnl_munmaplockedpages(vaddr, buf)
  * lock here because there is no complimentary KeFreeSpinLock()
  * function. Instead, we grab a mutex from the mutex pool.
  */
-__stdcall static void
+__stdcall void
 ntoskrnl_init_lock(lock)
 	kspin_lock		*lock;
 {