From owner-svn-src-head@FreeBSD.ORG Sun Feb 1 23:28:53 2009 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 421D0106566C; Sun, 1 Feb 2009 23:28:53 +0000 (UTC) (envelope-from sbruno@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 2B4328FC16; Sun, 1 Feb 2009 23:28:53 +0000 (UTC) (envelope-from sbruno@FreeBSD.org) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id n11NSrN4069214; Sun, 1 Feb 2009 23:28:53 GMT (envelope-from sbruno@svn.freebsd.org) Received: (from sbruno@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id n11NSqYG069209; Sun, 1 Feb 2009 23:28:52 GMT (envelope-from sbruno@svn.freebsd.org) Message-Id: <200902012328.n11NSqYG069209@svn.freebsd.org> From: Sean Bruno Date: Sun, 1 Feb 2009 23:28:52 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r187993 - head/sys/dev/firewire X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 01 Feb 2009 23:28:53 -0000 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 -#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