[src/trunk]: src/sys/dev only start the timeout machinery once the I/O is com...

[jdolecek <jdolecek%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/87cd6706a9b4
branches:  trunk
changeset: 972192:87cd6706a9b4
user:      jdolecek <jdolecek%NetBSD.org@localhost>
date:      Tue May 19 08:08:51 2020 +0000

description:
only start the timeout machinery once the I/O is completely setup
and successful, particularly after PIO write is finished

fixes crashes in case the setup is so slow that timeout is triggered
e.g. while still waiting in wdc_wait_for_unbusy() or shortly after, without
drive actually having chance to complete the I/O, as seen in some
configuration under QEMU by Paul Ripke

diffstat:

 sys/dev/ata/ata_wdc.c |  17 ++++++-----------
 sys/dev/ic/mvsata.c   |  30 ++++++++++++------------------
 2 files changed, 18 insertions(+), 29 deletions(-)

diffs (132 lines):

diff -r f45d73249f2a -r 87cd6706a9b4 sys/dev/ata/ata_wdc.c
--- a/sys/dev/ata/ata_wdc.c     Tue May 19 08:07:44 2020 +0000
+++ b/sys/dev/ata/ata_wdc.c     Tue May 19 08:08:51 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ata_wdc.c,v 1.116 2020/05/15 16:58:28 jdolecek Exp $   */
+/*     $NetBSD: ata_wdc.c,v 1.117 2020/05/19 08:08:51 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.116 2020/05/15 16:58:28 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.117 2020/05/19 08:08:51 jdolecek Exp $");
 
 #include "opt_ata.h"
 #include "opt_wdc.h"
@@ -478,10 +478,6 @@
                                (*wdc->dma_start)(wdc->dma_arg,
                                    chp->ch_channel, xfer->c_drive);
                        chp->ch_flags |= ATACH_DMA_WAIT;
-                       /* start timeout machinery */
-                       if ((xfer->c_flags & C_POLL) == 0)
-                               callout_reset(&chp->c_timo_callout,
-                                   ATA_DELAY / 1000 * hz, wdctimeout, chp);
                        /* wait for irq */
                        goto intr;
                } /* else not DMA */
@@ -549,10 +545,6 @@
                        (drvp->lp->d_type == DKTYPE_ST506) ?
                        drvp->lp->d_precompcyl / 4 : 0);
                }
-               /* start timeout machinery */
-               if ((xfer->c_flags & C_POLL) == 0)
-                       callout_reset(&chp->c_timo_callout,
-                           ATA_DELAY / 1000 * hz, wdctimeout, chp);
        } else if (ata_bio->nblks > 1) {
                /* The number of blocks in the last stretch may be smaller. */
                nblks = xfer->c_bcount / drvp->lp->d_secsize;
@@ -598,7 +590,10 @@
 intr:
 #endif
        /* Wait for IRQ (either real or polled) */
-       if ((ata_bio->flags & ATA_POLL) == 0) {
+       if ((xfer->c_flags & C_POLL) == 0) {
+               /* start timeout machinery */
+               callout_reset(&chp->c_timo_callout,
+                   ATA_DELAY / 1000 * hz, wdctimeout, chp);
                chp->ch_flags |= ATACH_IRQ_WAIT;
                return ATASTART_STARTED;
        } else {
diff -r f45d73249f2a -r 87cd6706a9b4 sys/dev/ic/mvsata.c
--- a/sys/dev/ic/mvsata.c       Tue May 19 08:07:44 2020 +0000
+++ b/sys/dev/ic/mvsata.c       Tue May 19 08:08:51 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: mvsata.c,v 1.56 2020/04/13 10:49:34 jdolecek Exp $     */
+/*     $NetBSD: mvsata.c,v 1.57 2020/05/19 08:08:51 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.56 2020/04/13 10:49:34 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: mvsata.c,v 1.57 2020/05/19 08:08:51 jdolecek Exp $");
 
 #include "opt_mvsata.h"
 
@@ -1108,10 +1108,6 @@
                                return ATASTART_ABORT;
                        }
                        chp->ch_flags |= ATACH_DMA_WAIT;
-                       /* start timeout machinery */
-                       if ((xfer->c_flags & C_POLL) == 0)
-                               callout_reset(&chp->c_timo_callout,
-                                   mstohz(ATA_DELAY), ata_timeout, chp);
                        /* wait for irq */
                        goto intr;
                } /* else not DMA */
@@ -1192,11 +1188,6 @@
                            head, sect, nblks,
                            (drvp->lp->d_type == DKTYPE_ST506) ?
                            drvp->lp->d_precompcyl / 4 : 0);
-
-               /* start timeout machinery */
-               if ((xfer->c_flags & C_POLL) == 0)
-                       callout_reset(&chp->c_timo_callout,
-                           mstohz(ATA_DELAY), wdctimeout, chp);
        } else if (ata_bio->nblks > 1) {
                /* The number of blocks in the last stretch may be smaller. */
                nblks = xfer->c_bcount / drvp->lp->d_secsize;
@@ -1238,9 +1229,12 @@
                xfer->c_flags, mvport->port_edmamode_curr, nodma); 
 
        /* Wait for IRQ (either real or polled) */
-       if ((ata_bio->flags & ATA_POLL) != 0)
+       if ((ata_bio->flags & ATA_POLL) != 0) {
+               /* start timeout machinery */
+               callout_reset(&chp->c_timo_callout,
+                   mstohz(ATA_DELAY), wdctimeout, chp);
                return ATASTART_POLL;
-       else
+       } else
                return ATASTART_STARTED;
 
 timeout:
@@ -2116,11 +2110,6 @@
                MVSATA_WDC_WRITE_1(mvport, SRB_CAS, WDCTL_4BIT);
                delay(10); /* some drives need a little delay here */
        }
-       /* start timeout machinery */
-       if ((sc_xfer->xs_control & XS_CTL_POLL) == 0)
-               callout_reset(&chp->c_timo_callout, mstohz(sc_xfer->timeout),
-                   wdctimeout, chp);
-
        MVSATA_WDC_WRITE_1(mvport, SRB_H, WDSD_IBM);
        if (wdc_wait_for_unbusy(chp, ATAPI_DELAY, wait_flags, &tfd) != 0) {
                aprint_error_dev(atac->atac_dev, "not ready, st = %02x\n",
@@ -2129,6 +2118,11 @@
                return ATASTART_ABORT;
        }
 
+       /* start timeout machinery */
+       if ((sc_xfer->xs_control & XS_CTL_POLL) == 0)
+               callout_reset(&chp->c_timo_callout, mstohz(sc_xfer->timeout),
+                   wdctimeout, chp);
+
        /*
         * Even with WDCS_ERR, the device should accept a command packet
         * Limit length to what can be stuffed into the cylinder register