From owner-p4-projects@FreeBSD.ORG Sun Jun 14 19:49:34 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 15EF710656EF; Sun, 14 Jun 2009 19:49:34 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BB9C510656D6 for ; Sun, 14 Jun 2009 19:49:33 +0000 (UTC) (envelope-from mav@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id A8C0E8FC1D for ; Sun, 14 Jun 2009 19:49:33 +0000 (UTC) (envelope-from mav@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n5EJnXYL069654 for ; Sun, 14 Jun 2009 19:49:33 GMT (envelope-from mav@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n5EJnX6Y069652 for perforce@freebsd.org; Sun, 14 Jun 2009 19:49:33 GMT (envelope-from mav@freebsd.org) Date: Sun, 14 Jun 2009 19:49:33 GMT Message-Id: <200906141949.n5EJnX6Y069652@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to mav@freebsd.org using -f From: Alexander Motin To: Perforce Change Reviews Cc: Subject: PERFORCE change 164373 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 14 Jun 2009 19:49:35 -0000 http://perforce.freebsd.org/chv.cgi?CH=164373 Change 164373 by mav@mav_mavbook on 2009/06/14 19:49:15 Tune error reporting. Do not do any error recovery here except controller itself, give all information to XPT and wait until it will handle everything. Affected files ... .. //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.c#30 edit .. //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.h#14 edit Differences ... ==== //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.c#30 (text+ko) ==== @@ -834,7 +834,7 @@ { device_t dev = (device_t)data; struct ahci_channel *ch = device_get_softc(dev); - uint32_t istatus, cstatus, sstatus, res, err; + uint32_t istatus, cstatus, sstatus, ok, err; enum ahci_err_type et; int i, ccs; @@ -851,42 +851,58 @@ ahci_phy_check_events(dev); /* Process command errors */ if (istatus & (AHCI_P_IX_IF|AHCI_P_IX_HBD|AHCI_P_IX_HBF|AHCI_P_IX_TFE)) { -device_printf(dev, "%s ERROR is %08x cs %08x ss %08x rs %08x\n", __func__, istatus, cstatus, sstatus, ch->rslots); +device_printf(dev, "%s ERROR is %08x cs %08x ss %08x rs %08x tfd %02x\n", + __func__, istatus, cstatus, sstatus, ch->rslots, ATA_INL(ch->r_mem, AHCI_P_TFD)); ccs = (ATA_INL(ch->r_mem, AHCI_P_CMD) & AHCI_P_CMD_CCS_MASK) >> AHCI_P_CMD_CCS_SHIFT; /* Kick controller into sane state */ ahci_stop(dev); ahci_clo(dev); ahci_start(dev); - res = ch->rslots; + ok = ch->rslots & ~(cstatus | sstatus); err = ch->rslots & (cstatus | sstatus); } else { ccs = 0; - res = ch->rslots & ~(cstatus | sstatus); + ok = ch->rslots & ~(cstatus | sstatus); err = 0; } - /* On error, requeue frozen command. */ - if (err && ch->frozen) { - union ccb *fccb = ch->frozen; - ch->frozen = NULL; - xpt_release_simq(ch->sim, TRUE); - fccb->ccb_h.status = CAM_REQUEUE_REQ; - xpt_done(fccb); + /* Complete all successfull commands. */ + for (i = 0; i < ch->numslots; i++) { + if ((ok >> i) & 1) + ahci_end_transaction(&ch->slot[i], AHCI_ERR_NONE); } - /* Check all slots. */ - for (i = 0; i < ch->numslots; i++) { - /* Do we have an event on slot? */ - if ((res & (1 << i)) == 0) - continue; - /* Process request completion. */ - et = AHCI_ERR_NONE; - if ((err >> i) & 1) { - if (i == ccs) - et = AHCI_ERR_REAL; - else - et = AHCI_ERR_BTW; + /* On error, complete the rest of commands with error statuses. */ + if (err) { + xpt_freeze_simq(ch->sim, ch->numrslots); + if (ch->frozen) { + union ccb *fccb = ch->frozen; + ch->frozen = NULL; + fccb->ccb_h.status = CAM_REQUEUE_REQ | CAM_RELEASE_SIMQ; + xpt_done(fccb); + } + for (i = 0; i < ch->numslots; i++) { + /* XXX: reqests in loading state. */ + if (((err >> i) & 1) == 0) + continue; + if (istatus & AHCI_P_IX_IF) { + /* SATA error */ + et = AHCI_ERR_SATA; + } else if (istatus & AHCI_P_IX_TFE) { + /* Task File Error */ + if (ch->numtslots == 0) { + /* Untagged operation. */ + if (i == ccs) + et = AHCI_ERR_TFE; + else + et = AHCI_ERR_INNOCENT; + } else { + /* Tagged operation. */ + et = AHCI_ERR_TFE; + } + } else + et = AHCI_ERR_INVALID; + ahci_end_transaction(&ch->slot[i], et); } - ahci_end_transaction(&ch->slot[i], et); } mtx_unlock(&ch->mtx); } @@ -990,6 +1006,7 @@ //device_printf(slot->dev, "%s slot %d\n", __func__, slot->slot); if (error) { device_printf(slot->dev, "DMA load error\n"); + xpt_freeze_simq(ch->sim, 1); ahci_end_transaction(slot, AHCI_ERR_INVALID); return; } @@ -1029,6 +1046,7 @@ /* Setup the FIS for this request */ if (!(fis_size = ahci_setup_fis(ctp, ccb, slot->slot))) { device_printf(ch->dev, "Setting up SATA FIS failed\n"); + xpt_freeze_simq(ch->sim, 1); ahci_end_transaction(slot, AHCI_ERR_INVALID); return; } @@ -1097,20 +1115,21 @@ device_printf(ch->dev, "Poll error on slot %d, TFD: %04x\n", slot->slot, ATA_INL(ch->r_mem, AHCI_P_TFD)); - et = AHCI_ERR_REAL; + et = AHCI_ERR_TFE; break; } } if (timeout && (count >= timeout)) { device_printf(ch->dev, "Poll timeout on slot %d\n", slot->slot); - et = CAM_CMD_TIMEOUT; + et = AHCI_ERR_TIMEOUT; } if (et != AHCI_ERR_NONE) { /* Kick controller into sane state */ ahci_stop(ch->dev); ahci_clo(ch->dev); ahci_start(ch->dev); + xpt_freeze_simq(ch->sim, 1); } ahci_end_transaction(slot, et); return; @@ -1127,11 +1146,31 @@ { device_t dev = slot->dev; struct ahci_channel *ch = device_get_softc(dev); + int i; device_printf(dev, "Timeout on slot %d\n", slot->slot); + + /* Kick controller into sane state. */ + ahci_stop(ch->dev); + ahci_clo(ch->dev); + ahci_start(ch->dev); + + xpt_freeze_simq(ch->sim, ch->numrslots); + /* Handle command with timeout. */ ahci_end_transaction(&ch->slot[slot->slot], AHCI_ERR_TIMEOUT); - /* XXX: This is wrong for NCQ error recovery. */ - ahci_reset(dev); + /* Handle the rest of commands. */ + if (ch->frozen) { + union ccb *fccb = ch->frozen; + ch->frozen = NULL; + fccb->ccb_h.status = CAM_REQUEUE_REQ | CAM_RELEASE_SIMQ; + xpt_done(fccb); + } + for (i = 0; i < ch->numslots; i++) { + /* Do we have a running request on slot? */ + if (ch->slot[i].state < AHCI_SLOT_RUNNING) + continue; + ahci_end_transaction(&ch->slot[i], AHCI_ERR_INNOCENT); + } } /* Must be called with channel locked. */ @@ -1149,19 +1188,20 @@ BUS_DMASYNC_POSTWRITE); /* Read result registers to the result struct * May be incorrect if several commands finished same time, - * so read only when sure. + * so read only when sure or have to. */ if (ccb->ccb_h.func_code == XPT_ATA_IO) { struct ata_res *res = &ccb->ataio.res; - if ((et == AHCI_ERR_REAL) || + if ((et == AHCI_ERR_TFE) || (ccb->ataio.cmd.flags & CAM_ATAIO_NEEDRESULT)) { u_int8_t *fis = ch->dma.rfis + 0x40; + uint16_t tfd = ATA_INL(ch->r_mem, AHCI_P_TFD); bus_dmamap_sync(ch->dma.rfis_tag, ch->dma.rfis_map, BUS_DMASYNC_POSTREAD); - res->status = fis[2]; - res->error = fis[3]; + res->status = tfd; + res->error = tfd >> 8; res->lba_low = fis[4]; res->lba_mid = fis[5]; res->lba_high = fis[6]; @@ -1182,6 +1222,8 @@ } /* Set proper result status. */ ccb->ccb_h.status &= ~CAM_STATUS_MASK; + if (et != AHCI_ERR_NONE) + ccb->ccb_h.status |= CAM_RELEASE_SIMQ; switch (et) { case AHCI_ERR_NONE: ccb->ccb_h.status |= CAM_REQ_CMP; @@ -1191,18 +1233,18 @@ case AHCI_ERR_INVALID: ccb->ccb_h.status |= CAM_REQ_INVALID; break; - case AHCI_ERR_REAL: + case AHCI_ERR_INNOCENT: + ccb->ccb_h.status |= CAM_REQUEUE_REQ; + break; + case AHCI_ERR_TFE: if (ccb->ccb_h.func_code == XPT_SCSI_IO) { ccb->ccb_h.status |= CAM_SCSI_STATUS_ERROR; ccb->csio.scsi_status = SCSI_STATUS_CHECK_COND; } else ccb->ccb_h.status |= CAM_REQ_CMP_ERR; break; - case AHCI_ERR_BTW: - ccb->ccb_h.status |= CAM_REQUEUE_REQ; - break; - case AHCI_ERR_RESET: - ccb->ccb_h.status |= CAM_SCSI_BUS_RESET; + case AHCI_ERR_SATA: + ccb->ccb_h.status |= CAM_UNCOR_PARITY; break; case AHCI_ERR_TIMEOUT: ccb->ccb_h.status |= CAM_CMD_TIMEOUT; @@ -1362,12 +1404,12 @@ if (bootverbose) device_printf(dev, "AHCI reset...\n"); + xpt_freeze_simq(ch->sim, ch->numrslots); /* Requeue freezed command. */ if (ch->frozen) { union ccb *fccb = ch->frozen; ch->frozen = NULL; - xpt_release_simq(ch->sim, TRUE); - fccb->ccb_h.status = CAM_SCSI_BUS_RESET; + fccb->ccb_h.status = CAM_REQUEUE_REQ | CAM_RELEASE_SIMQ; xpt_done(fccb); } /* Kill the engine and requeue all running commands. */ @@ -1376,7 +1418,8 @@ /* Do we have a running request on slot? */ if (ch->slot[i].state < AHCI_SLOT_RUNNING) continue; - ahci_end_transaction(&ch->slot[i], AHCI_ERR_RESET); + /* XXX; Commands in loading state. */ + ahci_end_transaction(&ch->slot[i], AHCI_ERR_INNOCENT); } /* Disable port interrupts */ ATA_OUTL(ch->r_mem, AHCI_P_IE, 0); ==== //depot/projects/scottl-camlock/src/sys/dev/ahci/ahci.h#14 (text+ko) ==== @@ -379,12 +379,12 @@ }; enum ahci_err_type { - AHCI_ERR_NONE, - AHCI_ERR_INVALID, - AHCI_ERR_REAL, - AHCI_ERR_BTW, - AHCI_ERR_RESET, - AHCI_ERR_TIMEOUT + AHCI_ERR_NONE, /* No error */ + AHCI_ERR_INVALID, /* Error detected by us before submitting. */ + AHCI_ERR_INNOCENT, /* Innocent victim. */ + AHCI_ERR_TFE, /* Task File Error. */ + AHCI_ERR_SATA, /* SATA error. */ + AHCI_ERR_TIMEOUT, /* Command execution timeout. */ }; /* macros to hide busspace uglyness */