Re: [PATCH] clipped disk reports clipped lba size

Re: [PATCH] clipped disk reports clipped lba size

[Andries.Brouwer]

On Wed, Jan 30, 2002 at 12:02:14PM -0300, Eduardo Trpani wrote:

> Since my BIOS does not support my disk (WD400) I had to clip to 33.8G.
> At boot time Linux (2.4.17) uses the lba size reported by the disk,
> that is 33.8 and does not allow me to access the rest of the disk.

> +     /* If the geometry has been forced recalculate lba_capacity */
> +     if ((id->capability & 2) && lba_capacity_is_ok(id) &&
> +      drive->forced_geom)
> +     {
> +             id->lba_capacity = drive->bios_head * drive->bios_sect * drive->bios_cyl;
> +     }

This is not really the right patch, although it may solve your problem.
It looks a bit silly to first test lba_capacity_is_ok() and then throw
away lba_capacity anyway.
Using a product of strange numbers is a bad idea.
Often this simple patch will not help, see below.

My point of view would be more like: we now have three levels of information.
First C/H/S - very outdated. On all large disks it will be 16383/16/63.
Then LBA. When it looks reasonable, we believe it, and let it override C/H/S.
Third what the disk returns for READ_NATIVE_MAX_ADDRESS. Especially when LBA
says 33.8GB we have a good reason to ask for this and see whether it is more.

Some disk types fake LBA at 33.8GB, but allow access past this point.
Some disks actually give I/O errors past the 33.8GB (when jumpered),
and a SETMAX command is needed to make the rest accessible.

Two years ago I wrote a tiny utility setmax that does this.
If I am not mistaken this stuff is now part of the 2.5 kernel.
No doubt some of it will eventually be backported to 2.4 / 2.2 / 2.0.
It is in 2.4.18-pre7-ac1.

[Just looked at patch-2.5.3-pre1. And indeed, some nice code.
Andre, others: is there a reason to make CONFIG_IDEDISK_STROKE
a configuration option?  I very much dislike configuration options,
especially in cases like this, where the user may choose between
things happening correctly and things not happening correctly.
And it looks like the option is undocumented as well.
Is setting CONFIG_IDEDISK_STROKE ever harmful?

It looks like the code

+       /* if OK, compute maximum address value */
+       if ((args.tfRegister[IDE_STATUS_OFFSET] & 0x01) == 0) {
+               addr = ((args.tfRegister[IDE_SELECT_OFFSET] & 0x0f) << 24)
+                    | ((args.tfRegister[  IDE_HCYL_OFFSET]       ) << 16)
+                    | ((args.tfRegister[  IDE_LCYL_OFFSET]       ) <<  8)
+                    | ((args.tfRegister[IDE_SECTOR_OFFSET]       ));
+       }
+       addr++; /* since the return value is (maxlba - 1), we add 1 */
+       return addr;

has a bug: the addr++ should be inside the if(), I suppose.
The same bug occurs a number of places.]

Andries

Re: [PATCH] clipped disk reports clipped lba size

[Paul Gortmaker]

Andries.Brouwer@cwi.nl wrote:

> Some disk types fake LBA at 33.8GB, but allow access past this point.
> Some disks actually give I/O errors past the 33.8GB (when jumpered),
> and a SETMAX command is needed to make the rest accessible.
> 
> Two years ago I wrote a tiny utility setmax that does this.
> If I am not mistaken this stuff is now part of the 2.5 kernel.
> No doubt some of it will eventually be backported to 2.4 / 2.2 / 2.0.
> It is in 2.4.18-pre7-ac1.

Alan has said (quite reasonably) that he is not interested in inclusion
of the big IDE patch that exists for 2.2.x -- however, a minimal cut and 
paste backport from 2.4.x IDE to just support HDIO_DRIVE_CMD_AEB (and thus
support setmax) is only about a 100 line diff which I did a while ago.

If there is any interest in this I can check it still applies cleanly to 
current 2.2 pre kernel and send it along for inclusion.

Paul.

Re: [PATCH] clipped disk reports clipped lba size

[Andries.Brouwer]

    From: Paul Gortmaker <p_gortmaker@yahoo.com>

    > (1) *_AEB is intended as private namespace for me, not for inclusion
    > in an official kernel. So, some official name, like HDIO_DRIVE_TASK,
    > must be better.

    Yes, DRIVE_TASK is of course the official name - I just knew you would
    immediately recognize *_AEB  & and know context/usage :)   BTW, if you
    didn't want it in an official kernel, it is a bit late for that. It has
    been in for some time in 2.4 .... <grep>  2.3.99pre9 to be exact.

Yes. But it is unnecessary and can be removed.

    > to solve both problems without involving ioctl's, or boot
    > parameters, or config parameters. All should just work
    > in the common case.

    This is a valid point - in an ideal situation things should just work
    without user intervention or setmax util, etc.  What are you currently
    recommending to 2.2.x kernel users that come across this limitation
    prior to such an ideal fix being released in a 2.2.x kernel?

Precisely what you also suggest: setmax plus the kernel patch that
makes it work. (Or: go to 2.4.)

    From Andre Hedrick <andre@linuxdiskcert.org>

    It is a valid and techincally correct operation; however, it is only one
    half of the solution.  Your "SETMAX" user-space IOCTL, is great if you do
    not have partitions that bridge that boundary, other wise TOAST.

If an extended partition table sector is located past the accessible
area of the disk, then probably accessing it yields an I/O error.
Later, using partx or "blockdev --rereadpt" or so, one can add the
missing partitions again. A bit ugly and kludgy, that is why the
kernel should do this itself, and automatically.

    Now the problem I have not addressed is the reboot issue,
    and neither have you.

Only partially. Details depend on type of disk and BIOS.
But roughly speaking, if one wants a warm reboot, one can
invoke setmax again to clip the disk (or hold F4 down
during the reboot).

Andries

Re: [PATCH] clipped disk reports clipped lba size

[Andries.Brouwer]

    From: Paul Gortmaker <p_gortmaker@yahoo.com>

    Andries.Brouwer@cwi.nl wrote:

    > Some disk types fake LBA at 33.8GB, but allow access past this point.
    > Some disks actually give I/O errors past the 33.8GB (when jumpered),
    > and a SETMAX command is needed to make the rest accessible.
    > 
    > Two years ago I wrote a tiny utility setmax that does this.
    > If I am not mistaken this stuff is now part of the 2.5 kernel.
    > No doubt some of it will eventually be backported to 2.4 / 2.2 / 2.0.
    > It is in 2.4.18-pre7-ac1.

    Alan has said (quite reasonably) that he is not interested in inclusion
    of the big IDE patch that exists for 2.2.x -- however, a minimal cut and 
    paste backport from 2.4.x IDE to just support HDIO_DRIVE_CMD_AEB (and thus
    support setmax) is only about a 100 line diff which I did a while ago.

    If there is any interest in this I can check it still applies cleanly to 
    current 2.2 pre kernel and send it along for inclusion.

(1) *_AEB is intended as private namespace for me, not for inclusion
in an official kernel. So, some official name, like HDIO_DRIVE_TASK,
must be better.

(2) Long ago very little information was available and I wrote a small
program that worked for me and solicited experiences from others.
By now we have a better idea of the variations that exist.
Moreover, this is beginning 2.5 time, so experiments are allowed.
That means that we can delete CONFIG_IDEDISK_STROKE, and make the
kernel do what we think is right. Once this gets to a state where
there are no complaints anymore we can move it to some 2.4 tree,
and if that still does not produce complaints to 2.2 and 2.0.

In the area of big disks there are two main hurdles these days:
(a) capacity-limiting jumpers to overcome BIOS problems
(b) disks larger than 137 GB, the old ATA limit.

Both problems can be solved with relatively small patches,
no big monolithic IDE patch required. And I would prefer
to solve both problems without involving ioctl's, or boot
parameters, or config parameters. All should just work
in the common case.

Andries

Re: [PATCH] clipped disk reports clipped lba size

[Andre Hedrick]

It is a valid and techincally correct operation; however, it is only one
half of the solution.  Your "SETMAX" user-space IOCTL, is great if you do
not have partitions that bridge that boundary, other wise TOAST.  Now the
problem I have not addressed is the reboot issue, and neither have you.
On a warm reboot after as SET_MAX_NATIVE_ADDRESS has been issued from the
previous time, the drive is now larger than the 32GB clip limit.  Thus the
warm reboot will fail.  Since it is a volitale command and does not
survive power cycles but does survive resets and reboots, all we have
both done is to insure a dead hang in warm reboot.  So the issue is to
destroke the drive on reboot or notification by undoing the capacity
expansion.  Since I have not gotten to completing the last stage, it is a
compile option.  An option is exactly what it means.

As for the monolithic patch ... there is a much grander end game for the
driver than what most can see or even invision.  Thus the goal is to
transform the driver to handle the current various archs with conflicts of
PIO v/s MMIO calling to the transport layer built in the same system.

The other more obvious point is to standardize the data-handlers where as
the addition of a new command is simple, not adding N-different handlers
for each new command Linux attempts to support.

Cheers,

Andre Hedrick
Linux Disk Certification Project                Linux ATA Development

On Fri, 1 Feb 2002 Andries.Brouwer@cwi.nl wrote:

>     From: Paul Gortmaker <p_gortmaker@yahoo.com>
> 
>     Andries.Brouwer@cwi.nl wrote:
> 
>     > Some disk types fake LBA at 33.8GB, but allow access past this point.
>     > Some disks actually give I/O errors past the 33.8GB (when jumpered),
>     > and a SETMAX command is needed to make the rest accessible.
>     > 
>     > Two years ago I wrote a tiny utility setmax that does this.
>     > If I am not mistaken this stuff is now part of the 2.5 kernel.
>     > No doubt some of it will eventually be backported to 2.4 / 2.2 / 2.0.
>     > It is in 2.4.18-pre7-ac1.
> 
>     Alan has said (quite reasonably) that he is not interested in inclusion
>     of the big IDE patch that exists for 2.2.x -- however, a minimal cut and 
>     paste backport from 2.4.x IDE to just support HDIO_DRIVE_CMD_AEB (and thus
>     support setmax) is only about a 100 line diff which I did a while ago.
> 
>     If there is any interest in this I can check it still applies cleanly to 
>     current 2.2 pre kernel and send it along for inclusion.
> 
> (1) *_AEB is intended as private namespace for me, not for inclusion
> in an official kernel. So, some official name, like HDIO_DRIVE_TASK,
> must be better.
> 
> (2) Long ago very little information was available and I wrote a small
> program that worked for me and solicited experiences from others.
> By now we have a better idea of the variations that exist.
> Moreover, this is beginning 2.5 time, so experiments are allowed.
> That means that we can delete CONFIG_IDEDISK_STROKE, and make the
> kernel do what we think is right. Once this gets to a state where
> there are no complaints anymore we can move it to some 2.4 tree,
> and if that still does not produce complaints to 2.2 and 2.0.
> 
> In the area of big disks there are two main hurdles these days:
> (a) capacity-limiting jumpers to overcome BIOS problems
> (b) disks larger than 137 GB, the old ATA limit.
> 
> Both problems can be solved with relatively small patches,
> no big monolithic IDE patch required. And I would prefer
> to solve both problems without involving ioctl's, or boot
> parameters, or config parameters. All should just work
> in the common case.
> 
> Andries
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH] clipped disk reports clipped lba size

[Paul Gortmaker]

Andries.Brouwer@cwi.nl wrote:
> 
>     From: Paul Gortmaker <p_gortmaker@yahoo.com>
>
>     Alan has said (quite reasonably) that he is not interested in inclusion
>     of the big IDE patch that exists for 2.2.x -- however, a minimal cut and
>     paste backport from 2.4.x IDE to just support HDIO_DRIVE_CMD_AEB (and thus
>     support setmax) is only about a 100 line diff which I did a while ago.
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^
>     If there is any interest in this I can check it still applies cleanly to
>     current 2.2 pre kernel and send it along for inclusion.
> 
> (1) *_AEB is intended as private namespace for me, not for inclusion
> in an official kernel. So, some official name, like HDIO_DRIVE_TASK,
> must be better.

Yes, DRIVE_TASK is of course the official name - I just knew you would
immediately recognize *_AEB  & and know context/usage :)   BTW, if you
didn't want it in an official kernel, it is a bit late for that. It has
been in for some time in 2.4 .... <grep>  2.3.99pre9 to be exact.

> Both problems can be solved with relatively small patches,
> no big monolithic IDE patch required. And I would prefer

Please see above - I agree, and was suggesting a small patch to 2.2 
rather than the massive one (which Alan already ruled out months ago).

> to solve both problems without involving ioctl's, or boot
> parameters, or config parameters. All should just work
> in the common case.

This is a valid point - in an ideal situation things should just work
without user intervention or setmax util, etc.  What are you currently
recommending to 2.2.x kernel users that come across this limitation
prior to such an ideal fix being released in a 2.2.x kernel?

Paul.

[PATCH] clipped disk reports clipped lba size

[Eduardo Trápani]

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


Since my BIOS does not support my disk (WD400) I had to clip to 33.8G.  At boot time Linux (2.4.17) uses the lba size reported by the disk, that is 33.8 and does not allow me to access the rest of the disk.

The problem is that even though the whole disk can be used after clipping, Linux uses only the reported lba size even if I force the geometry.

Here is a patch to solve that.  I am sure there is a more elegant solution, I guess we could add a "lba_size=" or something like that as a boot parameter.

The patch against 2.4.17 does this:  if the geometry has been forced then use it to calculate the lba size and ignore the (possibly clipped) answer from the disk.

Eduardo.

[-- Attachment #2: patch-clipped-ide.txt --]
[-- Type: text/plain, Size: 552 bytes --]

--- drivers/ide/ide-disk.c.orig	Wed Jan 30 11:26:51 2002
+++ drivers/ide/ide-disk.c	Wed Jan 30 11:30:26 2002
@@ -511,6 +511,12 @@
 
 	drive->select.b.lba = 0;
 
+	/* If the geometry has been forced recalculate lba_capacity */
+	if ((id->capability & 2) && lba_capacity_is_ok(id) &&
+	 drive->forced_geom)
+	{
+		id->lba_capacity = drive->bios_head * drive->bios_sect * drive->bios_cyl;
+	}
 	/* Determine capacity, and use LBA if the drive properly supports it */
 	if ((id->capability & 2) && lba_capacity_is_ok(id)) {
 		capacity = id->lba_capacity;

Re: [PATCH] clipped disk reports clipped lba size

[Alan Cox]

> Here is a patch to solve that.  I am sure there is a more elegant solution, I guess we could add a "lba_size=" or something like that as a boot parameter.
> The patch against 2.4.17 does this:  if the geometry has been forced then use it to calculate the lba size and ignore the (possibly clipped) answer from the disk.

If you get the 2.4.18pre-ac patches that have the new IDE code in them this
should already be covered, along with such nice details as 160Gb disks.

Alan

Re: [PATCH] clipped disk reports clipped lba size

[Andre Hedrick]

On Wed, 30 Jan 2002, Eduardo Trápani wrote:

> 
> Since my BIOS does not support my disk (WD400) I had to clip to 33.8G.  At boot time Linux (2.4.17) uses the lba size reported by the disk, that is 33.8 and does not allow me to access the rest of the disk.
> 
> The problem is that even though the whole disk can be used after clipping, Linux uses only the reported lba size even if I force the geometry.
> 
> Here is a patch to solve that.  I am sure there is a more elegant solution, I guess we could add a "lba_size=" or something like that as a boot parameter.
> 
> The patch against 2.4.17 does this:  if the geometry has been forced then use it to calculate the lba size and ignore the (possibly clipped) answer from the disk.
> 
> Eduardo.

Maybe if 2.4 would ever take a patch ... You would have this option at
compile time.

Regards,

Andre Hedrick
Linux Disk Certification Project                Linux ATA Development