Add locking to the twe(4) driver and make it MPSAFE:

- Add per-controller configuration (sx) and I/O (mutex) locks.  The
  configuration lock protects the relationship of volumes and drives
  while the I/O lock protects access to the controller's registers and
  the main I/O path.
- Remove some checks for M_WAITOK malloc()'s failing.
- Remove the explicit bus space tag/handle from the softc and use
  bus_*() rather than bus_space_*().
- Reuse the existing new-bus sysctl context instead of creating a
  new one.
- Remove compat shims for FreeBSD 4.x.
- Use pci_enable_busmaster() rather than doing it by hand, and rely
  on bus_alloc_resource() to enable PCI I/O decoding.

Tested by:	Mike Tancsa  mike sentex net
Reviewed by:	scottl (partially)
MFC after:	1 month

4 files changed, 196 insertions, 180 deletions

diff --git a/sys/dev/twe/twe.c b/sys/dev/twe/twe.c
index 6994cb4..d7c8c77 100644
--- a/sys/dev/twe/twe.c
+++ b/sys/dev/twe/twe.c
@@ -151,8 +151,11 @@ twe_setup(struct twe_softc *sc)
 	/*
 	 * Put command onto the freelist.
 	 */
+	TWE_IO_LOCK(sc);
 	twe_release_request(tr);
+	TWE_IO_UNLOCK(sc);
     }
+    TWE_IO_LOCK(sc);
 
     /*
      * Check status register for errors, clear them.
@@ -164,6 +167,7 @@ twe_setup(struct twe_softc *sc)
      * Wait for the controller to come ready.
      */
     if (twe_wait_status(sc, TWE_STATUS_MICROCONTROLLER_READY, 60)) {
+	TWE_IO_UNLOCK(sc);
 	twe_printf(sc, "microcontroller not ready\n");
 	return(ENXIO);
     }
@@ -185,6 +189,7 @@ twe_setup(struct twe_softc *sc)
 	if (!twe_soft_reset(sc))
 	    break;			/* reset process complete */
     }
+    TWE_IO_UNLOCK(sc);
     /* did we give up? */
     if (i >= TWE_MAX_RESET_TRIES) {
 	twe_printf(sc, "can't initialise controller, giving up\n");
@@ -203,14 +208,17 @@ twe_add_unit(struct twe_softc *sc, int unit)
     TWE_Param			*drives = NULL, *param = NULL;
     TWE_Array_Descriptor	*ud;
 
+    TWE_CONFIG_ASSERT_LOCKED(sc);
     if (unit < 0 || unit > TWE_MAX_UNITS)
 	return (EINVAL);
 
     /*
      * The controller is in a safe state, so try to find drives attached to it.
      */
+    TWE_IO_LOCK(sc);
     if ((drives = twe_get_param(sc, TWE_PARAM_UNITSUMMARY, TWE_PARAM_UNITSUMMARY_Status,
 				TWE_MAX_UNITS, NULL)) == NULL) {
+	TWE_IO_UNLOCK(sc);
 	twe_printf(sc, "can't detect attached units\n");
 	return (EIO);
     }
@@ -218,6 +226,7 @@ twe_add_unit(struct twe_softc *sc, int unit)
     dr = &sc->twe_drive[unit];
     /* check that the drive is online */
     if (!(drives->data[unit] & TWE_PARAM_UNITSTATUS_Online)) {
+	TWE_IO_UNLOCK(sc);
 	error = ENXIO;
 	goto out;
     }
@@ -225,21 +234,25 @@ twe_add_unit(struct twe_softc *sc, int unit)
     table = TWE_PARAM_UNITINFO + unit;
 
     if (twe_get_param_4(sc, table, TWE_PARAM_UNITINFO_Capacity, &dr->td_size)) {
+	TWE_IO_UNLOCK(sc);
 	twe_printf(sc, "error fetching capacity for unit %d\n", unit);
 	error = EIO;
 	goto out;
     }
     if (twe_get_param_1(sc, table, TWE_PARAM_UNITINFO_Status, &dr->td_state)) {
+	TWE_IO_UNLOCK(sc);
 	twe_printf(sc, "error fetching state for unit %d\n", unit);
 	error = EIO;
 	goto out;
     }
     if (twe_get_param_2(sc, table, TWE_PARAM_UNITINFO_DescriptorSize, &dsize)) {
+	TWE_IO_UNLOCK(sc);
 	twe_printf(sc, "error fetching descriptor size for unit %d\n", unit);
 	error = EIO;
 	goto out;
     }
     if ((param = twe_get_param(sc, table, TWE_PARAM_UNITINFO_Descriptor, dsize - 3, NULL)) == NULL) {
+	TWE_IO_UNLOCK(sc);
 	twe_printf(sc, "error fetching descriptor for unit %d\n", unit);
 	error = EIO;
 	goto out;
@@ -258,6 +271,7 @@ twe_add_unit(struct twe_softc *sc, int unit)
     }
     dr->td_cylinders = dr->td_size / (dr->td_heads * dr->td_sectors);
     dr->td_twe_unit = unit;
+    TWE_IO_UNLOCK(sc);
 
     error = twe_attach_drive(sc, dr);
 
@@ -274,6 +288,7 @@ twe_del_unit(struct twe_softc *sc, int unit)
 {
     int error;
 
+    TWE_CONFIG_ASSERT_LOCKED(sc);
     if (unit < 0 || unit >= TWE_MAX_UNITS)
 	return (ENXIO);
 
@@ -295,12 +310,15 @@ twe_init(struct twe_softc *sc)
     /*
      * Scan for drives
      */
+    TWE_CONFIG_LOCK(sc);
     for (i = 0; i < TWE_MAX_UNITS; i++)
 	twe_add_unit(sc, i);
+    TWE_CONFIG_UNLOCK(sc);
 
     /*
      * Initialise connection with controller.
      */
+    TWE_IO_LOCK(sc);
     twe_init_connection(sc, TWE_INIT_MESSAGE_CREDITS);
 
 #ifdef TWE_SHUTDOWN_NOTIFICATION
@@ -319,6 +337,7 @@ twe_init(struct twe_softc *sc)
      * Finally enable interrupts.
      */
     twe_enable_interrupts(sc);
+    TWE_IO_UNLOCK(sc);
 }
 
 /********************************************************************************
@@ -330,6 +349,7 @@ twe_deinit(struct twe_softc *sc)
     /*
      * Mark the controller as shutting down, and disable any further interrupts.
      */
+    TWE_IO_ASSERT_LOCKED(sc);
     sc->twe_state |= TWE_STATE_SHUTDOWN;
     twe_disable_interrupts(sc);
 
@@ -385,6 +405,7 @@ twe_startio(struct twe_softc *sc)
 
     debug_called(4);
 
+    TWE_IO_ASSERT_LOCKED(sc);
     if (sc->twe_state & (TWE_STATE_CTLR_BUSY | TWE_STATE_FRZN))
 	return;
 
@@ -500,15 +521,32 @@ twe_ioctl(struct twe_softc *sc, u_long ioctlcmd, void *addr)
     u_int16_t			*aen_code = (u_int16_t *)addr;
     struct twe_request		*tr;
     u_int8_t			srid;
-    int				s, error;
+    int				error;
+    size_t			tr_length;
 
     error = 0;
     switch(ioctlcmd) {
 	/* handle a command from userspace */
     case TWEIO_COMMAND:
+	/*
+	 * if there's a data buffer, allocate and copy it in.
+	 * Must be in multipled of 512 bytes.
+	 */
+	tr_length = roundup2(tu->tu_size, 512);
+	if (tr_length > 0) {
+	    data = malloc(tr_length, M_DEVBUF, M_WAITOK);
+	    error = copyin(tu->tu_data, data, tu->tu_size);
+	    if (error) {
+		free(data, M_DEVBUF);
+		break;
+	    }
+	} else
+	    data = NULL;
+
 	/* get a request */
+	TWE_IO_LOCK(sc);
 	while (twe_get_request(sc, &tr))
-	    tsleep(sc, PPAUSE, "twioctl", hz);
+	    mtx_sleep(sc, &sc->twe_io_lock, PPAUSE, "twioctl", hz);
 
 	/*
 	 * Save the command's request ID, copy the user-supplied command in,
@@ -519,23 +557,15 @@ twe_ioctl(struct twe_softc *sc, u_long ioctlcmd, void *addr)
 	bcopy(&tu->tu_command, cmd, sizeof(TWE_Command));
 	cmd->generic.request_id = srid;
 
-	/*
-	 * if there's a data buffer, allocate and copy it in.
-	 * Must be in multipled of 512 bytes.
-	 */
-	tr->tr_length = (tu->tu_size + 511) & ~511;
+	tr->tr_length = tr_length;
+	tr->tr_data = data;
 	if (tr->tr_length > 0) {
-	    if ((tr->tr_data = malloc(tr->tr_length, M_DEVBUF, M_WAITOK)) == NULL) {
-		error = ENOMEM;
-		goto cmd_done;
-	    }
-	    if ((error = copyin(tu->tu_data, tr->tr_data, tu->tu_size)) != 0)
-		goto cmd_done;
 	    tr->tr_flags |= TWE_CMD_DATAIN | TWE_CMD_DATAOUT;
 	}
 
 	/* run the command */
 	error = twe_wait_request(tr);
+	TWE_IO_UNLOCK(sc);
 	if (error)
 	    goto cmd_done;
 
@@ -550,8 +580,9 @@ twe_ioctl(struct twe_softc *sc, u_long ioctlcmd, void *addr)
 	/* free resources */
 	if (tr->tr_data != NULL)
 	    free(tr->tr_data, M_DEVBUF);
-	if (tr != NULL)
-	    twe_release_request(tr);
+	TWE_IO_LOCK(sc);
+	twe_release_request(tr);
+	TWE_IO_UNLOCK(sc);
 
 	break;
 
@@ -564,7 +595,9 @@ twe_ioctl(struct twe_softc *sc, u_long ioctlcmd, void *addr)
 	case TWEQ_READY:
 	case TWEQ_BUSY:
 	case TWEQ_COMPLETE:
+	    TWE_IO_LOCK(sc);
 	    bcopy(&sc->twe_qstat[ts->ts_item], &ts->ts_qstat, sizeof(struct twe_qstat));
+	    TWE_IO_UNLOCK(sc);
 	    break;
 #endif
 	default:
@@ -575,22 +608,28 @@ twe_ioctl(struct twe_softc *sc, u_long ioctlcmd, void *addr)
 
 	/* poll for an AEN */
     case TWEIO_AEN_POLL:
+	TWE_IO_LOCK(sc);
 	*aen_code = twe_dequeue_aen(sc);
+	TWE_IO_UNLOCK(sc);
 	break;
 
 	/* wait for another AEN to show up */
     case TWEIO_AEN_WAIT:
-	s = splbio();
+	TWE_IO_LOCK(sc);
 	while ((*aen_code = twe_dequeue_aen(sc)) == TWE_AEN_QUEUE_EMPTY) {
-	    error = tsleep(&sc->twe_aen_queue, PRIBIO | PCATCH, "tweaen", 0);
+	    error = mtx_sleep(&sc->twe_aen_queue, &sc->twe_io_lock, PRIBIO | PCATCH,
+		"tweaen", 0);
 	    if (error == EINTR)
 		break;
 	}
-	splx(s);
+	TWE_IO_UNLOCK(sc);
 	break;
 
     case TWEIO_GET_PARAM:
-	if ((param = twe_get_param(sc, tp->tp_table_id, tp->tp_param_id, tp->tp_size, NULL)) == NULL) {
+	TWE_IO_LOCK(sc);
+	param = twe_get_param(sc, tp->tp_table_id, tp->tp_param_id, tp->tp_size, NULL);
+	TWE_IO_UNLOCK(sc);
+	if (param == NULL) {
 	    twe_printf(sc, "TWEIO_GET_PARAM failed for 0x%x/0x%x/%d\n", 
 		       tp->tp_table_id, tp->tp_param_id, tp->tp_size);
 	    error = EINVAL;
@@ -607,26 +646,32 @@ twe_ioctl(struct twe_softc *sc, u_long ioctlcmd, void *addr)
 	break;
 
     case TWEIO_SET_PARAM:
-	if ((data = malloc(tp->tp_size, M_DEVBUF, M_WAITOK)) == NULL) {
-	    error = ENOMEM;
-	} else {
-	    error = copyin(tp->tp_data, data, tp->tp_size);
-	    if (error == 0)
-		error = twe_set_param(sc, tp->tp_table_id, tp->tp_param_id, tp->tp_size, data);
-	    free(data, M_DEVBUF);
+	data = malloc(tp->tp_size, M_DEVBUF, M_WAITOK);
+	error = copyin(tp->tp_data, data, tp->tp_size);
+	if (error == 0) {
+	    TWE_IO_LOCK(sc);
+	    error = twe_set_param(sc, tp->tp_table_id, tp->tp_param_id, tp->tp_size, data);
+	    TWE_IO_UNLOCK(sc);
 	}
+	free(data, M_DEVBUF);
 	break;
 
     case TWEIO_RESET:
+	TWE_IO_LOCK(sc);
 	twe_reset(sc);
+	TWE_IO_UNLOCK(sc);
 	break;
 
     case TWEIO_ADD_UNIT:
+	TWE_CONFIG_LOCK(sc);
 	error = twe_add_unit(sc, td->td_unit);
+	TWE_CONFIG_UNLOCK(sc);
 	break;
 
     case TWEIO_DEL_UNIT:
+	TWE_CONFIG_LOCK(sc);
 	error = twe_del_unit(sc, td->td_unit);
+	TWE_CONFIG_UNLOCK(sc);
 	break;
 
 	/* XXX implement ATA PASSTHROUGH */
@@ -724,6 +769,7 @@ twe_get_param(struct twe_softc *sc, int table_id, int param_id, size_t param_siz
 
     debug_called(4);
 
+    TWE_IO_ASSERT_LOCKED(sc);
     tr = NULL;
     param = NULL;
 
@@ -817,6 +863,7 @@ twe_set_param(struct twe_softc *sc, int table_id, int param_id, int param_size,
 
     debug_called(4);
 
+    TWE_IO_ASSERT_LOCKED(sc);
     tr = NULL;
     param = NULL;
     error = ENOMEM;
@@ -874,6 +921,8 @@ twe_init_connection(struct twe_softc *sc, int mode)
     
     debug_called(4);
 
+    TWE_IO_ASSERT_LOCKED(sc);
+
     /* get a command */
     if (twe_get_request(sc, &tr))
 	return(0);
@@ -903,18 +952,16 @@ twe_init_connection(struct twe_softc *sc, int mode)
 static int
 twe_wait_request(struct twe_request *tr)
 {
-    int		s;
 
     debug_called(4);
 
+    TWE_IO_ASSERT_LOCKED(tr->tr_sc);
     tr->tr_flags |= TWE_CMD_SLEEPER;
     tr->tr_status = TWE_CMD_BUSY;
     twe_enqueue_ready(tr);
     twe_startio(tr->tr_sc);
-    s = splbio();
     while (tr->tr_status == TWE_CMD_BUSY)
-	tsleep(tr, PRIBIO, "twewait", 0);
-    splx(s);
+	mtx_sleep(tr, &tr->tr_sc->twe_io_lock, PRIBIO, "twewait", 0);
     
     return(tr->tr_status != TWE_CMD_COMPLETE);
 }
@@ -991,12 +1038,12 @@ static void
 twe_reset(struct twe_softc *sc)
 {
     struct twe_request	*tr;
-    int			i, s;
+    int			i;
 
     /*
      * Sleep for a short period to allow AENs to be signalled.
      */
-    tsleep(sc, PRIBIO, "twereset", hz);
+    mtx_sleep(sc, &sc->twe_io_lock, PRIBIO, "twereset", hz);
 
     /*
      * Disable interrupts from the controller, and mask any accidental entry
@@ -1004,7 +1051,6 @@ twe_reset(struct twe_softc *sc)
      */
     twe_printf(sc, "controller reset in progress...\n");
     twe_disable_interrupts(sc);
-    s = splbio();
 
     /*
      * Try to soft-reset the controller.
@@ -1036,11 +1082,9 @@ twe_reset(struct twe_softc *sc)
      * Kick the controller to start things going again, then re-enable interrupts.
      */
     twe_startio(sc);
-    twe_enable_interrupts(sc);
     twe_printf(sc, "controller reset done, %d commands restarted\n", i);
 
 out:
-    splx(s);
     twe_enable_interrupts(sc);
 }
 
@@ -1060,11 +1104,14 @@ twe_start(struct twe_request *tr)
 {
     struct twe_softc	*sc = tr->tr_sc;
     TWE_Command		*cmd;
-    int			i, s, done;
+    int			i;
     u_int32_t		status_reg;
 
     debug_called(4);
 
+    if (!dumping)
+	TWE_IO_ASSERT_LOCKED(sc);
+
     /* mark the command as currently being processed */
     tr->tr_status = TWE_CMD_BUSY;
     cmd = TWE_FIND_COMMAND(tr);
@@ -1075,8 +1122,7 @@ twe_start(struct twe_request *tr)
      * XXX it might be more efficient to return EBUSY immediately
      *     and let the command be rescheduled.
      */
-    for (i = 100000, done = 0; (i > 0) && !done; i--) {
-	s = splbio();
+    for (i = 100000; (i > 0); i--) {
 	
 	/* check to see if we can post a command */
 	status_reg = TWE_STATUS(sc);
@@ -1086,7 +1132,7 @@ twe_start(struct twe_request *tr)
 	    twe_enqueue_busy(tr);
 
 	    TWE_COMMAND_QUEUE(sc, TWE_FIND_COMMANDPHYS(tr));
-	    done = 1;
+
 	    /* move command to work queue */
 #ifdef TWE_DEBUG
 	    if (tr->tr_complete != NULL) {
@@ -1097,14 +1143,10 @@ twe_start(struct twe_request *tr)
 		debug(3, "queued request %d for polling caller", cmd->generic.request_id);
 	    }
 #endif
+	    return(0);
 	}
-	splx(s);	/* drop spl to allow completion interrupts */
     }
 
-    /* command is enqueued */
-    if (done)
-	return(0);
-
     /* 
      * We couldn't get the controller to take the command; try submitting it again later.
      * This should only happen if something is wrong with the controller, or if we have
@@ -1125,14 +1167,13 @@ twe_done(struct twe_softc *sc)
     TWE_Response_Queue	rq;
     TWE_Command		*cmd;
     struct twe_request	*tr;
-    int			s, found;
+    int			found;
     u_int32_t		status_reg;
     
     debug_called(5);
 
     /* loop collecting completed commands */
     found = 0;
-    s = splbio();
     for (;;) {
 	status_reg = TWE_STATUS(sc);
 	twe_check_bits(sc, status_reg);		/* XXX should this fail? */
@@ -1155,7 +1196,6 @@ twe_done(struct twe_softc *sc)
 	    break;					/* no response ready */
 	}
     }
-    splx(s);
 
     /* if we've completed any commands, try posting some more */
     if (found)
@@ -1259,6 +1299,7 @@ twe_soft_reset(struct twe_softc *sc)
 
     debug_called(2);
 
+    TWE_IO_ASSERT_LOCKED(sc);
     TWE_SOFT_RESET(sc);
 
     if (twe_wait_status(sc, TWE_STATUS_ATTENTION_INTERRUPT, 30)) {
@@ -1396,6 +1437,7 @@ twe_drain_aen_queue(struct twe_softc *sc)
 {
     u_int16_t	aen;
 
+    TWE_IO_ASSERT_LOCKED(sc);
     for (;;) {
 	if (twe_get_param_2(sc, TWE_PARAM_AEN, TWE_PARAM_AEN_UnitCode, &aen))
 	    return(1);
@@ -1417,14 +1459,14 @@ static void
 twe_enqueue_aen(struct twe_softc *sc, u_int16_t aen)
 {
     char	*msg;
-    int		s, next, nextnext;
+    int		next, nextnext;
 
     debug_called(4);
 
+    TWE_IO_ASSERT_LOCKED(sc);
     if ((msg = twe_format_aen(sc, aen)) != NULL)
 	twe_printf(sc, "AEN: <%s>\n", msg);
 
-    s = splbio();
     /* enqueue the AEN */
     next = ((sc->twe_aen_head + 1) % TWE_Q_LENGTH);
     nextnext = ((sc->twe_aen_head + 2) % TWE_Q_LENGTH);
@@ -1447,7 +1489,6 @@ twe_enqueue_aen(struct twe_softc *sc, u_int16_t aen)
 	sc->twe_wait_aen = -1;
 	wakeup(&sc->twe_wait_aen);
     }
-    splx(s);
 }
 
 /********************************************************************************
@@ -1462,6 +1503,7 @@ twe_dequeue_aen(struct twe_softc *sc)
     
     debug_called(4);
 
+    TWE_IO_ASSERT_LOCKED(sc);
     if (sc->twe_aen_tail == sc->twe_aen_head) {
 	result = TWE_AEN_QUEUE_EMPTY;
     } else {
@@ -1479,15 +1521,13 @@ twe_dequeue_aen(struct twe_softc *sc)
 static int
 twe_find_aen(struct twe_softc *sc, u_int16_t aen)
 {
-    int		i, s, missing;
+    int		i, missing;
 
     missing = 1;
-    s = splbio();
     for (i = sc->twe_aen_tail; (i != sc->twe_aen_head) && missing; i = (i + 1) % TWE_Q_LENGTH) {
 	if (sc->twe_aen_queue[i] == aen)
 	    missing = 0;
     }
-    splx(s);
     return(missing);
 }
 
@@ -1504,22 +1544,20 @@ static int
 twe_wait_aen(struct twe_softc *sc, int aen, int timeout)
 {
     time_t	expiry;
-    int		found, s;
+    int		found;
 
     debug_called(4);
 
     expiry = time_second + timeout;
     found = 0;
 
-    s = splbio();
     sc->twe_wait_aen = aen;
     do {
 	twe_fetch_aen(sc);
-	tsleep(&sc->twe_wait_aen, PZERO, "twewaen", hz);
+	mtx_sleep(&sc->twe_wait_aen, &sc->twe_io_lock, PZERO, "twewaen", hz);
 	if (sc->twe_wait_aen == -1)
 	    found = 1;
     } while ((time_second <= expiry) && !found);
-    splx(s);
     return(!found);
 }
 #endif
@@ -1541,6 +1579,9 @@ twe_get_request(struct twe_softc *sc, struct twe_request **tr)
     TWE_Command		*cmd;
     debug_called(4);
 
+    if (!dumping)
+	TWE_IO_ASSERT_LOCKED(sc);
+
     /* try to reuse an old buffer */
     *tr = twe_dequeue_free(sc);
 
@@ -1567,6 +1608,8 @@ twe_release_request(struct twe_request *tr)
 {
     debug_called(4);
 
+    if (!dumping)
+	TWE_IO_ASSERT_LOCKED(tr->tr_sc);
     if (tr->tr_private != NULL)
 	twe_panic(tr->tr_sc, "tr_private != NULL");
     twe_enqueue_free(tr);
@@ -1591,6 +1634,8 @@ twe_describe_controller(struct twe_softc *sc)
 
     debug_called(2);
 
+    TWE_IO_LOCK(sc);
+
     /* get the port count */
     twe_get_param_1(sc, TWE_PARAM_CONTROLLER, TWE_PARAM_CONTROLLER_PortCount, &ports);
 
@@ -1641,6 +1686,7 @@ twe_describe_controller(struct twe_softc *sc)
 	if (p[0])
 	    free(p[0], M_DEVBUF);
     }
+    TWE_IO_UNLOCK(sc);
 }
 
 /********************************************************************************
diff --git a/sys/dev/twe/twe_compat.h b/sys/dev/twe/twe_compat.h
index 1bd46f0..b9cbddb 100644
--- a/sys/dev/twe/twe_compat.h
+++ b/sys/dev/twe/twe_compat.h
@@ -43,8 +43,11 @@
 #include <sys/systm.h>
 #include <sys/malloc.h>
 #include <sys/kernel.h>
+#include <sys/lock.h>
 #include <sys/module.h>
+#include <sys/mutex.h>
 #include <sys/sysctl.h>
+#include <sys/sx.h>
 
 #include <sys/bus.h>
 #include <sys/conf.h>
@@ -65,10 +68,10 @@
 /* 
  * Wrappers for bus-space actions
  */
-#define TWE_CONTROL(sc, val)		bus_space_write_4((sc)->twe_btag, (sc)->twe_bhandle, 0x0, (u_int32_t)val)
-#define TWE_STATUS(sc)			(u_int32_t)bus_space_read_4((sc)->twe_btag, (sc)->twe_bhandle, 0x4)
-#define TWE_COMMAND_QUEUE(sc, val)	bus_space_write_4((sc)->twe_btag, (sc)->twe_bhandle, 0x8, (u_int32_t)val)
-#define TWE_RESPONSE_QUEUE(sc)		(TWE_Response_Queue)bus_space_read_4((sc)->twe_btag, (sc)->twe_bhandle, 0xc)
+#define TWE_CONTROL(sc, val)		bus_write_4((sc)->twe_io, 0x0, (u_int32_t)val)
+#define TWE_STATUS(sc)			(u_int32_t)bus_read_4((sc)->twe_io, 0x4)
+#define TWE_COMMAND_QUEUE(sc, val)	bus_write_4((sc)->twe_io, 0x8, (u_int32_t)val)
+#define TWE_RESPONSE_QUEUE(sc)		(TWE_Response_Queue)bus_read_4((sc)->twe_io, 0xc)
 
 /*
  * FreeBSD-specific softc elements
@@ -79,8 +82,6 @@
     device_t			twe_dev;		/* bus device */		\
     struct cdev *twe_dev_t;		/* control device */		\
     struct resource		*twe_io;		/* register interface window */	\
-    bus_space_handle_t		twe_bhandle;		/* bus space handle */		\
-    bus_space_tag_t		twe_btag;		/* bus space tag */		\
     bus_dma_tag_t		twe_parent_dmat;	/* parent DMA tag */		\
     bus_dma_tag_t		twe_buffer_dmat;	/* data buffer DMA tag */	\
     bus_dma_tag_t		twe_cmd_dmat;		/* command buffer DMA tag */	\
@@ -91,8 +92,8 @@
     void			*twe_cmd;		/* command structures */	\
     void			*twe_immediate;		/* immediate commands */	\
     bus_dmamap_t		twe_immediate_map;					\
-    struct sysctl_ctx_list	sysctl_ctx;						\
-    struct sysctl_oid		*sysctl_tree;
+    struct mtx			twe_io_lock;						\
+    struct sx			twe_config_lock;
 
 /*
  * FreeBSD-specific request elements
@@ -107,31 +108,6 @@
 #define twe_printf(sc, fmt, args...)	device_printf(sc->twe_dev, fmt , ##args)
 #define twed_printf(twed, fmt, args...)	device_printf(twed->twed_dev, fmt , ##args)
 
-#if __FreeBSD_version < 500003
-# include <machine/clock.h>
-# define INTR_ENTROPY			0
-# define FREEBSD_4
-
-# include <sys/buf.h>			/* old buf style */
-typedef struct buf			twe_bio;
-typedef struct buf_queue_head		twe_bioq;
-# define TWE_BIO_QINIT(bq)		bufq_init(&bq);
-# define TWE_BIO_QINSERT(bq, bp)	bufq_insert_tail(&bq, bp)
-# define TWE_BIO_QFIRST(bq)		bufq_first(&bq)
-# define TWE_BIO_QREMOVE(bq, bp)	bufq_remove(&bq, bp)
-# define TWE_BIO_IS_READ(bp)		((bp)->b_flags & B_READ)
-# define TWE_BIO_DATA(bp)		(bp)->b_data
-# define TWE_BIO_LENGTH(bp)		(bp)->b_bcount
-# define TWE_BIO_LBA(bp)		(bp)->b_pblkno
-# define TWE_BIO_SOFTC(bp)		(bp)->b_dev->si_drv1
-# define TWE_BIO_UNIT(bp)		*(int *)((bp)->b_dev->si_drv2)
-# define TWE_BIO_SET_ERROR(bp, err)	do { (bp)->b_error = err; (bp)->b_flags |= B_ERROR;} while(0)
-# define TWE_BIO_HAS_ERROR(bp)		((bp)->b_flags & B_ERROR)
-# define TWE_BIO_RESID(bp)		(bp)->b_resid
-# define TWE_BIO_DONE(bp)		biodone(bp)
-# define TWE_BIO_STATS_START(bp)	devstat_start_transaction(&((struct twed_softc *)TWE_BIO_SOFTC(bp))->twed_stats)
-# define TWE_BIO_STATS_END(bp)		devstat_end_transaction_buf(&((struct twed_softc *)TWE_BIO_SOFTC(bp))->twed_stats, bp)
-#else
 # include <sys/bio.h>
 # include <geom/geom_disk.h>
 typedef struct bio			twe_bio;
@@ -152,7 +128,13 @@ typedef struct bio_queue_head		twe_bioq;
 # define TWE_BIO_DONE(bp)		biodone(bp)
 # define TWE_BIO_STATS_START(bp)
 # define TWE_BIO_STATS_END(bp)
-#endif
+
+#define	TWE_IO_LOCK(sc)			mtx_lock(&(sc)->twe_io_lock)
+#define	TWE_IO_UNLOCK(sc)		mtx_unlock(&(sc)->twe_io_lock)
+#define	TWE_IO_ASSERT_LOCKED(sc)	mtx_assert(&(sc)->twe_io_lock, MA_OWNED)
+#define	TWE_CONFIG_LOCK(sc)		sx_xlock(&(sc)->twe_config_lock)
+#define	TWE_CONFIG_UNLOCK(sc)		sx_xunlock(&(sc)->twe_config_lock)
+#define	TWE_CONFIG_ASSERT_LOCKED(sc)	sx_assert(&(sc)->twe_config_lock, SA_XLOCKED)
 
 #endif /* FreeBSD */
 
diff --git a/sys/dev/twe/twe_freebsd.c b/sys/dev/twe/twe_freebsd.c
index 6c31d6a..61de85c 100644
--- a/sys/dev/twe/twe_freebsd.c
+++ b/sys/dev/twe/twe_freebsd.c
@@ -69,7 +69,6 @@ static	d_ioctl_t		twe_ioctl_wrapper;
 
 static struct cdevsw twe_cdevsw = {
 	.d_version =	D_VERSION,
-	.d_flags =	D_NEEDGIANT,
 	.d_open =	twe_open,
 	.d_close =	twe_close,
 	.d_ioctl =	twe_ioctl_wrapper,
@@ -84,7 +83,13 @@ twe_open(struct cdev *dev, int flags, int fmt, struct thread *td)
 {
     struct twe_softc		*sc = (struct twe_softc *)dev->si_drv1;
 
+    TWE_IO_LOCK(sc);
+    if (sc->twe_state & TWE_STATE_DETACHING) {
+	TWE_IO_UNLOCK(sc);
+	return (ENXIO);
+    }
     sc->twe_state |= TWE_STATE_OPEN;
+    TWE_IO_UNLOCK(sc);
     return(0);
 }
 
@@ -96,7 +101,9 @@ twe_close(struct cdev *dev, int flags, int fmt, struct thread *td)
 {
     struct twe_softc		*sc = (struct twe_softc *)dev->si_drv1;
 
+    TWE_IO_LOCK(sc);
     sc->twe_state &= ~TWE_STATE_OPEN;
+    TWE_IO_UNLOCK(sc);
     return (0);
 }
 
@@ -172,8 +179,8 @@ static int
 twe_attach(device_t dev)
 {
     struct twe_softc	*sc;
+    struct sysctl_oid	*sysctl_tree;
     int			rid, error;
-    u_int32_t		command;
 
     debug_called(4);
 
@@ -182,32 +189,30 @@ twe_attach(device_t dev)
      */
     sc = device_get_softc(dev);
     sc->twe_dev = dev;
+    mtx_init(&sc->twe_io_lock, "twe I/O", NULL, MTX_DEF);
+    sx_init(&sc->twe_config_lock, "twe config");
 
-    sysctl_ctx_init(&sc->sysctl_ctx);
-    sc->sysctl_tree = SYSCTL_ADD_NODE(&sc->sysctl_ctx,
+    /*
+     * XXX: This sysctl tree must stay at hw.tweX rather than using
+     * the device_get_sysctl_tree() created by new-bus because
+     * existing 3rd party binary tools such as tw_cli and 3dm2 use the
+     * existence of this sysctl node to discover controllers.
+     */
+    sysctl_tree = SYSCTL_ADD_NODE(device_get_sysctl_ctx(dev),
 	SYSCTL_STATIC_CHILDREN(_hw), OID_AUTO,
 	device_get_nameunit(dev), CTLFLAG_RD, 0, "");
-    if (sc->sysctl_tree == NULL) {
+    if (sysctl_tree == NULL) {
 	twe_printf(sc, "cannot add sysctl tree node\n");
 	return (ENXIO);
     }
-    SYSCTL_ADD_STRING(&sc->sysctl_ctx, SYSCTL_CHILDREN(sc->sysctl_tree),
+    SYSCTL_ADD_STRING(device_get_sysctl_ctx(dev), SYSCTL_CHILDREN(sysctl_tree),
 	OID_AUTO, "driver_version", CTLFLAG_RD, TWE_DRIVER_VERSION_STRING, 0,
 	"TWE driver version");
 
     /*
-     * Make sure we are going to be able to talk to this board.
-     */
-    command = pci_read_config(dev, PCIR_COMMAND, 2);
-    if ((command & PCIM_CMD_PORTEN) == 0) {
-	twe_printf(sc, "register window not available\n");
-	return(ENXIO);
-    }
-    /*
      * Force the busmaster enable bit on, in case the BIOS forgot.
      */
-    command |= PCIM_CMD_BUSMASTEREN;
-    pci_write_config(dev, PCIR_COMMAND, command, 2);
+    pci_enable_busmaster(dev);
 
     /*
      * Allocate the PCI register window.
@@ -219,8 +224,6 @@ twe_attach(device_t dev)
 	twe_free(sc);
 	return(ENXIO);
     }
-    sc->twe_btag = rman_get_bustag(sc->twe_io);
-    sc->twe_bhandle = rman_get_bushandle(sc->twe_io);
 
     /*
      * Allocate the parent bus DMA tag appropriate for PCI.
@@ -251,7 +254,7 @@ twe_attach(device_t dev)
 	twe_free(sc);
 	return(ENXIO);
     }
-    if (bus_setup_intr(sc->twe_dev, sc->twe_irq, INTR_TYPE_BIO | INTR_ENTROPY,  
+    if (bus_setup_intr(sc->twe_dev, sc->twe_irq, INTR_TYPE_BIO | INTR_ENTROPY | INTR_MPSAFE,  
 		       NULL, twe_pci_intr, sc, &sc->twe_intr)) {
 	twe_printf(sc, "can't set up interrupt\n");
 	twe_free(sc);
@@ -302,7 +305,7 @@ twe_attach(device_t dev)
 			   BUS_SPACE_MAXSIZE_32BIT,	/* maxsegsize */
 			   BUS_DMA_ALLOCNOW,		/* flags */
 			   busdma_lock_mutex,		/* lockfunc */
-			   &Giant,			/* lockarg */
+			   &sc->twe_io_lock,		/* lockarg */
 			   &sc->twe_buffer_dmat)) {
 	twe_printf(sc, "can't allocate data buffer DMA tag\n");
 	twe_free(sc);
@@ -424,7 +427,8 @@ twe_free(struct twe_softc *sc)
     if (sc->twe_dev_t != (struct cdev *)NULL)
 	destroy_dev(sc->twe_dev_t);
 
-    sysctl_ctx_free(&sc->sysctl_ctx);
+    sx_destroy(&sc->twe_config_lock);
+    mtx_destroy(&sc->twe_io_lock);
 }
 
 /********************************************************************************
@@ -434,27 +438,30 @@ static int
 twe_detach(device_t dev)
 {
     struct twe_softc	*sc = device_get_softc(dev);
-    int			s, error;
 
     debug_called(4);
 
-    error = EBUSY;
-    s = splbio();
-    if (sc->twe_state & TWE_STATE_OPEN)
-	goto out;
+    TWE_IO_LOCK(sc);
+    if (sc->twe_state & TWE_STATE_OPEN) {
+	TWE_IO_UNLOCK(sc);
+	return (EBUSY);
+    }
+    sc->twe_state |= TWE_STATE_DETACHING;
+    TWE_IO_UNLOCK(sc);
 
     /*	
      * Shut the controller down.
      */
-    if (twe_shutdown(dev))
-	goto out;
+    if (twe_shutdown(dev)) {
+	TWE_IO_LOCK(sc);
+	sc->twe_state &= ~TWE_STATE_DETACHING;
+	TWE_IO_UNLOCK(sc);
+	return (EBUSY);
+    }
 
     twe_free(sc);
 
-    error = 0;
- out:
-    splx(s);
-    return(error);
+    return(0);
 }
 
 /********************************************************************************
@@ -467,30 +474,32 @@ static int
 twe_shutdown(device_t dev)
 {
     struct twe_softc	*sc = device_get_softc(dev);
-    int			i, s, error = 0;
+    int			i, error = 0;
 
     debug_called(4);
 
-    s = splbio();
-
     /* 
      * Delete all our child devices.
      */
+    TWE_CONFIG_LOCK(sc);
     for (i = 0; i < TWE_MAX_UNITS; i++) {
 	if (sc->twe_drive[i].td_disk != 0) {
-	    if ((error = twe_detach_drive(sc, i)) != 0)
-		goto out;
+	    if ((error = twe_detach_drive(sc, i)) != 0) {
+		TWE_CONFIG_UNLOCK(sc);
+		return (error);
+	    }
 	}
     }
+    TWE_CONFIG_UNLOCK(sc);
 
     /*
      * Bring the controller down.
      */
+    TWE_IO_LOCK(sc);
     twe_deinit(sc);
+    TWE_IO_UNLOCK(sc);
 
-out:
-    splx(s);
-    return(error);
+    return(0);
 }
 
 /********************************************************************************
@@ -500,15 +509,14 @@ static int
 twe_suspend(device_t dev)
 {
     struct twe_softc	*sc = device_get_softc(dev);
-    int			s;
 
     debug_called(4);
 
-    s = splbio();
+    TWE_IO_LOCK(sc);
     sc->twe_state |= TWE_STATE_SUSPEND;
     
     twe_disable_interrupts(sc);
-    splx(s);
+    TWE_IO_UNLOCK(sc);
 
     return(0);
 }
@@ -523,8 +531,10 @@ twe_resume(device_t dev)
 
     debug_called(4);
 
+    TWE_IO_LOCK(sc);
     sc->twe_state &= ~TWE_STATE_SUSPEND;
     twe_enable_interrupts(sc);
+    TWE_IO_UNLOCK(sc);
 
     return(0);
 }
@@ -536,7 +546,11 @@ twe_resume(device_t dev)
 static void
 twe_pci_intr(void *arg)
 {
-    twe_intr((struct twe_softc *)arg);
+    struct twe_softc *sc = arg;
+
+    TWE_IO_LOCK(sc);
+    twe_intr(sc);
+    TWE_IO_UNLOCK(sc);
 }
 
 /********************************************************************************
@@ -565,8 +579,10 @@ twe_attach_drive(struct twe_softc *sc, struct twe_drive *dr)
     char	buf[80];
     int		error;
 
+    mtx_lock(&Giant);
     dr->td_disk =  device_add_child(sc->twe_dev, NULL, -1);
     if (dr->td_disk == NULL) {
+	mtx_unlock(&Giant);
 	twe_printf(sc, "Cannot add unit\n");
 	return (EIO);
     }
@@ -582,7 +598,9 @@ twe_attach_drive(struct twe_softc *sc, struct twe_drive *dr)
 	    twe_describe_code(twe_table_unitstate, dr->td_state & TWE_PARAM_UNITSTATUS_MASK));
     device_set_desc_copy(dr->td_disk, buf);
 
-    if ((error = bus_generic_attach(sc->twe_dev)) != 0) {
+    error = device_probe_and_attach(dr->td_disk);
+    mtx_unlock(&Giant);
+    if (error != 0) {
 	twe_printf(sc, "Cannot attach unit to controller. error = %d\n", error);
 	return (EIO);
     }
@@ -599,7 +617,11 @@ twe_detach_drive(struct twe_softc *sc, int unit)
 {
     int error = 0;
 
-    if ((error = device_delete_child(sc->twe_dev, sc->twe_drive[unit].td_disk)) != 0) {
+    TWE_CONFIG_ASSERT_LOCKED(sc);
+    mtx_lock(&Giant);
+    error = device_delete_child(sc->twe_dev, sc->twe_drive[unit].td_disk);
+    mtx_unlock(&Giant);
+    if (error != 0) {
 	twe_printf(sc, "failed to delete unit %d\n", unit);
 	return(error);
     }
@@ -671,10 +693,6 @@ DRIVER_MODULE(twed, twe, twed_driver, twed_devclass, 0, 0);
  * Disk device control interface.
  */
 
-#ifdef FREEBSD_4
-static int		disks_registered = 0;
-#endif
-
 /********************************************************************************
  * Handle open from generic layer.
  *
@@ -724,10 +742,12 @@ twed_strategy(twe_bio *bp)
     TWE_BIO_STATS_START(bp);
 
     /* queue the bio on the controller */
+    TWE_IO_LOCK(sc->twed_controller);
     twe_enqueue_bio(sc->twed_controller, bp);
 
     /* poke the controller to start I/O */
     twe_startio(sc->twed_controller);
+    TWE_IO_UNLOCK(sc->twed_controller);
     return;
 }
 
@@ -827,14 +847,9 @@ twed_attach(device_t dev)
     sc->twed_disk->d_fwsectors = sc->twed_drive->td_sectors;
     sc->twed_disk->d_fwheads = sc->twed_drive->td_heads;
     sc->twed_disk->d_unit = sc->twed_drive->td_sys_unit;
-    sc->twed_disk->d_flags = DISKFLAG_NEEDSGIANT;
 
     disk_create(sc->twed_disk, DISK_VERSION);
 
-#ifdef FREEBSD_4
-    disks_registered++;
-#endif
-
     /* set the maximum I/O size to the theoretical maximum allowed by the S/G list size */
 
     return (0);
@@ -855,10 +870,6 @@ twed_detach(device_t dev)
 
     disk_destroy(sc->twed_disk);
 
-#ifdef FREEBSD_4
-    if (--disks_registered == 0)
-	cdevsw_remove(&tweddisk_cdevsw);
-#endif
     return(0);
 }
 
@@ -878,11 +889,7 @@ twe_allocate_request(struct twe_softc *sc, int tag)
 {
     struct twe_request	*tr;
 
-    if ((tr = malloc(sizeof(struct twe_request), TWE_MALLOC_CLASS, M_WAITOK)) == NULL) {
-	twe_printf(sc, "unable to allocate memory for tag %d\n", tag);
-	return(NULL);
-    }
-    bzero(tr, sizeof(*tr));
+    tr = malloc(sizeof(struct twe_request), TWE_MALLOC_CLASS, M_WAITOK | M_ZERO);
     tr->tr_sc = sc;
     tr->tr_tag = tag;
     if (bus_dmamap_create(sc->twe_buffer_dmat, 0, &tr->tr_dmamap)) {
@@ -1044,6 +1051,8 @@ twe_map_request(struct twe_request *tr)
 
     debug_called(4);
 
+    if (!dumping)
+	TWE_IO_ASSERT_LOCKED(sc);
     if (sc->twe_state & (TWE_STATE_CTLR_BUSY | TWE_STATE_FRZN)) {
 	twe_requeue_ready(tr);
 	return (EBUSY);
@@ -1101,6 +1110,8 @@ twe_unmap_request(struct twe_request *tr)
 
     debug_called(4);
 
+    if (!dumping)
+	TWE_IO_ASSERT_LOCKED(sc);
     bus_dmamap_sync(sc->twe_cmd_dmat, sc->twe_cmdmap, BUS_DMASYNC_POSTWRITE);
 
     /*
@@ -1153,12 +1164,10 @@ void
 twe_report(void)
 {
     struct twe_softc	*sc;
-    int			i, s;
+    int			i;
 
-    s = splbio();
     for (i = 0; (sc = devclass_get_softc(twe_devclass, i)) != NULL; i++)
 	twe_print_controller(sc);
     printf("twed: total bio count in %u  out %u\n", twed_bio_in, twed_bio_out);
-    splx(s);
 }
 #endif
diff --git a/sys/dev/twe/twevar.h b/sys/dev/twe/twevar.h
index 4990ca8..d2017aa 100644
--- a/sys/dev/twe/twevar.h
+++ b/sys/dev/twe/twevar.h
@@ -134,6 +134,7 @@ struct twe_softc
 #define TWE_STATE_SUSPEND	(1<<3)	/* controller is suspended */
 #define TWE_STATE_FRZN		(1<<4)	/* got EINPROGRESS */
 #define TWE_STATE_CTLR_BUSY	(1<<5)	/* controller cmd queue full */
+#define	TWE_STATE_DETACHING	(1<<6)	/* controller is being shut down */
     int			twe_host_id;
     struct twe_qstat	twe_qstat[TWEQ_COUNT];	/* queue statistics */
 
@@ -209,46 +210,31 @@ twe_initq_ ## name (struct twe_softc *sc)				\
 static __inline void							\
 twe_enqueue_ ## name (struct twe_request *tr)				\
 {									\
-    int		s;							\
-									\
-    s = splbio();							\
     TAILQ_INSERT_TAIL(&tr->tr_sc->twe_ ## name, tr, tr_link);		\
     TWEQ_ADD(tr->tr_sc, index);						\
-    splx(s);								\
 }									\
 static __inline void							\
 twe_requeue_ ## name (struct twe_request *tr)				\
 {									\
-    int		s;							\
-									\
-    s = splbio();							\
     TAILQ_INSERT_HEAD(&tr->tr_sc->twe_ ## name, tr, tr_link);		\
     TWEQ_ADD(tr->tr_sc, index);						\
-    splx(s);								\
 }									\
 static __inline struct twe_request *					\
 twe_dequeue_ ## name (struct twe_softc *sc)				\
 {									\
     struct twe_request	*tr;						\
-    int			s;						\
 									\
-    s = splbio();							\
     if ((tr = TAILQ_FIRST(&sc->twe_ ## name)) != NULL) {		\
 	TAILQ_REMOVE(&sc->twe_ ## name, tr, tr_link);			\
 	TWEQ_REMOVE(sc, index);						\
     }									\
-    splx(s);								\
     return(tr);								\
 }									\
 static __inline void							\
 twe_remove_ ## name (struct twe_request *tr)				\
 {									\
-    int			s;						\
-									\
-    s = splbio();							\
     TAILQ_REMOVE(&tr->tr_sc->twe_ ## name, tr, tr_link);		\
     TWEQ_REMOVE(tr->tr_sc, index);					\
-    splx(s);								\
 }
 
 TWEQ_REQUEST_QUEUE(free, TWEQ_FREE)
@@ -269,25 +255,18 @@ twe_initq_bio(struct twe_softc *sc)
 static __inline void
 twe_enqueue_bio(struct twe_softc *sc, twe_bio *bp)
 {
-    int		s;
-
-    s = splbio();
     TWE_BIO_QINSERT(sc->twe_bioq, bp);
     TWEQ_ADD(sc, TWEQ_BIO);
-    splx(s);
 }
 
 static __inline twe_bio *
 twe_dequeue_bio(struct twe_softc *sc)
 {
-    int		s;
     twe_bio	*bp;
 
-    s = splbio();
     if ((bp = TWE_BIO_QFIRST(sc->twe_bioq)) != NULL) {
 	TWE_BIO_QREMOVE(sc->twe_bioq, bp);
 	TWEQ_REMOVE(sc, TWEQ_BIO);
     }
-    splx(s);
     return(bp);
 }