[PATCH] Bogus LBA48 drives

[PATCH] Bogus LBA48 drives

[Geert Uytterhoeven]

Apparently some IDE drives (e.g. a pile of 80 GB ST380020ACE drives I have
access to) advertise to support LBA48, but don't, causing kernels that support
LBA48 (i.e. anything newer than 2.4.18, including 2.4.25 and 2.6.4) to fail on
them.  Older kernels (including 2.2.20 on the Debian woody CDs) work fine.

One problem with those drives is that the lba_capacity_2 field in their drive
identification is set to 0, making the IDE driver think the disk is 0 bytes
large. At first I tried modifying the driver to use lba_capacity if
lba_capacity_2 is set to 0, but this caused disk errors. So it looks like those
drives don't support the increased transfer size of LBA48 neither.

I added a workaround for these drives to both 2.4.25 and 2.6.4. I'll send
patches in follow-up emails.

BTW, this problem (incl. a small patch to fix it for 2.4.19, which doesn't work
on 2.4.25 anymore) was reported a while ago by JunHyeok Heo, cfr.
http://www.cs.helsinki.fi/linux/linux-kernel/2002-42/0312.html

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] Bogus LBA48 drives

[Geert Uytterhoeven]

On Tue, 30 Mar 2004, Geert Uytterhoeven wrote:
> Apparently some IDE drives (e.g. a pile of 80 GB ST380020ACE drives I have
> access to) advertise to support LBA48, but don't, causing kernels that support
> LBA48 (i.e. anything newer than 2.4.18, including 2.4.25 and 2.6.4) to fail on
> them.  Older kernels (including 2.2.20 on the Debian woody CDs) work fine.
>
> One problem with those drives is that the lba_capacity_2 field in their drive
> identification is set to 0, making the IDE driver think the disk is 0 bytes
> large. At first I tried modifying the driver to use lba_capacity if
> lba_capacity_2 is set to 0, but this caused disk errors. So it looks like those
> drives don't support the increased transfer size of LBA48 neither.
>
> I added a workaround for these drives to both 2.4.25 and 2.6.4. I'll send
> patches in follow-up emails.

Patch for 2.4.25 (and 2.4.26-rc1):
  - Remove useless check for LBA48 from lba_capacity_is_ok(), which is called
    for non-LBA48 drives only
  - Add idedisk_supports_lba48() (cfr. 2.6) and check for lba_capacity_2 being
    non-zero
  - Use idedisk_supports_lba48() to check for LBA48 at various places

Caveat: originally I made the patch for the Debianized 2.4.25, and I had to
modify it a bit since the Debian kernel changed some logic.

--- linux-2.4.25/drivers/ide/ide-disk.c.orig	2003-10-23 13:35:27.000000000 +0200
+++ linux-2.4.25/drivers/ide/ide-disk.c	2004-03-30 17:21:07.000000000 +0200
@@ -105,11 +105,6 @@
 {
 	unsigned long lba_sects, chs_sects, head, tail;

-	if ((id->command_set_2 & 0x0400) && (id->cfs_enable_2 & 0x0400)) {
-		printk("48-bit Drive: %llu \n", id->lba_capacity_2);
-		return 1;
-	}
-
 	/*
 	 * The ATA spec tells large drives to return
 	 * C/H/S = 16383/16/63 independent of their size.
@@ -142,6 +137,12 @@
 	return 0;	/* lba_capacity value may be bad */
 }

+static inline int idedisk_supports_lba48(const ide_drive_t *drive)
+{
+	return (drive->id->command_set_2 & 0x0400) &&
+	       (drive->id->cfs_enable_2 & 0x0400) && drive->id->lba_capacity_2;
+}
+
 #ifndef CONFIG_IDE_TASKFILE_IO

 /*
@@ -560,7 +561,7 @@

 static task_ioreg_t get_command (ide_drive_t *drive, int cmd)
 {
-	int lba48bit = (drive->id->cfs_enable_2 & 0x0400) ? 1 : 0;
+	int lba48bit = idedisk_supports_lba48(drive);

 #if 1
 	lba48bit = (drive->addressing == 1) ? 1 : 0;
@@ -1170,7 +1171,7 @@

 	(void) idedisk_supports_host_protected_area(drive);

-	if (id->cfs_enable_2 & 0x0400) {
+	if (idedisk_supports_lba48(drive)) {
 		capacity_2 = id->lba_capacity_2;
 		drive->head		= drive->bios_head = 255;
 		drive->sect		= drive->bios_sect = 63;
@@ -1222,7 +1223,7 @@

 	drive->capacity = capacity;

-	if ((id->command_set_2 & 0x0400) && (id->cfs_enable_2 & 0x0400)) {
+	if (idedisk_supports_lba48(drive)) {
 		drive->capacity48 = id->lba_capacity_2;
 		drive->head = 255;
 		drive->sect = 63;
@@ -1241,7 +1242,7 @@

 static unsigned long idedisk_capacity (ide_drive_t *drive)
 {
-	if (drive->id->cfs_enable_2 & 0x0400)
+	if (idedisk_supports_lba48(drive))
 		return (drive->capacity48 - drive->sect0);
 	return (drive->capacity - drive->sect0);
 }
@@ -1567,7 +1568,7 @@
 	if (HWIF(drive)->addressing)
 		return 0;

-	if (!(drive->id->cfs_enable_2 & 0x0400))
+	if (!idedisk_supports_lba48(drive))
                 return -EIO;
 	drive->addressing = arg;
 	return 0;

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] Bogus LBA48 drives

[Geert Uytterhoeven]

On Tue, 30 Mar 2004, Geert Uytterhoeven wrote:
> Apparently some IDE drives (e.g. a pile of 80 GB ST380020ACE drives I have
> access to) advertise to support LBA48, but don't, causing kernels that support
> LBA48 (i.e. anything newer than 2.4.18, including 2.4.25 and 2.6.4) to fail on
> them.  Older kernels (including 2.2.20 on the Debian woody CDs) work fine.
>
> One problem with those drives is that the lba_capacity_2 field in their drive
> identification is set to 0, making the IDE driver think the disk is 0 bytes
> large. At first I tried modifying the driver to use lba_capacity if
> lba_capacity_2 is set to 0, but this caused disk errors. So it looks like those
> drives don't support the increased transfer size of LBA48 neither.
>
> I added a workaround for these drives to both 2.4.25 and 2.6.4. I'll send
> patches in follow-up emails.

Patch for 2.6.4 (and 2.6.5-rc3):
  - Check for lba_capacity_2 being non-zero in idedisk_supports_lba48()

--- linux-2.6.4/drivers/ide/ide-disk.c.orig	2004-03-12 12:02:53.000000000 +0100
+++ linux-2.6.4/drivers/ide/ide-disk.c	2004-03-26 13:54:39.000000000 +0100
@@ -1058,7 +1058,8 @@
  */
 static inline int idedisk_supports_lba48(const struct hd_driveid *id)
 {
-	return (id->command_set_2 & 0x0400) && (id->cfs_enable_2 & 0x0400);
+	return (id->command_set_2 & 0x0400) && (id->cfs_enable_2 & 0x0400)
+	       && id->lba_capacity_2;
 }

 static inline void idedisk_check_hpa(ide_drive_t *drive)

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] Bogus LBA48 drives

[Bartlomiej Zolnierkiewicz]

On Tuesday 30 of March 2004 17:22, Geert Uytterhoeven wrote:
> Apparently some IDE drives (e.g. a pile of 80 GB ST380020ACE drives I have
> access to) advertise to support LBA48, but don't, causing kernels that
> support LBA48 (i.e. anything newer than 2.4.18, including 2.4.25 and 2.6.4)
> to fail on them.  Older kernels (including 2.2.20 on the Debian woody CDs)
> work fine.
>
> One problem with those drives is that the lba_capacity_2 field in their
> drive identification is set to 0, making the IDE driver think the disk is 0
> bytes large. At first I tried modifying the driver to use lba_capacity if
> lba_capacity_2 is set to 0, but this caused disk errors. So it looks like
> those drives don't support the increased transfer size of LBA48 neither.

I think somebody should make Seagate aware of the issue.

> I added a workaround for these drives to both 2.4.25 and 2.6.4. I'll send
> patches in follow-up emails.

They look okay but some comment about this issue would be useful.

Thanks,
Bartlomiej

Re: [PATCH] Bogus LBA48 drives

[Andre Hedrick]

Actually this issue an errata correction in ATA-6 and changed in ATA-7.

48-bit command set support is different than capacity.

A fix that address the erratium is prefered, just need to take some time
to read it.

Cheers,

Andre Hedrick
LAD Storage Consulting Group

On Tue, 30 Mar 2004, Geert Uytterhoeven wrote:

> 
> Apparently some IDE drives (e.g. a pile of 80 GB ST380020ACE drives I have
> access to) advertise to support LBA48, but don't, causing kernels that support
> LBA48 (i.e. anything newer than 2.4.18, including 2.4.25 and 2.6.4) to fail on
> them.  Older kernels (including 2.2.20 on the Debian woody CDs) work fine.
> 
> One problem with those drives is that the lba_capacity_2 field in their drive
> identification is set to 0, making the IDE driver think the disk is 0 bytes
> large. At first I tried modifying the driver to use lba_capacity if
> lba_capacity_2 is set to 0, but this caused disk errors. So it looks like those
> drives don't support the increased transfer size of LBA48 neither.
> 
> I added a workaround for these drives to both 2.4.25 and 2.6.4. I'll send
> patches in follow-up emails.
> 
> BTW, this problem (incl. a small patch to fix it for 2.4.19, which doesn't work
> on 2.4.25 anymore) was reported a while ago by JunHyeok Heo, cfr.
> http://www.cs.helsinki.fi/linux/linux-kernel/2002-42/0312.html
> 
> Gr{oetje,eeting}s,
> 
> 						Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> 							    -- Linus Torvalds
> 

Re: [PATCH] Bogus LBA48 drives

[Jeff Garzik]

Andre Hedrick wrote:
> Actually this issue an errata correction in ATA-6 and changed in ATA-7.
> 
> 48-bit command set support is different than capacity.
> 
> A fix that address the erratium is prefered, just need to take some time
> to read it.

Got a URL?

I only see an ATA-6 errata that talks about Packet DMA...

	Jeff

Re: [PATCH] Bogus LBA48 drives

[Andries Brouwer]

Geert Uytterhoeven wrote:

>> Apparently some IDE drives (e.g. a pile of 80 GB ST380020ACE drives I have
>> access to) advertise to support LBA48, but don't.
>> One problem with those drives is that the lba_capacity_2 field in their drive
>> identification is set to 0.


Andre Hedrick replies:

> Actually this issue an errata correction in ATA-6 and changed in ATA-7.
> 
> 48-bit command set support is different than capacity.


Hmm. I read in my copy of ATA7:

 6.16.55 Words (103:100): Maximum user LBA for 48-bit Address feature set
 Words (103:100) contain a value that is one greater than the maximum LBA
 in user accessable space when the 48-bit Addressing feature set is supported.
 The maximum value that shall be placed in this field is 0000FFFFFFFFFFFFh.
 Support of these words is mandatory if the 48-bit Address feature set is supported.

Do you read differently?

Re: [PATCH] Bogus LBA48 drives

[Jeff Garzik]

Andries Brouwer wrote:
> Hmm. I read in my copy of ATA7:
> 
>  6.16.55 Words (103:100): Maximum user LBA for 48-bit Address feature set
>  Words (103:100) contain a value that is one greater than the maximum LBA
>  in user accessable space when the 48-bit Addressing feature set is supported.
>  The maximum value that shall be placed in this field is 0000FFFFFFFFFFFFh.
>  Support of these words is mandatory if the 48-bit Address feature set is supported.
> 
> Do you read differently?

The errata is, one needs to check that field for zero, and use the other 
one if so...

	Jeff

Re: [PATCH] Bogus LBA48 drives

[Geert Uytterhoeven]

On Wed, 31 Mar 2004, Jeff Garzik wrote:
> Andries Brouwer wrote:
> > Hmm. I read in my copy of ATA7:
> >
> >  6.16.55 Words (103:100): Maximum user LBA for 48-bit Address feature set
> >  Words (103:100) contain a value that is one greater than the maximum LBA
> >  in user accessable space when the 48-bit Addressing feature set is supported.
> >  The maximum value that shall be placed in this field is 0000FFFFFFFFFFFFh.
> >  Support of these words is mandatory if the 48-bit Address feature set is supported.
> >
> > Do you read differently?
>
> The errata is, one needs to check that field for zero, and use the other
> one if so...

Which is not sufficient for `my' drives, since I get disk errors if I just use
the other capacity field and don't disable LBA48 completely.

I'll check the ATA specs myself, if I find some time...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] Bogus LBA48 drives

[Jeff Garzik]

Geert Uytterhoeven wrote:
> On Wed, 31 Mar 2004, Jeff Garzik wrote:
> 
>>Andries Brouwer wrote:
>>
>>>Hmm. I read in my copy of ATA7:
>>>
>>> 6.16.55 Words (103:100): Maximum user LBA for 48-bit Address feature set
>>> Words (103:100) contain a value that is one greater than the maximum LBA
>>> in user accessable space when the 48-bit Addressing feature set is supported.
>>> The maximum value that shall be placed in this field is 0000FFFFFFFFFFFFh.
>>> Support of these words is mandatory if the 48-bit Address feature set is supported.
>>>
>>>Do you read differently?
>>
>>The errata is, one needs to check that field for zero, and use the other
>>one if so...
> 
> 
> Which is not sufficient for `my' drives, since I get disk errors if I just use
> the other capacity field and don't disable LBA48 completely.
> 
> I'll check the ATA specs myself, if I find some time...

If it's reporting the "48-bit feature set supported" but doesn't really 
support it, I'd vote for broken drive :)  Maybe check for a firmware 
update on the manufacturer's web site?

	Jeff