Source-Changes-HG archive

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

[src/trunk]: src/sys/dev Merge jdolecek-ncqfixes branch



details:   https://anonhg.NetBSD.org/src/rev/9f4f9057d652
branches:  trunk
changeset: 836535:9f4f9057d652
user:      jdolecek <jdolecek%NetBSD.org@localhost>
date:      Mon Oct 22 20:13:47 2018 +0000

description:
Merge jdolecek-ncqfixes branch

- ata_xfer's are dynamicall allocated as needed using a pool, no longer
  limited to number of possible openings supported by controller; dump
  and recovery paths use dedicated pre-allocated storage
- moved callouts and condvars from ata_xfer to queue or channel, so that
  ata_xfer does not need special initialization
- slot allocation now done when xfer is being activated, uncoupled
  from memory allocation; active slots are no longer tracked by controller
  code
- channel and drive reset is done always via the atabus thread, and
  now executes with channel locked the whole time
- NCQ recovery moved to shared function, and run via the thread also
- added some workarounds for buggy error recovery AHCI emulation in QEMU
  and Parallels

designed to primarily fix kern/52614, but might also help with kern/47041
and kern/53183

diffstat:

 sys/dev/ata/TODO.ncq       |    7 +-
 sys/dev/ata/ata.c          |  565 +++++++++++++++++++++++---------------------
 sys/dev/ata/ata_recovery.c |  250 +++++++++++++++++++
 sys/dev/ata/ata_subr.c     |  333 +++++++++----------------
 sys/dev/ata/ata_wdc.c      |   28 +-
 sys/dev/ata/atavar.h       |   80 +++--
 sys/dev/ata/files.ata      |    3 +-
 sys/dev/ata/satapmp_subr.c |   12 +-
 sys/dev/ata/wd.c           |  246 +++++++++---------
 sys/dev/ata/wdvar.h        |   16 +-
 sys/dev/ic/ahcisata_core.c |  349 +++++++++-----------------
 sys/dev/ic/ahcisatavar.h   |    5 +-
 sys/dev/ic/mvsata.c        |  394 ++++++++++---------------------
 sys/dev/ic/mvsatavar.h     |    8 +-
 sys/dev/ic/siisata.c       |  359 +++++++++++-----------------
 sys/dev/ic/siisatavar.h    |    6 +-
 sys/dev/ic/wdc.c           |   81 +++---
 sys/dev/scsipi/atapi_wdc.c |   58 ++--
 sys/dev/usb/umass_isdata.c |    7 +-
 19 files changed, 1351 insertions(+), 1456 deletions(-)

diffs (truncated from 5606 to 300 lines):

diff -r 735592b03de3 -r 9f4f9057d652 sys/dev/ata/TODO.ncq
--- a/sys/dev/ata/TODO.ncq      Mon Oct 22 19:53:25 2018 +0000
+++ b/sys/dev/ata/TODO.ncq      Mon Oct 22 20:13:47 2018 +0000
@@ -12,11 +12,6 @@
 for hw which supports it, adjust error handling in controller drivers to handle
 xfers for several different drives
 
-maybe do device error handling in not-interrupt-context (maybe this should be
-done on a mpata branch?)
-
-queue is allocated regardless if there are any drives, fix? 
-
 dump to unopened disk fails (e.g. dump do wd1b when wd1a not mounted), due
 to the open path executing ata_get_params(), which eventually tsleeps()
 while waiting for the command to finish; specifically, if WDF_LOADED is not
@@ -30,7 +25,7 @@
 
 implement DIOCGCACHE/DIOCCACHESYNC for ld@ataraid? just passthrough, like ccd
 
-MSI/MSI-X support for AHCI and mvsata(4)
+MSI/MSI-X support for ahcisata(4), siisata(4) and mvsata(4)
 
 mvsata - move pci-specific code to the pci attach code
 
diff -r 735592b03de3 -r 9f4f9057d652 sys/dev/ata/ata.c
--- a/sys/dev/ata/ata.c Mon Oct 22 19:53:25 2018 +0000
+++ b/sys/dev/ata/ata.c Mon Oct 22 20:13:47 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ata.c,v 1.141 2017/10/28 04:53:54 riastradh Exp $      */
+/*     $NetBSD: ata.c,v 1.142 2018/10/22 20:13:47 jdolecek Exp $       */
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.  All rights reserved.
@@ -25,14 +25,13 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.141 2017/10/28 04:53:54 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.142 2018/10/22 20:13:47 jdolecek Exp $");
 
 #include "opt_ata.h"
 
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/kernel.h>
-#include <sys/malloc.h>
 #include <sys/device.h>
 #include <sys/conf.h>
 #include <sys/fcntl.h>
@@ -85,6 +84,7 @@
 #endif
 
 static ONCE_DECL(ata_init_ctrl);
+static struct pool ata_xfer_pool;
 
 /*
  * A queue of atabus instances, used to ensure the same bus probe order
@@ -129,7 +129,6 @@
 static void atabusconfig_thread(void *);
 
 static void ata_channel_idle(struct ata_channel *);
-static void ata_channel_thaw_locked(struct ata_channel *);
 static void ata_activate_xfer_locked(struct ata_channel *, struct ata_xfer *);
 static void ata_channel_freeze_locked(struct ata_channel *);
 static void ata_thread_wake_locked(struct ata_channel *);
@@ -143,6 +142,8 @@
 atabus_init(void)
 {
 
+       pool_init(&ata_xfer_pool, sizeof(struct ata_xfer), 0, 0, 0,
+           "ataspl", NULL, IPL_BIO);
        TAILQ_INIT(&atabus_initq_head);
        mutex_init(&atabus_qlock, MUTEX_DEFAULT, IPL_NONE);
        cv_init(&atabus_qcv, "atainitq");
@@ -292,7 +293,7 @@
        cv_broadcast(&atabus_qcv);
        mutex_exit(&atabus_qlock);
 
-       free(atabus_initq, M_DEVBUF);
+       kmem_free(atabus_initq, sizeof(*atabus_initq));
 
        ata_delref(chp);
 
@@ -418,7 +419,7 @@
        cv_broadcast(&atabus_qcv);
        mutex_exit(&atabus_qlock);
 
-       free(atabus_initq, M_DEVBUF);
+       kmem_free(atabus_initq, sizeof(*atabus_initq));
 
        ata_delref(chp);
 
@@ -438,7 +439,7 @@
        struct ata_channel *chp = sc->sc_chan;
        struct ata_queue *chq = chp->ch_queue;
        struct ata_xfer *xfer;
-       int i, rv, s;
+       int i, rv;
 
        ata_channel_lock(chp);
        chp->ch_flags |= ATACH_TH_RUN;
@@ -460,7 +461,8 @@
 
        ata_channel_lock(chp);
        for (;;) {
-               if ((chp->ch_flags & (ATACH_TH_RESET | ATACH_SHUTDOWN)) == 0 &&
+               if ((chp->ch_flags & (ATACH_TH_RESET | ATACH_TH_DRIVE_RESET
+                   | ATACH_TH_RECOVERY | ATACH_SHUTDOWN)) == 0 &&
                    (chq->queue_active == 0 || chq->queue_freeze == 0)) {
                        chp->ch_flags &= ~ATACH_TH_RUN;
                        cv_wait(&chp->ch_thr_idle, &chp->ch_lock);
@@ -476,12 +478,32 @@
                        ata_channel_lock(chp);
                }
                if (chp->ch_flags & ATACH_TH_RESET) {
-                       /* ata_reset_channel() will unfreeze the channel */
-                       ata_channel_unlock(chp);
-                       s = splbio();
-                       ata_reset_channel(chp, AT_WAIT | chp->ch_reset_flags);
+                       /* this will unfreeze the channel */
+                       ata_thread_run(chp, AT_WAIT,
+                           ATACH_TH_RESET, ATACH_NODRIVE);
+               } else if (chp->ch_flags & ATACH_TH_DRIVE_RESET) {
+                       /* this will unfreeze the channel */
+                       for (i = 0; i < chp->ch_ndrives; i++) {
+                               struct ata_drive_datas *drvp;
+
+                               drvp = &chp->ch_drive[i];
+
+                               if (drvp->drive_flags & ATA_DRIVE_TH_RESET) {
+                                       ata_thread_run(chp,
+                                           AT_WAIT, ATACH_TH_DRIVE_RESET, i);
+                               }
+                       }
+                       chp->ch_flags &= ~ATACH_TH_DRIVE_RESET;
+               } else if (chp->ch_flags & ATACH_TH_RECOVERY) {
+                       /*
+                        * This will unfreeze the channel; drops locks during
+                        * run, so must wrap in splbio()/splx() to avoid
+                        * spurious interrupts. XXX MPSAFE
+                        */
+                       int s = splbio();
+                       ata_thread_run(chp, AT_WAIT, ATACH_TH_RECOVERY,
+                           chp->recovery_tfd);
                        splx(s);
-                       ata_channel_lock(chp);
                } else if (chq->queue_active > 0 && chq->queue_freeze == 1) {
                        /*
                         * Caller has bumped queue_freeze, decrease it. This
@@ -509,6 +531,13 @@
                        }
                } else if (chq->queue_freeze > 1)
                        panic("%s: queue_freeze", __func__);
+
+               /* Try to run down the queue once channel is unfrozen */
+               if (chq->queue_freeze == 0) {
+                       ata_channel_unlock(chp);
+                       atastart(chp);
+                       ata_channel_lock(chp);
+               }
        }
        chp->ch_thread = NULL;
        cv_signal(&chp->ch_thr_idle);
@@ -569,13 +598,14 @@
 
        RUN_ONCE(&ata_init_ctrl, atabus_init);
 
-       initq = malloc(sizeof(*initq), M_DEVBUF, M_WAITOK);
+       initq = kmem_zalloc(sizeof(*initq), KM_SLEEP);
        initq->atabus_sc = sc;
        mutex_enter(&atabus_qlock);
        TAILQ_INSERT_TAIL(&atabus_initq_head, initq, atabus_initq);
        mutex_exit(&atabus_qlock);
        config_pending_incr(sc->sc_dev);
 
+       /* XXX MPSAFE - no KTHREAD_MPSAFE, so protected by KERNEL_LOCK() */
        if ((error = kthread_create(PRI_NONE, 0, NULL, atabus_thread, sc,
            &chp->ch_thread, "%s", device_xname(self))) != 0)
                aprint_error_dev(self,
@@ -716,9 +746,8 @@
        if (chp->ch_ndrives != ndrives)
                atabus_free_drives(chp);
        if (chp->ch_drive == NULL) {
-               chp->ch_drive = malloc(
-                   sizeof(struct ata_drive_datas) * ndrives,
-                   M_DEVBUF, M_NOWAIT | M_ZERO);
+               chp->ch_drive = kmem_zalloc(
+                   sizeof(struct ata_drive_datas) * ndrives, KM_NOSLEEP);
        }
        if (chp->ch_drive == NULL) {
            aprint_error_dev(chp->ch_atac->atac_dev,
@@ -761,8 +790,9 @@
 
        if (chp->ch_drive == NULL)
                return;
+       kmem_free(chp->ch_drive,
+           sizeof(struct ata_drive_datas) * chp->ch_ndrives);
        chp->ch_ndrives = 0;
-       free(chp->ch_drive, M_DEVBUF);
        chp->ch_drive = NULL;
 }
 
@@ -780,7 +810,7 @@
 
        ATADEBUG_PRINT(("%s\n", __func__), DEBUG_FUNCS);
 
-       xfer = ata_get_xfer(chp);
+       xfer = ata_get_xfer(chp, false);
        if (xfer == NULL) {
                ATADEBUG_PRINT(("%s: no xfer\n", __func__),
                    DEBUG_FUNCS|DEBUG_PROBE);
@@ -885,7 +915,7 @@
 
        ATADEBUG_PRINT(("ata_set_mode=0x%x\n", mode), DEBUG_FUNCS);
 
-       xfer = ata_get_xfer(chp);
+       xfer = ata_get_xfer(chp, false);
        if (xfer == NULL) {
                ATADEBUG_PRINT(("%s: no xfer\n", __func__),
                    DEBUG_FUNCS|DEBUG_PROBE);
@@ -916,89 +946,6 @@
        return rv;
 }
 
-int
-ata_read_log_ext_ncq(struct ata_drive_datas *drvp, uint8_t flags,
-    uint8_t *slot, uint8_t *status, uint8_t *err)
-{
-       struct ata_xfer *xfer;
-       int rv;
-       struct ata_channel *chp = drvp->chnl_softc;
-       struct atac_softc *atac = chp->ch_atac;
-       uint8_t *tb, cksum, page;
-
-       ATADEBUG_PRINT(("%s\n", __func__), DEBUG_FUNCS);
-
-       /* Only NCQ ATA drives support/need this */
-       if (drvp->drive_type != ATA_DRIVET_ATA ||
-           (drvp->drive_flags & ATA_DRIVE_NCQ) == 0)
-               return EOPNOTSUPP;
-
-       xfer = ata_get_xfer_ext(chp, C_RECOVERY, 0);
-
-       tb = drvp->recovery_blk;
-       memset(tb, 0, sizeof(drvp->recovery_blk));
-
-       /*
-        * We could use READ LOG DMA EXT if drive supports it (i.e.
-        * when it supports Streaming feature) to avoid PIO command,
-        * and to make this a little faster. Realistically, it
-        * should not matter.
-        */
-       xfer->c_flags |= C_RECOVERY;
-       xfer->c_ata_c.r_command = WDCC_READ_LOG_EXT;
-       xfer->c_ata_c.r_lba = page = WDCC_LOG_PAGE_NCQ;
-       xfer->c_ata_c.r_st_bmask = WDCS_DRDY;
-       xfer->c_ata_c.r_st_pmask = WDCS_DRDY;
-       xfer->c_ata_c.r_count = 1;
-       xfer->c_ata_c.r_device = WDSD_LBA;
-       xfer->c_ata_c.flags = AT_READ | AT_LBA | AT_LBA48 | flags;
-       xfer->c_ata_c.timeout = 1000; /* 1s */
-       xfer->c_ata_c.data = tb;
-       xfer->c_ata_c.bcount = sizeof(drvp->recovery_blk);
-
-       if ((*atac->atac_bustype_ata->ata_exec_command)(drvp,
-                                               xfer) != ATACMD_COMPLETE) {
-               rv = EAGAIN;
-               goto out;
-       }
-       if (xfer->c_ata_c.flags & (AT_ERROR | AT_TIMEOU | AT_DF)) {
-               rv = EINVAL;
-               goto out;
-       }
-
-       cksum = 0;
-       for (int i = 0; i < sizeof(drvp->recovery_blk); i++)
-               cksum += tb[i];
-       if (cksum != 0) {
-               aprint_error_dev(drvp->drv_softc,
-                   "invalid checksum %x for READ LOG EXT page %x\n",
-                   cksum, page);
-               rv = EINVAL;
-               goto out;
-       }
-
-       if (tb[0] & WDCC_LOG_NQ) {
-               /* not queued command */
-               rv = EOPNOTSUPP;
-               goto out;
-       }
-
-       *slot = tb[0] & 0x1f;
-       *status = tb[2];
-       *err = tb[3];
-
-       KASSERTMSG((*status & WDCS_ERR),
-           "%s: non-error command slot %d reported by READ LOG EXT page %x: "
-           "err %x status %x\n",
-           device_xname(drvp->drv_softc), *slot, page, *err, *status);
-



Home | Main Index | Thread Index | Old Index