Some updates and bug squashing in the firewire stack.

Move the interupt handler to a driver_intr_t type function as it was trying
to do way to much for a lightweight filter interrupt function.

Introduce much more locking around fc->mtx.  Tested this for lock reversals
and other such lockups.  Locking seems to be working better, but there
is much more to do with regard to locking.  The most significant lock is
in the BUS RESET handler.  It was possible, before this checkin, to set
a bus reset via "fwcontrol -r" and have the BUS RESET handler fire before
the code responsible for asserting BUS RESET was complete.  This locking
fixes that issue.

Move some of the memory allocations in the fc struct to the attach function
in firewire.c

Rework the businfo.generation indicator to be merely a on/off bit now.
It's purpose according to spec is to notify the bus that the config ROM
has changed.  That's it.

Catch and squash a possible panic in SBP where in the SBP_LOCK was held
during a possible error case.  The error handling code would definitely
panic as it would try to acquire the SBP_LOCK on entrance.

Catch and squash a camcontrol/device lockup when firewire drives go away.
When a firewire device was powered off or disconnected from the firewire
bus, a "camcontrol rescan all" would hang trying to poll removed devices
as they were not properly detached.  Don't do that.

Approved by:	scottl
MFC after:	2 weeks

5 files changed, 88 insertions, 69 deletions

diff --git a/sys/dev/firewire/firewire.c b/sys/dev/firewire/firewire.c
index ae0d5d3..8137df6 100644
--- a/sys/dev/firewire/firewire.c
+++ b/sys/dev/firewire/firewire.c
@@ -430,6 +430,31 @@ firewire_attach(device_t dev)
 
 	fwdev_makedev(sc);
 
+	fc->crom_src_buf = (struct crom_src_buf *)malloc(
+				sizeof(struct crom_src_buf),
+				M_FW, M_NOWAIT | M_ZERO);
+	if (fc->crom_src_buf == NULL) {
+		device_printf(fc->dev, "%s: Malloc Failure crom src buff\n", __func__);
+		return ENOMEM;
+	}
+	fc->topology_map = (struct fw_topology_map *)malloc(
+				sizeof(struct fw_topology_map),
+				M_FW, M_NOWAIT | M_ZERO);
+	if (fc->topology_map == NULL) {
+		device_printf(fc->dev, "%s: Malloc Failure topology map\n", __func__);
+		free(fc->crom_src_buf, M_FW);
+		return ENOMEM;
+	}
+	fc->speed_map = (struct fw_speed_map *)malloc(
+				sizeof(struct fw_speed_map),
+				M_FW, M_NOWAIT | M_ZERO);
+	if (fc->speed_map == NULL) {
+		device_printf(fc->dev, "%s: Malloc Failure speed map\n", __func__);
+		free(fc->crom_src_buf, M_FW);
+		free(fc->topology_map, M_FW);
+		return ENOMEM;
+	}
+
 	mtx_init(&fc->wait_lock, "fwwait", NULL, MTX_DEF);
 	mtx_init(&fc->tlabel_lock, "fwtlabel", NULL, MTX_DEF);
 	CALLOUT_INIT(&fc->timeout_callout);
@@ -451,7 +476,9 @@ firewire_attach(device_t dev)
 	bus_generic_attach(dev);
 
 	/* bus_reset */
+	FW_GLOCK(fc);
 	fw_busreset(fc, FWBUSNOTREADY);
+	FW_GUNLOCK(fc);
 	fc->ibr(fc);
 
 	return 0;
@@ -642,11 +669,6 @@ fw_init_crom(struct firewire_comm *fc)
 {
 	struct crom_src *src;
 
-	fc->crom_src_buf = (struct crom_src_buf *)
-		malloc(sizeof(struct crom_src_buf), M_FW, M_WAITOK | M_ZERO);
-	if (fc->crom_src_buf == NULL)
-		return;
-
 	src = &fc->crom_src_buf->src;
 	bzero(src, sizeof(struct crom_src));
 
@@ -663,7 +685,7 @@ fw_init_crom(struct firewire_comm *fc)
 	src->businfo.cyc_clk_acc = 100;
 	src->businfo.max_rec = fc->maxrec;
 	src->businfo.max_rom = MAXROM_4;
-	src->businfo.generation = 1;
+	src->businfo.generation = 0;
 	src->businfo.link_spd = fc->speed;
 
 	src->businfo.eui64.hi = fc->eui.hi;
@@ -682,9 +704,6 @@ fw_reset_crom(struct firewire_comm *fc)
 	struct crom_src *src;
 	struct crom_chunk *root;
 
-	if (fc->crom_src_buf == NULL)
-		fw_init_crom(fc);
-
 	buf =  fc->crom_src_buf;
 	src = fc->crom_src;
 	root = fc->crom_root;
@@ -715,18 +734,18 @@ fw_busreset(struct firewire_comm *fc, uint32_t new_status)
 	struct firewire_dev_comm *fdc;
 	struct crom_src *src;
 	device_t *devlistp;
-	void *newrom;
 	int i, devcnt;
 
-	switch(fc->status){
-	case FWBUSMGRELECT:
+	FW_GLOCK_ASSERT(fc);
+	if (fc->status == FWBUSMGRELECT)
 		callout_stop(&fc->bmr_callout);
-		break;
-	default:
-		break;
-	}
+
 	fc->status = new_status;
 	fw_reset_csr(fc);
+
+	if (fc->status == FWBUSNOTREADY)
+		fw_init_crom(fc);
+
 	fw_reset_crom(fc);
 
 	if (device_get_children(fc->bdev, &devlistp, &devcnt) == 0) {
@@ -739,19 +758,19 @@ fw_busreset(struct firewire_comm *fc, uint32_t new_status)
 		free(devlistp, M_TEMP);
 	}
 
-	newrom = malloc(CROMSIZE, M_FW, M_NOWAIT | M_ZERO);
 	src = &fc->crom_src_buf->src;
-	crom_load(src, (uint32_t *)newrom, CROMSIZE);
-	if (bcmp(newrom, fc->config_rom, CROMSIZE) != 0) {
-		/* bump generation and reload */
-		src->businfo.generation ++;
-		/* generation must be between 0x2 and 0xF */
-		if (src->businfo.generation < 2)
-			src->businfo.generation ++;
-		crom_load(src, (uint32_t *)newrom, CROMSIZE);
-		bcopy(newrom, (void *)fc->config_rom, CROMSIZE);
-	}
-	free(newrom, M_FW);
+	/*
+	 * If the old config rom needs to be overwritten,
+	 * bump the businfo.generation indicator to 
+	 * indicate that we need to be reprobed
+	 */
+	if (bcmp(src, fc->config_rom, CROMSIZE) != 0) {
+		/* generation is a 2 bit field */
+		/* valid values are only from 0 - 3 */
+		src->businfo.generation = 1;
+		bcopy(src, (void *)fc->config_rom, CROMSIZE);
+	} else
+		src->businfo.generation = 0;
 }
 
 /* Call once after reboot */
@@ -807,13 +826,7 @@ void fw_init(struct firewire_comm *fc)
 		fc->ir[i]->maxq = FWMAXQUEUE;
 		fc->it[i]->maxq = FWMAXQUEUE;
 	}
-/* Initialize csr registers */
-	fc->topology_map = (struct fw_topology_map *)malloc(
-				sizeof(struct fw_topology_map),
-				M_FW, M_NOWAIT | M_ZERO);
-	fc->speed_map = (struct fw_speed_map *)malloc(
-				sizeof(struct fw_speed_map),
-				M_FW, M_NOWAIT | M_ZERO);
+
 	CSRARC(fc, TOPO_MAP) = 0x3f1 << 16;
 	CSRARC(fc, TOPO_MAP + 4) = 1;
 	CSRARC(fc, SPED_MAP) = 0x3f1 << 16;
@@ -1559,7 +1572,7 @@ fw_explore_node(struct fw_device *dfwdev)
 	/* unchanged ? */
 	if (bcmp(&csr[0], &fwdev->csrrom[0], sizeof(uint32_t) * 5) == 0) {
 		if (firewire_debug)
-			printf("node%d: crom unchanged\n", node);
+			device_printf(fc->dev, "node%d: crom unchanged\n", node);
 		return (0);
 	}
 
diff --git a/sys/dev/firewire/fwohci.c b/sys/dev/firewire/fwohci.c
index 1c2573b..258ba1b 100644
--- a/sys/dev/firewire/fwohci.c
+++ b/sys/dev/firewire/fwohci.c
@@ -1003,7 +1003,7 @@ again:
 	if (maxdesc < db_tr->dbcnt) {
 		maxdesc = db_tr->dbcnt;
 		if (firewire_debug)
-			device_printf(sc->fc.dev, "maxdesc: %d\n", maxdesc);
+			device_printf(sc->fc.dev, "%s: maxdesc %d\n", __func__, maxdesc);
 	}
 	/* last db */
 	LAST_DB(db_tr, db);
@@ -1842,6 +1842,7 @@ fwohci_intr_core(struct fwohci_softc *sc, uint32_t stat, int count)
 	struct firewire_comm *fc = (struct firewire_comm *)sc;
 	uint32_t node_id, plen;
 
+	FW_GLOCK_ASSERT(fc);
 	if ((stat & OHCI_INT_PHY_BUS_R) && (fc->status != FWBUSRESET)) {
 		fc->status = FWBUSRESET;
 		/* Disable bus reset interrupt until sid recv. */
@@ -1884,8 +1885,8 @@ fwohci_intr_core(struct fwohci_softc *sc, uint32_t stat, int count)
 		plen = OREAD(sc, OHCI_SID_CNT);
 
 		fc->nodeid = node_id & 0x3f;
-		device_printf(fc->dev, "node_id=0x%08x, gen=%d, ",
-			node_id, (plen >> 16) & 0xff);
+		device_printf(fc->dev, "node_id=0x%08x, SelfID Count=%d, ",
+				fc->nodeid, (plen >> 16) & 0xff);
 		if (!(node_id & OHCI_NODE_VALID)) {
 			printf("Bus reset failure\n");
 			goto sidout;
@@ -1996,9 +1997,11 @@ fwohci_task_busreset(void *arg, int pending)
 {
 	struct fwohci_softc *sc = (struct fwohci_softc *)arg;
 
+	FW_GLOCK(&sc->fc);
 	fw_busreset(&sc->fc, FWBUSRESET);
 	OWRITE(sc, OHCI_CROMHDR, ntohl(sc->fc.config_rom[0]));
 	OWRITE(sc, OHCI_BUS_OPT, ntohl(sc->fc.config_rom[2]));
+	FW_GUNLOCK(&sc->fc);
 }
 
 static void
@@ -2010,6 +2013,10 @@ fwohci_task_sid(void *arg, int pending)
 	int i, plen;
 
 
+	/*
+	 * We really should have locking
+	 * here.  Not sure why it's not
+	 */
 	plen = OREAD(sc, OHCI_SID_CNT);
 
 	if (plen & OHCI_SID_ERR) {
@@ -2029,14 +2036,13 @@ fwohci_task_sid(void *arg, int pending)
 	}
 	for (i = 0; i < plen / 4; i ++)
 		buf[i] = FWOHCI_DMA_READ(sc->sid_buf[i+1]);
-#if 1 /* XXX needed?? */
+
 	/* pending all pre-bus_reset packets */
 	fwohci_txd(sc, &sc->atrq);
 	fwohci_txd(sc, &sc->atrs);
 	fwohci_arcv(sc, &sc->arrs, -1);
 	fwohci_arcv(sc, &sc->arrq, -1);
 	fw_drain_txq(fc);
-#endif
 	fw_sidrcv(fc, buf, plen);
 	free(buf, M_FW);
 }
@@ -2061,6 +2067,7 @@ fwohci_check_stat(struct fwohci_softc *sc)
 {
 	uint32_t stat, irstat, itstat;
 
+	FW_GLOCK_ASSERT(&sc->fc);
 	stat = OREAD(sc, FWOHCI_INTSTAT);
 	if (stat == 0xffffffff) {
 		device_printf(sc->fc.dev, 
@@ -2090,29 +2097,24 @@ fwohci_check_stat(struct fwohci_softc *sc)
 	return (FILTER_HANDLED);
 }
 
-int
-fwohci_filt(void *arg)
-{
-	struct fwohci_softc *sc = (struct fwohci_softc *)arg;
-
-	if (!(sc->intmask & OHCI_INT_EN)) {
-		/* polling mode */
-		return (FILTER_STRAY);
-	}
-	return (fwohci_check_stat(sc));
-}
-
 void
 fwohci_intr(void *arg)
 {
-	fwohci_filt(arg);
+	struct fwohci_softc *sc = (struct fwohci_softc *)arg;
+
+	FW_GLOCK(&sc->fc);
+	fwohci_check_stat(sc);
+	FW_GUNLOCK(&sc->fc);
 }
 
 void
 fwohci_poll(struct firewire_comm *fc, int quick, int count)
 {
 	struct fwohci_softc *sc = (struct fwohci_softc *)fc;
+
+	FW_GLOCK(fc);
 	fwohci_check_stat(sc);
+	FW_GUNLOCK(fc);
 }
 
 static void
@@ -2466,6 +2468,7 @@ fwohci_ibr(struct firewire_comm *fc)
 	device_printf(fc->dev, "Initiate bus reset\n");
 	sc = (struct fwohci_softc *)fc;
 
+	FW_GLOCK(fc);
 	/*
 	 * Make sure our cached values from the config rom are
 	 * initialised.
@@ -2486,6 +2489,7 @@ fwohci_ibr(struct firewire_comm *fc)
 	fun |= FW_PHY_ISBR | FW_PHY_RHB;
 	fun = fwphy_wrdata(sc, FW_PHY_ISBR_REG, fun);
 #endif
+	FW_GUNLOCK(fc);
 }
 
 void
diff --git a/sys/dev/firewire/fwohci_pci.c b/sys/dev/firewire/fwohci_pci.c
index b21bb28..ea46dbc 100644
--- a/sys/dev/firewire/fwohci_pci.c
+++ b/sys/dev/firewire/fwohci_pci.c
@@ -334,14 +334,11 @@ fwohci_pci_attach(device_t self)
 		return ENXIO;
 	}
 
-
 	err = bus_setup_intr(self, sc->irq_res,
-			INTR_TYPE_NET | INTR_MPSAFE,
-#if FWOHCI_INTFILT
-		     fwohci_filt, NULL, sc, &sc->ih);
-#else
-		     NULL, (driver_intr_t *) fwohci_intr, sc, &sc->ih);
-#endif
+				INTR_TYPE_NET | INTR_MPSAFE,
+				NULL, (driver_intr_t *) fwohci_intr,
+				sc, &sc->ih);
+
 #if defined(__DragonFly__) || __FreeBSD_version < 500000
 	/* XXX splcam() should mask this irq for sbp.c*/
 	err = bus_setup_intr(self, sc->irq_res, INTR_TYPE_CAM,
diff --git a/sys/dev/firewire/fwohcivar.h b/sys/dev/firewire/fwohcivar.h
index d0ee9f6..7e24ab0 100644
--- a/sys/dev/firewire/fwohcivar.h
+++ b/sys/dev/firewire/fwohcivar.h
@@ -37,12 +37,6 @@
 
 #include <sys/taskqueue.h>
 
-#if defined(__DragonFly__) ||  __FreeBSD_version < 700043
-#define FWOHCI_INTFILT	0
-#else
-#define FWOHCI_INTFILT	1
-#endif
-
 typedef struct fwohci_softc {
 	struct firewire_comm fc;
 	bus_space_tag_t bst;
@@ -84,7 +78,7 @@ typedef struct fwohci_softc {
 } fwohci_softc_t;
 
 void fwohci_intr (void *arg);
-int fwohci_filt (void *arg);
+void fwohci_filt (void *arg);
 int fwohci_init (struct fwohci_softc *, device_t);
 void fwohci_poll (struct firewire_comm *, int, int);
 void fwohci_reset (struct fwohci_softc *, device_t);
diff --git a/sys/dev/firewire/sbp.c b/sys/dev/firewire/sbp.c
index b4d89d5..e13ec85 100644
--- a/sys/dev/firewire/sbp.c
+++ b/sys/dev/firewire/sbp.c
@@ -493,6 +493,7 @@ END_DEBUG
 			/* lost device */
 			sbp_cam_detach_sdev(sdev);
 			sbp_free_sdev(sdev);
+			target->luns[lun] = NULL;
 		}
 	}
 
@@ -781,7 +782,9 @@ END_DEBUG
 					SBP_UNLOCK(sbp);
 				}
 				sdev->status = SBP_DEV_RETRY;
-				sbp_abort_all_ocbs(sdev, CAM_SCSI_BUS_RESET);
+				sbp_cam_detach_sdev(sdev);
+				sbp_free_sdev(sdev);
+				target->luns[i] = NULL;
 				break;
 			case SBP_DEV_PROBE:
 			case SBP_DEV_TOATTACH:
@@ -1255,11 +1258,18 @@ END_DEBUG
 		htonl(((sdev->target->sbp->fd.fc->nodeid | FWLOCALBUS )<< 16));
 	xfer->send.payload[1] = htonl((uint32_t)ocb->bus_addr);
 
+	/*
+	 * sbp_xfer_free() will attempt to acquire
+	 * the SBP lock on entrance.  Also, this removes
+	 * a LOR between the firewire layer and sbp
+	 */
+	SBP_UNLOCK(sdev->target->sbp);
 	if(fw_asyreq(xfer->fc, -1, xfer) != 0){
 			sbp_xfer_free(xfer);
 			ocb->ccb->ccb_h.status = CAM_REQ_INVALID;
 			xpt_done(ocb->ccb);
 	}
+	SBP_LOCK(sdev->target->sbp);
 }
 
 static void
@@ -2123,6 +2133,7 @@ sbp_free_sdev(struct sbp_dev *sdev)
 		    sdev->ocb[i].dmamap);
 	fwdma_free(sdev->target->sbp->fd.fc, &sdev->dma);
 	free(sdev, M_SBP);
+	sdev = NULL;
 }
 
 static void