ST340823A disk size issue

ST340823A disk size issue

[Mikko Rapeli]

Hello, 

(More details at http://bugzilla.kernel.org/show_bug.cgi?id=8816 )

My home server disk doesn't work with latest kernels since its size is
reported or probed correctly. With 2.6.22.1 I get:

Probing IDE interface ide1...
hdd: ST340823A, ATA DISK drive
hdd: selected mode 0x42
...
hdd: max request size: 128KiB
hdd: Host Protected Area detected.
        current capacity is 78165360 sectors (40020 MB)
        native  capacity is 78165361 sectors (40020 MB)
hdd: Host Protected Area disabled.
hdd: 78165361 sectors (40020 MB) w/1024KiB Cache, CHS=65535/16/63,
UDMA(33)
hdd: cache flushes not supported
 hdd: hdd1 hdd2 hdd3
hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360, 
sector=78165360
ide: failed opcode was: unknown
hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360,
sector=78165360
ide: failed opcode was: unknown
hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360,
sector=78165360
...

And as said shows in bugzilla, badblocks, fsck and smartctl think the
drive is not broken or going to die too soon.

What's the difference between the size formats of
WIN_READ_NATIVE_MAX_EXT read by
ide-disk.c/idedisk_read_native_max_address_ext() and (hdreg.h) 
hd_driveid->lba_capacity_2 read by, if I read drivers/ide/* code
correctly, ide-probe.c/do_identify()?

If this disk is reporting one of these wrong, which fix could be
applied? 

Both disabling 'stroke' and removing addr++ in
ide-disk.c/idedisk_read_native_max_address*() fix the problem, but if
kernel code is correct and standard compliant, these fixes are out of
the question.

Perhaps a boot parameter to kernel could force the disk sector count to
some value for buggy drives?

-Mikko

Re: ST340823A disk size issue

[Alan Cox]

On Wed, 1 Aug 2007 12:29:04 +0300
Mikko Rapeli <mikko.rapeli@iki.fi> wrote:

> Hello, 
> 
> (More details at http://bugzilla.kernel.org/show_bug.cgi?id=8816 )
> 
> My home server disk doesn't work with latest kernels since its size is
> reported or probed correctly. With 2.6.22.1 I get:

You've got an odd sized disk. If you have that, old IDE, and you also have
anything which tries to read the last sector (eg GPT partitioning) it'll
break as it tries to read 1K block sizes.

Vendors normally clip the drive to an even size which seems to be the
case on your box with the HPA left alone.

It *should* all "just work" with the libata drivers but needs more
testing of odd sizes to be 100% sure

Re: ST340823A disk size issue

[Mikko Rapeli]

On Wed, Aug 01, 2007 at 02:05:56PM +0100, Alan Cox wrote:
> You've got an odd sized disk. If you have that, old IDE, and you also have
> anything which tries to read the last sector (eg GPT partitioning) it'll
> break as it tries to read 1K block sizes.
>
> Vendors normally clip the drive to an even size which seems to be the
> case on your box with the HPA left alone.

Sorry, but I don't quite get the odd size and 1k block thing.

Drive manual (
http://www.seagate.com/support/disc/manuals/ata/u5pmb01.pdf ) says on
page 12 that the drive has 78165360 sectors of size 512 bytes of which 
0 to 78165359 are addressable in LBA mode.

The drive reports its size as 78165360 and falsely, according to kernels
addr++, with HPA as 78165361. So instead of reporting the maximum
addressable sector 78165359, the drive reports its size 78165360.

Kernel now picks the larger value 78165361 as the drive size and thinks 
sector 78165360 should exist, but it doesn't. Partition code tries to
read the last sector 78165360 and fails:

hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360,
sector=78165360
ide: failed opcode was: unknown

Shouldn't the partition code also try to read 78165361 if there was a 1k
read issued?

> It *should* all "just work" with the libata drivers but needs more
> testing of odd sizes to be 100% sure

The drive just worked with libata and 2.6.22.1, but I'd like to get a
back protable fix to ide-disk.c, so distro kernels like 2.6.18 could be
made to work too.

-Mikko

Re: ST340823A disk size issue

[Bartlomiej Zolnierkiewicz]

Hi,

On Wednesday 01 August 2007, Mikko Rapeli wrote:
> Hello, 
> 
> (More details at http://bugzilla.kernel.org/show_bug.cgi?id=8816 )
> 
> My home server disk doesn't work with latest kernels since its size is
> reported or probed correctly. With 2.6.22.1 I get:
> 
> Probing IDE interface ide1...
> hdd: ST340823A, ATA DISK drive
> hdd: selected mode 0x42
> ...
> hdd: max request size: 128KiB
> hdd: Host Protected Area detected.
>         current capacity is 78165360 sectors (40020 MB)
>         native  capacity is 78165361 sectors (40020 MB)
> hdd: Host Protected Area disabled.
> hdd: 78165361 sectors (40020 MB) w/1024KiB Cache, CHS=65535/16/63,
> UDMA(33)
> hdd: cache flushes not supported
>  hdd: hdd1 hdd2 hdd3
> hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
> hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360, 
> sector=78165360
> ide: failed opcode was: unknown
> hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
> hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360,
> sector=78165360
> ide: failed opcode was: unknown
> hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
> hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360,
> sector=78165360
> ...
> 
> And as said shows in bugzilla, badblocks, fsck and smartctl think the
> drive is not broken or going to die too soon.
> 
> What's the difference between the size formats of
> WIN_READ_NATIVE_MAX_EXT read by
> ide-disk.c/idedisk_read_native_max_address_ext() and (hdreg.h) 
> hd_driveid->lba_capacity_2 read by, if I read drivers/ide/* code
> correctly, ide-probe.c/do_identify()?
> 
> If this disk is reporting one of these wrong, which fix could be
> applied? 
> 
> Both disabling 'stroke' and removing addr++ in
> ide-disk.c/idedisk_read_native_max_address*() fix the problem, but if
> kernel code is correct and standard compliant, these fixes are out of
> the question.
> 
> Perhaps a boot parameter to kernel could force the disk sector count to
> some value for buggy drives?

Could you try attached patch?

[PATCH] ide-disk: workaround for buggy HPA support on ST340823A

This disk reports total number of sectors instead of maximum sector address
in response to READ_NATIVE_MAX_ADDRESS command and also happily accepts
SET_MAX_ADDRESS command with the bogus value.  This results in +1 sector
capacity being used and errors on attempts to use the last sector.

...
hdd: Host Protected Area detected.
        current capacity is 78165360 sectors (40020 MB)
        native  capacity is 78165361 sectors (40020 MB)
hdd: Host Protected Area disabled.
...
hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360, sector=78165360
...

Add hpa_list[] table and workaround the issue in idedisk_check_hpa().

Fixes kernel bugzilla bug #8816.

Thanks to Mikko for investigating the issue and testing this patch.

Cc: Mikko Rapeli <mikko.rapeli@iki.fi>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-disk.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Index: b/drivers/ide/ide-disk.c
===================================================================
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -481,6 +481,14 @@ static inline int idedisk_supports_lba48
 	       && id->lba_capacity_2;
 }
 
+/*
+ * Some disks report total number of sectors instead of
+ * maximum sector address.  We list them here.
+ */
+static const struct drive_list_entry hpa_list[] = {
+	{ "ST340823A",	NULL },
+};
+
 static void idedisk_check_hpa(ide_drive_t *drive)
 {
 	unsigned long long capacity, set_max;
@@ -492,6 +500,15 @@ static void idedisk_check_hpa(ide_drive_
 	else
 		set_max = idedisk_read_native_max_address(drive);
 
+	if (ide_in_drive_list(drive->id, hpa_list)) {
+		/*
+		 * Since we are inclusive wrt to firmware revisions do this
+		 * extra check and apply the workaround only when needed.
+		 */
+		if (set_max == capacity + 1)
+			set_max--;
+	}
+
 	if (set_max <= capacity)
 		return;
 

Re: ST340823A disk size issue

[Mikko Rapeli]

On Wed, Aug 01, 2007 at 10:34:03PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Could you try attached patch?
> 
> [PATCH] ide-disk: workaround for buggy HPA support on ST340823A

Umh, it's getting late but some makefile magic maybe missing, or I
should do a clean build?

$ make drivers/ide/ide-disk.ko V=1 
rm -f include/config/kernel.release
echo 2.6.22.1 > include/config/kernel.release
set -e; echo '  CHK     include/linux/version.h'; mkdir -p
include/linux/;     (echo \#define LINUX_VERSION_CODE 132630; echo
'#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))';) <
/home/mcfrisk/src/linux/linux-2.6.22.1.new/Makefile >
include/linux/version.h.tmp; if [ -r include/linux/version.h ] && cmp -s
include/linux/version.h include/linux/version.h.tmp; then rm -f
include/linux/version.h.tmp; else echo '  UPD
include/linux/version.h'; mv -f include/linux/version.h.tmp
include/linux/version.h; fi
  CHK     include/linux/version.h
set -e; echo '  CHK     include/linux/utsrelease.h'; mkdir -p
include/linux/;  if [ `echo -n "2.6.22.1" | wc -c ` -gt 64 ]; then echo
'"2.6.22.1" exceeds 64 characters' >&2; exit 1; fi; (echo \#define
UTS_RELEASE \"2.6.22.1\";) < include/config/kernel.release >
include/linux/utsrelease.h.tmp; if [ -r include/linux/utsrelease.h ] &&
cmp -s include/linux/utsrelease.h include/linux/utsrelease.h.tmp; then
rm -f include/linux/utsrelease.h.tmp; else echo '  UPD
include/linux/utsrelease.h'; mv -f include/linux/utsrelease.h.tmp
include/linux/utsrelease.h; fi
  CHK     include/linux/utsrelease.h
make -f scripts/Makefile.build obj=scripts/basic
make -f scripts/Makefile.build obj=.
mkdir -p arch/i386/kernel/
make -f scripts/Makefile.build obj=. missing-syscalls
  /bin/sh scripts/checksyscalls.sh gcc -m32
-Wp,-MD,./.missing-syscalls.d  -nostdinc -isystem
/usr/lib/gcc/i486-linux-gnu/4.1.3/include -D__KERNEL__ -Iinclude
-include include/linux/autoconf.h -Wall -Wundef -Wstrict-prototypes
-Wno-trigraphs -fno-strict-aliasing -fno-common -Os -pipe -msoft-float
-mregparm=3 -freg-struct-return -mpreferred-stack-boundary=2
-march=i486 -mtune=generic -ffreestanding -maccumulate-outgoing-args
-DCONFIG_AS_CFI=1 -DCONFIG_AS_CFI_SIGNAL_FRAME=1
-Iinclude/asm-i386/mach-default -fomit-frame-pointer -g
-fno-stack-protector -Wdeclaration-after-statement -Wno-pointer-sign
-D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(missing_syscalls)"
-D"KBUILD_MODNAME=KBUILD_STR(missing_syscalls)"
make -f scripts/Makefile.build obj=scripts
make -f scripts/Makefile.build obj=scripts/genksyms
make -f scripts/Makefile.build obj=scripts/mod
make KBUILD_MODULES=1   \
        -f scripts/Makefile.build obj=drivers/ide drivers/ide/ide-disk.o
make -f
/home/mcfrisk/src/linux/linux-2.6.22.1.new/scripts/Makefile.modpost
  scripts/mod/modpost -m  -o
/home/mcfrisk/src/linux/linux-2.6.22.1.new/Module.symvers
ERROR: "ide_in_drive_list" [drivers/ide/ide-disk.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [drivers/ide/ide-disk.ko] Error 2

Based on Alan's comments I'm already running 2.6.22.1 with this:

diff -ru linux-2.6.22.1/drivers/ide/ide-disk.c linux-2.6.22.1.new/drivers/ide/ide-disk.c
--- linux-2.6.22.1/drivers/ide/ide-disk.c	2007-07-10 21:56:30.000000000 +0300
+++ linux-2.6.22.1.new/drivers/ide/ide-disk.c	2007-08-01 22:59:47.000000000 +0300
@@ -502,6 +502,17 @@
 			 capacity, sectors_to_MB(capacity),
 			 set_max, sectors_to_MB(set_max));
 
+	if ((set_max % 2) && (set_max == capacity + 1)) {
+		printk(KERN_INFO "Old drive detected, keeping current capacity.\n");
+		return;
+	}
+
 	if (lba48)
 		set_max = idedisk_set_max_address_ext(drive, set_max);
 	else

Re: ST340823A disk size issue

[Bartlomiej Zolnierkiewicz]

On Wednesday 01 August 2007, Mikko Rapeli wrote:
> On Wed, Aug 01, 2007 at 10:34:03PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Could you try attached patch?
> > 
> > [PATCH] ide-disk: workaround for buggy HPA support on ST340823A
> 
> Umh, it's getting late but some makefile magic maybe missing, or I
> should do a clean build?

[...]

> ERROR: "ide_in_drive_list" [drivers/ide/ide-disk.ko] undefined!
> make[1]: *** [__modpost] Error 1
> make: *** [drivers/ide/ide-disk.ko] Error 2

There is "EXPORT_SYMBOL(ide_in_drive_list);" missing from ide-dma.c.

I overlooked it since here ide-disk is built-in, sorry for that.

> Based on Alan's comments I'm already running 2.6.22.1 with this:

Ah, I see it now.

> diff -ru linux-2.6.22.1/drivers/ide/ide-disk.c linux-2.6.22.1.new/drivers/ide/ide-disk.c
> --- linux-2.6.22.1/drivers/ide/ide-disk.c	2007-07-10 21:56:30.000000000 +0300
> +++ linux-2.6.22.1.new/drivers/ide/ide-disk.c	2007-08-01 22:59:47.000000000 +0300
> @@ -502,6 +502,17 @@
>  			 capacity, sectors_to_MB(capacity),
>  			 set_max, sectors_to_MB(set_max));
>  
> +	if ((set_max % 2) && (set_max == capacity + 1)) {
> +		printk(KERN_INFO "Old drive detected, keeping current capacity.\n");

Please either remove this printk() or change it to:

"Buggy HPA implementation, keeping current capacity."

The disk is not odd sized since the command to read the last sectors fail
with "SectorIdNotFound". This is a buggy HPA implementation as explained in
the description of my patch.

BTW libata wouldn't help since HPA code is almost a direct copy of the code
from ide-disk (I did the comparision before writing my patch)

> +		return;
> +	}
> +
>  	if (lba48)
>  		set_max = idedisk_set_max_address_ext(drive, set_max);
>  	else

Otherwise this patch looks fine and is a bit simpler than my patch.

If you fix the printk, add patch description (can use the one from my patch)
and add "Signed-off-by:" I would happily apply it and dump mine version.

Thanks,
Bart

Re: ST340823A disk size issue

[Alan Cox]

> If you fix the printk, add patch description (can use the one from my patch)
> and add "Signed-off-by:" I would happily apply it and dump mine version.

Sorry but the patch is wrong - plain and simple. It works over the bug
but its not the real problem. ide-disk needs to spot a 1K request for the
last 512 bytes, issue a 512 byte request and complete the 512 bytes only.

Alan

Re: ST340823A disk size issue

[Bartlomiej Zolnierkiewicz]

On Thursday 02 August 2007, Alan Cox wrote:
> > If you fix the printk, add patch description (can use the one from my patch)
> > and add "Signed-off-by:" I would happily apply it and dump mine version.
> 
> Sorry but the patch is wrong - plain and simple. It works over the bug
> but its not the real problem. ide-disk needs to spot a 1K request for the
> last 512 bytes, issue a 512 byte request and complete the 512 bytes only.

block/ll_rw_blk.c:

static inline void __generic_make_request(struct bio *bio)
{
...
        int ret, nr_sectors = bio_sectors(bio);
...
        /* Test device or partition size, when known. */
        maxsector = bio->bi_bdev->bd_inode->i_size >> 9;
        if (maxsector) {
                sector_t sector = bio->bi_sector;

                if (maxsector < nr_sectors || maxsector - nr_sectors < sector) {
                        /*
                         * This may well happen - the kernel calls bread()
                         * without checking the size of the device, e.g., when
                         * mounting a device.
                         */
                        handle_bad_sector(bio);
                        goto end_io;

so low-level-driver should never see such requests.

Additionally fs/partitions/check.c:rescan_partitions() should warn if the
partition exceeds device size and we are not seeing any such warning.

Mikko, could you please revert any patches that you have applied, uncomment
#define DEBUG in ide-disk.c, recompile and note the sectors count in the

"hdd: reading: block=78165360 sectors=..."

debug message?

Thanks,
Bart

Re: ST340823A disk size issue

[Alan Cox]

> Additionally fs/partitions/check.c:rescan_partitions() should warn if the
> partition exceeds device size and we are not seeing any such warning.

Why would it if there isn't anything wrong ?

The same disks just work in libata even with the hpa being disabled. It
closed a pile of long standing bugzillas about such problems with odd
sized disks. This therefore (and instrumenting the requests being issued
when I first looked at it) made me fairly sure its not the drive. I was
seeing 1K requests for the last 512 byte sector.

Alan

Re: ST340823A disk size issue

[Bartlomiej Zolnierkiewicz]

On Thursday 02 August 2007, Alan Cox wrote:
> > Additionally fs/partitions/check.c:rescan_partitions() should warn if the
> > partition exceeds device size and we are not seeing any such warning.
> 
> Why would it if there isn't anything wrong ?

Look at the __generic_make_requests() snippet (which was in my original mail).

...
       /* Test device or partition size, when known. */
        maxsector = bio->bi_bdev->bd_inode->i_size >> 9;
        if (maxsector) {
                sector_t sector = bio->bi_sector;

                if (maxsector < nr_sectors || maxsector - nr_sectors < sector) {
                        /*
                         * This may well happen - the kernel calls bread()
                         * without checking the size of the device, e.g., when
                         * mounting a device.
                         */
                        handle_bad_sector(bio);
                        goto end_io;
...

This check prevents requests exceeding device/partition size to ever hit LLD.

Device size is set by LLD itself so we can be sure that it is OK and for
the partition size the code in rescan_partitions() would warn if the size
of partition exceeds size of the device.

> The same disks just work in libata even with the hpa being disabled. It
> closed a pile of long standing bugzillas about such problems with odd

Logs / bug numbers please.

> sized disks. This therefore (and instrumenting the requests being issued
> when I first looked at it) made me fairly sure its not the drive. I was
> seeing 1K requests for the last 512 byte sector.

We should never ever see 1K requests for the last 512 bytes sector coming
from the block layer.  If we make driver handle such cases we may be just
papering over some other issue.

Bart

Re: ST340823A disk size issue

[Alan Cox]

> We should never ever see 1K requests for the last 512 bytes sector coming
> from the block layer.  If we make driver handle such cases we may be just
> papering over some other issue.

Quite possibly but at least in early 2.6 that issue was there, and that
means without actually doing proper tests I don't think the approach
involved is the right one. If a specific 512 byte read for that last
sector fails on that specific drive/firmware then blacklist it, but the
general case of "odd size = error" is definitely not true.

Alan

Re: ST340823A disk size issue

[Mikko Rapeli]

On Thu, Aug 02, 2007 at 01:17:58AM +0200, Bartlomiej Zolnierkiewicz wrote:
> Mikko, could you please revert any patches that you have applied, uncomment
> #define DEBUG in ide-disk.c, recompile and note the sectors count in the
> 
> "hdd: reading: block=78165360 sectors=..."
> 
> debug message?

Full log at http://bugzilla.kernel.org/attachment.cgi?id=12235

Here's a summary:

$ egrep hdd\|ide1\|current\ capacity\|native\ \ capacity
dmesg_ide_debug_26221.txt

    ide1: BM-DMA at 0xe008-0xe00f, BIOS settings: hdc:pio, hdd:pio
Probing IDE interface ide1...
probing for hdd: present=0, media=32, probetype=ATA
hdd: ST340823A, ATA DISK drive
hdd: selected mode 0x42
ide1 at 0x170-0x177,0x376 on irq 15
hdd: max request size: 128KiB
ide1: start_request: current=0xc18f5c28
hdd: do_special: 0x03
ide1: start_request: current=0xc18f5c28
hdd: do_special: 0x02
ide1: start_request: current=0xc18f5c28
hdd: Host Protected Area detected.
        current capacity is 78165360 sectors (40020 MB)
        native  capacity is 78165361 sectors (40020 MB)
ide1: start_request: current=0xc18f5c28
hdd: Host Protected Area disabled.
hdd: 78165361 sectors (40020 MB) w/1024KiB Cache, CHS=65535/16/63,
UDMA(33)
hdd: cache flushes not supported
 hdd:ide1: start_request: current=0xc1e0b578
hdd: reading: block=0, sectors=8, buffer=0xc18b7000
 hdd1 hdd2 hdd3
ide1: start_request: current=0xc1e0b578
hdd: reading: block=78165120, sectors=8, buffer=0xc1e54000
ide1: start_request: current=0xc1e0b578
hdd: reading: block=78165352, sectors=1, buffer=0xc1e2e000
ide1: start_request: current=0xc1e0b408
hdd: reading: block=78165353, sectors=7, buffer=0xc1e2e200
ide1: start_request: current=0xc1e0b408
hdd: reading: block=78165360, sectors=1, buffer=0xc1e63000
hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360,
sector=78165360
ide1: start_request: current=0xc1e0b408
hdd: reading: block=78165360, sectors=1, buffer=0xc1e63000
hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360,
sector=78165360
ide1: start_request: current=0xc1e0b408
hdd: do_special: 0x02
ide1: start_request: current=0xc1e0b408
hdd: reading: block=78165360, sectors=1, buffer=0xc1e63000
hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360,
sector=78165360
ide1: start_request: current=0xc1e0b408
hdd: reading: block=78165360, sectors=1, buffer=0xc1e63000
hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360,
sector=78165360
hdd: DMA disabled
ide1: reset: master: error (0x00?)
ide1: start_request: current=0xc1e0b408
hdd: do_special: 0x03
ide1: start_request: current=0xc1e0b408
hdd: do_special: 0x02
ide1: start_request: current=0xc1e0b408
hdd: reading: block=78165360, sectors=1, buffer=0xc1e63000
hdd: task_in_intr: status=0x59 { DriveReady SeekComplete DataRequest
Error }
hdd: task_in_intr: error=0x10 { SectorIdNotFound }, LBAsect=78230639,
sector=78165360
ide1: start_request: current=0xc1e0b408
hdd: reading: block=78165360, sectors=1, buffer=0xc1e63000
hdd: task_in_intr: status=0x59 { DriveReady SeekComplete DataRequest
Error }
hdd: task_in_intr: error=0x10 { SectorIdNotFound }, LBAsect=78230639,
sector=78165360
ide1: start_request: current=0xc1e0b408
hdd: do_special: 0x02
ide1: start_request: current=0xc1e0b408
hdd: reading: block=78165360, sectors=1, buffer=0xc1e63000
hdd: task_in_intr: status=0x59 { DriveReady SeekComplete DataRequest
Error }
hdd: task_in_intr: error=0x10 { SectorIdNotFound }, LBAsect=78230639,
sector=781
65360
ide1: start_request: current=0xc1e0b408
hdd: reading: block=78165360, sectors=1, buffer=0xc1e63000
hdd: task_in_intr: status=0x59 { DriveReady SeekComplete DataRequest
Error }
hdd: task_in_intr: error=0x10 { SectorIdNotFound }, LBAsect=78230639,
sector=78165360
ide1: reset: master: error (0x00?)
ide1: start_request: current=0xc1e0b408
end_request: I/O error, dev hdd, sector 78165360
Buffer I/O error on device hdd, logical block 78165360
ide1: start_request: current=0xc1e0b408
end_request: I/O error, dev hdd, sector 78165360
Buffer I/O error on device hdd, logical block 78165360
ide1: start_request: current=0xc1e0b408
end_request: I/O error, dev hdd, sector 78165360
Buffer I/O error on device hdd, logical block 78165360
ide1: start_request: current=0xc1e0b408
end_request: I/O error, dev hdd, sector 78165360
Buffer I/O error on device hdd, logical block 78165360
ide1: start_request: current=0xc1e0b408
end_request: I/O error, dev hdd, sector 78165296
...

Re: ST340823A disk size issue

[Alan Cox]

> hdd: reading: block=78165360, sectors=1, buffer=0xc1e63000
> hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
> hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360,

Ok this one does look like its actually a specific drive firmware error

> ide1: reset: master: error (0x00?)
> ide1: start_request: current=0xc1e0b408

and the drive then shits itself in style. Bart's right - this drive does
need a blacklisting. Presumably thats why it shipped with a default HPA
to clip the non-sector 

>

Re: ST340823A disk size issue

[Bartlomiej Zolnierkiewicz]

On Thursday 02 August 2007, Alan Cox wrote:
> > hdd: reading: block=78165360, sectors=1, buffer=0xc1e63000
> > hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
> > hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360,
> 
> Ok this one does look like its actually a specific drive firmware error
> 
> > ide1: reset: master: error (0x00?)
> > ide1: start_request: current=0xc1e0b408
> 
> and the drive then shits itself in style. Bart's right - this drive does
> need a blacklisting. Presumably thats why it shipped with a default HPA
> to clip the non-sector 

Mikko, please verify that the following patch fixes the issue.

[PATCH] ide-disk: workaround for buggy HPA support on ST340823A (take 2)

This disk reports total number of sectors instead of maximum sector address
in response to READ_NATIVE_MAX_ADDRESS command and also happily accepts
SET_MAX_ADDRESS command with the bogus value.  This results in +1 sector
capacity being used and errors on attempts to use the last sector.

...
hdd: Host Protected Area detected.
        current capacity is 78165360 sectors (40020 MB)
        native  capacity is 78165361 sectors (40020 MB)
hdd: Host Protected Area disabled.
...
hdd: reading: block=78165360, sectors=1, buffer=0xc1e63000
hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360, sector=78165360
...

Add hpa_list[] table and workaround the issue in idedisk_check_hpa().

Fixes kernel bugzilla bug #8816.

Thanks to Mikko for investigating the issue and testing this patch.

v2:
* Add missing export and improve patch description a bit.

Cc: Mikko Rapeli <mikko.rapeli@iki.fi>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
This patch depends on CONFIG_BLK_DEV_IDEDMA=y but for testing purposes
it is OK.  Final version wouldn't depend on CONFIG_BLK_DEV_IDEDMA,
"[PATCH] ide: add cable detection for early UDMA66 devices (take 3)"
which is in IDE quilt tree moves ide_in_drive_list() out of ide-dma.c.

 drivers/ide/ide-disk.c |   17 +++++++++++++++++
 drivers/ide/ide-dma.c  |    2 ++
 2 files changed, 19 insertions(+)

Index: b/drivers/ide/ide-disk.c
===================================================================
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -481,6 +481,14 @@ static inline int idedisk_supports_lba48
 	       && id->lba_capacity_2;
 }
 
+/*
+ * Some disks report total number of sectors instead of
+ * maximum sector address.  We list them here.
+ */
+static const struct drive_list_entry hpa_list[] = {
+	{ "ST340823A",	NULL },
+};
+
 static void idedisk_check_hpa(ide_drive_t *drive)
 {
 	unsigned long long capacity, set_max;
@@ -492,6 +500,15 @@ static void idedisk_check_hpa(ide_drive_
 	else
 		set_max = idedisk_read_native_max_address(drive);
 
+	if (ide_in_drive_list(drive->id, hpa_list)) {
+		/*
+		 * Since we are inclusive wrt to firmware revisions do this
+		 * extra check and apply the workaround only when needed.
+		 */
+		if (set_max == capacity + 1)
+			set_max--;
+	}
+
 	if (set_max <= capacity)
 		return;
 
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -153,6 +153,8 @@ int ide_in_drive_list(struct hd_driveid 
 	return 0;
 }
 
+EXPORT_SYMBOL_GPL(ide_in_drive_list);
+
 /**
  *	ide_dma_intr	-	IDE DMA interrupt handler
  *	@drive: the drive the interrupt is for

Re: ST340823A disk size issue

[Mikko Rapeli]

On Fri, Aug 03, 2007 at 12:34:13AM +0200, Bartlomiej Zolnierkiewicz wrote:
> Mikko, please verify that the following patch fixes the issue.

A few oopses later and this actually works with vanilla 2.6.22.1 and Debian
2.6.18 (2.6.18.dfsg.1-12etch2), latter without EXPORT_SYMBOL_GPL since
it's there already.

diff -X linux-2.6.22.1/Documentation/dontdiff -upr linux-2.6.22.1.orig/drivers/ide/ide-disk.c linux-2.6.22.1/drivers/ide/ide-disk.c
--- linux-2.6.22.1.orig/drivers/ide/ide-disk.c	2007-07-10 21:56:30.000000000 +0300
+++ linux-2.6.22.1/drivers/ide/ide-disk.c	2007-08-03 00:22:46.000000000 +0300
@@ -481,6 +481,15 @@ static inline int idedisk_supports_lba48
 	       && id->lba_capacity_2;
 }
 
+/*
+ * Some disks report total number of sectors instead of
+ * maximum sector address.  We list them here.
+ */
+static const struct drive_list_entry hpa_list[] = {
+	{ "ST340823A",	"ALL" },
+	{ NULL       ,  NULL  }
+};
+
 static void idedisk_check_hpa(ide_drive_t *drive)
 {
 	unsigned long long capacity, set_max;
@@ -492,6 +501,15 @@ static void idedisk_check_hpa(ide_drive_
 	else
 		set_max = idedisk_read_native_max_address(drive);
 
+	if (ide_in_drive_list(drive->id, hpa_list)) {
+		/*
+		 * Since we are inclusive wrt to firmware revisions do this
+		 * extra check and apply the workaround only when needed.
+		 */
+		if (set_max == capacity + 1)
+			set_max--;
+	}
+
 	if (set_max <= capacity)
 		return;
 
diff -X linux-2.6.22.1/Documentation/dontdiff -upr linux-2.6.22.1.orig/drivers/ide/ide-dma.c linux-2.6.22.1/drivers/ide/ide-dma.c
--- linux-2.6.22.1.orig/drivers/ide/ide-dma.c	2007-07-10 21:56:30.000000000 +0300
+++ linux-2.6.22.1/drivers/ide/ide-dma.c	2007-08-03 01:10:25.000000000 +0300
@@ -152,6 +152,7 @@ int ide_in_drive_list(struct hd_driveid 
 			return 1;
 	return 0;
 }
+EXPORT_SYMBOL_GPL(ide_in_drive_list);
 
 /**
  *	ide_dma_intr	-	IDE DMA interrupt handler

Re: ST340823A disk size issue

[Bartlomiej Zolnierkiewicz]

On Friday 03 August 2007, Mikko Rapeli wrote:
> On Fri, Aug 03, 2007 at 12:34:13AM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Mikko, please verify that the following patch fixes the issue.
> 
> A few oopses later and this actually works with vanilla 2.6.22.1 and Debian
> 2.6.18 (2.6.18.dfsg.1-12etch2), latter without EXPORT_SYMBOL_GPL since
> it's there already.
> 
> diff -X linux-2.6.22.1/Documentation/dontdiff -upr linux-2.6.22.1.orig/drivers/ide/ide-disk.c linux-2.6.22.1/drivers/ide/ide-disk.c
> --- linux-2.6.22.1.orig/drivers/ide/ide-disk.c	2007-07-10 21:56:30.000000000 +0300
> +++ linux-2.6.22.1/drivers/ide/ide-disk.c	2007-08-03 00:22:46.000000000 +0300
> @@ -481,6 +481,15 @@ static inline int idedisk_supports_lba48
>  	       && id->lba_capacity_2;
>  }
>  
> +/*
> + * Some disks report total number of sectors instead of
> + * maximum sector address.  We list them here.
> + */
> +static const struct drive_list_entry hpa_list[] = {
> +	{ "ST340823A",	"ALL" },

Doh, I forgot that the patch to change "ALL" to NULL was merged after 2.6.22.
I should have asked you from the beginning to test against 2.6.23-rc1 but it
is not that bad since now you have also working patches for 2.6.18 / 2.6.22.

;-)

> +	{ NULL       ,  NULL  }

Thanks, added this to the patch.

Final version (for completness) below.

[PATCH] ide-disk: workaround for buggy HPA support on ST340823A (take 3)

This disk reports total number of sectors instead of maximum sector address
in response to READ_NATIVE_MAX_ADDRESS command and also happily accepts
SET_MAX_ADDRESS command with the bogus value.  This results in +1 sector
capacity being used and errors on attempts to use the last sector.

...
hdd: Host Protected Area detected.
        current capacity is 78165360 sectors (40020 MB)
        native  capacity is 78165361 sectors (40020 MB)
hdd: Host Protected Area disabled.
...
hdd: reading: block=78165360, sectors=1, buffer=0xc1e63000
hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360, sector=78165360
...

Add hpa_list[] table and workaround the issue in idedisk_check_hpa().

v2:
* Add missing export and improve patch description a bit.

v3:
* Add list termination.  (From Mikko)

Fixes kernel bugzilla bug #8816.

Thanks to Mikko for investigating the issue and helping with this patch.

Cc: Mikko Rapeli <mikko.rapeli@iki.fi>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
This patch depends on CONFIG_BLK_DEV_IDEDMA=y but for testing purposes
it is OK.  Final version wouldn't depend on CONFIG_BLK_DEV_IDEDMA,
"[PATCH] ide: add cable detection for early UDMA66 devices (take 3)"
which is in IDE quilt tree moves ide_in_drive_list() out of ide-dma.c.

 drivers/ide/ide-disk.c |   18 ++++++++++++++++++
 drivers/ide/ide-dma.c  |    2 ++
 2 files changed, 20 insertions(+)

Index: b/drivers/ide/ide-disk.c
===================================================================
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -481,6 +481,15 @@ static inline int idedisk_supports_lba48
 	       && id->lba_capacity_2;
 }
 
+/*
+ * Some disks report total number of sectors instead of
+ * maximum sector address.  We list them here.
+ */
+static const struct drive_list_entry hpa_list[] = {
+	{ "ST340823A",	NULL },
+	{ NULL,		NULL }
+};
+
 static void idedisk_check_hpa(ide_drive_t *drive)
 {
 	unsigned long long capacity, set_max;
@@ -492,6 +501,15 @@ static void idedisk_check_hpa(ide_drive_
 	else
 		set_max = idedisk_read_native_max_address(drive);
 
+	if (ide_in_drive_list(drive->id, hpa_list)) {
+		/*
+		 * Since we are inclusive wrt to firmware revisions do this
+		 * extra check and apply the workaround only when needed.
+		 */
+		if (set_max == capacity + 1)
+			set_max--;
+	}
+
 	if (set_max <= capacity)
 		return;
 
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -153,6 +153,8 @@ int ide_in_drive_list(struct hd_driveid 
 	return 0;
 }
 
+EXPORT_SYMBOL_GPL(ide_in_drive_list);
+
 /**
  *	ide_dma_intr	-	IDE DMA interrupt handler
  *	@drive: the drive the interrupt is for

[PATCH] ST340823A, HPA and libata

[Mikko Rapeli]

Hello again,

Had some time to play with libata and this ST340823A drive. I had some
problems with 2.6.23-rc2 setup failing at early boot so I kept on
going with 2.6.22.1.

First, vanilla 2.6.22.1 which works as HPA is not disabled by default:

http://bugzilla.kernel.org/attachment.cgi?id=12251&action=view

$ egrep "Host Prote|current size|native size|sdb" dmesg_ide_bug_26221_pata.txt
ata2.01: Host Protected Area detected:
        current size: 78165360 sectors
        native size: 78165361 sectors
ata2.01: Host Protected Area detected:
        current size: 78165360 sectors
        native size: 78165361 sectors
sd 1:0:1:0: [sdb] 78165360 512-byte hardware sectors (40021 MB)
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't
support DPO or FUA
sd 1:0:1:0: [sdb] 78165360 512-byte hardware sectors (40021 MB)
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't
support DPO or FUA
 sdb: sdb1 sdb2 sdb3
sd 1:0:1:0: [sdb] Attached SCSI disk

Then I went to disabled HPA by default. Actually the
drive and filesystem work altough boot looks nasty:

http://bugzilla.kernel.org/attachment.cgi?id=12252&action=view 

$ egrep "Host Prote|current size|native size|sdb" 
dmesg_ide_bug_26221_pata_hpa.txt
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
sd 1:0:1:0: [sdb] 78165361 512-byte hardware sectors (40021 MB)
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 1:0:1:0: [sdb] 78165361 512-byte hardware sectors (40021 MB)
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
 sdb: sdb1 sdb2 sdb3
sd 1:0:1:0: [sdb] Attached SCSI disk
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
sd 1:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
sd 1:0:1:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
sd 1:0:1:0: [sdb] Add. Sense: Recorded entity not found
end_request: I/O error, dev sdb, sector 78165360
Buffer I/O error on device sdb, logical block 78165360
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
sd 1:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
sd 1:0:1:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
sd 1:0:1:0: [sdb] Add. Sense: Recorded entity not found
end_request: I/O error, dev sdb, sector 78165360
Buffer I/O error on device sdb, logical block 78165360
sd 1:0:1:0: [sdb] 78165361 512-byte hardware sectors (40021 MB)
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
sd 1:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
sd 1:0:1:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
sd 1:0:1:0: [sdb] Add. Sense: Recorded entity not found
end_request: I/O error, dev sdb, sector 78165360
Buffer I/O error on device sdb, logical block 78165360
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
sd 1:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
sd 1:0:1:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
sd 1:0:1:0: [sdb] Add. Sense: Recorded entity not found
end_request: I/O error, dev sdb, sector 78165360
Buffer I/O error on device sdb, logical block 78165360
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 1:0:1:0: [sdb] 78165361 512-byte hardware sectors (40021 MB)
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
sd 1:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
sd 1:0:1:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
sd 1:0:1:0: [sdb] Add. Sense: Recorded entity not found
end_request: I/O error, dev sdb, sector 78165360
Buffer I/O error on device sdb, logical block 78165360
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
sd 1:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
sd 1:0:1:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
sd 1:0:1:0: [sdb] Add. Sense: Recorded entity not found
end_request: I/O error, dev sdb, sector 78165360
Buffer I/O error on device sdb, logical block 78165360
sd 1:0:1:0: [sdb] 78165361 512-byte hardware sectors (40021 MB)
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 1:0:1:0: [sdb] 78165361 512-byte hardware sectors (40021 MB)
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
sd 1:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
sd 1:0:1:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
sd 1:0:1:0: [sdb] Add. Sense: Recorded entity not found
end_request: I/O error, dev sdb, sector 78165360
Buffer I/O error on device sdb, logical block 78165360
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
sd 1:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
sd 1:0:1:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
sd 1:0:1:0: [sdb] Add. Sense: Recorded entity not found
end_request: I/O error, dev sdb, sector 78165360
Buffer I/O error on device sdb, logical block 78165360
sd 1:0:1:0: [sdb] 78165361 512-byte hardware sectors (40021 MB)
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
sd 1:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
sd 1:0:1:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
sd 1:0:1:0: [sdb] Add. Sense: Recorded entity not found
end_request: I/O error, dev sdb, sector 78165360
Buffer I/O error on device sdb, logical block 78165360
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
sd 1:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
sd 1:0:1:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
sd 1:0:1:0: [sdb] Add. Sense: Recorded entity not found
end_request: I/O error, dev sdb, sector 78165360
Buffer I/O error on device sdb, logical block 78165360
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 1:0:1:0: [sdb] 78165361 512-byte hardware sectors (40021 MB)
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
sd 1:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
sd 1:0:1:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
sd 1:0:1:0: [sdb] Add. Sense: Recorded entity not found
end_request: I/O error, dev sdb, sector 78165360
Buffer I/O error on device sdb, logical block 78165360
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
ata2.01: Host Protected Area detected:
	current size: 78165360 sectors
	native size: 78165361 sectors
ata2.01: native size increased to 78165361 sectors
sd 1:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
sd 1:0:1:0: [sdb] Sense Key : Aborted Command [current] [descriptor]
sd 1:0:1:0: [sdb] Add. Sense: Recorded entity not found
end_request: I/O error, dev sdb, sector 78165360
sd 1:0:1:0: [sdb] 78165361 512-byte hardware sectors (40021 MB)
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 1:0:1:0: [sdb] 78165361 512-byte hardware sectors (40021 MB)
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA

Tried to hack a blacklist, which gives the log:

http://bugzilla.kernel.org/attachment.cgi?id=12254&action=view 

$ egrep "Host Prote|current size|native size|sdb"
dmesg_ide_bug_26221_pata_hpa_fix.txt
ata2.01: Host Protected Area detected:
        current size: 78165360 sectors
        native size: 78165361 sectors
ata2.01: Host Protected Area detected:
        current size: 78165360 sectors
        native size: 78165361 sectors
sd 1:0:1:0: [sdb] 78165360 512-byte hardware sectors (40021 MB)
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't
support DPO or FUA
sd 1:0:1:0: [sdb] 78165360 512-byte hardware sectors (40021 MB)
sd 1:0:1:0: [sdb] Write Protect is off
sd 1:0:1:0: [sdb] Mode Sense: 00 3a 00 00
sd 1:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't
support DPO or FUA
 sdb: sdb1 sdb2 sdb3
sd 1:0:1:0: [sdb] Attached SCSI disk

The patch is simply:

diff -X linux-2.6.22.1/Documentation/exception.txt -rup linux-2.6.22.1.orig/drivers/ata/libata-core.c linux-2.6.22.1/drivers/ata/libata-core.c
--- linux-2.6.22.1.orig/drivers/ata/libata-core.c	2007-07-10 21:56:30.000000000 +0300
+++ linux-2.6.22.1/drivers/ata/libata-core.c	2007-08-05 11:38:29.000000000 +0300
@@ -991,7 +991,10 @@ static u64 ata_hpa_resize(struct ata_dev
 			"\tnative size: %lld sectors\n",
 			(long long)sectors, (long long)hpa_sectors);
 
-		if (ata_ignore_hpa) {
+		if (ata_device_blacklisted(dev) & ATA_HORKAGE_HPA) {
+			ata_dev_printk(dev, KERN_INFO,
+				"Device blacklisted, ignoring HPA.\n");
+		} else if (ata_ignore_hpa) {
 			if (ata_id_has_lba48(dev->id))
 				hpa_sectors = ata_set_native_max_address_ext(dev, hpa_sectors);
 			else
@@ -3803,6 +3806,10 @@ static const struct ata_blacklist_entry 
 
 	/* Devices with NCQ limits */
 
+	/* Devices which fail when HPA is disable, e.g. READ_NATIVE_MAX_ADDRESS
+	   returns maximum size instead of maximum addressable sector. */
+	{ "ST340823A",	NULL,	ATA_HORKAGE_HPA },
+
 	/* End Marker */
 	{ }
 };
diff -X linux-2.6.22.1/Documentation/exception.txt -rup linux-2.6.22.1.orig/include/linux/libata.h linux-2.6.22.1/include/linux/libata.h
--- linux-2.6.22.1.orig/include/linux/libata.h	2007-07-10 21:56:30.000000000 +0300
+++ linux-2.6.22.1/include/linux/libata.h	2007-08-05 00:43:59.000000000 +0300
@@ -298,6 +298,7 @@ enum {
 	ATA_HORKAGE_NODMA	= (1 << 1),	/* DMA problems */
 	ATA_HORKAGE_NONCQ	= (1 << 2),	/* Don't use NCQ */
 	ATA_HORKAGE_MAX_SEC_128	= (1 << 3),	/* Limit max sects to 128 */
+	ATA_HORKAGE_HPA         = (1 << 4),     /* Don't disable HPA */
 };
 
 enum hsm_task_states {

Re: [PATCH] ST340823A, HPA and libata

[Alan Cox]

> The patch is simply:
> 
> diff -X linux-2.6.22.1/Documentation/exception.txt -rup linux-2.6.22.1.orig/drivers/ata/libata-core.c

Looks good although I'd rather apply a different version which handles
HPA_OFF_BY_ONE. The reason being someone, somewhere is going to come
along later and annoy me otherwise by having a disk which is BIOS clipped
and where we need to restore the HPA minus 1.

Alan

[PATCH] ST340823A, HPA and libata

[Alan Cox]

Ok this is a different approach to handling it - knowing it is an off by
one so we can handle the drive if clipped

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.23rc1-mm1/drivers/ata/libata-core.c linux-2.6.23rc1-mm1/drivers/ata/libata-core.c
--- linux.vanilla-2.6.23rc1-mm1/drivers/ata/libata-core.c	2007-07-26 15:02:57.000000000 +0100
+++ linux-2.6.23rc1-mm1/drivers/ata/libata-core.c	2007-08-08 13:38:39.257549616 +0100
@@ -988,6 +992,10 @@
 	else
 		hpa_sectors = ata_read_native_max_address(dev);
 
+	/* Reports one sector too many */
+	if (hpa_sectors && (dev->horkage & ATA_HORKAGE_HPA_SIZE))
+		hpa_sectors--;
+
 	if (hpa_sectors > sectors) {
 		ata_dev_printk(dev, KERN_INFO,
 			"Host Protected Area detected:\n"
@@ -3858,6 +3926,9 @@
 
 	/* Devices with NCQ limits */
 
+	/* Devices which report 1 sector over size HPA */
+	{ "ST340823A",		NULL,		ATA_HORKAGE_HPA_SIZE, },
+
 	/* End Marker */
 	{ }
 };
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.23rc1-mm1/include/linux/libata.h linux-2.6.23rc1-mm1/include/linux/libata.h
--- linux.vanilla-2.6.23rc1-mm1/include/linux/libata.h	2007-07-26 15:02:58.000000000 +0100
+++ linux-2.6.23rc1-mm1/include/linux/libata.h	2007-08-08 13:38:09.000000000 +0100
@@ -314,6 +314,7 @@
 	ATA_HORKAGE_NONCQ	= (1 << 2),	/* Don't use NCQ */
 	ATA_HORKAGE_MAX_SEC_128	= (1 << 3),	/* Limit max sects to 128 */
 	ATA_HORKAGE_ALPM	= (1 << 4), 	/* ALPM problems */
+	ATA_HORKAGE_HPA_SIZE	= (1 << 5), 	/* Reports native size off by one */
 };
 
 enum hsm_task_states {

Re: [PATCH] ST340823A, HPA and libata

[Mikko Rapeli]

On Wed, Aug 08, 2007 at 02:25:05PM +0100, Alan Cox wrote:
> Ok this is a different approach to handling it - knowing it is an off by
> one so we can handle the drive if clipped

Patch works with 2.6.23-rc2 after the boot problem was solved.

-Mikko

Re: [PATCH] ST340823A, HPA and libata

[Jeff Garzik]

Alan Cox wrote:
> Ok this is a different approach to handling it - knowing it is an off by
> one so we can handle the drive if clipped
> 
> Signed-off-by: Alan Cox <alan@redhat.com>
> 
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.23rc1-mm1/drivers/ata/libata-core.c linux-2.6.23rc1-mm1/drivers/ata/libata-core.c
> --- linux.vanilla-2.6.23rc1-mm1/drivers/ata/libata-core.c	2007-07-26 15:02:57.000000000 +0100
> +++ linux-2.6.23rc1-mm1/drivers/ata/libata-core.c	2007-08-08 13:38:39.257549616 +0100
> @@ -988,6 +992,10 @@
>  	else
>  		hpa_sectors = ata_read_native_max_address(dev);
>  
> +	/* Reports one sector too many */
> +	if (hpa_sectors && (dev->horkage & ATA_HORKAGE_HPA_SIZE))
> +		hpa_sectors--;
> +
>  	if (hpa_sectors > sectors) {
>  		ata_dev_printk(dev, KERN_INFO,
>  			"Host Protected Area detected:\n"
> @@ -3858,6 +3926,9 @@
>  
>  	/* Devices with NCQ limits */
>  
> +	/* Devices which report 1 sector over size HPA */
> +	{ "ST340823A",		NULL,		ATA_HORKAGE_HPA_SIZE, },
> +
>  	/* End Marker */
>  	{ }
>  };
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.23rc1-mm1/include/linux/libata.h linux-2.6.23rc1-mm1/include/linux/libata.h
> --- linux.vanilla-2.6.23rc1-mm1/include/linux/libata.h	2007-07-26 15:02:58.000000000 +0100
> +++ linux-2.6.23rc1-mm1/include/linux/libata.h	2007-08-08 13:38:09.000000000 +0100
> @@ -314,6 +314,7 @@
>  	ATA_HORKAGE_NONCQ	= (1 << 2),	/* Don't use NCQ */
>  	ATA_HORKAGE_MAX_SEC_128	= (1 << 3),	/* Limit max sects to 128 */
>  	ATA_HORKAGE_ALPM	= (1 << 4), 	/* ALPM problems */
> +	ATA_HORKAGE_HPA_SIZE	= (1 << 5), 	/* Reports native size off by one */

the horkage patch I sent upstream to Linus just now for 2.6.23-rc 
included some of the HPA size blacklist entries, so I dropped that 
not-for-2.6.23 patch chunk.

If this issue has settled, let's roll everything into a single patch, 
and I'll apply that.

Re: ST340823A disk size issue

[Bartlomiej Zolnierkiewicz]

On Thursday 02 August 2007, Bartlomiej Zolnierkiewicz wrote:

> Otherwise this patch looks fine and is a bit simpler than my patch.
> 
> If you fix the printk, add patch description (can use the one from my patch)
> and add "Signed-off-by:" I would happily apply it and dump mine version.

On the second thought - there may be real devices which have +1 native
capacity so we are better off with sticking to the version with extra drive
model check (this is of course given that the approach is right - yet to
be verified).

Thanks,
Bart

Re: ST340823A disk size issue

[Sergei Shtylyov]

Mikko Rapeli wrote:

> diff -ru linux-2.6.22.1/drivers/ide/ide-disk.c linux-2.6.22.1.new/drivers/ide/ide-disk.c
> --- linux-2.6.22.1/drivers/ide/ide-disk.c	2007-07-10 21:56:30.000000000 +0300
> +++ linux-2.6.22.1.new/drivers/ide/ide-disk.c	2007-08-01 22:59:47.000000000 +0300
> @@ -502,6 +502,17 @@
>  			 capacity, sectors_to_MB(capacity),
>  			 set_max, sectors_to_MB(set_max));
>  
> +	if ((set_max % 2)

    I hope gcc will convert this to (set_max & 1) but it wouldn't hurt to code 
this explicitly.

>  && (set_max == capacity + 1)) {

    Wait, isn't set_max an address of a last sector, and isn't capacity a 
sector count? Shouldn't it be (set_max == capacity)?

> +		printk(KERN_INFO "Old drive detected, keeping current capacity.\n");
> +		return;
> +	}
> +
>  	if (lba48)
>  		set_max = idedisk_set_max_address_ext(drive, set_max);
>  	else

MBR, Sergei

Re: ST340823A disk size issue

[Sergei Shtylyov]

Hello, I wrote:

>> diff -ru linux-2.6.22.1/drivers/ide/ide-disk.c 
>> linux-2.6.22.1.new/drivers/ide/ide-disk.c
>> --- linux-2.6.22.1/drivers/ide/ide-disk.c    2007-07-10 
>> 21:56:30.000000000 +0300
>> +++ linux-2.6.22.1.new/drivers/ide/ide-disk.c    2007-08-01 
>> 22:59:47.000000000 +0300
>> @@ -502,6 +502,17 @@
>>               capacity, sectors_to_MB(capacity),
>>               set_max, sectors_to_MB(set_max));
>>  
>> +    if ((set_max % 2)

>    I hope gcc will convert this to (set_max & 1) but it wouldn't hurt to 
> code this explicitly.

>>  && (set_max == capacity + 1)) {

>    Wait, isn't set_max an address of a last sector,

    Well, I was wrong: idedisk_read_native_max_address*() increments this 
value bofore returning.

> and isn't capacity a 
> sector count? Shouldn't it be (set_max == capacity)?

    No, it was OK.

  MBR, Sergei

Re: ST340823A disk size issue

[Alan Cox]

> Could you try attached patch?
> 
> [PATCH] ide-disk: workaround for buggy HPA support on ST340823A

Title is wrong. The disc is performing correctly but if you dump the
request you should see a 1024 byte request for the last odd sector number
and the drive response is likewise correct.


Thus NAK the change

Fix ide-disk to issue requests for the last sector correctly instead (or
at least complete them properly)