From: Simon Schubert <corecode@dragonflybsd.org>
Date: Thu, 29 Jan 2009 09:34:03 +0000 (+0100)
Subject: nata: process interrupt even if busmaster active bit is set
X-Git-Tag: v2.3.0~9
X-Git-Url: https://gitweb.dragonflybsd.org/dragonfly.git/commitdiff_plain/9f2212d66176922cab4310c64dc12b991018c2f5

nata: process interrupt even if busmaster active bit is set

The nata driver previously assumed that an interrupt which has the
interrupt and active bits set in the busmaster status register is not a
valid/completed interrupt.  In these situations if would return without
acknowledging the irq, leading to an interrupt livelock situation.

This condition of INTERRUPT|ACTIVE occured on two different machines
both equipped with different versions of an ICH SATA controller, when
accessing an attached SATA ATAPI (DVD) drive by TSScorp (Samsung
OEM/bulk).  Specifically, natapicam seems to trigger this condition on
attach.  It seems that the drive is unhappy with the INQUIRY command
sent by natapicam.  Related artifacts appear when trying to access a
video DVD (possibly due to missing region/css key).

Under the hood it seems that the drive reports an error condition
(visible in the ALT/STATUS register), but does not clear the ACTIVE bit
of the busmaster status.  As a result the nata driver could be waiting
for the completion of the transfer, while the SATA busmaster would keep
the interrupt line asserted.

We work around this issue by following the precedent of other operating
systems and ignoring the ACTIVE bit if the INTERRUPT bit is set.
Specifically, we wait a little bit to accomodate for possibly faulty
hardware which might assert the irq before the transfer is completely
finished.  Nevertheless, we do not reject interrupt proecessing if the
INTERRUPT bit is set in the busmaster status register.
---

diff --git a/sys/dev/disk/nata/ata-pci.c b/sys/dev/disk/nata/ata-pci.c
index cd7a5cd265..b0d8d43772 100644
--- a/sys/dev/disk/nata/ata-pci.c
+++ b/sys/dev/disk/nata/ata-pci.c
@@ -471,8 +471,22 @@ ata_pci_status(device_t dev)
 		    (ch->dma->flags & ATA_DMA_ACTIVE))) {
 	int bmstat = ATA_IDX_INB(ch, ATA_BMSTAT_PORT) & ATA_BMSTAT_MASK;
 
-	if ((bmstat & (ATA_BMSTAT_ACTIVE | ATA_BMSTAT_INTERRUPT)) !=
-	    ATA_BMSTAT_INTERRUPT)
+	/*
+	 * Strictly speaking the DMA engine should already be stopped
+	 * once we receive the interrupt.
+	 * However at least ICH controllers seem to have the habbit
+	 * of not clearing the active bit even though the interrupt
+	 * is valid.
+	 * To make sure we wait a little bit (to make sure that other
+	 * buggy systems actually have a chance of finishing their
+	 * DMA transaction) and then ignore the active bit.
+	 */
+	if ((bmstat & (ATA_BMSTAT_ACTIVE | ATA_BMSTAT_INTERRUPT)) ==
+		(ATA_BMSTAT_ACTIVE | ATA_BMSTAT_INTERRUPT)) {
+	    DELAY(100);
+	    bmstat = ATA_IDX_INB(ch, ATA_BMSTAT_PORT) & ATA_BMSTAT_MASK;
+	}
+	if ((bmstat & ATA_BMSTAT_INTERRUPT) == 0)
 	    return 0;
 	ATA_IDX_OUTB(ch, ATA_BMSTAT_PORT, bmstat & ~ATA_BMSTAT_ERROR);
 	DELAY(1);