Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/jdolecek-ncq]: src/sys/dev add NCQ error recovery for mvsata(4)



details:   https://anonhg.NetBSD.org/src/rev/b7c371648cc7
branches:  jdolecek-ncq
changeset: 823002:b7c371648cc7
user:      jdolecek <jdolecek%NetBSD.org@localhost>
date:      Sat Aug 12 22:43:22 2017 +0000

description:
add NCQ error recovery for mvsata(4)

fix wait flags for mvsata_bio_start(), AT_* constants are only valid for cmd

change the debug logging to similar form as ahcisata/siisata, so it's
easier to selectively show

diffstat:

 sys/dev/ata/TODO.ncq   |    3 -
 sys/dev/ic/mvsata.c    |  306 +++++++++++++++++++++++++++++++++++++-----------
 sys/dev/ic/mvsatavar.h |    6 +-
 3 files changed, 242 insertions(+), 73 deletions(-)

diffs (truncated from 728 to 300 lines):

diff -r 5bc940b4c81a -r b7c371648cc7 sys/dev/ata/TODO.ncq
--- a/sys/dev/ata/TODO.ncq      Sat Aug 12 22:31:50 2017 +0000
+++ b/sys/dev/ata/TODO.ncq      Sat Aug 12 22:43:22 2017 +0000
@@ -5,9 +5,6 @@
 
 test wd* at umass?, confirm the ata_channel kludge works
 
-do proper NCQ error recovery
-- update mvsata to do same as ahcisata/siisata (read log ext, timeouts, et.al)
-
 do biodone() in wddone() starting the dump to not leak bufs when dumping from
 active system? make sure to not trigger atastart()
 - call ata_kill_active() + ata_kill_pending() when dumping
diff -r 5bc940b4c81a -r b7c371648cc7 sys/dev/ic/mvsata.c
--- a/sys/dev/ic/mvsata.c       Sat Aug 12 22:31:50 2017 +0000
+++ b/sys/dev/ic/mvsata.c       Sat Aug 12 22:43:22 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: mvsata.c,v 1.35.6.21 2017/08/12 14:41:54 jdolecek Exp $        */
+/*     $NetBSD: mvsata.c,v 1.35.6.22 2017/08/12 22:43:22 jdolecek Exp $        */
 /*
  * Copyright (c) 2008 KIYOHARA Takashi
  * All rights reserved.
@@ -26,7 +26,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: mvsata.c,v 1.35.6.21 2017/08/12 14:41:54 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: mvsata.c,v 1.35.6.22 2017/08/12 22:43:22 jdolecek Exp $");
 
 #include "opt_mvsata.h"
 
@@ -86,12 +86,16 @@
        SHADOW_REG_BLOCK_OFFSET + (reg), (val))
 
 #ifdef MVSATA_DEBUG
-#define DPRINTF(x)     if (mvsata_debug) printf x
-#define        DPRINTFN(n,x)   if (mvsata_debug >= (n)) printf x
-int    mvsata_debug = 2;
+
+#define DEBUG_INTR   0x01
+#define DEBUG_XFERS  0x02
+#define DEBUG_FUNCS  0x08
+#define DEBUG_PROBE  0x10
+
+#define        DPRINTF(n,x)    if (mvsata_debug & (n)) printf x
+int    mvsata_debug = 0;
 #else
-#define DPRINTF(x)
-#define DPRINTFN(n,x)
+#define DPRINTF(n,x)
 #endif
 
 #define ATA_DELAY              10000   /* 10s for a drive I/O */
@@ -160,6 +164,7 @@
 #endif
 
 static int mvsata_nondma_handle(struct mvsata_port *);
+static void mvsata_channel_recover(struct mvsata_port *);
 
 static int mvsata_port_init(struct mvsata_hc *, int);
 static int mvsata_wdc_reg_init(struct mvsata_port *, struct wdc_regs *);
@@ -398,7 +403,7 @@
 
        cause = MVSATA_HC_READ_4(mvhc, SATAHC_IC);
 
-       DPRINTFN(3, ("%s:%d: mvsata_intr: cause=0x%08x\n",
+       DPRINTF(DEBUG_INTR, ("%s:%d: mvsata_intr: cause=0x%08x\n",
            device_xname(MVSATA_DEV(sc)), mvhc->hc, cause));
 
        if (cause & SATAHC_IC_SAINTCOAL)
@@ -432,22 +437,23 @@
 {
        struct ata_channel *chp = &mvport->port_ata_channel;
        struct ata_xfer *xfer;
-       int ret, quetag;
+       int ret;
 
        /*
         * The chip doesn't support several pending non-DMA commands,
         * and the ata middle layer never issues several non-NCQ commands,
         * so there must be exactly one active command at this moment.
         */
-       for (quetag = 0; quetag < MVSATA_EDMAQ_LEN; quetag++) {
-               if ((mvport->port_quetagidx & __BIT(quetag)) == 0)
-                       continue;
-
-               break;
+       xfer = ata_queue_get_active_xfer(chp);
+       if (xfer == NULL) {
+               /* Can happen after error recovery, ignore */
+               DPRINTF(DEBUG_FUNCS|DEBUG_XFERS,
+                   ("%s:%d: %s: intr without xfer\n",
+                   device_xname(MVSATA_DEV2(mvport)), chp->ch_channel,
+                   __func__));
+               return 0;
        }
-       KASSERT(quetag < MVSATA_EDMAQ_LEN);
-
-       xfer = ata_queue_hwslot_to_xfer(chp, quetag);
+
        ret = xfer->c_intr(chp, xfer, 1);
        return (ret);
 }
@@ -473,7 +479,7 @@
        }
        MVSATA_EDMA_WRITE_4(mvport, EDMA_IEC, ~cause);
 
-       DPRINTFN(3, ("%s:%d:%d:"
+       DPRINTF(DEBUG_INTR, ("%s:%d:%d:"
            " mvsata_error: cause=0x%08x, mask=0x%08x, status=0x%08x\n",
            device_xname(MVSATA_DEV2(mvport)), mvport->port_hc->hc,
            mvport->port, cause, MVSATA_EDMA_READ_4(mvport, EDMA_IEM),
@@ -492,8 +498,9 @@
                if (sc->sc_gen == gen1)
                        mvsata_devconn_gen1(mvport);
 
-               DPRINTFN(3, ("    device connected\n"));
+               DPRINTF(DEBUG_INTR, ("    device connected\n"));
        }
+
 #ifndef MVSATA_WITHOUTDMA
        if ((sc->sc_gen == gen1 && cause & EDMA_IE_ETRANSINT) ||
            (sc->sc_gen != gen1 && cause & EDMA_IE_ESELFDIS)) {
@@ -524,10 +531,138 @@
                    device_xname(MVSATA_DEV2(mvport)),
                    mvport->port_hc->hc, mvport->port);
        }
+       if (cause & EDMA_IE_EDEVERR) {
+               aprint_error("%s:%d:%d: device error, recovering\n",
+                   device_xname(MVSATA_DEV2(mvport)),
+                   mvport->port_hc->hc, mvport->port);
+
+               if (!mvport->port_recovering)
+                       mvsata_channel_recover(mvport);
+       }
 
        return 1;
 }
 
+static void
+mvsata_hold(struct mvsata_port *mvport)
+{
+       mvport->port_hold_slots |= mvport->port_quetagidx;
+       mvport->port_quetagidx = 0;
+}
+
+static void
+mvsata_unhold(struct mvsata_port *mvport)
+{
+       mvport->port_quetagidx = mvport->port_hold_slots;
+       mvport->port_hold_slots = 0;
+}
+
+static void
+mvsata_channel_recover(struct mvsata_port *mvport)
+{
+       struct ata_channel *chp = &mvport->port_ata_channel;
+       struct ata_drive_datas *drvp;
+       int drive, error;
+       uint8_t eslot, slot, st, err;
+       struct ata_xfer *xfer;
+
+       KASSERT(!mvport->port_recovering);
+
+       mvport->port_recovering = true;
+
+       if (chp->ch_ndrives > PMP_PORT_CTL) {
+               /* Get PM port number for the device in error. This device
+                * doesn't seem to have dedicated register for this, so just
+                * assume last selected port was the one. */
+               /* XXX FIS-based switching */
+               drive = MVSATA_EDMA_READ_4(mvport, SATA_SATAICTL) & 0xf;
+       } else
+               drive = 0;
+
+       drvp = &chp->ch_drive[drive];
+
+       /*
+        * Controller doesn't need any special action. Simply execute
+        * READ LOG EXT for NCQ to unblock device processing, then continue
+        * as if nothing happened.
+        */
+       KASSERT(drive >= 0);
+
+       mvsata_hold(mvport);
+
+       /*
+        * When running NCQ commands, READ LOG EXT is necessary to clear the
+        * error condition and unblock the device.
+        */
+       error = ata_read_log_ext_ncq(drvp, AT_POLL, &eslot, &st, &err);
+
+       mvsata_unhold(mvport);
+
+       switch (error) {
+       case 0:
+               /* Error out the particular NCQ xfer, then requeue the others */
+               if ((mvport->port_quetagidx & (1 << eslot)) != 0) {
+                       xfer = ata_queue_hwslot_to_xfer(chp, eslot);
+                       xfer->c_flags |= C_RECOVERED;
+                       xfer->c_bio.error = ERROR;
+                       xfer->c_bio.r_error = err;
+                       xfer->c_intr(chp, xfer, 1);
+               }
+               break;
+
+       case EOPNOTSUPP:
+               /*
+                * Non-NCQ command error, just find the slot and end it with
+                * an error. Handler figures the error itself.
+                */
+               for (slot = 0; slot < MVSATA_EDMAQ_LEN; slot++) {
+                       if ((mvport->port_quetagidx & (1 << slot)) != 0) {
+                               xfer = ata_queue_hwslot_to_xfer(chp, slot);
+                               if (xfer->c_drive != drive)
+                                       continue;
+
+                               xfer->c_intr(chp, xfer, 1);
+                       }
+               }
+               break;
+
+       case EAGAIN:
+               /*
+                * Failed to get resources to run the recovery command, must
+                * reset the drive. This will also kill all still outstanding
+                * transfers.
+                */
+               mvsata_reset_channel(chp, AT_POLL);
+               goto out;
+               /* NOTREACHED */
+
+       default:
+               /*
+                * The command to get the slot failed. Kill outstanding
+                * commands for the same drive only. No need to reset
+                * the drive, it's unblocked nevertheless.
+                */
+               break;
+       }
+
+       /* Requeue the non-errorred commands */ 
+       for (slot = 0; slot < MVSATA_EDMAQ_LEN; slot++) {
+               if (((mvport->port_quetagidx >> slot) & 1) == 0)
+                       continue;
+
+               xfer = ata_queue_hwslot_to_xfer(chp, slot);
+               if (xfer->c_drive != drive)
+                       continue;
+
+               xfer->c_kill_xfer(chp, xfer,
+                   (error == 0) ? KILL_REQUEUE : KILL_RESET);
+       }
+
+out:
+       /* Drive unblocked, back to normal operation */
+       mvport->port_recovering = false;
+       atastart(chp);
+}
 
 /*
  * ATA callback entry points
@@ -561,7 +696,8 @@
        struct atac_softc *atac = chp->ch_atac;
        struct ata_bio *ata_bio = &xfer->c_bio;
 
-       DPRINTFN(1, ("%s:%d: mvsata_bio: drive=%d, blkno=%" PRId64
+       DPRINTF(DEBUG_FUNCS|DEBUG_XFERS,
+           ("%s:%d: mvsata_bio: drive=%d, blkno=%" PRId64
            ", bcount=%ld\n", device_xname(atac->atac_dev), chp->ch_channel,
            drvp->drive, ata_bio->blkno, ata_bio->bcount));
 
@@ -594,7 +730,8 @@
 
        edma_c = MVSATA_EDMA_READ_4(mvport, EDMA_CMD);
 
-       DPRINTF(("%s:%d: mvsata_reset_drive: drive=%d (EDMA %sactive)\n",
+       DPRINTF(DEBUG_FUNCS,
+           ("%s:%d: mvsata_reset_drive: drive=%d (EDMA %sactive)\n",
            device_xname(MVSATA_DEV2(mvport)), chp->ch_channel, drvp->drive,
            (edma_c & EDMA_CMD_EENEDMA) ? "" : "not "));
 
@@ -622,7 +759,7 @@
        struct mvsata_softc *sc = device_private(MVSATA_DEV2(mvport));
        uint32_t sstat, ctrl;
 
-       DPRINTF(("%s: mvsata_reset_channel: channel=%d\n",
+       DPRINTF(DEBUG_FUNCS, ("%s: mvsata_reset_channel: channel=%d\n",
            device_xname(MVSATA_DEV2(mvport)), chp->ch_channel));
 
        mvsata_hreset_port(mvport);
@@ -664,7 +801,8 @@
        struct ata_command *ata_c = &xfer->c_ata_c;
        int rv, s;
 
-       DPRINTFN(1, ("%s:%d: mvsata_exec_command: drive=%d, bcount=%d,"
+       DPRINTF(DEBUG_FUNCS|DEBUG_XFERS,
+           ("%s:%d: mvsata_exec_command: drive=%d, bcount=%d,"
            " r_lba=0x%012"PRIx64", r_count=0x%04x, r_features=0x%04x,"
            " r_device=0x%02x, r_command=0x%02x\n",



Home | Main Index | Thread Index | Old Index