Working around bogus HPAs in libata
Since SCSI has no concept of the Host Protected Area (HPA) supported by ATA, ATA disks handled by libata have no set_capacity() operation and it appears to be impossible to override an HPA except through the libata module parameter.
In particular, this means that the workaround for bogus HPAs in rescan_partitions() does not work:
if (bdops->set_capacity &&
(disk->flags & GENHD_FL_NATIVE_CAPACITY) == 0) {
printk(KERN_CONT "enabling native capacity\n");
capacity = bdops->set_capacity(disk, ~0ULL);
disk->flags |= GENHD_FL_NATIVE_CAPACITY;
if (capacity > get_capacity(disk)) {
set_capacity(disk, capacity);
check_disk_size_change(disk, bdev);
bdev->bd_invalidated = 0;
}
...This creates a serious problem when transitioning from IDE drivers to libata-based drivers: partitions may disappear or be truncated. For example, see <http://bugs.debian.org/574795>.
One workaround is to ignore the HPA by default, but this can result in data loss:
<https://bugs.launchpad.net/ubuntu/+source/linux/+bug/380138>.
I think that sd should implement set_capacity() and somehow pass through requests to libata. This could perhaps be done using the new ATA PASS-THROUGH commands in the SATL, but I regret that I do not know enough about either ATA or SCSI to produce a patch.
Ben.
Signature · 2 lines
Ben Hutchings If you seem to know what you are doing, you'll be given more to do.
Quoted 8 lines · Ben Hutchings
Since SCSI has no concept of the Host Protected Area (HPA) supported by ATA, ATA disks handled by libata have no
set_capacity()operation and it appears to be impossible to override an HPA except through the libata module parameter.In particular, this means that the workaround for bogus HPAs in
rescan_partitions()does not work:
Quoted 10 lines
if (bdops->set_capacity &&
(disk->flags & GENHD_FL_NATIVE_CAPACITY) == 0) {
printk(KERN_CONT "enabling native capacity\n");
capacity = bdops->set_capacity(disk, ~0ULL);
disk->flags |= GENHD_FL_NATIVE_CAPACITY;
if (capacity > get_capacity(disk)) {
set_capacity(disk, capacity);
check_disk_size_change(disk, bdev);
bdev->bd_invalidated = 0;
}Hmmm, yeah, this is much better than the dumb switch we have now. I wonder why nobody pointed it out before. Does the following patch work as expected?
drivers/ata/libata-core.c +45 -29
@@ -1493,6 +1493,7 @@ static int ata_hpa_resize(struct ata_device *dev) {struct ata_eh_context *ehc = &dev->link->eh_context;int print_info = ehc->i.flags &ATA_EHI_PRINTINFO ; +bool unlock_hpa = ata_ignore_hpa || dev->flags &ATA_DFLAG_UNLOCK_HPA ;u64 sectors =ata_id_n_sectors (dev->id);u64 native_sectors;int rc;
@@ -1509,7 +1510,7 @@ static int ata_hpa_resize(struct ata_device *dev)/* If device aborted the command or HPA isn't going to * be unlocked, skip HPA resizing. */ -if (rc == -EACCES || !ata_ignore_hpa) { +if (rc == -EACCES || !unlock_hpa) {ata_dev_printk (dev,KERN_WARNING ,"HPA support seems " "broken, skipping HPA handling\n" ); dev->horkage |=ATA_HORKAGE_BROKEN_HPA ;
@@ -1524,7 +1525,7 @@ static int ata_hpa_resize(struct ata_device *dev) dev->n_native_sectors = native_sectors;/* nothing to do? */ -if (native_sectors <= sectors || !ata_ignore_hpa) { +if (native_sectors <= sectors || !unlock_hpa) {if (!print_info || native_sectors == sectors)return 0 ;
@@ -4185,36 +4186,50 @@ int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class, goto fail;/* verify n_sectors hasn't changed */ -if (dev->class ==ATA_DEV_ATA && n_sectors && - dev->n_sectors != n_sectors) { -ata_dev_printk (dev,KERN_WARNING ,"n_sectors mismatch " -"%llu != %llu\n" , - (unsigned long long )n_sectors, - (unsigned long long )dev->n_sectors); -/* -* Something could have caused HPA to be unlocked -* involuntarily. If n_native_sectors hasn't changed -* and the new size matches it, keep the device. -*/ -if (dev->n_native_sectors == n_native_sectors && - dev->n_sectors > n_sectors && - dev->n_sectors == n_native_sectors) { -ata_dev_printk (dev,KERN_WARNING , -"new n_sectors matches native, probably " -"late HPA unlock, continuing\n" ); -/* keep using the old n_sectors */ - dev->n_sectors = n_sectors; - }else { -/* restore original n_[native]_sectors and fail */ - dev->n_native_sectors = n_native_sectors; - dev->n_sectors = n_sectors; - rc = -ENODEV ; - goto fail; - } +if (dev->class !=ATA_DEV_ATA || !n_sectors || + dev->n_sectors == n_sectors) +return 0 ; + +/* n_sectors has changed */ +ata_dev_printk (dev,KERN_WARNING ,"n_sectors mismatch %llu != %llu\n" , + (unsigned long long )n_sectors, + (unsigned long long )dev->n_sectors); + +/* +* Something could have caused HPA to be unlocked +* involuntarily. If n_native_sectors hasn't changed and the +* new size matches it, keep the device. +*/ +if (dev->n_native_sectors == n_native_sectors && + dev->n_sectors > n_sectors && dev->n_sectors == n_native_sectors) { +ata_dev_printk (dev,KERN_WARNING , +"new n_sectors matches native, probably " +"late HPA unlock, n_sectors adjusted\n" ); +/* keep using the larger n_sectors */ +return 0 ; } -return 0 ; +/* +* Some BIOSes boot w/o HPA but resume w/ HPA locked. Try +* unlocking HPA in those cases. +* +* https://bugzilla.kernel.org/show_bug.cgi?id=15396 +*/ +if (dev->n_native_sectors == n_native_sectors && + dev->n_sectors < n_sectors && n_sectors == n_native_sectors && + !(dev->horkage &ATA_HORKAGE_BROKEN_HPA )) { +ata_dev_printk (dev,KERN_WARNING , +"old n_sectors matches native, probably " +"late HPA lock, will try to unlock HPA\n" ); +/* try unlocking HPA */ + dev->flags |=ATA_DFLAG_UNLOCK_HPA ; + rc = -EIO ; + }else + rc = -ENODEV ; +/* restore original n_[native_]sectors and fail */ + dev->n_native_sectors = n_native_sectors; + dev->n_sectors = n_sectors; fail:ata_dev_printk (dev,KERN_ERR ,"revalidation failed (errno=%d)\n" , rc);return rc;
@@ -6785,6 +6800,7 @@ EXPORT_SYMBOL_GPL(ata_dummy_port_info);EXPORT_SYMBOL_GPL (ata_link_next);EXPORT_SYMBOL_GPL (ata_dev_next);EXPORT_SYMBOL_GPL (ata_std_bios_param); +EXPORT_SYMBOL_GPL (ata_scsi_set_capacity);EXPORT_SYMBOL_GPL (ata_host_init);EXPORT_SYMBOL_GPL (ata_host_alloc);EXPORT_SYMBOL_GPL (ata_host_alloc_pinfo);
drivers/ata/libata-scsi.c +26 -0
@@ -413,6 +413,32 @@ int ata_std_bios_param(struct scsi_device *sdev, struct block_device *bdev,return 0 ; } +sector_t ata_scsi_set_capacity (struct scsi_device *sdev,sector_t new_capacity) +{ +struct ata_port *ap =ata_shost_to_port (sdev->host); +sector_t capacity =0 ; +struct ata_device *dev; +unsigned long flags; + +spin_lock_irqsave (ap->lock, flags); + + dev =ata_scsi_find_dev (ap, sdev); +if (dev && dev->n_sectors < new_capacity && + dev->n_sectors < dev->n_native_sectors) { +struct ata_eh_info *ehi = &dev->link->eh_info; + + dev->flags |=ATA_DFLAG_UNLOCK_HPA ; + ehi->action |=ATA_EH_RESET ; +ata_port_schedule_eh (ap); +spin_unlock_irqrestore (ap->lock, flags); +ata_port_wait_eh (ap); + capacity = dev->n_sectors; + }else +spin_unlock_irqrestore (ap->lock, flags); + +return capacity; +} +/** * ata_get_identity - Handler for HDIO_GET_IDENTITY ioctl * @ap: target port
drivers/scsi/sd.c +14 -0
@@ -96,6 +96,8 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC);#endif static int sd_revalidate_disk (struct gendisk *); +static unsigned long long sd_set_capacity (struct gendisk *disk, +unsigned long long new_capacity);static int sd_probe (struct device *);static int sd_remove (struct device *);static void sd_shutdown (struct device *);
@@ -1099,6 +1101,7 @@ static const struct block_device_operations sd_fops = {#endif .media_changed = sd_media_changed, .revalidate_disk = sd_revalidate_disk, + .set_capacity = sd_set_capacity, };static unsigned int sd_completed_bytes (struct scsi_cmnd *scmd)
@@ -2099,6 +2102,17 @@ static int sd_revalidate_disk(struct gendisk *disk)return 0 ; } +static unsigned long long sd_set_capacity (struct gendisk *disk, +unsigned long long new_capacity) +{ +struct scsi_device *sdev =scsi_disk (disk)->device; +sector_t capacity =0 ; + +if (sdev->host->hostt->set_capacity) + capacity = sdev->host->hostt->set_capacity (sdev, new_capacity); +return capacity ?:get_capacity (disk); +} +/** * sd_format_disk_name - format disk name * @prefix: name prefix - ie. "sd" for SCSI disks
include/linux/libata.h +5 -1
@@ -146,6 +146,7 @@ enum {ATA_DFLAG_SLEEPING = (1 <<15 ),/* device is sleeping */ ATA_DFLAG_DUBIOUS_XFER = (1 <<16 ),/* data transfer not verified */ ATA_DFLAG_NO_UNLOAD = (1 <<17 ),/* device doesn't support unload */ +ATA_DFLAG_UNLOCK_HPA = (1 <<18 ),/* unlock HPA */ ATA_DFLAG_INIT_MASK = (1 <<24 ) -1 , ATA_DFLAG_DETACH = (1 << 24),
@@ -328,7 +329,7 @@ enum {ATA_EH_HARDRESET = (1 <<2 ),/* meaningful only in ->prereset */ ATA_EH_RESET =ATA_EH_SOFTRESET |ATA_EH_HARDRESET ,ATA_EH_ENABLE_LINK = (1 <<3 ), -ATA_EH_LPM = (1 <<4 ),/* link power management action */ +ATA_EH_LPM = (1 <<4 ),/* link power management action */ ATA_EH_PARK = (1 <<5 ),/* unload heads and stop I/O */ ATA_EH_PERDEV_MASK = ATA_EH_REVALIDATE | ATA_EH_PARK,
@@ -1025,6 +1026,8 @@ extern void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd,extern int ata_std_bios_param (struct scsi_device *sdev,struct block_device *bdev,sector_t capacity,int geom[]); +extern sector_t ata_scsi_set_capacity (struct scsi_device *sdev, +sector_t new_capacity);extern int ata_scsi_slave_config (struct scsi_device *sdev);extern void ata_scsi_slave_destroy (struct scsi_device *sdev); extern int ata_scsi_change_queue_depth(struct scsi_device *sdev,
@@ -1179,6 +1182,7 @@ extern struct device_attribute *ata_common_sdev_attrs[]; .slave_configure = ata_scsi_slave_config, \ .slave_destroy = ata_scsi_slave_destroy, \ .bios_param = ata_std_bios_param, \ + .set_capacity = ata_scsi_set_capacity, \ .sdev_attrs = ata_common_sdev_attrs#define ATA_NCQ_SHT (drv_name) \
include/scsi/scsi_host.h +2 -0
@@ -326,6 +326,8 @@ struct scsi_host_template {int (* bios_param)(struct scsi_device *,struct block_device *,sector_t ,int []); +sector_t (*set_capacity)(struct scsi_device *,sector_t ); +/* * Can be used to export driver statistics and other infos to the * world outside the kernel ie. userspace and it also provides an
Signature · 1 line
tejun
Quoted 20 lines · Tejun Heo
Quoted 8 lines · Ben Hutchings
Since SCSI has no concept of the Host Protected Area (HPA) supported by ATA, ATA disks handled by libata have no
set_capacity()operation and it appears to be impossible to override an HPA except through the libata module parameter.In particular, this means that the workaround for bogus HPAs in
rescan_partitions()does not work:Quoted 10 lines
if (bdops->set_capacity && (disk->flags & GENHD_FL_NATIVE_CAPACITY) == 0) { printk(KERN_CONT "enabling native capacity\n"); capacity = bdops->set_capacity(disk, ~0ULL); disk->flags |= GENHD_FL_NATIVE_CAPACITY; if (capacity > get_capacity(disk)) { set_capacity(disk, capacity); check_disk_size_change(disk, bdev); bdev->bd_invalidated = 0; }
Quoted 2 lines
Hmmm, yeah, this is much better than the dumb switch we have now. I wonder why nobody pointed it out before.
Several distributions have turned on the switch by default. As I said previously, this turned out to be a bad idea.
Quoted 1 line
Does the following patch work as expected?
Without the patch:
[ 2.853930] sd 0:0:1:0: [sdb] 150000000 512-byte logical blocks: (76.8 GB/71.5 GiB) [ 2.854220] sd 0:0:1:0: [sdb] Write Protect is off [ 2.856273] sd 0:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA [ 2.856786] sdb: sdb1 sdb2 < > [ 2.914459] sdb: p1 size 150866352 exceeds device capacity, limited to end of disk [ 2.914631] sdb: p2 ignored, start 150866415 is behind the end of the disk [ 2.938499] sd 0:0:1:0: [sdb] Attached SCSI disk
With the patch:
[ 2.871114] sd 0:0:1:0: [sdb] 150000000 512-byte logical blocks: (76.8 GB/71.5 GiB) [ 2.871395] sd 0:0:1:0: [sdb] Write Protect is off [ 2.871601] sd 0:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA [ 2.872177] sdb: sdb1 sdb2 < > [ 2.914556] sdb: p1 size 150866352 exceeds device capacity, enabling native capacity [ 2.914753] ata1: soft resetting link [ 3.116372] ata1.01: n_sectors mismatch 150000000 != 156301488 [ 3.116433] ata1.01: new n_sectors matches native, probably late HPA unlock, n_sectors adjusted [ 3.164409] ata1.01: configured for UDMA/100 [ 3.164471] ata1: EH complete [ 3.164723] sdb: detected capacity change from 76800000000 to 80026361856 [ 3.179393] sd 0:0:1:0: [sdb] 156301488 512-byte logical blocks: (80.0 GB/74.5 GiB) [ 3.179842] sd 0:0:1:0: [sdb] Attached SCSI disk [ 3.213602] sdb: detected capacity change from 0 to 80026361856 [ 3.214000] sdb: detected capacity change from 0 to 80026361856
For comparison, using an IDE driver:
[ 4.172438] hdb: max request size: 128KiB [ 4.173740] hdb: Host Protected Area detected. [ 4.173743] current capacity is 150000000 sectors (76800 MB) [ 4.173745] native capacity is 156301488 sectors (80026 MB) [ 4.173922] hdb: 150000000 sectors (76800 MB) w/8192KiB Cache, CHS=65535/16/63 [ 4.177393] hdb: cache flushes supported [ 4.177549] hdb: hdb1 hdb2 < > [ 4.183025] hdb: p1 size 150866352 exceeds device capacity, enabling native capacity [ 4.183842] hdb: detected capacity change from 76800000000 to 80026361856
So, this looks good, but I'm slightly disturbed by the multiple 'capacity change' messages when using libata. Also, the warnings in ata_dev_revalidate() should not be shown when we are deliberately changing capacity.
My test case also revealed a bug in the partition scanning retry:
partition 2 is an extended partition and wholly within the HPA, so the extended partition table is not accessible until the HPA is disabled. But it is not rescanned after the HPA is disabled. However, this is is consistent between IDE and libata drivers and will not be a regression when making the transition.
Ben.
Signature · 2 lines
Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse.
Hello,
Quoted 2 lines · Ben Hutchings
Several distributions have turned on the switch by default. As I said previously, this turned out to be a bad idea.
Heh, yeah, I did that on openSUSE which was mandatory for backward compatibility because ide unlocked HPA by default (at least on openSUSE). Do you know how common are these problem cases w/ default HPA unlocking? Diddling with HPAs and writing stuff to disks from BIOS is generally a very bad idea because it means that switching BIOS options or attaching a hard drive to a different motherboard can lead to data corruption.
Quoted 1 line
Quoted 1 line
Does the following patch work as expected?
...
Quoted 2 lines
So, this looks good, but I'm slightly disturbed by the multiple 'capacity change' messages when using libata.
I'll see where those multiple messages are coming from.
Quoted 2 lines
Also, the warnings in
ata_dev_revalidate()should not be shown when we are deliberately changing capacity.
Given that this is an ugly situation, I don't think it's a bad idea to emit ugly messages.
Quoted 6 lines
My test case also revealed a bug in the partition scanning retry:
partition 2 is an extended partition and wholly within the HPA, so the extended partition table is not accessible until the HPA is disabled. But it is not rescanned after the HPA is disabled. However, this is is consistent between IDE and libata drivers and will not be a regression when making the transition.
Ah... I see, but let's fix that up too. It could be worse to have half working workaround than not working around at all. I'll update the patch once the currently pending HPA updates are in.
Thanks.
Signature · 1 line
tejun
Quoted 10 lines · Tejun Heo
Hello,
Quoted 2 lines · Ben Hutchings
Several distributions have turned on the switch by default. As I said previously, this turned out to be a bad idea.
Heh, yeah, I did that on openSUSE which was mandatory for backward compatibility because ide unlocked HPA by default (at least on openSUSE). Do you know how common are these problem cases w/ default HPA unlocking?
No idea, but they look serious enough to worry about.
Quoted 4 lines
Diddling with HPAs and writing stuff to disks from
BIOS is generally a very bad idea because it means that switching BIOS options or attaching a hard drive to a different motherboard can lead to data corruption.
If a system vendor puts its own name or model numbers on the disks it ships then I think the BIOS or other platform firmware can reasonably assume that it 'owns' and can write to the HPA on a disk with the vendor's identification. (I wouldn't be at all surprised to find that some vendors take shortcuts though.)
[...]
Quoted 10 lines
Quoted 6 lines
My test case also revealed a bug in the partition scanning retry:
partition 2 is an extended partition and wholly within the HPA, so the extended partition table is not accessible until the HPA is disabled. But it is not rescanned after the HPA is disabled. However, this is is consistent between IDE and libata drivers and will not be a regression when making the transition.Ah... I see, but let's fix that up too. It could be worse to have half working workaround than not working around at all. I'll update the patch once the currently pending HPA updates are in.
Oh, what are those?
I want to apply some version of this fix in Debian soon so we can complete the transition to libata. I would very much appreciate it if you could answer whether or not the multiple 'capacity change' messages may indicate a problem.
Ben.
Signature · 2 lines
Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse.
Hello,
Quoted 5 lines · Ben Hutchings
If a system vendor puts its own name or model numbers on the disks it ships then I think the BIOS or other platform firmware can reasonably assume that it 'owns' and can write to the HPA on a disk with the vendor's identification. (I wouldn't be at all surprised to find that some vendors take shortcuts though.)
I would be very surprised if no vendor took shortcuts. :-)
Quoted 5 lines
Quoted 3 lines
Ah... I see, but let's fix that up too. It could be worse to have half working workaround than not working around at all. I'll update the patch once the currently pending HPA updates are in.
Oh, what are those?
The patch I posted is rolled up version which contained currently pending HPA unlock on shrink patch.
http://article.gmane.org/gmane.linux.ide/45662
Quoted 4 lines
I want to apply some version of this fix in Debian soon so we can complete the transition to libata. I would very much appreciate it if you could answer whether or not the multiple 'capacity change' messages may indicate a problem.
Multiple messages are probably sd doing revalidation of the device. I don't think it will cause any problem but I'll look into it later today and let you know.
Thanks.
Signature · 1 line
tejun
Quoted 15 lines · Tejun Heo
Hello,
Quoted 5 lines · Ben Hutchings
If a system vendor puts its own name or model numbers on the disks it ships then I think the BIOS or other platform firmware can reasonably assume that it 'owns' and can write to the HPA on a disk with the vendor's identification. (I wouldn't be at all surprised to find that some vendors take shortcuts though.)
I would be very surprised if no vendor took shortcuts. :-)
Quoted 4 lines
Quoted 3 lines
Ah... I see, but let's fix that up too. It could be worse to have half working workaround than not working around at all. I'll update the patch once the currently pending HPA updates are in.
Quoted 6 lines
Quoted 1 line
Oh, what are those?
The patch I posted is rolled up version which contained currently pending HPA unlock on shrink patch.
It sounds like that should go to stable as well.
Quoted 8 lines
Quoted 4 lines
I want to apply some version of this fix in Debian soon so we can complete the transition to libata. I would very much appreciate it if you could answer whether or not the multiple 'capacity change' messages may indicate a problem.
Multiple messages are probably sd doing revalidation of the device. I don't think it will cause any problem but I'll look into it later today and let you know.
What did you find?
Ben.
Signature · 2 lines
Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse.
Hello,
Quoted 6 lines · Ben Hutchings
Quoted 4 lines
The patch I posted is rolled up version which contained currently pending HPA unlock on shrink patch.
It sounds like that should go to stable as well.
I think it's a bit too pervasive for -stable inclusion. The number of affected systems is very low after all.
Quoted 5 lines
Quoted 3 lines
Multiple messages are probably sd doing revalidation of the device. I don't think it will cause any problem but I'll look into it later today and let you know.
What did you find?
Nothing yet. I just moved and my stuff are still in transit, so I couldn't test anything. I'll get to it once my things arrive (they're expected to next week).
Thanks.
Signature · 1 line
tejun