[PATCH 1/3] ide: add "ignore_hpa" module parameter

[PATCH 1/3] ide: add "ignore_hpa" module parameter

[Bartlomiej Zolnierkiewicz]

Add "ignore_hpa" module parameter (modelled after corresponding
libata's parameter) to allow preserving of Host Protected Area.

While at it make some Documentation/kernel-parameters.txt fixes:
- remove stale "idebus=" entry
- s/ide-core/ide_core/g

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
This patchset is for Linus' tree and is destined for 2.6.30-rc8.

 Documentation/kernel-parameters.txt |   10 +++++++---
 drivers/ide/ide-disk.c              |    3 +++
 drivers/ide/ide.c                   |    7 +++++++
 include/linux/ide.h                 |    1 +
 4 files changed, 18 insertions(+), 3 deletions(-)

Index: b/Documentation/kernel-parameters.txt
===================================================================
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -833,14 +833,18 @@ and is between 256 and 4096 characters. 
 	icn=		[HW,ISDN]
 			Format: <io>[,<membase>[,<icn_id>[,<icn_id2>]]]
 
-	ide-core.nodma=	[HW] (E)IDE subsystem
+	ide_core.nodma=	[HW] (E)IDE subsystem
 			Format: =0.0 to prevent dma on hda, =0.1 hdb =1.0 hdc
 			.vlb_clock .pci_clock .noflush .noprobe .nowerr .cdrom
 			.chs .ignore_cable are additional options
 			See Documentation/ide/ide.txt.
 
-	idebus=		[HW] (E)IDE subsystem - VLB/PCI bus speed
-			See Documentation/ide/ide.txt.
+	ide_core.ignore_hpa=
+			[HW] (E)IDE subsystem
+			Ignore HPA limit
+			Format: { "0" | "1" }
+			0 -- keep BIOS limits
+			1 -- ignore limits, using full disk (default)
 
 	ide-pci-generic.all-generic-ide [HW] (E)IDE subsystem
 			Claim all unknown PCI IDE storage controllers.
Index: b/drivers/ide/ide-disk.c
===================================================================
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -330,6 +330,9 @@ static void idedisk_check_hpa(ide_drive_
 			 capacity, sectors_to_MB(capacity),
 			 set_max, sectors_to_MB(set_max));
 
+	if (ide_ignore_hpa == 0)
+		return;
+
 	set_max = idedisk_set_max_address(drive, set_max, lba48);
 
 	if (set_max) {
Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -166,6 +166,13 @@ struct bus_type ide_bus_type = {
 
 EXPORT_SYMBOL_GPL(ide_bus_type);
 
+int ide_ignore_hpa = 1;
+EXPORT_SYMBOL_GPL(ide_ignore_hpa);
+
+module_param_named(ignore_hpa, ide_ignore_hpa, int, 0);
+MODULE_PARM_DESC(ignore_hpa,
+"Ignore HPA limit (0=keep BIOS limits, 1=ignore limits, using full disk)");
+
 int ide_vlb_clk;
 EXPORT_SYMBOL_GPL(ide_vlb_clk);
 
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1072,6 +1072,7 @@ int ide_setting_ioctl(ide_drive_t *, str
 
 int generic_ide_ioctl(ide_drive_t *, struct block_device *, unsigned, unsigned long);
 
+extern int ide_ignore_hpa;
 extern int ide_vlb_clk;
 extern int ide_pci_clk;
 

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Frans Pop]

Bartlomiej Zolnierkiewicz wrote:
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
[...]
> +       ide_core.ignore_hpa=
> +                       [HW] (E)IDE subsystem
> +                       Ignore HPA limit

Add a short explanation of what HPA is maybe (or at least expand the 
acronym)?

It's also not obvious (at least not to a dumb user like me) that "ignore 
HPA limit" is equivalent to "preserving the Host Protected Area" as the 
commit commit says.
After reading the wiki page on HPA it looks like the actual meaning is to
"make the HPA visible to the OS"?

> +                       Format: { "0" | "1" }
> +                       0 -- keep BIOS limits
> +                       1 -- ignore limits, using full disk (default)

Hmm. This looks weird. Is "BIOS limits" a synonym for "HPA limit"?
Same goes for the MODULE_PARM_DESC.

Cheers,
FJP

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Alan Cox]

> It's also not obvious (at least not to a dumb user like me) that "ignore 
> HPA limit" is equivalent to "preserving the Host Protected Area" as the 
> commit commit says.

It isn't - its the exact reverse.

Ignoring the HPA limit tells the kernel to ignore the system BIOS and
firmware set defaults and to stomp the whole disk regardless. On a modern
system thats almost always a really bad idea. Unfortunately on ancient
boxes with disk jumpers set to lie about the disk size (32GB clipping
etc) its the right thing.

Having the same parameter in both stacks seems a good idea but really we
need Tejun's patch exposing the values and then to propogate the hpa
ignore into sysfs and trigger a revalidate of the disk if you change it.
Libata has all the framework for that ready just needing the final bits.
I don't see anything problematic in old IDE also having that interface.

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Frans Pop]

On Tuesday 26 May 2009, Alan Cox wrote:
> > It's also not obvious (at least not to a dumb user like me) that
> > "ignore HPA limit" is equivalent to "preserving the Host Protected
> > Area" as the commit commit says.
>
> It isn't - its the exact reverse.

Right.

> Ignoring the HPA limit tells the kernel to ignore the system BIOS and
> firmware set defaults and to stomp the whole disk regardless. On a
> modern system thats almost always a really bad idea. Unfortunately on
> ancient boxes with disk jumpers set to lie about the disk size (32GB
> clipping etc) its the right thing.
>
> Having the same parameter in both stacks seems a good idea but really
> we need Tejun's patch exposing the values and then to propogate the hpa
> ignore into sysfs and trigger a revalidate of the disk if you change
> it. Libata has all the framework for that ready just needing the final
> bits. I don't see anything problematic in old IDE also having that
> interface.

That also sounds as if it would better protect (or at least inform) 
existing users who have file systems on disks where the HPA is currently 
being ignored.

Seems to me this whole issue would also be worth an LWN (BCCed) article to 
raise awareness, explain the issue, maybe give practical info how to test 
whether you're affected, and maybe add some advice to distributions how 
to handle it. Seems to me like it's something that should be mentioned in 
distribution release notes.

Cheers,
FJP

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Bartlomiej Zolnierkiewicz]

On Tuesday 26 May 2009 01:06:12 Frans Pop wrote:
> On Tuesday 26 May 2009, Alan Cox wrote:
> > > It's also not obvious (at least not to a dumb user like me) that
> > > "ignore HPA limit" is equivalent to "preserving the Host Protected
> > > Area" as the commit commit says.
> >
> > It isn't - its the exact reverse.
> 
> Right.
> 
> > Ignoring the HPA limit tells the kernel to ignore the system BIOS and
> > firmware set defaults and to stomp the whole disk regardless. On a
> > modern system thats almost always a really bad idea. Unfortunately on
> > ancient boxes with disk jumpers set to lie about the disk size (32GB
> > clipping etc) its the right thing.
> >
> > Having the same parameter in both stacks seems a good idea but really
> > we need Tejun's patch exposing the values and then to propogate the hpa
> > ignore into sysfs and trigger a revalidate of the disk if you change
> > it. Libata has all the framework for that ready just needing the final
> > bits. I don't see anything problematic in old IDE also having that
> > interface.
> 
> That also sounds as if it would better protect (or at least inform) 
> existing users who have file systems on disks where the HPA is currently 
> being ignored.
> 
> Seems to me this whole issue would also be worth an LWN (BCCed) article to 
> raise awareness, explain the issue, maybe give practical info how to test 
> whether you're affected, and maybe add some advice to distributions how 
> to handle it. Seems to me like it's something that should be mentioned in 
> distribution release notes.

Ironically, some distributions were a well aware of the problem, yet they
chose to ignore it *twice* over the last few years.

First time with the introduction of the default IDE HPA behavior of always
giving the access to the full capacity of a disk and leaving decisions about
HPA preservation/removal up to the installer & user.

Second time when said distributions switched from IDE to libata (which uses
the different default behavior as it limits the capacity to non-HPA part).

Please note that no fancy sysfs support was needed to prevent the problem:
both stacks (ide & libata) support HDIO_DRIVE_TASK ioctl which can be used
to execute commands needed to retrieve/change HPA setting.

Moreover the (much needed) work from Tejun doesn't help a tiny bit in case
of people migrating their *working* setups from IDE to libata (at least
in distro upgrade case this could have been handled by distro installer but
see above) and hitting the compatibility issue mentioned in bug #13365.

Anyway I'm putting all HPA fixes on hold as I have enough (thanks for your
feedback on patches - all points are valid and I will address them if I ever
take patches out of freezer).

Thanks.
Bart

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Frans Pop]

On Tuesday 26 May 2009, Bartlomiej Zolnierkiewicz wrote:
> First time with the introduction of the default IDE HPA behavior of
> always giving the access to the full capacity of a disk and leaving
> decisions about HPA preservation/removal up to the installer & user.
>
> Second time when said distributions switched from IDE to libata (which
> uses the different default behavior as it limits the capacity to
> non-HPA part).

Has there really been something like a deliberate switch?

In my experience with Debian there's been no such thing. ATA has just been 
gradually enabled. (And initially that resulted in quite a mess because 
some devices would be supported by two drivers and the kernel couldn't 
even make up its own mind which one it preferred, resulting in random 
switches between drivers. At least that was resolved at some point.)

However, I must admit that Debian has not handled the IDE-ATA switch very 
well. So far users have mostly been left to deal with any fallout for 
themselves.

> Please note that no fancy sysfs support was needed to prevent the
> problem: both stacks (ide & libata) support HDIO_DRIVE_TASK ioctl which
> can be used to execute commands needed to retrieve/change HPA setting.

I don't really see how that would help. The fact that fancy ioctls exist 
does not mean users or even distros have the knowledge needed to use 
them. IMHO such issues should be dealt with at the level where that 
knowledge is available: the kernel community.

> Moreover the (much needed) work from Tejun doesn't help a tiny bit in
> case of people migrating their *working* setups from IDE to libata

>From what I've seen now that seems the most important case that needs 
supporting, as well as the case where a working setup would migrate from 
IDE with old HPA default to IDE with new HPA default.

Naive thought: wouldn't it be possible to check whether a partition 
overlaps the HPA boundary and warn or even bug out in that case?

> (at least in distro upgrade case this could have been handled by distro
> installer but see above) and hitting the compatibility issue mentioned
> in bug #13365.

At least in the case of Debian there is no such thing as a "distro 
installer" involved in upgrades. Debian only uses its installer for new 
installs, not for upgrades between releases. The last is purely done by 
regular package management.

It's also not sufficient to rely on that anyway because users sometimes do 
install newer upstream kernels (whether because of issues or to help with 
kernel development/testing), which means no distro upgrade is involved.

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Alan Cox]

> Naive thought: wouldn't it be possible to check whether a partition 
> overlaps the HPA boundary and warn or even bug out in that case?

No - believe me we all thought about that. The PC partition format has
starts but not ends so you need detailed internal file system knowledge
to play guessing games (which thus needs to be user space). Some of the
non PC formats use the middle or end of disk so if you don't know the
size you can't *find* the partition data.

Alan

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Andries E. Brouwer]

On Tue, May 26, 2009 at 02:01:32PM +0100, Alan Cox wrote:
> > Naive thought: wouldn't it be possible to check whether a partition 
> > overlaps the HPA boundary and warn or even bug out in that case?
> 
> No - believe me we all thought about that. The PC partition format has
> starts but not ends so you need detailed internal file system knowledge
> to play guessing games (which thus needs to be user space). Some of the
> non PC formats use the middle or end of disk so if you don't know the
> size you can't *find* the partition data.
> 
> Alan

Hi Alan,

Probably I am missing some context, but the above seems to be
a false statement. The PC partition format does give you
start and size, and no filesystem knowledge is needed to
find out where a partition ends.

Andries

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Alan Cox]

> Probably I am missing some context, but the above seems to be
> a false statement. The PC partition format does give you
> start and size, and no filesystem knowledge is needed to
> find out where a partition ends.

I am indeed in error. The PC DOS format is indeed fine. I was
remembering from the magneto-opticals and the problem there was you
couldn't tell the size because there were two conflicting standards for
partition encoding.

So that means a distro can detect some of the cases which is even better
and makes a sysfs interface even more useful.

Alan

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Bartlomiej Zolnierkiewicz]

On Tuesday 26 May 2009 14:53:21 Frans Pop wrote:
> On Tuesday 26 May 2009, Bartlomiej Zolnierkiewicz wrote:
> > First time with the introduction of the default IDE HPA behavior of
> > always giving the access to the full capacity of a disk and leaving
> > decisions about HPA preservation/removal up to the installer & user.
> >
> > Second time when said distributions switched from IDE to libata (which
> > uses the different default behavior as it limits the capacity to
> > non-HPA part).
> 
> Has there really been something like a deliberate switch?

This heavily depends on distribution.

> In my experience with Debian there's been no such thing. ATA has just been 
> gradually enabled. (And initially that resulted in quite a mess because 
> some devices would be supported by two drivers and the kernel couldn't 
> even make up its own mind which one it preferred, resulting in random 
> switches between drivers. At least that was resolved at some point.)
> 
> However, I must admit that Debian has not handled the IDE-ATA switch very 
> well. So far users have mostly been left to deal with any fallout for 
> themselves.

I think that Debian still sticks to IDE (which is correct choice given
goals of this distribution).

> > Please note that no fancy sysfs support was needed to prevent the
> > problem: both stacks (ide & libata) support HDIO_DRIVE_TASK ioctl which
> > can be used to execute commands needed to retrieve/change HPA setting.
> 
> I don't really see how that would help. The fact that fancy ioctls exist 
> does not mean users or even distros have the knowledge needed to use 
> them. IMHO such issues should be dealt with at the level where that 
> knowledge is available: the kernel community.

The coordination from distributions was necessary and the answer from
some distributions was to continuously ignore or downplay the issue.

This has changed only recently with Tejun's work.

> > Moreover the (much needed) work from Tejun doesn't help a tiny bit in
> > case of people migrating their *working* setups from IDE to libata
> 
> From what I've seen now that seems the most important case that needs 
> supporting, as well as the case where a working setup would migrate from 
> IDE with old HPA default to IDE with new HPA default.

There is no IDE with new HPA default at the moment.

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Robert Hancock]

Frans Pop wrote:
>> Moreover the (much needed) work from Tejun doesn't help a tiny bit in
>> case of people migrating their *working* setups from IDE to libata
> 
> From what I've seen now that seems the most important case that needs 
> supporting, as well as the case where a working setup would migrate from 
> IDE with old HPA default to IDE with new HPA default.
> 
> Naive thought: wouldn't it be possible to check whether a partition 
> overlaps the HPA boundary and warn or even bug out in that case?

It does, you get a warning about the partition exceeding device 
capacity. Easy to miss that, however.

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Alan Cox]

> Please note that no fancy sysfs support was needed to prevent the problem:
> both stacks (ide & libata) support HDIO_DRIVE_TASK ioctl which can be used
> to execute commands needed to retrieve/change HPA setting.

Wrong. You need sysfs support because you may not be able to retrieve the
HPA settings after the drive has removed them. You also need a trigger of
some form because the block layer needs to see the volume resized and it
doesn't snoop the innards of the ATA command structure.
 
> Moreover the (much needed) work from Tejun doesn't help a tiny bit in case
> of people migrating their *working* setups from IDE to libata (at least
> in distro upgrade case this could have been handled by distro installer but
> see above) and hitting the compatibility issue mentioned in bug #13365.

Fedora handled it with clear documentation at the time. I'm not sure how
other distros handled it back then but given the patch was written by
Ubuntu folk I suspect they knew about it too 8)

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Bartlomiej Zolnierkiewicz]

On Tuesday 26 May 2009 14:53:43 Alan Cox wrote:
> > Please note that no fancy sysfs support was needed to prevent the problem:
> > both stacks (ide & libata) support HDIO_DRIVE_TASK ioctl which can be used
> > to execute commands needed to retrieve/change HPA setting.
> 
> Wrong. You need sysfs support because you may not be able to retrieve the
> HPA settings after the drive has removed them. You also need a trigger of

All information was there (kernel logs).

Moreover the default in libata is to not remove HPA settings.

I don't recommend parsing kernels logs in search of such information but
you're are stretching the reality too far to match with your arguments.

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Alan Cox]

> All information was there (kernel logs).
> 
> Moreover the default in libata is to not remove HPA settings.
> 
> I don't recommend parsing kernels logs in search of such information but
> you're are stretching the reality too far to match with your arguments.

So you disagree that sysfs is needed and propose an alternative that you
say you don't recommend (and which doesn't solve the block problem).

I used to work for a distro, and telling Bill Nottingham that the
reliable long term way for Fedora to obtain some interface data was by
grepping dmesg wouldn't have gone down well.

Alan

Re: [PATCH 1/3] ide: add "ignore_hpa" module parameter

[Bartlomiej Zolnierkiewicz]

On Wednesday 27 May 2009 12:38:57 Alan Cox wrote:
> > All information was there (kernel logs).
> > 
> > Moreover the default in libata is to not remove HPA settings.
> > 
> > I don't recommend parsing kernels logs in search of such information but
> > you're are stretching the reality too far to match with your arguments.
> 
> So you disagree that sysfs is needed and propose an alternative that you
> say you don't recommend (and which doesn't solve the block problem).

This is not what I said (if this is a straw man attempt, please don't do it).

> I used to work for a distro, and telling Bill Nottingham that the
> reliable long term way for Fedora to obtain some interface data was by
> grepping dmesg wouldn't have gone down well.

Knowing the severity of accidentally removing data from HPA (which you
described yourself in the other mail) and also not being able to access
valid data from HPA (bug #13365) I would simply use in the installer
the alternative [*] (that I *don't* recommend as the long-term solution)
and then start working on adding proper sysyfs kernel interfaces (that
I *don't* consider not needed) and proper installer support.

[*] it would of course require passing "libata.ignore_hpa=1" to kernel
    during installation time

This would result in much less hassle for users, much less support costs
for distribution and having proper in-kernel / installer support faster
than doing it the non-pragmatic way (since we would save a lot of time on
handling bug-reports and discussions like this one).