From 8cf32ac6578a70025be1103466da9d1d6141429e Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sat, 8 Dec 2007 08:45:27 +0900 Subject: [PATCH 01/16] sata_sil: fix spurious IRQ handling Interestingly, sata_sil raises spurious interrupts if it's coupled with Sil SATA_PATA bridge. Currently, sata_sil interrupt handler is strict about spurious interrupts and freezes the port when it occurs. This patch makes it more forgiving. * On SATA PHY event interrupt, serror value is checked to see whether it really is PHYRDY CHG event. If not, SATA PHY event interrupt is ignored. * If ATA interrupt occurs while no command is in progress, it's cleared and ignored. This fixes bugzilla bug 9505. http://bugzilla.kernel.org/show_bug.cgi?id=9505 Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/sata_sil.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/ata/sata_sil.c b/drivers/ata/sata_sil.c index 4e6e381279cc..025622b14efb 100644 --- a/drivers/ata/sata_sil.c +++ b/drivers/ata/sata_sil.c @@ -390,23 +390,28 @@ static void sil_host_intr(struct ata_port *ap, u32 bmdma2) sil_scr_read(ap, SCR_ERROR, &serror); sil_scr_write(ap, SCR_ERROR, serror); - /* Trigger hotplug and accumulate SError only if the - * port isn't already frozen. Otherwise, PHY events - * during hardreset makes controllers with broken SIEN - * repeat probing needlessly. + /* Sometimes spurious interrupts occur, double check + * it's PHYRDY CHG. */ - if (!(ap->pflags & ATA_PFLAG_FROZEN)) { - ata_ehi_hotplugged(&ap->link.eh_info); - ap->link.eh_info.serror |= serror; + if (serror & SERR_PHYRDY_CHG) { + /* Trigger hotplug and accumulate SError only + * if the port isn't already frozen. + * Otherwise, PHY events during hardreset + * makes controllers with broken SIEN repeat + * probing needlessly. + */ + if (!(ap->pflags & ATA_PFLAG_FROZEN)) { + ata_ehi_hotplugged(&ap->link.eh_info); + ap->link.eh_info.serror |= serror; + } + goto freeze; } - goto freeze; + if (!(bmdma2 & SIL_DMA_COMPLETE)) + return; } - if (unlikely(!qc)) - goto freeze; - - if (unlikely(qc->tf.flags & ATA_TFLAG_POLLING)) { + if (unlikely(!qc || (qc->tf.flags & ATA_TFLAG_POLLING))) { /* this sometimes happens, just clear IRQ */ ata_chk_status(ap); return; From f7fe7ad4bcaba17f05d5cbf1119772c645783b08 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sat, 8 Dec 2007 08:47:01 +0900 Subject: [PATCH 02/16] libata: clear link->eh_info.serror from ata_std_postreset() link->eh_info.serror is used to cache SError for controllers which need it cleared from interrupt handler to clear IRQ. It also should be cleared after reset just like SError itself. Make ata_std_postreset() clear link->eh_info.serror too and update sata_sil such that it doesn't care about bookkeeping the value. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-core.c | 1 + drivers/ata/sata_sil.c | 11 +---------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index e4dea8623a71..b0d1dc3cdb2d 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -3923,6 +3923,7 @@ void ata_std_postreset(struct ata_link *link, unsigned int *classes) /* clear SError */ if (sata_scr_read(link, SCR_ERROR, &serror) == 0) sata_scr_write(link, SCR_ERROR, serror); + link->eh_info.serror = 0; /* is double-select really necessary? */ if (classes[0] != ATA_DEV_NONE) diff --git a/drivers/ata/sata_sil.c b/drivers/ata/sata_sil.c index 025622b14efb..f5119bf40c24 100644 --- a/drivers/ata/sata_sil.c +++ b/drivers/ata/sata_sil.c @@ -394,16 +394,7 @@ static void sil_host_intr(struct ata_port *ap, u32 bmdma2) * it's PHYRDY CHG. */ if (serror & SERR_PHYRDY_CHG) { - /* Trigger hotplug and accumulate SError only - * if the port isn't already frozen. - * Otherwise, PHY events during hardreset - * makes controllers with broken SIEN repeat - * probing needlessly. - */ - if (!(ap->pflags & ATA_PFLAG_FROZEN)) { - ata_ehi_hotplugged(&ap->link.eh_info); - ap->link.eh_info.serror |= serror; - } + ap->link.eh_info.serror |= serror; goto freeze; } From e41bd3e854e3536de847d5831c0e25a47f394885 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 9 Dec 2007 19:45:39 +0900 Subject: [PATCH 03/16] libata: add ST3160023AS / 3.42 to NCQ blacklist Like ST380817AS / 3.42, ST3160023AS / 3.42 times out commands if NCQ is used. Blacklist it. This is reported by Matheus Izvekov in the following thread. http://thread.gmane.org/gmane.linux.ide/24202 Signed-off-by: Tejun Heo Cc: Matheus Izvekov Signed-off-by: Jeff Garzik --- drivers/ata/libata-core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index b0d1dc3cdb2d..99a18cea316a 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -4150,6 +4150,7 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = { { "HITACHI HDS7250SASUN500G*", NULL, ATA_HORKAGE_NONCQ }, { "HITACHI HDS7225SBSUN250G*", NULL, ATA_HORKAGE_NONCQ }, { "ST380817AS", "3.42", ATA_HORKAGE_NONCQ }, + { "ST3160023AS", "3.42", ATA_HORKAGE_NONCQ }, /* Blacklist entries taken from Silicon Image 3124/3132 Windows driver .inf file - also several Linux problem reports */ From 4e5200334e03e5620aa19d538300c13db270a063 Mon Sep 17 00:00:00 2001 From: Mark Lord Date: Tue, 11 Dec 2007 12:58:05 -0500 Subject: [PATCH 04/16] sata_mv: improve warnings about Highpoint RocketRAID 23xx cards Improve the existing boot/load time warnings from sata_mv for Highpoint RocketRAID 23xx cards, based on new knowledge about where the BIOS likes to overwrite sectors with metadata. Harmless to us, but very useful for end users. Signed-off-by: Mark Lord Signed-off-by: Jeff Garzik --- drivers/ata/sata_mv.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c index fe0105d35bae..37b850ae0845 100644 --- a/drivers/ata/sata_mv.c +++ b/drivers/ata/sata_mv.c @@ -2506,11 +2506,31 @@ static int mv_chip_id(struct ata_host *host, unsigned int board_idx) if (pdev->vendor == PCI_VENDOR_ID_TTI && (pdev->device == 0x2300 || pdev->device == 0x2310)) { - printk(KERN_WARNING "sata_mv: Highpoint RocketRAID BIOS" - " will CORRUPT DATA on attached drives when" - " configured as \"Legacy\". BEWARE!\n"); - printk(KERN_WARNING "sata_mv: Use BIOS \"JBOD\" volumes" - " instead for safety.\n"); + /* + * Highpoint RocketRAID PCIe 23xx series cards: + * + * Unconfigured drives are treated as "Legacy" + * by the BIOS, and it overwrites sector 8 with + * a "Lgcy" metadata block prior to Linux boot. + * + * Configured drives (RAID or JBOD) leave sector 8 + * alone, but instead overwrite a high numbered + * sector for the RAID metadata. This sector can + * be determined exactly, by truncating the physical + * drive capacity to a nice even GB value. + * + * RAID metadata is at: (dev->n_sectors & ~0xfffff) + * + * Warn the user, lest they think we're just buggy. + */ + printk(KERN_WARNING DRV_NAME ": Highpoint RocketRAID" + " BIOS CORRUPTS DATA on all attached drives," + " regardless of if/how they are configured." + " BEWARE!\n"); + printk(KERN_WARNING DRV_NAME ": For data safety, do not" + " use sectors 8-9 on \"Legacy\" drives," + " and avoid the final two gigabytes on" + " all RocketRAID BIOS initialized drives.\n"); } case chip_6042: hpriv->ops = &mv6xxx_ops; From 0d02f0b22b678b9d6c8ac8cad7b4cfbbdf6fab18 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sat, 15 Dec 2007 15:04:57 +0900 Subject: [PATCH 05/16] libata-acpi: adjust constness in ata_acpi_gtm/stm() parameters * No internal function uses const ata_port. Drop const from @ap. * Make ata_acpi_stm() copy @stm before using it and change @stm to const. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-acpi.c | 7 ++++--- include/linux/libata.h | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index 545ea865ceb5..8ae36ad029c5 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -200,7 +200,7 @@ void ata_acpi_associate(struct ata_host *host) * RETURNS: * 0 on success, -ENOENT if _GTM doesn't exist, -errno on failure. */ -int ata_acpi_gtm(const struct ata_port *ap, struct ata_acpi_gtm *gtm) +int ata_acpi_gtm(struct ata_port *ap, struct ata_acpi_gtm *gtm) { struct acpi_buffer output = { .length = ACPI_ALLOCATE_BUFFER }; union acpi_object *out_obj; @@ -259,15 +259,16 @@ EXPORT_SYMBOL_GPL(ata_acpi_gtm); * RETURNS: * 0 on success, -ENOENT if _STM doesn't exist, -errno on failure. */ -int ata_acpi_stm(const struct ata_port *ap, struct ata_acpi_gtm *stm) +int ata_acpi_stm(struct ata_port *ap, const struct ata_acpi_gtm *stm) { acpi_status status; + struct ata_acpi_gtm stm_buf = *stm; struct acpi_object_list input; union acpi_object in_params[3]; in_params[0].type = ACPI_TYPE_BUFFER; in_params[0].buffer.length = sizeof(struct ata_acpi_gtm); - in_params[0].buffer.pointer = (u8 *)stm; + in_params[0].buffer.pointer = (u8 *)&stm_buf; /* Buffers for id may need byteswapping ? */ in_params[1].type = ACPI_TYPE_BUFFER; in_params[1].buffer.length = 512; diff --git a/include/linux/libata.h b/include/linux/libata.h index ef52a07c43d8..1ca9b89632f9 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -940,8 +940,8 @@ enum { /* libata-acpi.c */ #ifdef CONFIG_ATA_ACPI extern int ata_acpi_cbl_80wire(struct ata_port *ap); -int ata_acpi_stm(const struct ata_port *ap, struct ata_acpi_gtm *stm); -int ata_acpi_gtm(const struct ata_port *ap, struct ata_acpi_gtm *stm); +int ata_acpi_stm(struct ata_port *ap, const struct ata_acpi_gtm *stm); +int ata_acpi_gtm(struct ata_port *ap, struct ata_acpi_gtm *stm); #else static inline int ata_acpi_cbl_80wire(struct ata_port *ap) { return 0; } #endif From c2e366a107e511ad00c2181c52e4150fc086ec0f Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sat, 15 Dec 2007 15:04:58 +0900 Subject: [PATCH 06/16] libata: update ata_*_printk() macros such that level can be a variable Make prink helpers format @lv together rather than prepending to the format string as constant. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- include/linux/libata.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/linux/libata.h b/include/linux/libata.h index 1ca9b89632f9..3784af395576 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1013,18 +1013,18 @@ extern void ata_do_eh(struct ata_port *ap, ata_prereset_fn_t prereset, * printk helpers */ #define ata_port_printk(ap, lv, fmt, args...) \ - printk(lv"ata%u: "fmt, (ap)->print_id , ##args) + printk("%sata%u: "fmt, lv, (ap)->print_id , ##args) #define ata_link_printk(link, lv, fmt, args...) do { \ if ((link)->ap->nr_pmp_links) \ - printk(lv"ata%u.%02u: "fmt, (link)->ap->print_id, \ + printk("%sata%u.%02u: "fmt, lv, (link)->ap->print_id, \ (link)->pmp , ##args); \ else \ - printk(lv"ata%u: "fmt, (link)->ap->print_id , ##args); \ + printk("%sata%u: "fmt, lv, (link)->ap->print_id , ##args); \ } while(0) #define ata_dev_printk(dev, lv, fmt, args...) \ - printk(lv"ata%u.%02u: "fmt, (dev)->link->ap->print_id, \ + printk("%sata%u.%02u: "fmt, lv, (dev)->link->ap->print_id, \ (dev)->link->pmp + (dev)->devno , ##args) /* From ce2e0abbd31b047ac7be740d28ef710f5bbdb105 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sat, 15 Dec 2007 15:04:59 +0900 Subject: [PATCH 07/16] libata: add more opcodes to ata.h Add constants for DEVICE CONFIGURATION OVERLAY and SET_MAX to include/linux/ata.h. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- include/linux/ata.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/include/linux/ata.h b/include/linux/ata.h index 5c4e54a2a8d6..72ab80801ef6 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -190,6 +190,8 @@ enum { ATA_CMD_READ_LOG_EXT = 0x2f, ATA_CMD_PMP_READ = 0xE4, ATA_CMD_PMP_WRITE = 0xE8, + ATA_CMD_CONF_OVERLAY = 0xB1, + ATA_CMD_SEC_FREEZE_LOCK = 0xF5, /* READ_LOG_EXT pages */ ATA_LOG_SATA_NCQ = 0x10, @@ -239,6 +241,19 @@ enum { SATA_AN = 0x05, /* Asynchronous Notification */ SATA_DIPM = 0x03, /* Device Initiated Power Management */ + /* feature values for SET_MAX */ + ATA_SET_MAX_ADDR = 0x00, + ATA_SET_MAX_PASSWD = 0x01, + ATA_SET_MAX_LOCK = 0x02, + ATA_SET_MAX_UNLOCK = 0x03, + ATA_SET_MAX_FREEZE_LOCK = 0x04, + + /* feature values for DEVICE CONFIGURATION OVERLAY */ + ATA_DCO_RESTORE = 0xC0, + ATA_DCO_FREEZE_LOCK = 0xC1, + ATA_DCO_IDENTIFY = 0xC2, + ATA_DCO_SET = 0xC3, + /* ATAPI stuff */ ATAPI_PKT_DMA = (1 << 0), ATAPI_DMADIR = (1 << 2), /* ATAPI data dir: From 7f9ad9b8b96855f529f4fe9db0bf32cd3f14c01b Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sat, 15 Dec 2007 15:05:00 +0900 Subject: [PATCH 08/16] libata: ata_dev_disable() should be called from EH context ata_port_detach() calls ata_dev_disable() with host lock held but ata_dev_disable() should be called from EH context. ata_port_detach() steals EH context by setting ATA_PFLAG_UNLOADAING and flushing EH. Drop locking around ata_dev_disable() and note that ata_port_detach() owns EH context at that point. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-core.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 99a18cea316a..c316eacbeddd 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -7210,18 +7210,14 @@ static void ata_port_detach(struct ata_port *ap) ata_port_wait_eh(ap); - /* EH is now guaranteed to see UNLOADING, so no new device - * will be attached. Disable all existing devices. + /* EH is now guaranteed to see UNLOADING - EH context belongs + * to us. Disable all existing devices. */ - spin_lock_irqsave(ap->lock, flags); - ata_port_for_each_link(link, ap) { ata_link_for_each_dev(dev, link) ata_dev_disable(dev); } - spin_unlock_irqrestore(ap->lock, flags); - /* Final freeze & EH. All in-flight commands are aborted. EH * will be skipped and retrials will be terminated with bad * target. From 562f0c2d771ee7be6b37fe015f94a929f8056120 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sat, 15 Dec 2007 15:05:01 +0900 Subject: [PATCH 09/16] libata-acpi: add new hooks ata_acpi_dissociate() and ata_acpi_on_disable() Add two hooks - ata_acpi_dissociate() which is called during driver detach after the whole host is shutdown and ata_acpi_on_disable() which is called when a device is disabled. Signed-off-by: Tejun heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-acpi.c | 28 ++++++++++++++++++++++++++++ drivers/ata/libata-core.c | 4 ++++ drivers/ata/libata.h | 8 ++++++-- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index 8ae36ad029c5..9e5fc5d75b2e 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -187,6 +187,21 @@ void ata_acpi_associate(struct ata_host *host) } } +/** + * ata_acpi_dissociate - dissociate ATA host from ACPI objects + * @host: target ATA host + * + * This function is called during driver detach after the whole host + * is shut down. + * + * LOCKING: + * EH context. + */ +void ata_acpi_dissociate(struct ata_host *host) +{ + /* nada */ +} + /** * ata_acpi_gtm - execute _GTM * @ap: target ATA port @@ -716,3 +731,16 @@ int ata_acpi_on_devcfg(struct ata_device *dev) dev->flags |= ATA_DFLAG_ACPI_FAILED; return rc; } + +/** + * ata_acpi_on_disable - ATA ACPI hook called when a device is disabled + * @dev: target ATA device + * + * This function is called when @dev is about to be disabled. + * + * LOCKING: + * EH context. + */ +void ata_acpi_on_disable(struct ata_device *dev) +{ +} diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index c316eacbeddd..4af939a00e54 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -622,6 +622,7 @@ void ata_dev_disable(struct ata_device *dev) if (ata_dev_enabled(dev)) { if (ata_msg_drv(dev->link->ap)) ata_dev_printk(dev, KERN_WARNING, "disabled\n"); + ata_acpi_on_disable(dev); ata_down_xfermask_limit(dev, ATA_DNXFER_FORCE_PIO0 | ATA_DNXFER_QUIET); dev->class++; @@ -7249,6 +7250,9 @@ void ata_host_detach(struct ata_host *host) for (i = 0; i < host->n_ports; i++) ata_port_detach(host->ports[i]); + + /* the host is dead now, dissociate ACPI */ + ata_acpi_dissociate(host); } /** diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h index 0e6cf3a484dc..bbe59c2fd1e2 100644 --- a/drivers/ata/libata.h +++ b/drivers/ata/libata.h @@ -108,15 +108,19 @@ extern void ata_lpm_schedule(struct ata_port *ap, enum link_pm); #ifdef CONFIG_ATA_ACPI extern void ata_acpi_associate_sata_port(struct ata_port *ap); extern void ata_acpi_associate(struct ata_host *host); +extern void ata_acpi_dissociate(struct ata_host *host); extern int ata_acpi_on_suspend(struct ata_port *ap); extern void ata_acpi_on_resume(struct ata_port *ap); -extern int ata_acpi_on_devcfg(struct ata_device *adev); +extern int ata_acpi_on_devcfg(struct ata_device *dev); +extern void ata_acpi_on_disable(struct ata_device *dev); #else static inline void ata_acpi_associate_sata_port(struct ata_port *ap) { } static inline void ata_acpi_associate(struct ata_host *host) { } +static inline void ata_acpi_dissociate(struct ata_host *host) { } static inline int ata_acpi_on_suspend(struct ata_port *ap) { return 0; } static inline void ata_acpi_on_resume(struct ata_port *ap) { } -static inline int ata_acpi_on_devcfg(struct ata_device *adev) { return 0; } +static inline int ata_acpi_on_devcfg(struct ata_device *dev) { return 0; } +static inline void ata_acpi_on_disable(struct ata_device *dev) { } #endif /* libata-scsi.c */ From c05e6ff035c1b25d17364a685432b33937d3dc23 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sat, 15 Dec 2007 15:05:02 +0900 Subject: [PATCH 10/16] libata-acpi: implement and use ata_acpi_init_gtm() _GTM fetches currently configured transfer mode while _STM configures controller according to _GTM parameter and prepares transfer mode configuration TFs for _GTF. In many cases _GTM and _STM implementations are quite brittle and can't cope with configuration changed by libata. libata does not depend on ATA ACPI to configure devices. The only reason libata performs _GTM and _STM are to make _GTF evaluation succeed and libata also doesn't care about how _GTF TFs configure transfer mode. It overrides that configuration anyway, so from libata's POV, it doesn't matter what value is feeded to _STM as long as evaluation succeeds for _STM and following _GTF. This patch adds dev->__acpi_init_gtm and store initial _GTM values on host initialization before modified by reset and mode configuration. If the field is valid, ata_acpi_init_gtm() returns pointer to the saved _GTM structure; otherwise, NULL. This saved value is used for _STM during resume and peek at BIOS/firmware programmed initial timing for later use. The accessor is there to make building w/o ACPI easy as dev->__acpi_init doesn't exist if ACPI is not enabled. On driver detach, the initial BIOS configuration is restored by executing _STM with the initial _GTM values such that the next driver can also use the initial BIOS configured values. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-acpi.c | 62 +++++++++++++++++---------------------- include/linux/libata.h | 14 +++++++-- 2 files changed, 39 insertions(+), 37 deletions(-) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index 9e5fc5d75b2e..b3aeca368774 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -94,6 +94,9 @@ static void ata_acpi_associate_ide_port(struct ata_port *ap) dev->acpi_handle = acpi_get_child(ap->acpi_handle, i); } + + if (ata_acpi_gtm(ap, &ap->__acpi_init_gtm) == 0) + ap->pflags |= ATA_PFLAG_INIT_GTM_VALID; } static void ata_acpi_handle_hotplug(struct ata_port *ap, struct kobject *kobj, @@ -199,7 +202,18 @@ void ata_acpi_associate(struct ata_host *host) */ void ata_acpi_dissociate(struct ata_host *host) { - /* nada */ + int i; + + /* Restore initial _GTM values so that driver which attaches + * afterward can use them too. + */ + for (i = 0; i < host->n_ports; i++) { + struct ata_port *ap = host->ports[i]; + const struct ata_acpi_gtm *gtm = ata_acpi_init_gtm(ap); + + if (ap->acpi_handle && gtm) + ata_acpi_stm(ap, gtm); + } } /** @@ -409,22 +423,21 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf, int ata_acpi_cbl_80wire(struct ata_port *ap) { - struct ata_acpi_gtm gtm; + const struct ata_acpi_gtm *gtm = ata_acpi_init_gtm(ap); int valid = 0; - /* No _GTM data, no information */ - if (ata_acpi_gtm(ap, >m) < 0) + if (!gtm) return 0; /* Split timing, DMA enabled */ - if ((gtm.flags & 0x11) == 0x11 && gtm.drive[0].dma < 55) + if ((gtm->flags & 0x11) == 0x11 && gtm->drive[0].dma < 55) valid |= 1; - if ((gtm.flags & 0x14) == 0x14 && gtm.drive[1].dma < 55) + if ((gtm->flags & 0x14) == 0x14 && gtm->drive[1].dma < 55) valid |= 2; /* Shared timing, DMA enabled */ - if ((gtm.flags & 0x11) == 0x01 && gtm.drive[0].dma < 55) + if ((gtm->flags & 0x11) == 0x01 && gtm->drive[0].dma < 55) valid |= 1; - if ((gtm.flags & 0x14) == 0x04 && gtm.drive[0].dma < 55) + if ((gtm->flags & 0x14) == 0x04 && gtm->drive[0].dma < 55) valid |= 2; /* Drive check */ @@ -612,27 +625,8 @@ static int ata_acpi_push_id(struct ata_device *dev) */ int ata_acpi_on_suspend(struct ata_port *ap) { - unsigned long flags; - int rc; - - /* proceed iff per-port acpi_handle is valid */ - if (!ap->acpi_handle) - return 0; - BUG_ON(ap->flags & ATA_FLAG_ACPI_SATA); - - /* store timing parameters */ - rc = ata_acpi_gtm(ap, &ap->acpi_gtm); - - spin_lock_irqsave(ap->lock, flags); - if (rc == 0) - ap->pflags |= ATA_PFLAG_GTM_VALID; - else - ap->pflags &= ~ATA_PFLAG_GTM_VALID; - spin_unlock_irqrestore(ap->lock, flags); - - if (rc == -ENOENT) - rc = 0; - return rc; + /* nada */ + return 0; } /** @@ -647,14 +641,12 @@ int ata_acpi_on_suspend(struct ata_port *ap) */ void ata_acpi_on_resume(struct ata_port *ap) { + const struct ata_acpi_gtm *gtm = ata_acpi_init_gtm(ap); struct ata_device *dev; - if (ap->acpi_handle && (ap->pflags & ATA_PFLAG_GTM_VALID)) { - BUG_ON(ap->flags & ATA_FLAG_ACPI_SATA); - - /* restore timing parameters */ - ata_acpi_stm(ap, &ap->acpi_gtm); - } + /* restore timing parameters */ + if (ap->acpi_handle && gtm) + ata_acpi_stm(ap, gtm); /* schedule _GTF */ ata_link_for_each_dev(dev, &ap->link) diff --git a/include/linux/libata.h b/include/linux/libata.h index 3784af395576..ba84d8a37545 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -211,7 +211,7 @@ enum { ATA_PFLAG_SUSPENDED = (1 << 17), /* port is suspended (power) */ ATA_PFLAG_PM_PENDING = (1 << 18), /* PM operation pending */ - ATA_PFLAG_GTM_VALID = (1 << 19), /* acpi_gtm data valid */ + ATA_PFLAG_INIT_GTM_VALID = (1 << 19), /* initial gtm data valid */ /* struct ata_queued_cmd flags */ ATA_QCFLAG_ACTIVE = (1 << 0), /* cmd not yet ack'd to scsi lyer */ @@ -653,7 +653,7 @@ struct ata_port { #ifdef CONFIG_ATA_ACPI acpi_handle acpi_handle; - struct ata_acpi_gtm acpi_gtm; + struct ata_acpi_gtm __acpi_init_gtm; /* use ata_acpi_init_gtm() */ #endif u8 sector_buf[ATA_SECT_SIZE]; /* owned by EH */ }; @@ -939,10 +939,20 @@ enum { /* libata-acpi.c */ #ifdef CONFIG_ATA_ACPI +static inline const struct ata_acpi_gtm *ata_acpi_init_gtm(struct ata_port *ap) +{ + if (ap->pflags & ATA_PFLAG_INIT_GTM_VALID) + return &ap->__acpi_init_gtm; + return NULL; +} extern int ata_acpi_cbl_80wire(struct ata_port *ap); int ata_acpi_stm(struct ata_port *ap, const struct ata_acpi_gtm *stm); int ata_acpi_gtm(struct ata_port *ap, struct ata_acpi_gtm *stm); #else +static inline const struct ata_acpi_gtm *ata_acpi_init_gtm(struct ata_port *ap) +{ + return NULL; +} static inline int ata_acpi_cbl_80wire(struct ata_port *ap) { return 0; } #endif From 398e07826b24cbeb5ff2f0a178367fc9d24cd475 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sat, 15 Dec 2007 15:05:03 +0900 Subject: [PATCH 11/16] libata-acpi: implement dev->gtf_cache and evaluate _GTF right after _STM during resume On certain implementations, _GTF evaluation depends on preceding _STM and both can be pretty picky about the configuration. Using _GTM result cached during controller initialization satisfies the most neurotic _STM implementation. However, libata evaluates _GTF after reset during device configuration and the hardware state can be different from what _GTF expects and can cause evaluation failure. This patch adds dev->gtf_cache and updates ata_dev_get_GTF() such that it uses the cached value if available. Cache is cleared with a call to ata_acpi_clear_gtf(). Because for SATA ACPI nodes _GTF must be evaluated after _SDD which can't be done till IDENTIFY is complete, _GTF caching from ata_acpi_on_resume() is used only for IDE ACPI nodes. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-acpi.c | 69 +++++++++++++++++++++++++++------------ include/linux/libata.h | 1 + 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index b3aeca368774..e0dd132fd490 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -41,6 +41,12 @@ static int is_pci_dev(struct device *dev) return (dev->bus == &pci_bus_type); } +static void ata_acpi_clear_gtf(struct ata_device *dev) +{ + kfree(dev->gtf_cache); + dev->gtf_cache = NULL; +} + /** * ata_acpi_associate_sata_port - associate SATA port with ACPI objects * @ap: target SATA port @@ -327,7 +333,6 @@ EXPORT_SYMBOL_GPL(ata_acpi_stm); * ata_dev_get_GTF - get the drive bootup default taskfile settings * @dev: target ATA device * @gtf: output parameter for buffer containing _GTF taskfile arrays - * @ptr_to_free: pointer which should be freed * * This applies to both PATA and SATA drives. * @@ -344,8 +349,7 @@ EXPORT_SYMBOL_GPL(ata_acpi_stm); * Number of taskfiles on success, 0 if _GTF doesn't exist or doesn't * contain valid data. */ -static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf, - void **ptr_to_free) +static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf) { struct ata_port *ap = dev->link->ap; acpi_status status; @@ -353,6 +357,12 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf, union acpi_object *out_obj; int rc = 0; + /* if _GTF is cached, use the cached value */ + if (dev->gtf_cache) { + out_obj = dev->gtf_cache; + goto done; + } + /* set up output buffer */ output.length = ACPI_ALLOCATE_BUFFER; output.pointer = NULL; /* ACPI-CA sets this; save/free it later */ @@ -363,6 +373,7 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf, /* _GTF has no input parameters */ status = acpi_evaluate_object(dev->acpi_handle, "_GTF", NULL, &output); + out_obj = dev->gtf_cache = output.pointer; if (ACPI_FAILURE(status)) { if (status != AE_NOT_FOUND) { @@ -383,7 +394,6 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf, goto out_free; } - out_obj = output.pointer; if (out_obj->type != ACPI_TYPE_BUFFER) { ata_dev_printk(dev, KERN_WARNING, "_GTF unexpected object type 0x%x\n", @@ -398,18 +408,19 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf, goto out_free; } - *ptr_to_free = out_obj; - *gtf = (void *)out_obj->buffer.pointer; + done: rc = out_obj->buffer.length / REGS_PER_GTF; - - if (ata_msg_probe(ap)) - ata_dev_printk(dev, KERN_DEBUG, "%s: returning " - "gtf=%p, gtf_count=%d, ptr_to_free=%p\n", - __FUNCTION__, *gtf, rc, *ptr_to_free); + if (gtf) { + *gtf = (void *)out_obj->buffer.pointer; + if (ata_msg_probe(ap)) + ata_dev_printk(dev, KERN_DEBUG, + "%s: returning gtf=%p, gtf_count=%d\n", + __FUNCTION__, *gtf, rc); + } return rc; out_free: - kfree(output.pointer); + ata_acpi_clear_gtf(dev); return rc; } @@ -533,11 +544,10 @@ static int taskfile_load_raw(struct ata_device *dev, static int ata_acpi_exec_tfs(struct ata_device *dev) { struct ata_acpi_gtf *gtf = NULL; - void *ptr_to_free = NULL; int gtf_count, i, rc; /* get taskfiles */ - gtf_count = ata_dev_get_GTF(dev, >f, &ptr_to_free); + gtf_count = ata_dev_get_GTF(dev, >f); /* execute them */ for (i = 0, rc = 0; i < gtf_count; i++) { @@ -551,7 +561,7 @@ static int ata_acpi_exec_tfs(struct ata_device *dev) rc = tmp; } - kfree(ptr_to_free); + ata_acpi_clear_gtf(dev); if (rc == 0) return gtf_count; @@ -644,13 +654,31 @@ void ata_acpi_on_resume(struct ata_port *ap) const struct ata_acpi_gtm *gtm = ata_acpi_init_gtm(ap); struct ata_device *dev; - /* restore timing parameters */ - if (ap->acpi_handle && gtm) + if (ap->acpi_handle && gtm) { + /* _GTM valid */ + + /* restore timing parameters */ ata_acpi_stm(ap, gtm); - /* schedule _GTF */ - ata_link_for_each_dev(dev, &ap->link) - dev->flags |= ATA_DFLAG_ACPI_PENDING; + /* _GTF should immediately follow _STM so that it can + * use values set by _STM. Cache _GTF result and + * schedule _GTF. + */ + ata_link_for_each_dev(dev, &ap->link) { + ata_acpi_clear_gtf(dev); + if (ata_dev_get_GTF(dev, NULL) >= 0) + dev->flags |= ATA_DFLAG_ACPI_PENDING; + } + } else { + /* SATA _GTF needs to be evaulated after _SDD and + * there's no reason to evaluate IDE _GTF early + * without _STM. Clear cache and schedule _GTF. + */ + ata_link_for_each_dev(dev, &ap->link) { + ata_acpi_clear_gtf(dev); + dev->flags |= ATA_DFLAG_ACPI_PENDING; + } + } } /** @@ -735,4 +763,5 @@ int ata_acpi_on_devcfg(struct ata_device *dev) */ void ata_acpi_on_disable(struct ata_device *dev) { + ata_acpi_clear_gtf(dev); } diff --git a/include/linux/libata.h b/include/linux/libata.h index ba84d8a37545..cb91280be9bd 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -498,6 +498,7 @@ struct ata_device { struct scsi_device *sdev; /* attached SCSI device */ #ifdef CONFIG_ATA_ACPI acpi_handle acpi_handle; + union acpi_object *gtf_cache; #endif /* n_sector is used as CLEAR_OFFSET, read comment above CLEAR_OFFSET */ u64 n_sectors; /* size of device, if ATA */ From 66fa7f2158e84530aa4a1839a3500d6bdb231301 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sat, 15 Dec 2007 15:05:04 +0900 Subject: [PATCH 12/16] libata-acpi: improve ACPI disabling * If _GTF evalution fails, it's pointless to retry. If nothing else is wrong, just ignore the error. * After disabling ACPI, return success iff the number of executed _GTF command equals zero. Otherwise, tell EH to retry. This change fixes bogus 1 return bug where ata_acpi_on_devcfg() expects the caller to reload IDENTIFY data and continue but the caller interprets it as an error. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-acpi.c | 59 +++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index e0dd132fd490..5932ae24ddc1 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -346,8 +346,8 @@ EXPORT_SYMBOL_GPL(ata_acpi_stm); * EH context. * * RETURNS: - * Number of taskfiles on success, 0 if _GTF doesn't exist or doesn't - * contain valid data. + * Number of taskfiles on success, 0 if _GTF doesn't exist. -EINVAL + * if _GTF is invalid. */ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf) { @@ -380,6 +380,7 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf) ata_dev_printk(dev, KERN_WARNING, "_GTF evaluation failed (AE 0x%x)\n", status); + rc = -EINVAL; } goto out_free; } @@ -391,6 +392,7 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf) __FUNCTION__, (unsigned long long)output.length, output.pointer); + rc = -EINVAL; goto out_free; } @@ -398,6 +400,7 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf) ata_dev_printk(dev, KERN_WARNING, "_GTF unexpected object type 0x%x\n", out_obj->type); + rc = -EINVAL; goto out_free; } @@ -405,6 +408,7 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf) ata_dev_printk(dev, KERN_WARNING, "unexpected _GTF length (%d)\n", out_obj->buffer.length); + rc = -EINVAL; goto out_free; } @@ -531,6 +535,7 @@ static int taskfile_load_raw(struct ata_device *dev, /** * ata_acpi_exec_tfs - get then write drive taskfile settings * @dev: target ATA device + * @nr_executed: out paramter for the number of executed commands * * Evaluate _GTF and excute returned taskfiles. * @@ -538,16 +543,19 @@ static int taskfile_load_raw(struct ata_device *dev, * EH context. * * RETURNS: - * Number of executed taskfiles on success, 0 if _GTF doesn't exist or - * doesn't contain valid data. -errno on other errors. + * Number of executed taskfiles on success, 0 if _GTF doesn't exist. + * -errno on other errors. */ -static int ata_acpi_exec_tfs(struct ata_device *dev) +static int ata_acpi_exec_tfs(struct ata_device *dev, int *nr_executed) { struct ata_acpi_gtf *gtf = NULL; int gtf_count, i, rc; /* get taskfiles */ - gtf_count = ata_dev_get_GTF(dev, >f); + rc = ata_dev_get_GTF(dev, >f); + if (rc < 0) + return rc; + gtf_count = rc; /* execute them */ for (i = 0, rc = 0; i < gtf_count; i++) { @@ -559,12 +567,12 @@ static int ata_acpi_exec_tfs(struct ata_device *dev) tmp = taskfile_load_raw(dev, gtf++); if (!rc) rc = tmp; + + (*nr_executed)++; } ata_acpi_clear_gtf(dev); - if (rc == 0) - return gtf_count; return rc; } @@ -700,6 +708,7 @@ int ata_acpi_on_devcfg(struct ata_device *dev) struct ata_port *ap = dev->link->ap; struct ata_eh_context *ehc = &ap->link.eh_context; int acpi_sata = ap->flags & ATA_FLAG_ACPI_SATA; + int nr_executed = 0; int rc; if (!dev->acpi_handle) @@ -718,14 +727,14 @@ int ata_acpi_on_devcfg(struct ata_device *dev) } /* do _GTF */ - rc = ata_acpi_exec_tfs(dev); - if (rc < 0) + rc = ata_acpi_exec_tfs(dev, &nr_executed); + if (rc) goto acpi_err; dev->flags &= ~ATA_DFLAG_ACPI_PENDING; /* refresh IDENTIFY page if any _GTF command has been executed */ - if (rc > 0) { + if (nr_executed) { rc = ata_dev_reread_id(dev, 0); if (rc < 0) { ata_dev_printk(dev, KERN_ERR, "failed to IDENTIFY " @@ -737,18 +746,26 @@ int ata_acpi_on_devcfg(struct ata_device *dev) return 0; acpi_err: - /* let EH retry on the first failure, disable ACPI on the second */ - if (dev->flags & ATA_DFLAG_ACPI_FAILED) { - ata_dev_printk(dev, KERN_WARNING, "ACPI on devcfg failed the " - "second time, disabling (errno=%d)\n", rc); + /* ignore evaluation failure if we can continue safely */ + if (rc == -EINVAL && !nr_executed && !(ap->pflags & ATA_PFLAG_FROZEN)) + return 0; - dev->acpi_handle = NULL; - - /* if port is working, request IDENTIFY reload and continue */ - if (!(ap->pflags & ATA_PFLAG_FROZEN)) - rc = 1; + /* fail and let EH retry once more for unknown IO errors */ + if (!(dev->flags & ATA_DFLAG_ACPI_FAILED)) { + dev->flags |= ATA_DFLAG_ACPI_FAILED; + return rc; } - dev->flags |= ATA_DFLAG_ACPI_FAILED; + + ata_dev_printk(dev, KERN_WARNING, + "ACPI: failed the second time, disabled\n"); + dev->acpi_handle = NULL; + + /* We can safely continue if no _GTF command has been executed + * and port is not frozen. + */ + if (!nr_executed && !(ap->pflags & ATA_PFLAG_FROZEN)) + return 0; + return rc; } From 0e8634bf8e48e50aa96c7e7becafcf9d98c1a28d Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sat, 15 Dec 2007 15:05:05 +0900 Subject: [PATCH 13/16] libata-acpi: improve _GTF execution error handling and reporting As _GTF commands can't transfer data, device error never signals transfer error. It indicates that the device vetoed the operation, so it's meaningless to retry. This patch makes libata-acpi to report and continue on device errors when executing _GTF commands. Also commands rejected by device don't contribute to the number of _GTF commands executed. While at it, update _GTF execution reporting such that all successful commands are logged at KERN_DEBUG and rename taskfile_load_raw() to ata_acpi_run_tf() for consistency. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-acpi.c | 77 +++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index 5932ae24ddc1..d4cb557a727d 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -466,7 +466,7 @@ int ata_acpi_cbl_80wire(struct ata_port *ap) EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire); /** - * taskfile_load_raw - send taskfile registers to host controller + * ata_acpi_run_tf - send taskfile registers to host controller * @dev: target ATA device * @gtf: raw ATA taskfile register set (0x1f1 - 0x1f7) * @@ -485,14 +485,17 @@ EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire); * EH context. * * RETURNS: - * 0 on success, -errno on failure. + * 1 if command is executed successfully. 0 if ignored or rejected, + * -errno on other errors. */ -static int taskfile_load_raw(struct ata_device *dev, - const struct ata_acpi_gtf *gtf) +static int ata_acpi_run_tf(struct ata_device *dev, + const struct ata_acpi_gtf *gtf) { - struct ata_port *ap = dev->link->ap; struct ata_taskfile tf, rtf; unsigned int err_mask; + const char *level; + char msg[60]; + int rc; if ((gtf->tf[0] == 0) && (gtf->tf[1] == 0) && (gtf->tf[2] == 0) && (gtf->tf[3] == 0) && (gtf->tf[4] == 0) && (gtf->tf[5] == 0) @@ -512,24 +515,39 @@ static int taskfile_load_raw(struct ata_device *dev, tf.device = gtf->tf[5]; /* 0x1f6 */ tf.command = gtf->tf[6]; /* 0x1f7 */ - if (ata_msg_probe(ap)) - ata_dev_printk(dev, KERN_DEBUG, "executing ACPI cmd " - "%02x/%02x:%02x:%02x:%02x:%02x:%02x\n", - tf.command, tf.feature, tf.nsect, - tf.lbal, tf.lbam, tf.lbah, tf.device); - rtf = tf; err_mask = ata_exec_internal(dev, &rtf, NULL, DMA_NONE, NULL, 0, 0); - if (err_mask) { - ata_dev_printk(dev, KERN_ERR, - "ACPI cmd %02x/%02x:%02x:%02x:%02x:%02x:%02x failed " - "(Emask=0x%x Stat=0x%02x Err=0x%02x)\n", - tf.command, tf.feature, tf.nsect, tf.lbal, tf.lbam, - tf.lbah, tf.device, err_mask, rtf.command, rtf.feature); - return -EIO; + + switch (err_mask) { + case 0: + level = KERN_DEBUG; + snprintf(msg, sizeof(msg), "succeeded"); + rc = 1; + break; + + case AC_ERR_DEV: + level = KERN_INFO; + snprintf(msg, sizeof(msg), + "rejected by device (Stat=0x%02x Err=0x%02x)", + rtf.command, rtf.feature); + rc = 0; + break; + + default: + level = KERN_ERR; + snprintf(msg, sizeof(msg), + "failed (Emask=0x%x Stat=0x%02x Err=0x%02x)", + err_mask, rtf.command, rtf.feature); + rc = -EIO; + break; } - return 0; + ata_dev_printk(dev, level, + "ACPI cmd %02x/%02x:%02x:%02x:%02x:%02x:%02x %s\n", + tf.command, tf.feature, tf.nsect, tf.lbal, + tf.lbam, tf.lbah, tf.device, msg); + + return rc; } /** @@ -558,22 +576,19 @@ static int ata_acpi_exec_tfs(struct ata_device *dev, int *nr_executed) gtf_count = rc; /* execute them */ - for (i = 0, rc = 0; i < gtf_count; i++) { - int tmp; - - /* ACPI errors are eventually ignored. Run till the - * end even after errors. - */ - tmp = taskfile_load_raw(dev, gtf++); - if (!rc) - rc = tmp; - - (*nr_executed)++; + for (i = 0; i < gtf_count; i++) { + rc = ata_acpi_run_tf(dev, gtf++); + if (rc < 0) + break; + if (rc) + (*nr_executed)++; } ata_acpi_clear_gtf(dev); - return rc; + if (rc < 0) + return rc; + return 0; } /** From 3264a8d8f95348e05cc6ac1ce747a8339ed7ab08 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sat, 15 Dec 2007 15:05:06 +0900 Subject: [PATCH 14/16] libata-acpi: implement _GTF command filtering Implement _GTF command filtering which can be controlled by libata.acpi_filter kernel parameter. Currently SETXFER and LOCK commands are filtered. libata configures transfer mode by itself and _GTF SETXFER commands can potentially disrupt device configuration. _GTM/_STM mechanism can't handle hotplugging too well and when _GTF is executed, controller is in PIO0 rather than the mode _STM configured. Note that detecting SET MAX LOCK requires looking at the previous command. This adds a bit to code complexity. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-acpi.c | 151 ++++++++++++++++++++++++++++---------- 1 file changed, 111 insertions(+), 40 deletions(-) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index d4cb557a727d..7bf4befd96bc 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -6,6 +6,7 @@ * Copyright (C) 2006 Randy Dunlap */ +#include #include #include #include @@ -25,6 +26,18 @@ #include #include +enum { + ATA_ACPI_FILTER_SETXFER = 1 << 0, + ATA_ACPI_FILTER_LOCK = 1 << 1, + + ATA_ACPI_FILTER_DEFAULT = ATA_ACPI_FILTER_SETXFER | + ATA_ACPI_FILTER_LOCK, +}; + +static unsigned int ata_acpi_gtf_filter = ATA_ACPI_FILTER_DEFAULT; +module_param_named(acpi_gtf_filter, ata_acpi_gtf_filter, int, 0644); +MODULE_PARM_DESC(acpi_gtf_filter, "filter mask for ACPI _GTF commands, set to filter out (0x1=set xfermode, 0x2=lock/freeze lock)"); + #define NO_PORT_MULT 0xffff #define SATA_ADR(root, pmp) (((root) << 16) | (pmp)) @@ -465,6 +478,60 @@ int ata_acpi_cbl_80wire(struct ata_port *ap) EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire); +static void ata_acpi_gtf_to_tf(struct ata_device *dev, + const struct ata_acpi_gtf *gtf, + struct ata_taskfile *tf) +{ + ata_tf_init(dev, tf); + + tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + tf->protocol = ATA_PROT_NODATA; + tf->feature = gtf->tf[0]; /* 0x1f1 */ + tf->nsect = gtf->tf[1]; /* 0x1f2 */ + tf->lbal = gtf->tf[2]; /* 0x1f3 */ + tf->lbam = gtf->tf[3]; /* 0x1f4 */ + tf->lbah = gtf->tf[4]; /* 0x1f5 */ + tf->device = gtf->tf[5]; /* 0x1f6 */ + tf->command = gtf->tf[6]; /* 0x1f7 */ +} + +static int ata_acpi_filter_tf(const struct ata_taskfile *tf, + const struct ata_taskfile *ptf) +{ + if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_SETXFER) { + /* libata doesn't use ACPI to configure transfer mode. + * It will only confuse device configuration. Skip. + */ + if (tf->command == ATA_CMD_SET_FEATURES && + tf->feature == SETFEATURES_XFER) + return 1; + } + + if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_LOCK) { + /* BIOS writers, sorry but we don't wanna lock + * features unless the user explicitly said so. + */ + + /* DEVICE CONFIGURATION FREEZE LOCK */ + if (tf->command == ATA_CMD_CONF_OVERLAY && + tf->feature == ATA_DCO_FREEZE_LOCK) + return 1; + + /* SECURITY FREEZE LOCK */ + if (tf->command == ATA_CMD_SEC_FREEZE_LOCK) + return 1; + + /* SET MAX LOCK and SET MAX FREEZE LOCK */ + if ((!ptf || ptf->command != ATA_CMD_READ_NATIVE_MAX) && + tf->command == ATA_CMD_SET_MAX && + (tf->feature == ATA_SET_MAX_LOCK || + tf->feature == ATA_SET_MAX_FREEZE_LOCK)) + return 1; + } + + return 0; +} + /** * ata_acpi_run_tf - send taskfile registers to host controller * @dev: target ATA device @@ -485,13 +552,15 @@ EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire); * EH context. * * RETURNS: - * 1 if command is executed successfully. 0 if ignored or rejected, - * -errno on other errors. + * 1 if command is executed successfully. 0 if ignored, rejected or + * filtered out, -errno on other errors. */ static int ata_acpi_run_tf(struct ata_device *dev, - const struct ata_acpi_gtf *gtf) + const struct ata_acpi_gtf *gtf, + const struct ata_acpi_gtf *prev_gtf) { - struct ata_taskfile tf, rtf; + struct ata_taskfile *pptf = NULL; + struct ata_taskfile tf, ptf, rtf; unsigned int err_mask; const char *level; char msg[60]; @@ -502,44 +571,44 @@ static int ata_acpi_run_tf(struct ata_device *dev, && (gtf->tf[6] == 0)) return 0; - ata_tf_init(dev, &tf); + ata_acpi_gtf_to_tf(dev, gtf, &tf); + if (prev_gtf) { + ata_acpi_gtf_to_tf(dev, prev_gtf, &ptf); + pptf = &ptf; + } - /* convert gtf to tf */ - tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; /* TBD */ - tf.protocol = ATA_PROT_NODATA; - tf.feature = gtf->tf[0]; /* 0x1f1 */ - tf.nsect = gtf->tf[1]; /* 0x1f2 */ - tf.lbal = gtf->tf[2]; /* 0x1f3 */ - tf.lbam = gtf->tf[3]; /* 0x1f4 */ - tf.lbah = gtf->tf[4]; /* 0x1f5 */ - tf.device = gtf->tf[5]; /* 0x1f6 */ - tf.command = gtf->tf[6]; /* 0x1f7 */ + if (!ata_acpi_filter_tf(&tf, pptf)) { + rtf = tf; + err_mask = ata_exec_internal(dev, &rtf, NULL, + DMA_NONE, NULL, 0, 0); - rtf = tf; - err_mask = ata_exec_internal(dev, &rtf, NULL, DMA_NONE, NULL, 0, 0); + switch (err_mask) { + case 0: + level = KERN_DEBUG; + snprintf(msg, sizeof(msg), "succeeded"); + rc = 1; + break; - switch (err_mask) { - case 0: - level = KERN_DEBUG; - snprintf(msg, sizeof(msg), "succeeded"); - rc = 1; - break; + case AC_ERR_DEV: + level = KERN_INFO; + snprintf(msg, sizeof(msg), + "rejected by device (Stat=0x%02x Err=0x%02x)", + rtf.command, rtf.feature); + rc = 0; + break; - case AC_ERR_DEV: + default: + level = KERN_ERR; + snprintf(msg, sizeof(msg), + "failed (Emask=0x%x Stat=0x%02x Err=0x%02x)", + err_mask, rtf.command, rtf.feature); + rc = -EIO; + break; + } + } else { level = KERN_INFO; - snprintf(msg, sizeof(msg), - "rejected by device (Stat=0x%02x Err=0x%02x)", - rtf.command, rtf.feature); + snprintf(msg, sizeof(msg), "filtered out"); rc = 0; - break; - - default: - level = KERN_ERR; - snprintf(msg, sizeof(msg), - "failed (Emask=0x%x Stat=0x%02x Err=0x%02x)", - err_mask, rtf.command, rtf.feature); - rc = -EIO; - break; } ata_dev_printk(dev, level, @@ -566,7 +635,7 @@ static int ata_acpi_run_tf(struct ata_device *dev, */ static int ata_acpi_exec_tfs(struct ata_device *dev, int *nr_executed) { - struct ata_acpi_gtf *gtf = NULL; + struct ata_acpi_gtf *gtf = NULL, *pgtf = NULL; int gtf_count, i, rc; /* get taskfiles */ @@ -576,12 +645,14 @@ static int ata_acpi_exec_tfs(struct ata_device *dev, int *nr_executed) gtf_count = rc; /* execute them */ - for (i = 0; i < gtf_count; i++) { - rc = ata_acpi_run_tf(dev, gtf++); + for (i = 0; i < gtf_count; i++, gtf++) { + rc = ata_acpi_run_tf(dev, gtf, pgtf); if (rc < 0) break; - if (rc) + if (rc) { (*nr_executed)++; + pgtf = gtf; + } } ata_acpi_clear_gtf(dev); From f2dfc1a12bb1a029df62b018a8e1882e91041025 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 12 Dec 2007 12:12:46 +0900 Subject: [PATCH 15/16] libata: update atapi_eh_request_sense() such that lbam/lbah contains buffer size While updating lbam/h for ATAPI commands, atapi_eh_request_sense() was left out. Update it. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-eh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index e6605f038647..f0124a8d3134 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -1264,8 +1264,8 @@ static unsigned int atapi_eh_request_sense(struct ata_queued_cmd *qc) tf.feature |= ATAPI_PKT_DMA; } else { tf.protocol = ATA_PROT_ATAPI; - tf.lbam = (8 * 1024) & 0xff; - tf.lbah = (8 * 1024) >> 8; + tf.lbam = SCSI_SENSE_BUFFERSIZE; + tf.lbah = 0; } return ata_exec_internal(dev, &tf, cdb, DMA_FROM_DEVICE, From 140b5e59119a172a91b5fa13d54ca4f79bbefee1 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 12 Dec 2007 12:21:52 +0900 Subject: [PATCH 16/16] libata: fix ATAPI draining With ATAPI transfer chunk size properly programmed, libata PIO HSM should be able to handle full spurious data chunks. Also, it's a good idea to suppress trailing data warning for misc ATAPI commands as there can be many of them per command - for example, if the chunk size is 16 and the drive tries to transfer 510 bytes, there can be 31 trailing data messages. This patch makes the following updates to libata ATAPI PIO HSM implementation. * Make it drain full spurious chunks. * Suppress trailing data warning message for misc commands. * Put limit on how many bytes can be drained. * If odd, round up consumed bytes and the number of bytes to be drained. This gets the number of bytes to drain right for drivers which do 16bit PIO. This patch is partial backport of improve-ATAPI-data-xfer patchset pending for #upstream. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/ata/libata-core.c | 87 +++++++++++++++++++++++++++++---------- include/linux/libata.h | 2 + 2 files changed, 67 insertions(+), 22 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 4af939a00e54..4753a1831dbc 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -64,6 +64,7 @@ #include #include #include +#include #include "libata.h" @@ -4651,6 +4652,43 @@ int ata_check_atapi_dma(struct ata_queued_cmd *qc) return 0; } +/** + * atapi_qc_may_overflow - Check whether data transfer may overflow + * @qc: ATA command in question + * + * ATAPI commands which transfer variable length data to host + * might overflow due to application error or hardare bug. This + * function checks whether overflow should be drained and ignored + * for @qc. + * + * LOCKING: + * None. + * + * RETURNS: + * 1 if @qc may overflow; otherwise, 0. + */ +static int atapi_qc_may_overflow(struct ata_queued_cmd *qc) +{ + if (qc->tf.protocol != ATA_PROT_ATAPI && + qc->tf.protocol != ATA_PROT_ATAPI_DMA) + return 0; + + if (qc->tf.flags & ATA_TFLAG_WRITE) + return 0; + + switch (qc->cdb[0]) { + case READ_10: + case READ_12: + case WRITE_10: + case WRITE_12: + case GPCMD_READ_CD: + case GPCMD_READ_CD_MSF: + return 0; + } + + return 1; +} + /** * ata_std_qc_defer - Check whether a qc needs to be deferred * @qc: ATA command in question @@ -5139,23 +5177,19 @@ static void atapi_send_cdb(struct ata_port *ap, struct ata_queued_cmd *qc) * Inherited from caller. * */ - -static void __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes) +static int __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes) { int do_write = (qc->tf.flags & ATA_TFLAG_WRITE); - struct scatterlist *sg = qc->__sg; - struct scatterlist *lsg = sg_last(qc->__sg, qc->n_elem); struct ata_port *ap = qc->ap; + struct ata_eh_info *ehi = &qc->dev->link->eh_info; + struct scatterlist *sg; struct page *page; unsigned char *buf; unsigned int offset, count; - int no_more_sg = 0; - - if (qc->curbytes + bytes >= qc->nbytes) - ap->hsm_task_state = HSM_ST_LAST; next_sg: - if (unlikely(no_more_sg)) { + sg = qc->cursg; + if (unlikely(!sg)) { /* * The end of qc->sg is reached and the device expects * more data to transfer. In order not to overrun qc->sg @@ -5164,21 +5198,28 @@ next_sg: * - for write case, padding zero data to the device */ u16 pad_buf[1] = { 0 }; - unsigned int words = bytes >> 1; unsigned int i; - if (words) /* warning if bytes > 1 */ - ata_dev_printk(qc->dev, KERN_WARNING, - "%u bytes trailing data\n", bytes); + if (bytes > qc->curbytes - qc->nbytes + ATAPI_MAX_DRAIN) { + ata_ehi_push_desc(ehi, "too much trailing data " + "buf=%u cur=%u bytes=%u", + qc->nbytes, qc->curbytes, bytes); + return -1; + } - for (i = 0; i < words; i++) + /* overflow is exptected for misc ATAPI commands */ + if (bytes && !atapi_qc_may_overflow(qc)) + ata_dev_printk(qc->dev, KERN_WARNING, "ATAPI %u bytes " + "trailing data (cdb=%02x nbytes=%u)\n", + bytes, qc->cdb[0], qc->nbytes); + + for (i = 0; i < (bytes + 1) / 2; i++) ap->ops->data_xfer(qc->dev, (unsigned char *)pad_buf, 2, do_write); - ap->hsm_task_state = HSM_ST_LAST; - return; - } + qc->curbytes += bytes; - sg = qc->cursg; + return 0; + } page = sg_page(sg); offset = sg->offset + qc->cursg_ofs; @@ -5213,19 +5254,20 @@ next_sg: } bytes -= count; + if ((count & 1) && bytes) + bytes--; qc->curbytes += count; qc->cursg_ofs += count; if (qc->cursg_ofs == sg->length) { - if (qc->cursg == lsg) - no_more_sg = 1; - qc->cursg = sg_next(qc->cursg); qc->cursg_ofs = 0; } if (bytes) goto next_sg; + + return 0; } /** @@ -5268,7 +5310,8 @@ static void atapi_pio_bytes(struct ata_queued_cmd *qc) VPRINTK("ata%u: xfering %d bytes\n", ap->print_id, bytes); - __atapi_pio_bytes(qc, bytes); + if (__atapi_pio_bytes(qc, bytes)) + goto err_out; ata_altstatus(ap); /* flush */ return; diff --git a/include/linux/libata.h b/include/linux/libata.h index cb91280be9bd..124033cb5e9b 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -119,6 +119,8 @@ enum { ATA_DEF_BUSY_WAIT = 10000, ATA_SHORT_PAUSE = (HZ >> 6) + 1, + ATAPI_MAX_DRAIN = 16 << 10, + ATA_SHT_EMULATED = 1, ATA_SHT_CMD_PER_LUN = 1, ATA_SHT_THIS_ID = -1,