AHCI - Fix interrupt enablement sequencing
authorMatthew Dillon <dillon@apollo.backplane.com>
Wed, 19 Aug 2009 17:10:41 +0000 (10:10 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Wed, 19 Aug 2009 17:10:41 +0000 (10:10 -0700)
* Interrupt enablement could race port initialization, causing the AHCI
  probe to fail and/or assert.

  Wait for basic port hardware initialization to complete (which is done by
  the port helper threads) before enabling interrupt processing on the chip.

Testing-by: Alexander Polakov <polachok@gmail.com>
sys/dev/disk/ahci/ahci.c
sys/dev/disk/ahci/ahci.h
sys/dev/disk/ahci/ahci_attach.c
sys/dev/disk/ahci/ahci_dragonfly.c

index 5741831..68cdabc 100644 (file)
@@ -443,10 +443,10 @@ nomem:
        ap->ap_intmask = data;
 
        /*
-        * Start the port.  The helper thread will call ahci_port_init()
-        * so the ports can all be started in parallel.  A failure by
-        * ahci_port_init() does not deallocate the port since we still
-        * want hot-plug events.
+        * Start the port helper thread.  The helper thread will call
+        * ahci_port_init() so the ports can all be started in parallel.
+        * A failure by ahci_port_init() does not deallocate the port
+        * since we still want hot-plug events.
         */
        ahci_os_start_port(ap);
        return(0);
@@ -477,6 +477,15 @@ ahci_port_init(struct ahci_port *ap)
                ahci_pwrite(ap, AHCI_PREG_SNTF, -1);
        ap->ap_probe = ATA_PROBE_NEED_HARD_RESET;
        ap->ap_pmcount = 0;
+
+       /*
+        * Cycle the port before enabling its interrupt.  This makes sure
+        * that the CI and SACT registers are clear.  It might not be
+        * necesary now that we sequence the interrupt enablement properly
+        * but I'm keeping it in.
+        */
+       ahci_port_start(ap);
+       ahci_port_stop(ap, 0);
        ahci_port_interrupt_enable(ap);
        return (0);
 }
index dd603a9..7d64a1a 100644 (file)
@@ -386,6 +386,7 @@ struct ahci_port {
 #define AP_SIGF_INIT           0x0001
 #define AP_SIGF_TIMEOUT                0x0002
 #define AP_SIGF_PORTINT                0x0004
+#define AP_SIGF_THREAD_SYNC    0x0008
 #define AP_SIGF_STOP           0x8000
        struct cam_sim          *ap_sim;
 
index 5f49134..b999595 100644 (file)
@@ -424,6 +424,19 @@ noccc:
        }
 
        /*
+        * Before marking the sc as good, which allows the interrupt
+        * subsystem to operate on the ports, wait for all the port threads
+        * to get past their initial pre-probe init.  Otherwise an interrupt
+        * may try to process the port before it has been initialized.
+        */
+       for (i = 0; i < AHCI_MAX_PORTS; i++) {
+               if ((ap = sc->sc_ports[i]) != NULL) {
+                       while (ap->ap_signal & AP_SIGF_THREAD_SYNC)
+                               tsleep(&ap->ap_signal, 0, "ahprb1", hz);
+               }
+       }
+
+       /*
         * Master interrupt enable, and call ahci_intr() in case we race
         * our AHCI_F_INT_GOOD flag.
         */
@@ -441,7 +454,7 @@ noccc:
        for (i = 0; i < AHCI_MAX_PORTS; i++) {
                if ((ap = sc->sc_ports[i]) != NULL) {
                        while (ap->ap_signal & AP_SIGF_INIT)
-                               tsleep(&ap->ap_signal, 0, "ahprb1", hz);
+                               tsleep(&ap->ap_signal, 0, "ahprb2", hz);
                        ahci_os_lock_port(ap);
                        if (ahci_cam_attach(ap) == 0) {
                                ahci_cam_changed(ap, NULL, -1);
index 7ecc319..9aa40b9 100644 (file)
@@ -192,7 +192,7 @@ ahci_os_hardsleep(int us)
 void
 ahci_os_start_port(struct ahci_port *ap)
 {
-       atomic_set_int(&ap->ap_signal, AP_SIGF_INIT);
+       atomic_set_int(&ap->ap_signal, AP_SIGF_INIT | AP_SIGF_THREAD_SYNC);
        lockinit(&ap->ap_lock, "ahcipo", 0, 0);
        kthread_create(ahci_port_thread, ap, &ap->ap_thread,
                       "%s", PORTNAME(ap));
@@ -281,6 +281,8 @@ ahci_port_thread(void *arg)
         */
        ahci_os_lock_port(ap);
        ahci_port_init(ap);
+       atomic_clear_int(&ap->ap_signal, AP_SIGF_THREAD_SYNC);
+       wakeup(&ap->ap_signal);
        ahci_port_state_machine(ap, 1);
        ahci_os_unlock_port(ap);
        atomic_clear_int(&ap->ap_signal, AP_SIGF_INIT);