Success(?) with SiI + Seagate pessimistic fix disabled.

Success(?) with SiI + Seagate pessimistic fix disabled.

[Eric Wong]

Hi, I got a generic SiI card at the computer fair over the weekend and a
pair of Seagate drives (not RAIDed in any way).  Disappointed by the
performance with the pessimistic errata fix, I took a chance and
commented it out... my computer is still running and the drives are a
lot faster :)

I have nothing important on the drives yet, but I'm copying data back
and forth between them (reiserfs 3.6) to test it.  MD5 checks don't show
any problems, either.  Let me know if you have any other ideas to test
(and try to break) my setup.

kernel: 2.6.1-rc3

lspci:
00:0e.0 RAID bus controller: CMD Technology Inc Silicon Image SiI 3112 SATARaid Controller (rev 02)
	Subsystem: CMD Technology Inc: Unknown device 6112
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66Mhz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 32, Cache Line Size: 0x08 (32 bytes)
	Interrupt: pin A routed to IRQ 10
	Region 0: I/O ports at b000 [size=8]
	Region 1: I/O ports at a800 [size=4]
	Region 2: I/O ports at a400 [size=8]
	Region 3: I/O ports at a000 [size=4]
	Region 4: I/O ports at 9800 [size=16]
	Region 5: Memory at de800000 (32-bit, non-prefetchable) [size=512]
	Expansion ROM at <unassigned> [disabled] [size=512K]
	Capabilities: [60] Power Management version 2
		Flags: PMEClk- DSI+ D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=2 PME-


dmesg:
ata1: SATA max UDMA/133 cmd 0xF8933080 ctl 0xF893308A bmdma 0xF8933000 irq 10
ata2: SATA max UDMA/133 cmd 0xF89330C0 ctl 0xF89330CA bmdma 0xF8933008 irq 10
ata1: dev 0 cfg 49:2f00 82:346b 83:7d01 84:4003 85:3469 86:3c01 87:4003 88:207f
ata1: dev 0 ATA, max UDMA/133, 312581808 sectors (lba48)
ata1: dev 0 configured for UDMA/133
scsi0 : ata_sil
ata2: dev 0 cfg 49:2f00 82:346b 83:7d01 84:4003 85:3469 86:3c01 87:4003 88:207f
ata2: dev 0 ATA, max UDMA/133, 312581808 sectors (lba48)
ata2: dev 0 configured for UDMA/133
scsi1 : ata_sil
  Vendor: ATA       Model: ST3160023AS       Rev: 0.81
  Type:   Direct-Access                      ANSI SCSI revision: 05
SCSI device sda: 312581808 512-byte hdwr sectors (160042 MB)
SCSI device sda: drive cache: write through
 sda: sda1
Attached scsi disk sda at scsi0, channel 0, id 0, lun 0
  Vendor: ATA       Model: ST3160023AS       Rev: 0.81
  Type:   Direct-Access                      ANSI SCSI revision: 05
SCSI device sdb: 312581808 512-byte hdwr sectors (160042 MB)

-- 
Eric Wong

Re: Success(?) with SiI + Seagate pessimistic fix disabled.

[Eric Wong]

Eric Wong <normalperson@yhbt.net> wrote:
<snip success story>

I'm not feeling any more pessimistic than usual at the moment, so I
might as well start a one item whitelist.  It works for me, but more
testing is always a good thing.

On a related note, I couldn't find a way to get the firmware revision
from hdparm or dmesg when using libata.

--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -65,6 +65,15 @@ static struct pci_device_id sil_pci_tbl[
 	{ }	/* terminate list */
 };
 
+
+/* TODO firmware versions should be added - eric */
+struct sil_drivelist {
+	char * product;
+} sil_whitelist [] = {
+	{ "ST3160023AS" }, /* firmware 3.05 */
+	{ 0 }
+};
+
 static struct pci_driver sil_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= sil_pci_tbl,
@@ -184,6 +194,7 @@ static void sil_scr_write (struct ata_po
  */
 static void sil_dev_config(struct ata_port *ap, struct ata_device *dev)
 {
+	int n;
 	const char *s = &dev->product[0];
 	unsigned int len = strnlen(s, sizeof(dev->product));
 
@@ -191,6 +202,12 @@ static void sil_dev_config(struct ata_po
 	while ((len > 0) && (s[len - 1] == ' '))
 		len--;
 
+	/* check against whitelist */
+	for (n = 0; sil_whitelist[n].product; n++)
+		if ( (len == strlen(sil_whitelist[n].product))
+		&& !strncmp(sil_whitelist[n].product,s,len) )
+			return;
+
 	/* limit to udma5 */
 	if (!memcmp(s, "Maxtor ", 7)) {
 		printk(KERN_INFO "ata%u(%u): applying pessimistic Maxtor errata fix\n",

-- 
Eric Wong

Re: Success(?) with SiI + Seagate pessimistic fix disabled.

[Jeff Garzik]

Eric Wong wrote:
> Eric Wong <normalperson@yhbt.net> wrote:
> <snip success story>
> 
> I'm not feeling any more pessimistic than usual at the moment, so I
> might as well start a one item whitelist.  It works for me, but more
> testing is always a good thing.
> 
> On a related note, I couldn't find a way to get the firmware revision
> from hdparm or dmesg when using libata.
> 
> --- a/drivers/scsi/sata_sil.c
> +++ b/drivers/scsi/sata_sil.c


Well, if you're motivated, there is a blacklist...

google for "mod15write" and click on the first link.

Re: Success(?) with SiI + Seagate pessimistic fix disabled.

[Eric Wong]

Jeff Garzik <jgarzik@pobox.com> wrote:
> Eric Wong wrote:
> >Eric Wong <normalperson@yhbt.net> wrote:
> 
> Well, if you're motivated, there is a blacklist...
> 
> google for "mod15write" and click on the first link.

Cool.  I found some info on the Maxtor problem as well.  The blacklist
is ready for the Maxtor fix, but I chickened out and kept the original
pessimistic fix for those drives for the time being.  Seagates should be
correct, however.

--- a/drivers/scsi/sata_sil.c	2004-01-11 22:47:39.000000000 -0800
+++ b/drivers/scsi/sata_sil.c	2004-01-12 01:56:28.231405184 -0800
@@ -37,6 +37,9 @@
 #define DRV_NAME	"ata_sil"
 #define DRV_VERSION	"0.51"
 
+#define SIL_QUIRK_MOD15WRITE	0x01
+#define SIL_QUIRK_UDMA5MAX	0x02
+
 enum {
 	sil_3112		= 0,
 
@@ -65,6 +68,28 @@ static struct pci_device_id sil_pci_tbl[
 	{ }	/* terminate list */
 };
 
+
+/* TODO firmware versions should be added - eric */
+struct sil_drivelist {
+	const char * product;
+	unsigned int quirk;
+} sil_blacklist [] = {
+	{ "ST320012AS",		SIL_QUIRK_MOD15WRITE },
+	{ "ST330013AS",		SIL_QUIRK_MOD15WRITE },
+	{ "ST340017AS",		SIL_QUIRK_MOD15WRITE },
+	{ "ST360015AS",		SIL_QUIRK_MOD15WRITE },
+	{ "ST380023AS",		SIL_QUIRK_MOD15WRITE },
+	{ "ST3120023AS",	SIL_QUIRK_MOD15WRITE },
+	{ "ST340014ASL",	SIL_QUIRK_MOD15WRITE },
+	{ "ST360014ASL",	SIL_QUIRK_MOD15WRITE },
+	{ "ST380011ASL",	SIL_QUIRK_MOD15WRITE },
+	{ "ST3120022ASL",	SIL_QUIRK_MOD15WRITE },
+	{ "ST3160021ASL",	SIL_QUIRK_MOD15WRITE },
+	{ "Maxtor 4D060H3                          DAK05GK0",
+		SIL_QUIRK_UDMA5MAX },
+	{ }
+};
+
 static struct pci_driver sil_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= sil_pci_tbl,
@@ -182,16 +207,52 @@ static void sil_scr_write (struct ata_po
  *	information on these errata, I will create a more exhaustive
  *	list, and apply the fixups to only the specific
  *	devices/hosts/firmwares that need it.
+ *
+ *	20040111 - Seagate drives affected by the Mod15Write bug are blacklisted
+ *	The Maxtor quirk is in the blacklist, but I'm keeping the original
+ *	pessimistic fix for the following reasons:
+ *	- There seems to be less info on it, only one device gleaned off the
+ *	Windows	driver, maybe only one is affected.  More info would be greatly
+ *	appreciated.
+ *	- But then again UDMA5 is hardly anything to complain about
  */
 static void sil_dev_config(struct ata_port *ap, struct ata_device *dev)
 {
+	unsigned int n, quirks = 0;
+	u32 class_rev = 0;
 	const char *s = &dev->product[0];
 	unsigned int len = strnlen(s, sizeof(dev->product));
 
+	pci_read_config_dword(ap->host_set->pdev, PCI_CLASS_REVISION, &class_rev);
+	class_rev &= 0xff;
+
 	/* ATAPI specifies that empty space is blank-filled; remove blanks */
 	while ((len > 0) && (s[len - 1] == ' '))
 		len--;
 
+	for (n = 0; sil_blacklist[n].product; n++) 
+		if ( ( strlen(sil_blacklist[n].product) == len )
+		&& strncmp(sil_blacklist[n].product,s,len) )
+			quirks = sil_blacklist[n].quirk;
+	
+	/* limit requests to 15 sectors */
+	if ( (class_rev <= 0x01) && (quirks & SIL_QUIRK_MOD15WRITE) ) {
+		printk(KERN_INFO "ata%u(%u): applying Seagate errata fix\n",
+		       ap->id, dev->devno);
+		ap->host->max_sectors = 15;
+		ap->host->hostt->max_sectors = 15;
+		return;
+	}
+
+	/* limit to udma5 */
+	/* is this for (class_rev <= 0x01) only, too? */
+	if ( quirks & SIL_QUIRK_UDMA5MAX ) {
+		printk(KERN_INFO "ata%u(%u): applying Maxtor errata fix %s\n",
+		       ap->id, dev->devno, s);
+		ap->udma_mask &= ATA_UDMA5;
+		return;
+	}
+
 	/* limit to udma5 */
 	if (!memcmp(s, "Maxtor ", 7)) {
 		printk(KERN_INFO "ata%u(%u): applying pessimistic Maxtor errata fix\n",
@@ -199,18 +260,6 @@ static void sil_dev_config(struct ata_po
 		ap->udma_mask &= ATA_UDMA5;
 		return;
 	}
-
-	/* limit requests to 15 sectors */
-	if ((len > 4) && (!memcmp(s, "ST", 2))) {
-		if ((!memcmp(s + len - 2, "AS", 2)) ||
-		    (!memcmp(s + len - 3, "ASL", 3))) {
-			printk(KERN_INFO "ata%u(%u): applying pessimistic Seagate errata fix\n",
-			       ap->id, dev->devno);
-			ap->host->max_sectors = 15;
-			ap->host->hostt->max_sectors = 15;
-			return;
-		}
-	}
 }
 
 static void sil_set_piomode (struct ata_port *ap, struct ata_device *adev,

-- 
Eric Wong

Re: Success(?) with SiI + Seagate pessimistic fix disabled.

[Jeff Garzik]

[-- Attachment #1: Type: text/plain, Size: 107 bytes --]

Thanks, here's what I checked in.

Anyone with an errata'd drive, that can help test this patch?

	Jeff




[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 4198 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1510  -> 1.1511 
#	drivers/scsi/sata_sil.c	1.5     -> 1.6    
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 04/01/14	normalperson@yhbt.net	1.1511
# [libata sata_sil] cleaner, better version of errata workarounds
# 
# No longer unfairly punishes non-errata Seagate and Maxtor drives.
# --------------------------------------------
#
diff -Nru a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
--- a/drivers/scsi/sata_sil.c	Wed Jan 14 18:19:53 2004
+++ b/drivers/scsi/sata_sil.c	Wed Jan 14 18:19:53 2004
@@ -64,6 +64,9 @@
 	SIL_IDE3_CTL		= 0x2CA,
 	SIL_IDE3_BMDMA		= 0x208,
 	SIL_IDE3_SCR		= 0x380,
+
+	SIL_QUIRK_MOD15WRITE	= (1 << 0),
+	SIL_QUIRK_UDMA5MAX	= (1 << 1),
 };
 
 static void sil_set_piomode (struct ata_port *ap, struct ata_device *adev,
@@ -82,6 +85,27 @@
 	{ }	/* terminate list */
 };
 
+
+/* TODO firmware versions should be added - eric */
+struct sil_drivelist {
+	const char * product;
+	unsigned int quirk;
+} sil_blacklist [] = {
+	{ "ST320012AS",		SIL_QUIRK_MOD15WRITE },
+	{ "ST330013AS",		SIL_QUIRK_MOD15WRITE },
+	{ "ST340017AS",		SIL_QUIRK_MOD15WRITE },
+	{ "ST360015AS",		SIL_QUIRK_MOD15WRITE },
+	{ "ST380023AS",		SIL_QUIRK_MOD15WRITE },
+	{ "ST3120023AS",	SIL_QUIRK_MOD15WRITE },
+	{ "ST340014ASL",	SIL_QUIRK_MOD15WRITE },
+	{ "ST360014ASL",	SIL_QUIRK_MOD15WRITE },
+	{ "ST380011ASL",	SIL_QUIRK_MOD15WRITE },
+	{ "ST3120022ASL",	SIL_QUIRK_MOD15WRITE },
+	{ "ST3160021ASL",	SIL_QUIRK_MOD15WRITE },
+	{ "Maxtor 4D060H3",	SIL_QUIRK_UDMA5MAX },
+	{ }
+};
+
 static struct pci_driver sil_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= sil_pci_tbl,
@@ -207,34 +231,52 @@
  *	information on these errata, I will create a more exhaustive
  *	list, and apply the fixups to only the specific
  *	devices/hosts/firmwares that need it.
+ *
+ *	20040111 - Seagate drives affected by the Mod15Write bug are blacklisted
+ *	The Maxtor quirk is in the blacklist, but I'm keeping the original
+ *	pessimistic fix for the following reasons:
+ *	- There seems to be less info on it, only one device gleaned off the
+ *	Windows	driver, maybe only one is affected.  More info would be greatly
+ *	appreciated.
+ *	- But then again UDMA5 is hardly anything to complain about
  */
 static void sil_dev_config(struct ata_port *ap, struct ata_device *dev)
 {
+	unsigned int n, quirks = 0;
+	u32 class_rev = 0;
 	const char *s = &dev->product[0];
 	unsigned int len = strnlen(s, sizeof(dev->product));
 
+	pci_read_config_dword(ap->host_set->pdev, PCI_CLASS_REVISION, &class_rev);
+	class_rev &= 0xff;
+
 	/* ATAPI specifies that empty space is blank-filled; remove blanks */
 	while ((len > 0) && (s[len - 1] == ' '))
 		len--;
 
-	/* limit to udma5 */
-	if (!memcmp(s, "Maxtor ", 7)) {
-		printk(KERN_INFO "ata%u(%u): applying pessimistic Maxtor errata fix\n",
+	for (n = 0; sil_blacklist[n].product; n++) 
+		if (!memcmp(sil_blacklist[n].product, s,
+			    strlen(sil_blacklist[n].product))) {
+			quirks = sil_blacklist[n].quirk;
+			break;
+		}
+	
+	/* limit requests to 15 sectors */
+	if ((class_rev <= 0x01) && (quirks & SIL_QUIRK_MOD15WRITE)) {
+		printk(KERN_INFO "ata%u(%u): applying Seagate errata fix\n",
 		       ap->id, dev->devno);
-		ap->udma_mask &= ATA_UDMA5;
+		ap->host->max_sectors = 15;
+		ap->host->hostt->max_sectors = 15;
 		return;
 	}
 
-	/* limit requests to 15 sectors */
-	if ((len > 4) && (!memcmp(s, "ST", 2))) {
-		if ((!memcmp(s + len - 2, "AS", 2)) ||
-		    (!memcmp(s + len - 3, "ASL", 3))) {
-			printk(KERN_INFO "ata%u(%u): applying pessimistic Seagate errata fix\n",
-			       ap->id, dev->devno);
-			ap->host->max_sectors = 15;
-			ap->host->hostt->max_sectors = 15;
-			return;
-		}
+	/* limit to udma5 */
+	/* is this for (class_rev <= 0x01) only, too? */
+	if (quirks & SIL_QUIRK_UDMA5MAX) {
+		printk(KERN_INFO "ata%u(%u): applying Maxtor errata fix %s\n",
+		       ap->id, dev->devno, s);
+		ap->udma_mask &= ATA_UDMA5;
+		return;
 	}
 }
 

Success(?) with SiI + Seagate pessimistic fix disabled.

[Eric Wong]

Hi, I got a generic SiI card at the computer fair over the weekend and a
pair of Seagate drives (not RAIDed in any way).  Disappointed by the
performance with the pessimistic errata fix, I took a chance and
commented it out, my computer is still running and the drives are a lot
faster :)

I have nothing important on the drives yet, but I'm copying data back
and forth between them (reiserfs 3.6) to test it.  MD5 checks don't show
any problems.  Let me know if you have any other ideas to test (and try
to break) my setup.

lspci:
00:0e.0 RAID bus controller: CMD Technology Inc Silicon Image SiI 3112 SATARaid Controller (rev 02)
	Subsystem: CMD Technology Inc: Unknown device 6112
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66Mhz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 32, Cache Line Size: 0x08 (32 bytes)
	Interrupt: pin A routed to IRQ 10
	Region 0: I/O ports at b000 [size=8]
	Region 1: I/O ports at a800 [size=4]
	Region 2: I/O ports at a400 [size=8]
	Region 3: I/O ports at a000 [size=4]
	Region 4: I/O ports at 9800 [size=16]
	Region 5: Memory at de800000 (32-bit, non-prefetchable) [size=512]
	Expansion ROM at <unassigned> [disabled] [size=512K]
	Capabilities: [60] Power Management version 2
		Flags: PMEClk- DSI+ D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=2 PME-


dmesg:
ata1: SATA max UDMA/133 cmd 0xF8933080 ctl 0xF893308A bmdma 0xF8933000 irq 10
ata2: SATA max UDMA/133 cmd 0xF89330C0 ctl 0xF89330CA bmdma 0xF8933008 irq 10
ata1: dev 0 cfg 49:2f00 82:346b 83:7d01 84:4003 85:3469 86:3c01 87:4003 88:207f
ata1: dev 0 ATA, max UDMA/133, 312581808 sectors (lba48)
ata1: dev 0 configured for UDMA/133
scsi0 : ata_sil
ata2: dev 0 cfg 49:2f00 82:346b 83:7d01 84:4003 85:3469 86:3c01 87:4003 88:207f
ata2: dev 0 ATA, max UDMA/133, 312581808 sectors (lba48)
ata2: dev 0 configured for UDMA/133
scsi1 : ata_sil
  Vendor: ATA       Model: ST3160023AS       Rev: 0.81
  Type:   Direct-Access                      ANSI SCSI revision: 05
SCSI device sda: 312581808 512-byte hdwr sectors (160042 MB)
SCSI device sda: drive cache: write through
 sda: sda1
Attached scsi disk sda at scsi0, channel 0, id 0, lun 0
  Vendor: ATA       Model: ST3160023AS       Rev: 0.81
  Type:   Direct-Access                      ANSI SCSI revision: 05
SCSI device sdb: 312581808 512-byte hdwr sectors (160042 MB)

-- 
Eric Wong