Source-Changes-HG archive

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

[src/jdolecek-ncq]: src/sys/dev remove all logic around ATACH_IRQ_WAIT and ch...



details:   https://anonhg.NetBSD.org/src/rev/5bc4ed2e4437
branches:  jdolecek-ncq
changeset: 822996:5bc4ed2e4437
user:      jdolecek <jdolecek%NetBSD.org@localhost>
date:      Sat Aug 12 09:52:28 2017 +0000

description:
remove all logic around ATACH_IRQ_WAIT and channel-global ch_error/ch_status,
so that there is less hidden state shared by commands; primary intent is
to make the NCQ and non-NCQ paths more similar, and remove possibility
of incorrect handling for the NCQ commands

tested both disk and ATAPI - piixide(4) on QEMU, and siisata(4),
ahcisata(4), mvsata(4) on real hw

diffstat:

 sys/dev/ata/TODO.ncq        |    5 +-
 sys/dev/ata/ata.c           |   23 ++-
 sys/dev/ata/ata_wdc.c       |  102 +++++++++--------
 sys/dev/ata/atavar.h        |   11 +-
 sys/dev/ic/mvsata.c         |  244 ++++++++++++++++++++++---------------------
 sys/dev/ic/wdc.c            |  147 +++++++++++++------------
 sys/dev/ic/wdcvar.h         |   16 +-
 sys/dev/pci/pciide_common.c |    7 +-
 sys/dev/scsipi/atapi_wdc.c  |   84 +++++++--------
 9 files changed, 320 insertions(+), 319 deletions(-)

diffs (truncated from 1900 to 300 lines):

diff -r 8613a5466244 -r 5bc4ed2e4437 sys/dev/ata/TODO.ncq
--- a/sys/dev/ata/TODO.ncq      Sat Aug 12 09:38:58 2017 +0000
+++ b/sys/dev/ata/TODO.ncq      Sat Aug 12 09:52:28 2017 +0000
@@ -7,9 +7,6 @@
 
 do proper NCQ error recovery
 - update mvsata to do same as ahcisata/siisata (read log ext, timeouts, et.al)
-- update also ic/wdc.c, scsipi/atapi_wdc.c, ata/ata_wdc.c to not use
-  ch_status/ch_error/ATACH_IRQ_WAIT
-- retest ATAPI
 
 do biodone() in wddone() starting the dump to not leak bufs when dumping from
 active system? make sure to not trigger atastart()
@@ -18,6 +15,8 @@
 kill active transfers after software drive reset - race timeout vs.
 error recovery
 
+atabus_thread() protect run by mutex/condvar
+
 Other random notes (do outside the NCQ branch):
 -----------------------------------------------------
 implement support for PM FIS-based switching, remove restriction in atastart()
diff -r 8613a5466244 -r 5bc4ed2e4437 sys/dev/ata/ata.c
--- a/sys/dev/ata/ata.c Sat Aug 12 09:38:58 2017 +0000
+++ b/sys/dev/ata/ata.c Sat Aug 12 09:52:28 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ata.c,v 1.132.8.24 2017/08/01 21:41:25 jdolecek Exp $  */
+/*     $NetBSD: ata.c,v 1.132.8.25 2017/08/12 09:52:28 jdolecek Exp $  */
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.  All rights reserved.
@@ -25,7 +25,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.132.8.24 2017/08/01 21:41:25 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.132.8.25 2017/08/12 09:52:28 jdolecek Exp $");
 
 #include "opt_ata.h"
 
@@ -622,15 +622,16 @@
                        ata_reset_channel(chp, AT_WAIT | chp->ch_reset_flags);
                } else if (chq->queue_active > 0 && chq->queue_freeze == 1) {
                        /*
-                        * Caller has bumped queue_freeze, decrease it.
+                        * Caller has bumped queue_freeze, decrease it. This
+                        * flow shalt never be executed for NCQ commands.
                         */
+                       KASSERT((chp->ch_flags & ATACH_NCQ) == 0);
+                       KASSERT(chq->queue_active == 1);
+
                        ata_channel_thaw(chp);
-                       u_int active __diagused = 0;
-                       TAILQ_FOREACH(xfer, &chq->active_xfers, c_activechain) {
-                               (*xfer->c_start)(xfer->c_chp, xfer);
-                               active++;
-                       }
-                       KASSERT(active == chq->queue_active);
+                       xfer = ata_queue_get_active_xfer(chp);
+                       KASSERT(xfer != NULL);
+                       (*xfer->c_start)(xfer->c_chp, xfer);
                } else if (chq->queue_freeze > 1)
                        panic("ata_thread: queue_freeze");
        }
@@ -1430,7 +1431,7 @@
                /* finish the busmastering PIO */
                (*wdc->piobm_done)(wdc->dma_arg,
                    chp->ch_channel, xfer->c_drive);
-               chp->ch_flags &= ~(ATACH_DMA_WAIT | ATACH_PIOBM_WAIT | ATACH_IRQ_WAIT);
+               chp->ch_flags &= ~(ATACH_DMA_WAIT | ATACH_PIOBM_WAIT);
        }
 #endif
 
@@ -1521,7 +1522,7 @@
 /*
  * Check for race of normal transfer handling vs. timeout.
  */
-static bool
+bool
 ata_timo_xfer_check(struct ata_xfer *xfer)
 {
        struct ata_channel *chp = xfer->c_chp;
diff -r 8613a5466244 -r 5bc4ed2e4437 sys/dev/ata/ata_wdc.c
--- a/sys/dev/ata/ata_wdc.c     Sat Aug 12 09:38:58 2017 +0000
+++ b/sys/dev/ata/ata_wdc.c     Sat Aug 12 09:52:28 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ata_wdc.c,v 1.105.6.6 2017/06/27 18:36:03 jdolecek Exp $       */
+/*     $NetBSD: ata_wdc.c,v 1.105.6.7 2017/08/12 09:52:28 jdolecek Exp $       */
 
 /*
  * Copyright (c) 1998, 2001, 2003 Manuel Bouyer.
@@ -54,7 +54,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.105.6.6 2017/06/27 18:36:03 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.105.6.7 2017/08/12 09:52:28 jdolecek Exp $");
 
 #include "opt_ata.h"
 #include "opt_wdc.h"
@@ -111,7 +111,7 @@
 static void    wdc_ata_bio_kill_xfer(struct ata_channel *,
                                      struct ata_xfer *, int);
 static void    wdc_ata_bio_done(struct ata_channel *, struct ata_xfer *);
-static int     wdc_ata_err(struct ata_drive_datas *, struct ata_bio *);
+static int     wdc_ata_err(struct ata_drive_datas *, struct ata_bio *, int);
 #define WDC_ATA_NOERR 0x00 /* Drive doesn't report an error */
 #define WDC_ATA_RECOV 0x01 /* There was a recovered error */
 #define WDC_ATA_ERR   0x02 /* Drive reports an error */
@@ -175,7 +175,7 @@
        struct wdc_regs *wdr = &wdc->regs[chp->ch_channel];
        struct ata_bio *ata_bio = &xfer->c_bio;
        struct ata_drive_datas *drvp = &chp->ch_drive[xfer->c_drive];
-       int wait_flags;
+       int wait_flags, tfd;
        const char *errstring;
 #ifdef WDC_NO_IDS
        wait_flags = AT_POLL;
@@ -216,15 +216,17 @@
                    WDSD_IBM | (xfer->c_drive << 4));
                DELAY(10);
                errstring = "wait";
-               if (wdcwait(chp, WDCS_DRDY, WDCS_DRDY, ATA_DELAY, wait_flags))
+               if (wdcwait(chp, WDCS_DRDY, WDCS_DRDY, ATA_DELAY, wait_flags,
+                   &tfd))
                        goto ctrltimeout;
                wdccommandshort(chp, xfer->c_drive, WDCC_RECAL);
                /* Wait for at last 400ns for status bit to be valid */
                DELAY(1);
                errstring = "recal";
-               if (wdcwait(chp, WDCS_DRDY, WDCS_DRDY, ATA_DELAY, wait_flags))
+               if (wdcwait(chp, WDCS_DRDY, WDCS_DRDY, ATA_DELAY, wait_flags,
+                   &tfd))
                        goto ctrltimeout;
-               if (chp->ch_status & (WDCS_ERR | WDCS_DWF))
+               if (ATACH_ST(tfd) & (WDCS_ERR | WDCS_DWF))
                        goto ctrlerror;
                /* Don't try to set modes if controller can't be adjusted */
                if (atac->atac_set_modes == NULL)
@@ -235,9 +237,10 @@
                wdccommand(chp, drvp->drive, SET_FEATURES, 0, 0, 0,
                    0x08 | drvp->PIO_mode, WDSF_SET_MODE);
                errstring = "piomode";
-               if (wdcwait(chp, WDCS_DRDY, WDCS_DRDY, ATA_DELAY, wait_flags))
+               if (wdcwait(chp, WDCS_DRDY, WDCS_DRDY, ATA_DELAY, wait_flags,
+                   &tfd))
                        goto ctrltimeout;
-               if (chp->ch_status & (WDCS_ERR | WDCS_DWF))
+               if (ATACH_ST(tfd) & (WDCS_ERR | WDCS_DWF))
                        goto ctrlerror;
 #if NATA_DMA
 #if NATA_UDMA
@@ -253,9 +256,10 @@
                        goto geometry;
                }
                errstring = "dmamode";
-               if (wdcwait(chp, WDCS_DRDY, WDCS_DRDY, ATA_DELAY, wait_flags))
+               if (wdcwait(chp, WDCS_DRDY, WDCS_DRDY, ATA_DELAY, wait_flags,
+                   &tfd))
                        goto ctrltimeout;
-               if (chp->ch_status & (WDCS_ERR | WDCS_DWF))
+               if (ATACH_ST(tfd) & (WDCS_ERR | WDCS_DWF))
                        goto ctrlerror;
 #endif /* NATA_DMA */
 geometry:
@@ -267,9 +271,10 @@
                    (drvp->lp->d_type == DKTYPE_ST506) ?
                        drvp->lp->d_precompcyl / 4 : 0);
                errstring = "geometry";
-               if (wdcwait(chp, WDCS_DRDY, WDCS_DRDY, ATA_DELAY, wait_flags))
+               if (wdcwait(chp, WDCS_DRDY, WDCS_DRDY, ATA_DELAY, wait_flags,
+                   &tfd))
                        goto ctrltimeout;
-               if (chp->ch_status & (WDCS_ERR | WDCS_DWF))
+               if (ATACH_ST(tfd) & (WDCS_ERR | WDCS_DWF))
                        goto ctrlerror;
 multimode:
                if (drvp->multi == 1)
@@ -277,9 +282,10 @@
                wdccommand(chp, xfer->c_drive, WDCC_SETMULTI, 0, 0, 0,
                    drvp->multi, 0);
                errstring = "setmulti";
-               if (wdcwait(chp, WDCS_DRDY, WDCS_DRDY, ATA_DELAY, wait_flags))
+               if (wdcwait(chp, WDCS_DRDY, WDCS_DRDY, ATA_DELAY, wait_flags,
+                   &tfd))
                        goto ctrltimeout;
-               if (chp->ch_status & (WDCS_ERR | WDCS_DWF))
+               if (ATACH_ST(tfd) & (WDCS_ERR | WDCS_DWF))
                        goto ctrlerror;
 ready:
                drvp->state = READY;
@@ -304,13 +310,13 @@
        printf("%s:%d:%d: %s ",
            device_xname(atac->atac_dev), chp->ch_channel, xfer->c_drive,
            errstring);
-       if (chp->ch_status & WDCS_DWF) {
+       if (ATACH_ST(tfd) & WDCS_DWF) {
                printf("drive fault\n");
                ata_bio->error = ERR_DF;
        } else {
-               printf("error (%x)\n", chp->ch_error);
-               ata_bio->r_error = chp->ch_error;
+               ata_bio->r_error = ATACH_ERR(tfd);
                ata_bio->error = ERROR;
+               printf("error (%x)\n", ata_bio->r_error);
        }
 ctrldone:
        drvp->state = 0;
@@ -335,7 +341,7 @@
        uint8_t head, sect, cmd = 0;
        int nblks;
 #if NATA_DMA || NATA_PIOBM
-       int error, dma_flags = 0;
+       int error, dma_flags = 0, tfd;
 #endif
 
        ATADEBUG_PRINT(("_wdc_ata_bio_start %s:%d:%d\n",
@@ -441,7 +447,8 @@
                                wdc->select(chp, xfer->c_drive);
                        bus_space_write_1(wdr->cmd_iot, wdr->cmd_iohs[wd_sdh],
                            0, WDSD_IBM | (xfer->c_drive << 4));
-                       switch(wdc_wait_for_ready(chp, ATA_DELAY, wait_flags)) {
+                       switch(wdc_wait_for_ready(chp, ATA_DELAY, wait_flags,
+                           &tfd)) {
                        case WDCWAIT_OK:
                                break;
                        case WDCWAIT_TOUT:
@@ -521,7 +528,7 @@
                        wdc->select(chp, xfer->c_drive);
                bus_space_write_1(wdr->cmd_iot, wdr->cmd_iohs[wd_sdh], 0,
                    WDSD_IBM | (xfer->c_drive << 4));
-               switch(wdc_wait_for_ready(chp, ATA_DELAY, wait_flags)) {
+               switch(wdc_wait_for_ready(chp, ATA_DELAY, wait_flags, &tfd)) {
                case WDCWAIT_OK:
                        break;
                case WDCWAIT_TOUT:
@@ -556,17 +563,18 @@
                 * we have to busy-wait here, we can't rely on running in
                 * thread context.
                 */
-               if (wdc_wait_for_drq(chp, ATA_DELAY, AT_POLL) != 0) {
+               if (wdc_wait_for_drq(chp, ATA_DELAY, AT_POLL, &tfd) != 0) {
                        printf("%s:%d:%d: timeout waiting for DRQ, "
                            "st=0x%02x, err=0x%02x\n",
                            device_xname(atac->atac_dev), chp->ch_channel,
-                           xfer->c_drive, chp->ch_status, chp->ch_error);
-                       if (wdc_ata_err(drvp, ata_bio) != WDC_ATA_ERR)
+                           xfer->c_drive,
+                           ATACH_ST(tfd), ATACH_ERR(tfd));
+                       if (wdc_ata_err(drvp, ata_bio, tfd) != WDC_ATA_ERR)
                                ata_bio->error = TIMEOUT;
                        wdc_ata_bio_done(chp, xfer);
                        return;
                }
-               if (wdc_ata_err(drvp, ata_bio) == WDC_ATA_ERR) {
+               if (wdc_ata_err(drvp, ata_bio, tfd) == WDC_ATA_ERR) {
                        wdc_ata_bio_done(chp, xfer);
                        return;
                }
@@ -588,9 +596,7 @@
 intr:
 #endif
        /* Wait for IRQ (either real or polled) */
-       if ((ata_bio->flags & ATA_POLL) == 0) {
-               chp->ch_flags |= ATACH_IRQ_WAIT;
-       } else {
+       if ((ata_bio->flags & ATA_POLL) != 0) {
                /* Wait for at last 400ns for status bit to be valid */
                delay(1);
 #if NATA_DMA
@@ -607,21 +613,22 @@
 timeout:
        printf("%s:%d:%d: not ready, st=0x%02x, err=0x%02x\n",
            device_xname(atac->atac_dev), chp->ch_channel, xfer->c_drive,
-           chp->ch_status, chp->ch_error);
-       if (wdc_ata_err(drvp, ata_bio) != WDC_ATA_ERR)
+           ATACH_ST(tfd), ATACH_ERR(tfd));
+       if (wdc_ata_err(drvp, ata_bio, tfd) != WDC_ATA_ERR)
                ata_bio->error = TIMEOUT;
        wdc_ata_bio_done(chp, xfer);
        return;
 }
 
 static int
-wdc_ata_bio_intr(struct ata_channel *chp, struct ata_xfer *xfer, int irq)
+wdc_ata_bio_intr(struct ata_channel *chp, struct ata_xfer *xfer, int is)
 {
        struct atac_softc *atac = chp->ch_atac;
        struct wdc_softc *wdc = CHAN_TO_WDC(chp);
        struct ata_bio *ata_bio = &xfer->c_bio;
        struct ata_drive_datas *drvp = &chp->ch_drive[xfer->c_drive];
-       int drv_err;
+       int drv_err, tfd;
+       bool poll = ((xfer->c_flags & C_POLL) != 0);
 
        ATADEBUG_PRINT(("wdc_ata_bio_intr %s:%d:%d\n",
            device_xname(atac->atac_dev), chp->ch_channel, xfer->c_drive),
@@ -655,8 +662,8 @@
 #endif
 



Home | Main Index | Thread Index | Old Index