linux-kernel - [PATCH #upstream] ahci: start engine only during soft/hard resets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110722094126.GI2622@htj.dyndns.org>
Date:	Fri, 22 Jul 2011 11:41:26 +0200
From:	Tejun Heo <tj@...nel.org>
To:	Jeff Garzik <jgarzik@...ox.com>,
	Brian Norris <computersforpeace@...il.com>
Cc:	linux-ide@...r.kernel.org, Valdis.Kletnieks@...edu,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Michael Leun <lkml20100708@...ton.leun.net>,
	linux-kernel@...r.kernel.org, Jian Peng <jipeng2005@...il.com>,
	Kevin Cernekee <cernekee@...il.com>
Subject: [PATCH #upstream] ahci: start engine only during soft/hard resets

This is another attempt at fixing the same problem that 270dac35c2
(libata: ahci_start_engine compliant to AHCI spec) tried to solve.
Unfortunately, 270dac35c2 created regressions for a lot more common
controllers and got reverted.

This specific AHCI IP block becomes a brick if the DMA engine is
started while DRQ is set.  It is not possible to avoid the condition
completely but the most common occurrence is caused by spurious use of
ahci_start_engine() from ahci_start_port() during init sequence.

DMA engine is started after both soft and hard resets and
ahci_start_port() is always followed by resets, so there is no reason
to start DMA engine from ahci_start_port().

This patch removes ahci_start_engine() invocation from
ahci_start_port().  This change makes failure path of
ahci_port_suspend() leave engine stopped without following resets.
This is resolved by replacing ahci_start_port() call with
ata_port_freeze() which forces resets afterwards, which is the better
behavior anyway.

Signed-off-by: Tejun Heo <tj@...nel.org>
Reported-by: Brian Norris <computersforpeace@...il.com>
Reported-by: Jian Peng <jipeng2005@...il.com>
---
Jeff, this needs to be tested for a while in linux-next before sending
it mainline.  Please apply it for 3.1 merge window and let's see
whether anything explodes.

Thanks.

 drivers/ata/libahci.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 41223c7..d57f4cf 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -745,9 +745,6 @@ static void ahci_start_port(struct ata_port *ap)
 	/* enable FIS reception */
 	ahci_start_fis_rx(ap);

-	/* enable DMA */
-	ahci_start_engine(ap);
-
 	/* turn on LEDs */
 	if (ap->flags & ATA_FLAG_EM) {
 		ata_for_each_link(link, ap, EDGE) {
@@ -1976,7 +1973,7 @@ static int ahci_port_suspend(struct ata_port *ap, pm_message_t mesg)
 		ahci_power_down(ap);
 	else {
 		ata_port_printk(ap, KERN_ERR, "%s (%d)\n", emsg, rc);
-		ahci_start_port(ap);
+		ata_port_freeze(ap);
 	}

 	return rc;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/