From patchwork Fri Apr 2 03:24:46 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tejun Heo X-Patchwork-Id: 49267 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 74083B7D00 for ; Fri, 2 Apr 2010 14:25:52 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752909Ab0DBDZo (ORCPT ); Thu, 1 Apr 2010 23:25:44 -0400 Received: from hera.kernel.org ([140.211.167.34]:42519 "EHLO hera.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752841Ab0DBDZn (ORCPT ); Thu, 1 Apr 2010 23:25:43 -0400 Received: from htj.dyndns.org (localhost [127.0.0.1]) by hera.kernel.org (8.14.3/8.14.3) with ESMTP id o323Okpr032730 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Fri, 2 Apr 2010 03:24:48 GMT X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.95.2 at hera.kernel.org Received: from [127.0.0.2] (htj.dyndns.org [127.0.0.2]) by htj.dyndns.org (Postfix) with ESMTPSA id 4CDC2100B6A0E; Fri, 2 Apr 2010 12:24:46 +0900 (KST) Message-ID: <4BB5637E.4050604@kernel.org> Date: Fri, 02 Apr 2010 12:24:46 +0900 From: Tejun Heo User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1.9) Gecko/20100317 Thunderbird/3.0.4 MIME-Version: 1.0 To: Ben Hutchings CC: linux-ide@vger.kernel.org, debian-kernel@lists.debian.org Subject: Re: Working around bogus HPAs in libata References: <1269192318.18314.218.camel@localhost> In-Reply-To: <1269192318.18314.218.camel@localhost> X-Spam-Status: No, score=-0.6 required=5.0 tests=ALL_TRUSTED,BAYES_00, DATE_IN_FUTURE_96_Q autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on hera.kernel.org X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.2.3 (hera.kernel.org [127.0.0.1]); Fri, 02 Apr 2010 03:24:49 +0000 (UTC) Sender: linux-ide-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ide@vger.kernel.org On 03/22/2010 02:25 AM, Ben Hutchings wrote: > 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; > } 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? diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 4a28420..81a0820 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -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); diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bea003a..e81dc54 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -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 diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 7b75c8a..a0a6b9e 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -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 diff --git a/include/linux/libata.h b/include/linux/libata.h index f8ea71e..4f0fb5b 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -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) \ diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index c50a97f..58048cb 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -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