[src/trunk]: src/sys/dev hold channel lock during whole ata_dmaerr()/ata_down...

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

details:   https://anonhg.NetBSD.org/src/rev/ab2fd585f7fc
branches:  trunk
changeset: 445788:ab2fd585f7fc
user:      jdolecek <jdolecek%NetBSD.org@localhost>
date:      Mon Nov 12 18:51:01 2018 +0000

description:
hold channel lock during whole ata_dmaerr()/ata_downgrade_mode() -
according to code inspection this is safe, none of the set_modes
hooks execute anything which would be taking the lock

adresses PR kern/53714 by Andreas Gustafsson

diffstat:

 sys/dev/ata/ata.c          |   8 ++++++--
 sys/dev/ata/ata_wdc.c      |   6 ++----
 sys/dev/ic/mvsata.c        |  18 +++++++++---------
 sys/dev/scsipi/atapi_wdc.c |  16 ++++++++--------
 4 files changed, 25 insertions(+), 23 deletions(-)

diffs (271 lines):

diff -r aa549704ee3c -r ab2fd585f7fc sys/dev/ata/ata.c
--- a/sys/dev/ata/ata.c Mon Nov 12 18:10:36 2018 +0000
+++ b/sys/dev/ata/ata.c Mon Nov 12 18:51:01 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ata.c,v 1.145 2018/10/24 20:25:52 jdolecek Exp $       */
+/*     $NetBSD: ata.c,v 1.146 2018/11/12 18:51:01 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.145 2018/10/24 20:25:52 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.146 2018/11/12 18:51:01 jdolecek Exp $");
 
 #include "opt_ata.h"
 
@@ -952,6 +952,8 @@
 void
 ata_dmaerr(struct ata_drive_datas *drvp, int flags)
 {
+       ata_channel_lock_owned(drvp->chnl_softc);
+
        /*
         * Downgrade decision: if we get NERRS_MAX in NXFER.
         * We start with n_dmaerrs set to NERRS_MAX-1 so that the
@@ -1772,6 +1774,8 @@
        device_t drv_dev = drvp->drv_softc;
        int cf_flags = device_cfdata(drv_dev)->cf_flags;
 
+       ata_channel_lock_owned(drvp->chnl_softc);
+
        /* if drive or controller don't know its mode, we can't do much */
        if ((drvp->drive_flags & ATA_DRIVE_MODE) == 0 ||
            (atac->atac_set_modes == NULL))
diff -r aa549704ee3c -r ab2fd585f7fc sys/dev/ata/ata_wdc.c
--- a/sys/dev/ata/ata_wdc.c     Mon Nov 12 18:10:36 2018 +0000
+++ b/sys/dev/ata/ata_wdc.c     Mon Nov 12 18:51:01 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ata_wdc.c,v 1.112 2018/10/22 20:13:47 jdolecek Exp $   */
+/*     $NetBSD: ata_wdc.c,v 1.113 2018/11/12 18:51:01 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.112 2018/10/22 20:13:47 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.113 2018/11/12 18:51:01 jdolecek Exp $");
 
 #include "opt_ata.h"
 #include "opt_wdc.h"
@@ -729,10 +729,8 @@
                if (drv_err != WDC_ATA_ERR)
                        goto end;
                if (ata_bio->r_error & WDCE_CRC || ata_bio->error == ERR_DMA) {
-                       ata_channel_unlock(chp);
                        ata_dmaerr(drvp,
                            (xfer->c_flags & C_POLL) ? AT_POLL : 0);
-                       ata_channel_lock(chp);
                        goto err;
                }
        }
diff -r aa549704ee3c -r ab2fd585f7fc sys/dev/ic/mvsata.c
--- a/sys/dev/ic/mvsata.c       Mon Nov 12 18:10:36 2018 +0000
+++ b/sys/dev/ic/mvsata.c       Mon Nov 12 18:51:01 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: mvsata.c,v 1.44 2018/10/22 20:30:52 jdolecek Exp $     */
+/*     $NetBSD: mvsata.c,v 1.45 2018/11/12 18:51:01 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.44 2018/10/22 20:30:52 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: mvsata.c,v 1.45 2018/11/12 18:51:01 jdolecek Exp $");
 
 #include "opt_mvsata.h"
 
@@ -1360,9 +1360,9 @@
                if (ata_bio->error == NOERROR)
                        goto end;
                if (ata_bio->error == ERR_DMA) {
-                       ata_channel_unlock(chp);
                        ata_dmaerr(drvp,
                            (xfer->c_flags & C_POLL) ? AT_POLL : 0);
+                       ata_channel_unlock(chp);
                        goto err;
                }
        }
@@ -2275,11 +2275,11 @@
                aprint_error_dev(atac->atac_dev,
                    "channel %d: device timeout, c_bcount=%d, c_skip=%d\n",
                    chp->ch_channel, xfer->c_bcount, xfer->c_skip);
-               ata_channel_unlock(chp);
                if (xfer->c_flags & C_DMA)
                        ata_dmaerr(drvp,
                            (xfer->c_flags & C_POLL) ? AT_POLL : 0);
                sc_xfer->error = XS_TIMEOUT;
+               ata_channel_unlock(chp);
                mvsata_atapi_reset(chp, xfer);
                return 1;
        }
@@ -2289,9 +2289,9 @@
         * and reset device.
         */
        if ((xfer->c_flags & C_TIMEOU) && (xfer->c_flags & C_DMA)) {
-               ata_channel_unlock(chp);
                ata_dmaerr(drvp, (xfer->c_flags & C_POLL) ? AT_POLL : 0);
                sc_xfer->error = XS_RESET;
+               ata_channel_unlock(chp);
                mvsata_atapi_reset(chp, xfer);
                return (1);
        }
@@ -2351,11 +2351,11 @@
                        aprint_error_dev(atac->atac_dev,
                            "channel %d drive %d: bad data phase DATAOUT\n",
                            chp->ch_channel, xfer->c_drive);
-                       ata_channel_unlock(chp);
                        if (xfer->c_flags & C_DMA)
                                ata_dmaerr(drvp,
                                    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
                        sc_xfer->error = XS_TIMEOUT;
+                       ata_channel_unlock(chp);
                        mvsata_atapi_reset(chp, xfer);
                        return 1;
                }
@@ -2387,10 +2387,10 @@
                        aprint_error_dev(atac->atac_dev,
                            "channel %d drive %d: bad data phase DATAIN\n",
                            chp->ch_channel, xfer->c_drive);
-                       ata_channel_unlock(chp);
                        if (xfer->c_flags & C_DMA)
                                ata_dmaerr(drvp,
                                    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
+                       ata_channel_unlock(chp);
                        sc_xfer->error = XS_TIMEOUT;
                        mvsata_atapi_reset(chp, xfer);
                        return 1;
@@ -2441,11 +2441,11 @@
                        sc_xfer->error = XS_SHORTSENSE;
                        sc_xfer->sense.atapi_sense = ATACH_ERR(tfd);
                } else {
-                       ata_channel_unlock(chp);
                        if (xfer->c_flags & C_DMA)
                                ata_dmaerr(drvp,
                                    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
                        sc_xfer->error = XS_RESET;
+                       ata_channel_unlock(chp);
                        mvsata_atapi_reset(chp, xfer);
                        return (1);
                }
@@ -2580,10 +2580,10 @@
                        sc_xfer->status = SCSI_CHECK;
                } else
                    if (wdc->dma_status & (WDC_DMAST_NOIRQ | WDC_DMAST_ERR)) {
-                       ata_channel_unlock(chp);
                        ata_dmaerr(drvp,
                            (xfer->c_flags & C_POLL) ? AT_POLL : 0);
                        sc_xfer->error = XS_RESET;
+                       ata_channel_unlock(chp);
                        mvsata_atapi_reset(chp, xfer);
                        return;
                }
diff -r aa549704ee3c -r ab2fd585f7fc sys/dev/scsipi/atapi_wdc.c
--- a/sys/dev/scsipi/atapi_wdc.c        Mon Nov 12 18:10:36 2018 +0000
+++ b/sys/dev/scsipi/atapi_wdc.c        Mon Nov 12 18:51:01 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: atapi_wdc.c,v 1.131 2018/10/22 20:13:47 jdolecek Exp $ */
+/*     $NetBSD: atapi_wdc.c,v 1.132 2018/11/12 18:51:01 jdolecek Exp $ */
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -25,7 +25,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: atapi_wdc.c,v 1.131 2018/10/22 20:13:47 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: atapi_wdc.c,v 1.132 2018/11/12 18:51:01 jdolecek Exp $");
 
 #ifndef ATADEBUG
 #define ATADEBUG
@@ -808,7 +808,6 @@
                printf("%s:%d:%d: device timeout, c_bcount=%d, c_skip=%d\n",
                    device_xname(atac->atac_dev), chp->ch_channel,
                    xfer->c_drive, xfer->c_bcount, xfer->c_skip);
-               ata_channel_unlock(chp);
 #if NATA_DMA
                if (xfer->c_flags & C_DMA) {
                        ata_dmaerr(drvp,
@@ -816,6 +815,7 @@
                }
 #endif
                sc_xfer->error = XS_TIMEOUT;
+               ata_channel_unlock(chp);
                wdc_atapi_reset(chp, xfer);
                return 1;
        }
@@ -828,9 +828,9 @@
         * and reset device.
         */
        if ((xfer->c_flags & C_TIMEOU) && (xfer->c_flags & C_DMA)) {
-               ata_channel_unlock(chp);
                ata_dmaerr(drvp, (xfer->c_flags & C_POLL) ? AT_POLL : 0);
                sc_xfer->error = XS_RESET;
+               ata_channel_unlock(chp);
                wdc_atapi_reset(chp, xfer);
                return (1);
        }
@@ -908,12 +908,12 @@
                if ((sc_xfer->xs_control & XS_CTL_DATA_OUT) == 0 ||
                    (xfer->c_flags & C_DMA) != 0) {
                        printf("wdc_atapi_intr: bad data phase DATAOUT\n");
-                       ata_channel_unlock(chp);
                        if (xfer->c_flags & C_DMA) {
                                ata_dmaerr(drvp,
                                    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
                        }
                        sc_xfer->error = XS_TIMEOUT;
+                       ata_channel_unlock(chp);
                        wdc_atapi_reset(chp, xfer);
                        return 1;
                }
@@ -962,12 +962,12 @@
                if ((sc_xfer->xs_control & XS_CTL_DATA_IN) == 0 ||
                    (xfer->c_flags & C_DMA) != 0) {
                        printf("wdc_atapi_intr: bad data phase DATAIN\n");
-                       ata_channel_unlock(chp);
                        if (xfer->c_flags & C_DMA) {
                                ata_dmaerr(drvp,
                                    (xfer->c_flags & C_POLL) ? AT_POLL : 0);
                        }
                        sc_xfer->error = XS_TIMEOUT;
+                       ata_channel_unlock(chp);
                        wdc_atapi_reset(chp, xfer);
                        return 1;
                }
@@ -1037,7 +1037,6 @@
                        sc_xfer->error = XS_SHORTSENSE;
                        sc_xfer->sense.atapi_sense = ATACH_ERR(tfd);
                } else {
-                       ata_channel_unlock(chp);
 #if NATA_DMA
                        if (xfer->c_flags & C_DMA) {
                                ata_dmaerr(drvp,
@@ -1045,6 +1044,7 @@
                        }
 #endif
                        sc_xfer->error = XS_RESET;
+                       ata_channel_unlock(chp);
                        wdc_atapi_reset(chp, xfer);
                        return (1);
                }
@@ -1123,12 +1123,12 @@
 #if NATA_DMA || NATA_PIOBM
                else if (wdc->dma_status &
                    (WDC_DMAST_NOIRQ | WDC_DMAST_ERR)) {
-                       ata_channel_unlock(chp);
 #if NATA_DMA
                        ata_dmaerr(drvp,
                            (xfer->c_flags & C_POLL) ? AT_POLL : 0);
 #endif
                        sc_xfer->error = XS_RESET;
+                       ata_channel_unlock(chp);
                        wdc_atapi_reset(chp, xfer);
                        return;
                }