svn commit: r187993 - head/sys/dev/firewire

Sean Bruno sbruno at FreeBSD.org
Sun Feb 1 15:28:53 PST 2009


Author: sbruno
Date: Sun Feb  1 23:28:52 2009
New Revision: 187993
URL: http://svn.freebsd.org/changeset/base/187993

Log:
  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

Modified:
  head/sys/dev/firewire/firewire.c
  head/sys/dev/firewire/fwohci.c
  head/sys/dev/firewire/fwohci_pci.c
  head/sys/dev/firewire/fwohcivar.h
  head/sys/dev/firewire/sbp.c

Modified: head/sys/dev/firewire/firewire.c
==============================================================================
--- head/sys/dev/firewire/firewire.c	Sun Feb  1 23:27:21 2009	(r187992)
+++ head/sys/dev/firewire/firewire.c	Sun Feb  1 23:28:52 2009	(r187993)
@@ -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, ui
 	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, ui
 		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);
 	}
 

Modified: head/sys/dev/firewire/fwohci.c
==============================================================================
--- head/sys/dev/firewire/fwohci.c	Sun Feb  1 23:27:21 2009	(r187992)
+++ head/sys/dev/firewire/fwohci.c	Sun Feb  1 23:28:52 2009	(r187993)
@@ -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
 	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
 		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 pend
 {
 	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 *s
 {
 	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 *s
 	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

Modified: head/sys/dev/firewire/fwohci_pci.c
==============================================================================
--- head/sys/dev/firewire/fwohci_pci.c	Sun Feb  1 23:27:21 2009	(r187992)
+++ head/sys/dev/firewire/fwohci_pci.c	Sun Feb  1 23:28:52 2009	(r187993)
@@ -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,

Modified: head/sys/dev/firewire/fwohcivar.h
==============================================================================
--- head/sys/dev/firewire/fwohcivar.h	Sun Feb  1 23:27:21 2009	(r187992)
+++ head/sys/dev/firewire/fwohcivar.h	Sun Feb  1 23:28:52 2009	(r187993)
@@ -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);

Modified: head/sys/dev/firewire/sbp.c
==============================================================================
--- head/sys/dev/firewire/sbp.c	Sun Feb  1 23:27:21 2009	(r187992)
+++ head/sys/dev/firewire/sbp.c	Sun Feb  1 23:28:52 2009	(r187993)
@@ -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


More information about the svn-src-head mailing list