sata_sil24: prevent hba lockup when pass-through ATA commands are used

PATCH In review Gwendal Grignou 3 participants 5 messages Jan 1, 2008 – Jan 10, 2008 libata

Share & tools

Gwendal Grignou

author

2008-01-01 16:51 UTC #0 lore raw

PATCH 1/1 sata_sil24: prevent hba lockup when pass-through ATA commands are used

Fix commands timeout with Sil3124/3132 based HBA when pass-through ATA commands [where ATA_QCFLAG_RESULT_TF is set] are used while other commands are active on other devices
connected to the same port with a Port Multiplier.
Due to a hardware bug, these commands must be sent alone, like ATAPI commands.

---
 drivers/ata/sata_sil24.c |   36 ++++++++++++++++++++++++++----------
 1 files changed, 26 insertions(+), 10 deletions(-)

drivers/ata/sata_sil24.c +25 ~~-10~~

index 96fd526..56cddc7 100644
@@ -832,16 +832,32 @@ static int sil24_qc_defer(struct ata_que
 	struct ata_link *link = qc->dev->link;
 	struct ata_port *ap = link->ap;
 	u8 prot = qc->tf.protocol;
-	int is_atapi = (prot == ATA_PROT_ATAPI ||
-			prot == ATA_PROT_ATAPI_NODATA ||
-			prot == ATA_PROT_ATAPI_DMA);
-
-	/* ATAPI commands completing with CHECK_SENSE cause various
-	 * weird problems if other commands are active.  PMP DMA CS
-	 * errata doesn't cover all and HSM violation occurs even with
-	 * only one other device active.  Always run an ATAPI command
-	 * by itself.
+
+	/*
+	 * There is a bug in the chip:
+	 * Port LRAM Causes the PRB/SGT Data to be Corrupted
+	 * If the host issues a read request for LRAM and SActive registers
+	 * while active commands are available in the port, PRB/SGT data in
+	 * the LRAM can become corrupted. This issue applies only when
+	 * reading from, but not writing to, the LRAM.
+	 *
+	 * Therefore, reading LRAM when there is no particular error [and
+	 * other commands may be outstanding] is prohibited.
+	 *
+	 * To avoid this bug there are two situations where a command must run
+	 * exclusive of any other commands on the port:
+	 *
+	 * - ATAPI commands which check the sense data
+	 * - Passthrough ATA commands which always have ATA_QCFLAG_RESULT_TF
+	 *   set.
+	 *
 	 */
+	int is_excl = (prot == ATA_PROT_ATAPI ||
+		       prot == ATA_PROT_ATAPI_NODATA ||
+		       prot == ATA_PROT_ATAPI_DMA ||
+		       (qc->flags & ATA_QCFLAG_RESULT_TF));
+
 	if (unlikely(ap->excl_link)) {
 		if (link == ap->excl_link) {
 			if (ap->nr_active_links)

@@ -849,7 +865,7 @@ static int sil24_qc_defer(struct ata_que
 			qc->flags |= ATA_QCFLAG_CLEAR_EXCL;
 		} else
 			return ATA_DEFER_PORT;
-	} else if (unlikely(is_atapi)) {
+	} else if (unlikely(is_excl)) {
 		ap->excl_link = link;
 		if (ap->nr_active_links)
 			return ATA_DEFER_PORT;

Tejun Heo

Acked-by on v1 1/1

2008-01-09 02:17 UTC #1 lore raw

Quoted 5 lines · Gwendal Grignou

Fix commands timeout with Sil3124/3132 based HBA when pass-through ATA commands [where ATA_QCFLAG_RESULT_TF is set] are used while other commands are active on other devices
connected to the same port with a Port Multiplier.
Due to a hardware bug, these commands must be sent alone, like ATAPI commands.

Thanks a lot. You forgot S-O-B.

Acked-by Tejun Heo <[email protected]>

Jeff, please queue this for 2.6.24. Gwendal, IIRC you guys use lots of PMPs on your server farms, right? How is the current code working there? Do you guys see any detection problems?

Signature · 1 line

tejun

Jeff Garzik

2008-01-10 04:54 UTC #2 lore raw

Quoted 7 lines · Tejun Heo

Quoted 5 lines · Gwendal Grignou
Fix commands timeout with Sil3124/3132 based HBA when pass-through ATA commands [where ATA_QCFLAG_RESULT_TF is set] are used while other commands are active on other devices
connected to the same port with a Port Multiplier.
Due to a hardware bug, these commands must be sent alone, like ATAPI commands.

Quoted 7 lines

Thanks a lot. You forgot S-O-B.
Acked-by Tejun Heo <[email protected]>
Jeff, please queue this for 2.6.24. Gwendal, IIRC you guys use lots of PMPs on your server farms, right? How is the current code working there? Do you guys see any detection problems?

hrm, I don't see the original message in my inbox. Would one of you mind resending the original?

Jeff

Tejun Heo

2008-01-10 06:48 UTC #3 lore raw

PATCH 1/1 sata_sil24: prevent hba lockup when pass-through ATA commands are used

From: Gwendal Grignou <[email protected]>

Fix commands timeout with Sil3124/3132 based HBA when pass-through ATA commands [where ATA_QCFLAG_RESULT_TF is set] are used while other commands are active on other devices connected to the same port with a Port Multiplier. Due to a hardware bug, these commands must be sent alone, like ATAPI commands.

Signed-off-by Gwendal Grignou <[email protected]>

Acked-by Tejun Heo <[email protected]>

---
 drivers/ata/sata_sil24.c |   37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

drivers/ata/sata_sil24.c +26 ~~-11~~

index 96fd526..b4c674d 100644
@@ -832,16 +832,31 @@ static int sil24_qc_defer(struct ata_queued_cmd *qc)
 	struct ata_link *link = qc->dev->link;
 	struct ata_port *ap = link->ap;
 	u8 prot = qc->tf.protocol;
-	int is_atapi = (prot == ATA_PROT_ATAPI ||
-			prot == ATA_PROT_ATAPI_NODATA ||
-			prot == ATA_PROT_ATAPI_DMA);
-
-	/* ATAPI commands completing with CHECK_SENSE cause various
-	 * weird problems if other commands are active.  PMP DMA CS
-	 * errata doesn't cover all and HSM violation occurs even with
-	 * only one other device active.  Always run an ATAPI command
-	 * by itself.
-	 */
+
+	/*
+	 * There is a bug in the chip:
+	 * Port LRAM Causes the PRB/SGT Data to be Corrupted
+	 * If the host issues a read request for LRAM and SActive registers
+	 * while active commands are available in the port, PRB/SGT data in
+	 * the LRAM can become corrupted. This issue applies only when
+	 * reading from, but not writing to, the LRAM.
+	 *
+	 * Therefore, reading LRAM when there is no particular error [and
+	 * other commands may be outstanding] is prohibited.
+	 *
+	 * To avoid this bug there are two situations where a command must run
+	 * exclusive of any other commands on the port:
+	 *
+	 * - ATAPI commands which check the sense data
+	 * - Passthrough ATA commands which always have ATA_QCFLAG_RESULT_TF
+	 *   set.
+	 *
+ 	 */
+	int is_excl = (prot == ATA_PROT_ATAPI ||
+		       prot == ATA_PROT_ATAPI_NODATA ||
+		       prot == ATA_PROT_ATAPI_DMA ||
+		       (qc->flags & ATA_QCFLAG_RESULT_TF));
+
 	if (unlikely(ap->excl_link)) {
 		if (link == ap->excl_link) {
 			if (ap->nr_active_links)

@@ -849,7 +864,7 @@ static int sil24_qc_defer(struct ata_queued_cmd *qc)
 			qc->flags |= ATA_QCFLAG_CLEAR_EXCL;
 		} else
 			return ATA_DEFER_PORT;
-	} else if (unlikely(is_atapi)) {
+	} else if (unlikely(is_excl)) {
 		ap->excl_link = link;
 		if (ap->nr_active_links)
 			return ATA_DEFER_PORT;

Jeff Garzik

Applied

2008-01-10 21:56 UTC #4 lore raw

Quoted 8 lines · Tejun Heo

From: Gwendal Grignou <[email protected]>
Fix commands timeout with Sil3124/3132 based HBA when pass-through ATA commands [where ATA_QCFLAG_RESULT_TF is set] are used while other commands are active on other devices connected to the same port with a Port Multiplier. Due to a hardware bug, these commands must be sent alone, like ATAPI commands.

Quoted 5 lines

Signed-off-by Gwendal Grignou <[email protected]>
Acked-by Tejun Heo <[email protected]>
---
 drivers/ata/sata_sil24.c |   37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

applied