d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

When you call MiniportInitialize() for an 802.11 driver, it will

Browse Source 
at some point result in a status event being triggered (it should
be a link down event: the Microsoft driver design guide says you
should generate one when the NIC is initialized). Some drivers
generate the event during MiniportInitialize(), such that by the
time MiniportInitialize() completes, the NIC is ready to go. But
some drivers, in particular the ones for Atheros wireless NICs,
don't generate the event until after a device interrupt occurs
at some point after MiniportInitialize() has completed.

The gotcha is that you have to wait until the link status event
occurs one way or the other before you try to fiddle with any
settings (ssid, channel, etc...). For the drivers that set the
event sycnhronously this isn't a problem, but for the others
we have to pause after calling ndis_init_nic() and wait for the event
to arrive before continuing. Failing to wait can cause big trouble:
on my SMP system, calling ndis_setstate_80211() after ndis_init_nic()
completes, but _before_ the link event arrives, will lock up or
reset the system.

What we do now is check to see if a link event arrived while
ndis_init_nic() was running, and if it didn't we msleep() until
it does.

Along the way, I discovered a few other problems:

- Defered procedure calls run at PASSIVE_LEVEL, not DISPATCH_LEVEL.
  ntoskrnl_run_dpc() has been fixed accordingly. (I read the documentation
  wrong.)

- Similarly, the NDIS interrupt handler, which is essentially a
  DPC, also doesn't need to run at DISPATCH_LEVEL. ndis_intrtask()
  has been fixed accordingly.

- MiniportQueryInformation() and MiniportSetInformation() run at
  DISPATCH_LEVEL, and each request must complete before another
  can be submitted. ndis_get_info() and ndis_set_info() have been
  fixed accordingly.

- Turned the sleep lock that guards the NDIS thread job list into
  a spin lock. We never do anything with this lock held except manage
  the job list (no other locks are held), so it's safe to do this,
  and it's possible that ndis_sched() and ndis_unsched() can be
  called from DISPATCH_LEVEL, so using a sleep lock here is
  semantically incorrect. Also updated subr_witness.c to add the
  lock to the order list.
This commit is contained in:
wpaul 2005-03-07 03:05:31 +00:00
parent 427fcfc7b2
commit fba490d861
4 changed files with 120 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

129
sys/compat/ndis/kern_ndis.c
View File

@ -239,19 +239,19 @@ ndis_runq(arg)
/* Look for any jobs on the work queue. */
mtx_lock(&ndis_thr_mtx);
mtx_lock_spin(&ndis_thr_mtx);
p->np_state = NDIS_PSTATE_RUNNING;
while(STAILQ_FIRST(p->np_q) != NULL) {
r = STAILQ_FIRST(p->np_q);
STAILQ_REMOVE_HEAD(p->np_q, link);
mtx_unlock(&ndis_thr_mtx);
mtx_unlock_spin(&ndis_thr_mtx);
/* Do the work. */
if (r->nr_func != NULL)
(*r->nr_func)(r->nr_arg);
mtx_lock(&ndis_thr_mtx);
mtx_lock_spin(&ndis_thr_mtx);
STAILQ_INSERT_HEAD(&ndis_free, r, link);
/* Check for a shutdown request */
@ -260,7 +260,7 @@ ndis_runq(arg)
die = r;
}
p->np_state = NDIS_PSTATE_SLEEPING;
mtx_unlock(&ndis_thr_mtx);
mtx_unlock_spin(&ndis_thr_mtx);
/* Bail if we were told to shut down. */
@ -282,10 +282,8 @@ ndis_create_kthreads()
struct ndis_req *r;
int i, error = 0;
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);
mtx_init(&ndis_thr_mtx, "NDIS thread lock", NULL, MTX_SPIN);
mtx_init(&ndis_req_mtx, "NDIS request lock", MTX_NDIS_LOCK, MTX_DEF);
STAILQ_INIT(&ndis_ttodo);
STAILQ_INIT(&ndis_itodo);
@ -368,14 +366,14 @@ ndis_stop_thread(t)
/* Create and post a special 'exit' job. */
mtx_lock(&ndis_thr_mtx);
mtx_lock_spin(&ndis_thr_mtx);
r = STAILQ_FIRST(&ndis_free);
STAILQ_REMOVE_HEAD(&ndis_free, link);
r->nr_func = NULL;
r->nr_arg = NULL;
r->nr_exit = TRUE;
STAILQ_INSERT_TAIL(q, r, link);
mtx_unlock(&ndis_thr_mtx);
mtx_unlock_spin(&ndis_thr_mtx);
ndis_thresume(p);
@ -385,12 +383,12 @@ ndis_stop_thread(t)
/* Now empty the job list. */
mtx_lock(&ndis_thr_mtx);
mtx_lock_spin(&ndis_thr_mtx);
while ((r = STAILQ_FIRST(q)) != NULL) {
STAILQ_REMOVE_HEAD(q, link);
STAILQ_INSERT_HEAD(&ndis_free, r, link);
}
mtx_unlock(&ndis_thr_mtx);
mtx_unlock_spin(&ndis_thr_mtx);
return;
}
@ -406,10 +404,10 @@ ndis_enlarge_thrqueue(cnt)
r = malloc(sizeof(struct ndis_req), M_DEVBUF, M_WAITOK);
if (r == NULL)
return(ENOMEM);
mtx_lock(&ndis_thr_mtx);
mtx_lock_spin(&ndis_thr_mtx);
STAILQ_INSERT_HEAD(&ndis_free, r, link);
ndis_jobs++;
mtx_unlock(&ndis_thr_mtx);
mtx_unlock_spin(&ndis_thr_mtx);
}
return(0);
@ -423,15 +421,15 @@ ndis_shrink_thrqueue(cnt)
int i;
for (i = 0; i < cnt; i++) {
mtx_lock(&ndis_thr_mtx);
mtx_lock_spin(&ndis_thr_mtx);
r = STAILQ_FIRST(&ndis_free);
if (r == NULL) {
mtx_unlock(&ndis_thr_mtx);
mtx_unlock_spin(&ndis_thr_mtx);
return(ENOMEM);
}
STAILQ_REMOVE_HEAD(&ndis_free, link);
ndis_jobs--;
mtx_unlock(&ndis_thr_mtx);
mtx_unlock_spin(&ndis_thr_mtx);
free(r, M_DEVBUF);
}
@ -456,17 +454,17 @@ ndis_unsched(func, arg, t)
p = ndis_iproc.np_p;
}
mtx_lock(&ndis_thr_mtx);
mtx_lock_spin(&ndis_thr_mtx);
STAILQ_FOREACH(r, q, link) {
if (r->nr_func == func && r->nr_arg == arg) {
STAILQ_REMOVE(q, r, ndis_req, link);
STAILQ_INSERT_HEAD(&ndis_free, r, link);
mtx_unlock(&ndis_thr_mtx);
mtx_unlock_spin(&ndis_thr_mtx);
return(0);
}
}
mtx_unlock(&ndis_thr_mtx);
mtx_unlock_spin(&ndis_thr_mtx);
return(ENOENT);
}
@ -490,20 +488,20 @@ ndis_sched(func, arg, t)
p = ndis_iproc.np_p;
}
mtx_lock(&ndis_thr_mtx);
mtx_lock_spin(&ndis_thr_mtx);
/*
* Check to see if an instance of this job is already
* pending. If so, don't bother queuing it again.
*/
STAILQ_FOREACH(r, q, link) {
if (r->nr_func == func && r->nr_arg == arg) {
mtx_unlock(&ndis_thr_mtx);
mtx_unlock_spin(&ndis_thr_mtx);
return(0);
}
}
r = STAILQ_FIRST(&ndis_free);
if (r == NULL) {
mtx_unlock(&ndis_thr_mtx);
mtx_unlock_spin(&ndis_thr_mtx);
return(EAGAIN);
}
STAILQ_REMOVE_HEAD(&ndis_free, link);
@ -515,7 +513,7 @@ ndis_sched(func, arg, t)
s = ndis_tproc.np_state;
else
s = ndis_iproc.np_state;
mtx_unlock(&ndis_thr_mtx);
mtx_unlock_spin(&ndis_thr_mtx);
/*
* Post the job, but only if the thread is actually blocked
@ -635,7 +633,7 @@ ndis_resetdone_func(adapter, status, addressingreset)
if (ifp->if_flags & IFF_DEBUG)
device_printf (sc->ndis_dev, "reset done...\n");
wakeup(ifp);
wakeup(sc);
return;
}
@ -1153,29 +1151,46 @@ ndis_set_info(arg, oid, buf, buflen)
int error;
uint8_t irql;
/*
* According to the NDIS spec, MiniportQueryInformation()
* and MiniportSetInformation() requests are handled serially:
* once one request has been issued, we must wait for it to
* finish before allowing another request to proceed.
*/
mtx_lock(&ndis_req_mtx);
sc = arg;
NDIS_LOCK(sc);
if (sc->ndis_block->nmb_pendingreq != NULL)
panic("ndis_set_info() called while other request pending");
else
sc->ndis_block->nmb_pendingreq = (ndis_request *)sc;
setfunc = sc->ndis_chars->nmc_setinfo_func;
adapter = sc->ndis_block->nmb_miniportadapterctx;
NDIS_UNLOCK(sc);
if (adapter == NULL || setfunc == NULL)
if (adapter == NULL || setfunc == NULL) {
mtx_unlock(&ndis_req_mtx);
return(ENXIO);
}
KeAcquireSpinLock(&sc->ndis_block->nmb_lock, &irql);
irql = KeRaiseIrql(DISPATCH_LEVEL);
rval = MSCALL6(setfunc, adapter, oid, buf, *buflen,
&byteswritten, &bytesneeded);
KeReleaseSpinLock(&sc->ndis_block->nmb_lock, irql);
KeLowerIrql(irql);
if (rval == NDIS_STATUS_PENDING) {
mtx_lock(&ndis_req_mtx);
error = msleep(&sc->ndis_block->nmb_setstat,
&ndis_req_mtx,
curthread->td_priority|PDROP,
curthread->td_priority,
"ndisset", 5 * hz);
rval = sc->ndis_block->nmb_setstat;
}
sc->ndis_block->nmb_pendingreq = NULL;
mtx_unlock(&ndis_req_mtx);
if (byteswritten)
*buflen = byteswritten;
if (bytesneeded)
@ -1211,7 +1226,7 @@ ndis_send_packets(arg, packets, cnt)
__stdcall ndis_senddone_func senddonefunc;
int i;
ndis_packet *p;
uint8_t irql;
int irql;
sc = arg;
adapter = sc->ndis_block->nmb_miniportadapterctx;
@ -1219,9 +1234,12 @@ ndis_send_packets(arg, packets, cnt)
return(ENXIO);
sendfunc = sc->ndis_chars->nmc_sendmulti_func;
senddonefunc = sc->ndis_block->nmb_senddone_func;
irql = KeRaiseIrql(DISPATCH_LEVEL);
if (!(sc->ndis_block->nmb_flags & NDIS_ATTRIBUTE_DESERIALIZE))
KeAcquireSpinLock(&sc->ndis_block->nmb_lock, &irql);
MSCALL3(sendfunc, adapter, packets, cnt);
KeLowerIrql(irql);
if (!(sc->ndis_block->nmb_flags & NDIS_ATTRIBUTE_DESERIALIZE))
KeReleaseSpinLock(&sc->ndis_block->nmb_lock, irql);
for (i = 0; i < cnt; i++) {
p = packets[i];
@ -1258,10 +1276,12 @@ ndis_send_packet(arg, packet)
sendfunc = sc->ndis_chars->nmc_sendsingle_func;
senddonefunc = sc->ndis_block->nmb_senddone_func;
irql = KeRaiseIrql(DISPATCH_LEVEL);
if (!(sc->ndis_block->nmb_flags & NDIS_ATTRIBUTE_DESERIALIZE))
KeAcquireSpinLock(&sc->ndis_block->nmb_lock, &irql);
status = MSCALL3(sendfunc, adapter, packet,
packet->np_private.npp_flags);
KeLowerIrql(irql);
if (!(sc->ndis_block->nmb_flags & NDIS_ATTRIBUTE_DESERIALIZE))
KeReleaseSpinLock(&sc->ndis_block->nmb_lock, irql);
if (status == NDIS_STATUS_PENDING)
return(0);
@ -1341,10 +1361,10 @@ ndis_reset_nic(arg)
sc = arg;
ifp = &sc->arpcom.ac_if;
NDIS_LOCK(sc);
adapter = sc->ndis_block->nmb_miniportadapterctx;
resetfunc = sc->ndis_chars->nmc_reset_func;
NDIS_UNLOCK(sc);
if (adapter == NULL || resetfunc == NULL)
return(EIO);
@ -1519,10 +1539,12 @@ ndis_isr(arg, ourintr, callhandler)
sc = arg;
adapter = sc->ndis_block->nmb_miniportadapterctx;
isrfunc = sc->ndis_chars->nmc_isr_func;
if (adapter == NULL || isrfunc == NULL)
return(ENXIO);
MSCALL3(isrfunc, &accepted, &queue, adapter);
*ourintr = accepted;
*callhandler = queue;
@ -1541,10 +1563,10 @@ ndis_intrhand(arg)
return(EINVAL);
sc = arg;
NDIS_LOCK(sc);
adapter = sc->ndis_block->nmb_miniportadapterctx;
intrfunc = sc->ndis_chars->nmc_interrupt_func;
NDIS_UNLOCK(sc);
if (adapter == NULL || intrfunc == NULL)
return(EINVAL);
@ -1568,31 +1590,41 @@ ndis_get_info(arg, oid, buf, buflen)
int error;
uint8_t irql;
mtx_lock(&ndis_req_mtx);
sc = arg;
NDIS_LOCK(sc);
if (sc->ndis_block->nmb_pendingreq != NULL)
panic("ndis_get_info() called while other request pending");
else
sc->ndis_block->nmb_pendingreq = (ndis_request *)sc;
queryfunc = sc->ndis_chars->nmc_queryinfo_func;
adapter = sc->ndis_block->nmb_miniportadapterctx;
NDIS_UNLOCK(sc);
if (adapter == NULL || queryfunc == NULL)
if (adapter == NULL || queryfunc == NULL) {
mtx_unlock(&ndis_req_mtx);
return(ENXIO);
}
KeAcquireSpinLock(&sc->ndis_block->nmb_lock, &irql);
irql = KeRaiseIrql(DISPATCH_LEVEL);
rval = MSCALL6(queryfunc, adapter, oid, buf, *buflen,
&byteswritten, &bytesneeded);
KeReleaseSpinLock(&sc->ndis_block->nmb_lock, irql);
KeLowerIrql(irql);
/* Wait for requests that block. */
if (rval == NDIS_STATUS_PENDING) {
mtx_lock(&ndis_req_mtx);
error = msleep(&sc->ndis_block->nmb_getstat,
&ndis_req_mtx,
curthread->td_priority|PDROP,
curthread->td_priority,
"ndisget", 5 * hz);
rval = sc->ndis_block->nmb_getstat;
}
sc->ndis_block->nmb_pendingreq = NULL;
mtx_unlock(&ndis_req_mtx);
if (byteswritten)
*buflen = byteswritten;
if (bytesneeded)
@ -1655,6 +1687,7 @@ NdisAddDevice(drv, pdo)
block->nmb_querydone_func = kernndis_functbl[3].ipt_wrap;
block->nmb_resetdone_func = kernndis_functbl[4].ipt_wrap;
block->nmb_sendrsrc_func = kernndis_functbl[5].ipt_wrap;
block->nmb_pendingreq = NULL;
ndis_enlarge_thrqueue(8);

3
sys/compat/ndis/subr_ntoskrnl.c
View File

@ -2423,14 +2423,11 @@ ntoskrnl_run_dpc(arg)
{
__stdcall kdpc_func dpcfunc;
kdpc *dpc;
uint8_t irql;
dpc = arg;
dpcfunc = dpc->k_deferedfunc;
irql = KeRaiseIrql(DISPATCH_LEVEL);
MSCALL4(dpcfunc, dpc, dpc->k_deferredctx,
dpc->k_sysarg1, dpc->k_sysarg2);
KeLowerIrql(irql);
return;
}

42
sys/dev/if_ndis/if_ndis.c
View File

@ -44,6 +44,7 @@ __FBSDID("$FreeBSD$");
#include <sys/socket.h>
#include <sys/queue.h>
#include <sys/module.h>
#include <sys/proc.h>
#if __FreeBSD_version < 502113
#include <sys/sysctl.h>
#endif
@ -1080,9 +1081,14 @@ ndis_linksts(adapter, status, sbuf, slen)
uint32_t slen;
{
ndis_miniport_block *block;
struct ndis_softc *sc;
block = adapter;
sc = device_get_softc(block->nmb_physdeviceobj->do_devext);
NDIS_LOCK(sc);
block->nmb_getstat = status;
NDIS_UNLOCK(sc);
return;
}
@ -1113,6 +1119,10 @@ ndis_linksts_done(adapter)
case NDIS_STATUS_MEDIA_DISCONNECT:
if (sc->ndis_link)
ndis_sched(ndis_ticktask, sc, NDIS_TASKQUEUE);
else {
if (sc->ndis_80211)
wakeup(&block->nmb_getstat);
}
break;
default:
break;
@ -1128,14 +1138,11 @@ ndis_intrtask(arg)
{
struct ndis_softc *sc;
struct ifnet *ifp;
uint8_t irql;
sc = arg;
ifp = &sc->arpcom.ac_if;
irql = KeRaiseIrql(DISPATCH_LEVEL);
ndis_intrhand(sc);
KeLowerIrql(irql);
ndis_enable_intr(sc);
@ -1170,7 +1177,7 @@ ndis_intr(arg)
KeReleaseSpinLock(&intr->ni_dpccountlock, irql);
if ((is_our_intr || call_isr))
ndis_sched(ndis_intrtask, ifp->if_softc, NDIS_SWI);
ndis_sched(ndis_intrtask, sc, NDIS_SWI);
return;
}
@ -1459,9 +1466,36 @@ ndis_init(xsc)
* Cancel pending I/O and free all RX/TX buffers.
*/
ndis_stop(sc);
NDIS_LOCK(sc);
sc->ndis_block->nmb_getstat = 0;
if (ndis_init_nic(sc))
return;
/*
* 802.11 NDIS drivers are supposed to generate a link
* down event right when you initialize them. You wait
* until this event occurs before trying to futz with
* the device. Some drivers will actually set the event
* during the course of MiniportInitialize(), which means
* by the time it completes, the device is ready for us
* to interact with it. But some drivers don't signal the
* event until after MiniportInitialize() (they probably
* need to wait for a device interrupt to occur first).
* We have to be careful to handle both cases. After we
* call ndis_init_nic(), we have to see if a status event
* was triggered. If it wasn't, we have to wait for it
* to occur before we can proceed.
*/
if (sc->ndis_80211 & !sc->ndis_block->nmb_getstat) {
error = msleep(&sc->ndis_block->nmb_getstat,
&sc->ndis_mtx, curthread->td_priority,
"ndiswait", 5 * hz);
}
sc->ndis_block->nmb_getstat = 0;
NDIS_UNLOCK(sc);
/* Init our MAC address */
/* Program the packet filter */

1
sys/kern/subr_witness.c
View File

@ -372,6 +372,7 @@ static struct witness_order_list_entry order_lists[] = {
#endif
#if defined(__i386__) || defined(__amd64__)
{ "pcicfg", &lock_class_mtx_spin },
{ "NDIS thread lock", &lock_class_mtx_spin },
#endif
{ NULL, NULL },
{ NULL, NULL }