[RFT] libata hpa support

[RFT] libata hpa support

[Kyle McMartin]

Rough first cut at Host Protected Area support for libata. Buyer beware, I
can't guarantee it won't club baby seals or kick your cat... In particular,
the non-LBA48 codepath hasn't been tested by me...

A few questions are raised, libata SATA hasn't been resizing disks in the
past, should we only disable HPA on PATA only, a la drivers/ide? In any
event, supporting HPA lets people who had partitions which extend beyond
the protected area boot with libata PATA. Some policy decision needs to be
made too... Right now, I'm just disabling it unilaterally.

I've added 0 && to the patch where we would attempt to resize the disk to
mitigate the risk testing the reading portion of the code. I don't have a
laptop with a jumpered setting, so I've been testing by halving the disk size
as you can see in the #if 0 section of code. It seems to work and recover
alright. The SET_MAX_ADDRESS{,_EXT} is temporary, as when I reboot and look
at the disk I've been testing with, it's still full sized.

I'd appreciate a few more eyes on the code, and maybe even a tester
or two. (The box I'm beating on to test this has buggy ACPI, so I've not
had the opportunity to test resume, for instance.)

In particular, the taskfiles could use a perusing, I've noticed on my
box when the ICH8 controller is in AHCI mode, everything is fine, but when
driving it with ata_piix, the first read_native_max returns something that is
incorrect (but successive attempts work... very bizarre.)

Cheers,
	Kyle M.

Signed-off-by: Kyle McMartin <kyle@canonical.com>

---
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 2cf8251..c3da3ea 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -804,6 +804,182 @@ void ata_id_c_string(const u16 *id, unsigned char *s,
 	*p = '\0';
 }
 
+static u64 ata_tf_to_lba48(struct ata_taskfile *tf)
+{
+	u64 sectors = 0;
+
+	sectors |= ((u64)(tf->hob_lbah & 0xff)) << 40;
+	sectors |= ((u64)(tf->hob_lbam & 0xff)) << 32;
+	sectors |= (tf->hob_lbal & 0xff) << 24;
+	sectors |= (tf->lbah & 0xff) << 16;
+	sectors |= (tf->lbam & 0xff) << 8;
+	sectors |= (tf->lbal & 0xff);
+
+	return ++sectors;
+}
+
+static u64 ata_tf_to_lba(struct ata_taskfile *tf)
+{
+	u64 sectors = 0;
+
+	sectors |= (tf->device & 0xff) << 24;
+	sectors |= (tf->lbah & 0xff) << 16;
+	sectors |= (tf->lbam & 0xff) << 8;
+	sectors |= (tf->lbal & 0xff);
+
+	return ++sectors;
+}
+
+static u64 ata_read_native_max_address_ext(struct ata_device *dev)
+{
+	unsigned int err;
+	struct ata_taskfile tf;
+
+	ata_tf_init(dev, &tf);
+
+	tf.command = ATA_CMD_READ_NATIVE_MAX_EXT;
+	tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR;
+	tf.protocol |= ATA_PROT_NODATA;
+	tf.device = 0x40;
+
+	err = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
+	if (err)
+		return 0;
+
+	return ata_tf_to_lba48(&tf);
+}
+
+static u64 ata_read_native_max_address(struct ata_device *dev)
+{
+	unsigned int err;
+	struct ata_taskfile tf;
+
+	ata_tf_init(dev, &tf);
+
+	tf.command = ATA_CMD_READ_NATIVE_MAX;
+	tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR;
+	tf.protocol |= ATA_PROT_NODATA;
+	tf.device = 0x40;
+
+	err = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
+	if (err)
+		return 0;
+
+	return ata_tf_to_lba(&tf);
+}
+
+static u64 ata_set_native_max_address_ext(struct ata_device *dev, u64 new_sectors)
+{
+	unsigned int err;
+	struct ata_taskfile tf;
+
+	new_sectors--;
+
+	ata_tf_init(dev, &tf);
+
+	tf.command = ATA_CMD_SET_MAX_EXT;
+	tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR;
+	tf.protocol |= ATA_PROT_NODATA;
+	tf.device = 0x40;
+
+	tf.lbal = (new_sectors >> 0) & 0xff;
+	tf.lbam = (new_sectors >> 8) & 0xff;
+	tf.lbah = (new_sectors >> 16) & 0xff;
+
+	tf.hob_lbal = (new_sectors >> 24) & 0xff;
+	tf.hob_lbam = (new_sectors >> 32) & 0xff;
+	tf.hob_lbah = (new_sectors >> 40) & 0xff;
+
+	err = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
+	if (err)
+		return 0;
+
+	return ata_tf_to_lba48(&tf);
+}
+
+static u64 ata_set_native_max_address(struct ata_device *dev, u64 new_sectors)
+{
+	unsigned int err;
+	struct ata_taskfile tf;
+
+	new_sectors--;
+
+	ata_tf_init(dev, &tf);
+
+	tf.command = ATA_CMD_SET_MAX;
+	tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR;
+	tf.protocol |= ATA_PROT_NODATA;
+
+	tf.lbal = (new_sectors >> 0) & 0xff;
+	tf.lbam = (new_sectors >> 8) & 0xff;
+	tf.lbah = (new_sectors >> 16) & 0xff;
+	tf.device = ((new_sectors >> 24) & 0xff) | 0x40;
+
+	err = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
+	if (err)
+		return 0;
+
+	return ata_tf_to_lba(&tf);
+}
+
+static u64 ata_hpa_resize(struct ata_device *dev)
+{
+	u64 sectors = dev->n_sectors;
+	u64 hpa_sectors;
+
+	if (ata_id_has_lba48(dev->id))
+		hpa_sectors = ata_read_native_max_address_ext(dev);
+	else
+		hpa_sectors = ata_read_native_max_address(dev);
+
+#if 0 /* testing, one, two, three... */
+	/* if no hpa, both should be equal */
+	ata_dev_printk(dev, KERN_INFO, "%s 1: sectors = %lld, hpa_sectors = %lld\n",
+		__FUNCTION__, sectors, hpa_sectors);
+
+	if (hpa_sectors == sectors) {
+		if (ata_id_has_lba48(dev->id))
+			hpa_sectors = ata_set_native_max_address_ext(dev, sectors >>= 1);
+		else
+			hpa_sectors = ata_set_native_max_address(dev, sectors >>= 1);
+	}
+
+	/* we just halved the disk, both should be equal */
+	ata_dev_printk(dev, KERN_INFO, "%s 2: sectors = %lld, hpa_sectors = %lld\n",
+		__FUNCTION__, sectors, hpa_sectors);
+
+	if (ata_id_has_lba48(dev->id))
+		hpa_sectors = ata_read_native_max_address_ext(dev);
+	else
+		hpa_sectors = ata_read_native_max_address(dev);
+
+	/* read native max, hpa_sectors should be original size */
+	ata_dev_printk(dev, KERN_INFO, "%s 3: sectors = %lld, hpa_sectors = %lld\n",
+		__FUNCTION__, sectors, hpa_sectors);
+#endif
+
+	if (hpa_sectors > sectors) {
+		ata_dev_printk(dev, KERN_INFO,
+			"Host Protected Area detected:\n"
+			"\tcurrent size: %lld sectors\n"
+			"\tnative size: %lld sectors\n",
+			sectors, hpa_sectors);
+
+		if (ata_id_has_lba48(dev->id))
+			hpa_sectors = ata_set_native_max_address_ext(dev, hpa_sectors);
+		else
+			hpa_sectors = ata_set_native_max_address(dev, hpa_sectors);
+
+		if (hpa_sectors) {
+			ata_dev_printk(dev, KERN_INFO,
+				"native size increased to %lld sectors\n", hpa_sectors);
+			return hpa_sectors;
+		}
+	}
+
+	return sectors;
+}
+
 static u64 ata_id_n_sectors(const u16 *id)
 {
 	if (ata_id_has_lba(id)) {
@@ -1630,6 +1806,9 @@ int ata_dev_configure(struct ata_device *dev)
 					dev->flags |= ATA_DFLAG_FLUSH_EXT;
 			}
 
+			if (ata_id_hpa_enabled(dev->id))
+				dev->n_sectors = ata_hpa_resize(dev);
+
 			/* config NCQ */
 			ata_dev_config_ncq(dev, ncq_desc, sizeof(ncq_desc));
 
diff --git a/include/linux/ata.h b/include/linux/ata.h
index 272736e..38113ba 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -158,6 +158,8 @@ enum {
 	ATA_CMD_INIT_DEV_PARAMS	= 0x91,
 	ATA_CMD_READ_NATIVE_MAX	= 0xF8,
 	ATA_CMD_READ_NATIVE_MAX_EXT = 0x27,
+	ATA_CMD_SET_MAX		= 0xF9,
+	ATA_CMD_SET_MAX_EXT	= 0x37,
 	ATA_CMD_READ_LOG_EXT	= 0x2f,
 
 	/* READ_LOG_EXT pages */

Re: [RFT] libata hpa support

[Alan]

> A few questions are raised, libata SATA hasn't been resizing disks in the
> past, should we only disable HPA on PATA only, a la drivers/ide? In any

drivers/ide disable HPA on SATA ports it manages too.

> event, supporting HPA lets people who had partitions which extend beyond
> the protected area boot with libata PATA. Some policy decision needs to be
> made too... Right now, I'm just disabling it unilaterally.

Last time we had a Red Hat bitching session about it the main thing the
installer guys wanted was a way to find out what size the disk thought it
was before we twiddled with it, so that they can make an intelligent
attempt to handle partitioning in the case where for example a disk is
unpartitioned. A partitioned disk isn't so much of a problem as the
partitioning itself tells you the a lot about the state of disk and
intended HPA.

What do the Ubuntu installer/partitioning folk think ?

Alan

Re: [RFT] libata hpa support

[Colin Watson]

Alan wrote:
> > A few questions are raised, libata SATA hasn't been resizing disks in the
> > past, should we only disable HPA on PATA only, a la drivers/ide? In any
> 
> drivers/ide disable HPA on SATA ports it manages too.
> 
> > event, supporting HPA lets people who had partitions which extend beyond
> > the protected area boot with libata PATA. Some policy decision needs to be
> > made too... Right now, I'm just disabling it unilaterally.
> 
> Last time we had a Red Hat bitching session about it the main thing the
> installer guys wanted was a way to find out what size the disk thought it
> was before we twiddled with it, so that they can make an intelligent
> attempt to handle partitioning in the case where for example a disk is
> unpartitioned. A partitioned disk isn't so much of a problem as the
> partitioning itself tells you the a lot about the state of disk and
> intended HPA.
> 
> What do the Ubuntu installer/partitioning folk think ?

Suspiciously similar to the Red Hat installer folks. :-) Our partitioner
(and Debian's) asks libparted to create a partition table in memory
(ped_disk_new_fresh) and then works entirely based on what libparted
reports about that partition table before actually committing it. This
obviously only works if you can find out the size of the disk before
writing anything to it.

libparted uses ioctl(BLKGETSIZE[64]) to get the size of the disk at the
moment. Making that report the non-HPA size would be enough to make
libparted create new partition tables properly, but it would defeat the
purpose of Kyle's patch in that you probably couldn't use libparted to
work with existing partition tables extending beyond the HPA. Adding say
ioctl(BLKGETNONHPASIZE) or something and teaching libparted to create
new partition tables excluding the HPA but still be able to read
existing partition tables extending into the HPA seems like it'd be the
best approach, and sounds pretty straightforward. I'd be happy to do the
libparted side of that.

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

Re: [RFT] libata hpa support

[Kyle McMartin]

On Fri, Feb 23, 2007 at 06:19:59PM +0000, Alan wrote:
> > A few questions are raised, libata SATA hasn't been resizing disks in the
> > past, should we only disable HPA on PATA only, a la drivers/ide? In any
> 
> drivers/ide disable HPA on SATA ports it manages too.
> 

Right, so we can leave the policy to be just disabling it by 40p/80p cable
type for now... Sigh.

> > event, supporting HPA lets people who had partitions which extend beyond
> > the protected area boot with libata PATA. Some policy decision needs to be
> > made too... Right now, I'm just disabling it unilaterally.
> 
> Last time we had a Red Hat bitching session about it the main thing the
> installer guys wanted was a way to find out what size the disk thought it
> was before we twiddled with it, so that they can make an intelligent
> attempt to handle partitioning in the case where for example a disk is
> unpartitioned. A partitioned disk isn't so much of a problem as the
> partitioning itself tells you the a lot about the state of disk and
> intended HPA.
> 

I suspect Fedora is getting fairly late in the F7 development cycle and this
kind of intrusive change to the installer, etc., would be frowned upon?

One thought I had was to read the native max size, stow it away somewhere,
and not resize the drive. [Disclaimer, I know almost nothing about the block
layer at this point...] Then, when a bio gets rejected by the drive because
the LBA is out of range, check if it's between the native capacity, and the
current capacity, if it is, disable HPA and re-submit.

That approach, while hackish, might be sufficient to keep people who've been
using drivers/ide from being unable to boot if they're got partitions
overlapping the HPA.

Cheers,
	Kyle M.

Re: [RFT] libata hpa support

[Jeff Garzik]

Alan wrote:
>> Right, so we can leave the policy to be just disabling it by 40p/80p cable
>> type for now... Sigh.
> 
> The only compatible policy is always act on it.

Agreed.

	Jeff

Re: [RFT] libata hpa support

[Alan]

> Right, so we can leave the policy to be just disabling it by 40p/80p cable
> type for now... Sigh.

The only compatible policy is always act on it.

> I suspect Fedora is getting fairly late in the F7 development cycle and this
> kind of intrusive change to the installer, etc., would be frowned upon?

HPA is needed by FC7, which is one reason I'm glad you are doing it ;)

Re: [RFT] libata hpa support

[Tejun Heo]

Jeff Garzik wrote:
> Alan wrote:
>>> Right, so we can leave the policy to be just disabling it by 40p/80p
>>> cable
>>> type for now... Sigh.
>>
>> The only compatible policy is always act on it.
> 
> Agreed.

The problem here is that "always acting" is incompatible with the
"current behavior".  So, we're screwed either way.

-- 
tejun

Re: [RFT] libata hpa support

[Kyle McMartin]

On Fri, Feb 23, 2007 at 12:06:25PM -0500, Kyle McMartin wrote:
> 

Bummer. It appears to fail horribly on a MacBook. Unfortunately I have
no clue what's special (but I assume something to do with EFI bringup) about
them, nor any hardware. Alan, Jeff, any ideas?

Cheers,
	Kyle

Re: [RFT] libata hpa support

[Matthew Garrett]

On Wed, Mar 21, 2007 at 06:34:55PM +0000, Alan Cox wrote:

> I'm testing a variant of it at the moment with no problems on x86. My
> first thought would be to check for endianness problems and the like but
> I don't see any.

Macbooks are x86.

> It may also fail to allow any resizing if the Macbook issues a security
> freeze from the firmware before the OS boots, if so we won't be able to
> resize the HPA or set/change passwords on the disk. If they are doing
> that then its a fairly sensible security choice.

https://launchpad.net/ubuntu/+source/linux-source-2.6.20/+bug/91940 
gives the traces we have.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFT] libata hpa support

[Jeff Garzik]

Alan Cox wrote:
> It may also fail to allow any resizing if the Macbook issues a security
> freeze from the firmware before the OS boots, if so we won't be able to
> resize the HPA or set/change passwords on the disk. If they are doing
> that then its a fairly sensible security choice.


That reminds me, there have been suggestions in the past that we should 
do a security freeze after probing and configuring.

	Jeff

Re: [RFT] libata hpa support

[Matthew Garrett]

On Wed, Mar 21, 2007 at 06:51:59PM +0000, Alan Cox wrote:
> > https://launchpad.net/ubuntu/+source/linux-source-2.6.20/+bug/91940 
> > gives the traces we have.
> 
> Wrong bug number ? - seems to be nothing there that looks HPA related at
> all

That's with Kyle's HPA code - reverting it results in that failure 
vanishing.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFT] libata hpa support

[Jeff Garzik]

Alan Cox wrote:
>> That reminds me, there have been suggestions in the past that we should 
>> do a security freeze after probing and configuring.
> 
> And as has been observed previously from a security perspective there is
> no point.
> 
> 	Break into box
> 	security freeze - annoying
> 	Patch boot block to load my disk destroyer
> 	Reboot
> 
> You need to the security freeze in the firmware at boot, or it is the
> same whether you do it in kernel or in the initrd or early boot, except
> that its pagable code, its configurable and its easier to work with when
> it is in user space.
> 
> Paranoid people get PCI boot roms that lock their disks.

Certainly.

But I argue that doing it late is better than not doing it at all.

	Jeff

Re: [RFT] libata hpa support

[Jeff Garzik]

Alan Cox wrote:
> And distributions (smart ones anyway) already do it in the initrd which
> is the correct place for this.


Do you have a list of distributions that do this?

	Jeff

Re: [RFT] libata hpa support

[Alan Cox]

On Wed, 21 Mar 2007 09:01:41 -0400
Kyle McMartin <kyle@canonical.com> wrote:

> On Fri, Feb 23, 2007 at 12:06:25PM -0500, Kyle McMartin wrote:
> > 
> 
> Bummer. It appears to fail horribly on a MacBook. Unfortunately I have
> no clue what's special (but I assume something to do with EFI bringup) about
> them, nor any hardware. Alan, Jeff, any ideas?

I'm testing a variant of it at the moment with no problems on x86. My
first thought would be to check for endianness problems and the like but
I don't see any.

It may also fail to allow any resizing if the Macbook issues a security
freeze from the firmware before the OS boots, if so we won't be able to
resize the HPA or set/change passwords on the disk. If they are doing
that then its a fairly sensible security choice.

Re: [RFT] libata hpa support

[Alan Cox]

> That reminds me, there have been suggestions in the past that we should 
> do a security freeze after probing and configuring.

And as has been observed previously from a security perspective there is
no point.

	Break into box
	security freeze - annoying
	Patch boot block to load my disk destroyer
	Reboot

You need to the security freeze in the firmware at boot, or it is the
same whether you do it in kernel or in the initrd or early boot, except
that its pagable code, its configurable and its easier to work with when
it is in user space.

Paranoid people get PCI boot roms that lock their disks.

Re: [RFT] libata hpa support

[Alan Cox]

> https://launchpad.net/ubuntu/+source/linux-source-2.6.20/+bug/91940 
> gives the traces we have.

Wrong bug number ? - seems to be nothing there that looks HPA related at
all

Re: [RFT] libata hpa support

[Alan Cox]

> > Paranoid people get PCI boot roms that lock their disks.
> 
> Certainly.
> 
> But I argue that doing it late is better than not doing it at all.

And distributions (smart ones anyway) already do it in the initrd which
is the correct place for this.