AHCI - Port Multiplier bug fixes
authorMatthew Dillon <dillon@apollo.backplane.com>
Sat, 13 Jun 2009 21:07:57 +0000 (14:07 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Sat, 13 Jun 2009 21:07:57 +0000 (14:07 -0700)
* The error ccb must be used in ahci_pm_softreset() to collect a reliable
  signature from the device.

* Release the error ccb before performing any ahci_pm_*() functions (which
  have to issue their own commands).

* Probe the CAM bus asynchronously.

* Proactively assert that the error ccb is not used in a reentrant fashion.

sys/dev/disk/ahci/ahci.c
sys/dev/disk/ahci/ahci.h
sys/dev/disk/ahci/ahci_cam.c
sys/dev/disk/ahci/ahci_pm.c

index c8be01f..a0e7a81 100644 (file)
@@ -76,9 +76,6 @@ void  ahci_end_exclusive_access(struct ahci_port *ap, struct ata_port *at);
 void   ahci_issue_pending_ncq_commands(struct ahci_port *);
 void   ahci_issue_pending_commands(struct ahci_port *, int);
 
-struct ahci_ccb        *ahci_get_err_ccb(struct ahci_port *);
-void   ahci_put_err_ccb(struct ahci_ccb *);
-
 int    ahci_port_read_ncq_error(struct ahci_port *, int *);
 
 struct ahci_dmamem *ahci_dmamem_alloc(struct ahci_softc *, bus_dma_tag_t tag);
@@ -2338,8 +2335,8 @@ ahci_port_intr(struct ahci_port *ap, int blockable)
 
                /* If device hasn't cleared its busy status, try to idle it. */
                if (tfd & (AHCI_PREG_TFD_STS_BSY | AHCI_PREG_TFD_STS_DRQ)) {
-                       kprintf("%s: Attempting to idle device ccb=%p\n",
-                               PORTNAME(ap), ccb_at);
+                       kprintf("%s: Attempting to idle device\n",
+                               ATANAME(ap, ccb_at));
                        if (ap->ap_flags & AP_F_IN_RESET)
                                goto fatal;
                        /*
@@ -2800,6 +2797,8 @@ ahci_get_err_ccb(struct ahci_port *ap)
        if (sact != 0)
                kprintf("ahci_get_err_ccb but SACT %08x != 0?\n", sact);
        KKASSERT(ahci_pread(ap, AHCI_PREG_CI) == 0);
+       KKASSERT((ap->ap_flags & AP_F_ERR_CCB_RESERVED) == 0);
+       ap->ap_flags |= AP_F_ERR_CCB_RESERVED;
 
 #ifdef DIAGNOSTIC
        KKASSERT(ap->ap_err_busy == 0);
@@ -2838,6 +2837,8 @@ ahci_put_err_ccb(struct ahci_ccb *ccb)
 #ifdef DIAGNOSTIC
        KKASSERT(ap->ap_err_busy);
 #endif
+       KKASSERT((ap->ap_flags & AP_F_ERR_CCB_RESERVED) != 0);
+
        /*
         * No commands may be active on the chip
         */
@@ -2864,6 +2865,7 @@ ahci_put_err_ccb(struct ahci_ccb *ccb)
 #ifdef DIAGNOSTIC
        ap->ap_err_busy = 0;
 #endif
+       ap->ap_flags &= ~AP_F_ERR_CCB_RESERVED;
 }
 
 /*
index cd93dde..154ecb1 100644 (file)
@@ -423,6 +423,7 @@ struct ahci_port {
 #define AP_F_IFS_IGNORED       0x0080
 #define AP_F_IFS_OCCURED       0x0100
 #define AP_F_EXCLUSIVE_ACCESS  0x0200
+#define AP_F_ERR_CCB_RESERVED  0x0400
        int                     ap_signal;      /* os per-port thread sig */
        thread_t                ap_thread;      /* os per-port thread */
        struct lock             ap_lock;        /* os per-port lock */
@@ -557,6 +558,8 @@ void        ahci_pm_check_good(struct ahci_port *ap, int target);
 void   ahci_ata_cmd_timeout(struct ahci_ccb *ccb);
 struct ahci_ccb *ahci_get_ccb(struct ahci_port *ap);
 void   ahci_put_ccb(struct ahci_ccb *ccb);
+struct ahci_ccb *ahci_get_err_ccb(struct ahci_port *);
+void   ahci_put_err_ccb(struct ahci_ccb *);
 int    ahci_poll(struct ahci_ccb *ccb, int timeout,
                        void (*timeout_fn)(struct ahci_ccb *));
 
index feaaeb8..e878d38 100644 (file)
@@ -693,7 +693,7 @@ ahci_xpt_rescan(struct ahci_port *ap)
        ccb->ccb_h.cbfcnp = ahci_cam_rescan_callback;
        ccb->ccb_h.sim_priv.entries[0].ptr = ap;
        ccb->crcn.flags = CAM_FLAG_NONE;
-       xpt_action(ccb);
+       xpt_action_async(ccb);
 }
 
 /*
index 5f487f0..54ca627 100644 (file)
@@ -253,7 +253,7 @@ int
 ahci_pm_softreset(struct ahci_port *ap, int target)
 {
        struct ata_port         *at;
-       struct ahci_ccb         *ccb = NULL;
+       struct ahci_ccb         *ccb;
        struct ahci_cmd_hdr     *cmd_slot;
        u_int8_t                *fis;
        int                     count;
@@ -276,11 +276,13 @@ retry:
         * NOTE: This cannot be safely done between the first and second
         *       softreset FISs.  It's now or never.
         */
+#if 1
        if (ahci_pm_phy_status(ap, target, &data)) {
                kprintf("%s: (B)Cannot clear phy status\n",
                        ATANAME(ap ,at));
        }
        ahci_pm_write(ap, target, AHCI_PMREG_SERR, -1);
+#endif
 
        /*
         * Prep first D2H command with SRST feature & clear busy/reset flags
@@ -292,7 +294,7 @@ retry:
         * non-NULL, assigning it to the ccb prevents the port interrupt
         * from hard-resetting the port if a problem crops up.
         */
-       ccb = ahci_get_ccb(ap);
+       ccb = ahci_get_err_ccb(ap);
        ccb->ccb_done = ahci_pm_empty_done;
        ccb->ccb_xa.flags = ATA_F_READ | ATA_F_POLL;
        ccb->ccb_xa.complete = ahci_pm_dummy_done;
@@ -332,10 +334,9 @@ retry:
                                count += 4;
                        ++tried_longer;
                }
-               if (--count) {
-                       ahci_put_ccb(ccb);
+               ahci_put_err_ccb(ccb);
+               if (--count)
                        goto retry;
-               }
                goto err;
        }
 
@@ -375,13 +376,15 @@ retry:
 
        if (ahci_poll(ccb, 1000, ahci_ata_cmd_timeout) != ATA_S_COMPLETE) {
                kprintf("%s: (PM) Second FIS failed\n", ATANAME(ap, at));
-               if (--count) {
-                       ahci_put_ccb(ccb);
+               ahci_put_err_ccb(ccb);
+#if 1
+               if (--count)
                        goto retry;
-               }
+#endif
                goto err;
        }
 
+       ahci_put_err_ccb(ccb);
        ahci_os_sleep(100);
        ahci_pm_write(ap, target, AHCI_PMREG_SERR, -1);
        if (ahci_pm_phy_status(ap, target, &data)) {
@@ -395,7 +398,6 @@ retry:
         */
        if (--count) {
                fis[15] = 0;
-               ahci_put_ccb(ccb);
                goto retry;
        }
 
@@ -424,15 +426,6 @@ retry:
         */
        ahci_os_sleep(100);
 err:
-       /*
-        * Clean up the CCB.  If the command failed it already went through
-        * the standard timeout handling and should no longer be active.
-        */
-       if (ccb) {
-               KKASSERT((ap->ap_active & (1 << ccb->ccb_slot)) == 0);
-               ahci_put_ccb(ccb);
-       }
-
        /*
         * Clear error status so we can detect removal.
         */
@@ -443,7 +436,6 @@ err:
        }
        ahci_pwrite(ap, AHCI_PREG_SERR, -1);
 
-
        at->at_probe = error ? ATA_PROBE_FAILED : ATA_PROBE_NEED_IDENT;
        return (error);
 }