| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 20 Feb 2016 12:01:53 +0100
From: Ondrej Zary <linux@...nbow-software.org>
To: linux-ide@...r.kernel.org
Cc: Tejun Heo <tj@...nel.org>, Joseph Chan <JosephChan@....com.tw>,
Kernel development list <linux-kernel@...r.kernel.org>
Subject: [PATCH v2] sata_via: Apply WD workaround only when needed on VT6421
Currently, workaround for broken WD drives is applied always, slowing
down all drives. And it has a bug - it's not applied after resume.
Apply the workaround only if the error really appears
(SErr == 0x1000500). This allows unaffected drives to run at full speed
(provided that no affected drive is connected to the controller).
Also make sure the workaround is re-applied on resume.
Tested on VT6421.
As SCR registers access is known to cause problems on VT6420 (and I
don't have it to test), keep the workaround applied always on VT6420.
Unaffected drive (Hitachi HDS721680PLA380):
Before:
$ hdparm -t --direct /dev/sdb
/dev/sdb:
Timing O_DIRECT disk reads: 160 MB in 3.01 seconds = 53.16 MB/sec
After:
$ hdparm -t --direct /dev/sdb
/dev/sdb:
Timing O_DIRECT disk reads: 200 MB in 3.01 seconds = 66.47 MB/sec
Affected drive (WDC WD5003ABYX-18WERA0):
Before:
$ hdparm -t --direct /dev/sda
/dev/sda:
Timing O_DIRECT disk reads: 180 MB in 3.02 seconds = 59.51 MB/sec
After:
$ hdparm -t --direct /dev/sdb
/dev/sdb:
Timing O_DIRECT disk reads: 156 MB in 3.03 seconds = 51.48 MB/sec
$ hdparm -t --direct /dev/sdb
/dev/sdb:
Timing O_DIRECT disk reads: 180 MB in 3.02 seconds = 59.64 MB/sec
The first hdparm is slower because of the error:
[ 50.408042] ata5: Incompatible drive: enabling workaround. This slows down transfer rate to ~60 MB/s
[ 50.728052] ata5: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
[ 50.744834] ata5.00: configured for UDMA/133
Signed-off-by: Ondrej Zary <linux@...nbow-software.org>
---
Changes in v2: added missing check in svia_pci_device_resume() so the
workaround is re-applied only when marked as such
---
drivers/ata/sata_via.c | 81 +++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 74 insertions(+), 7 deletions(-)
diff --git a/drivers/ata/sata_via.c b/drivers/ata/sata_via.c
index 17d31fc..9804054 100644
--- a/drivers/ata/sata_via.c
+++ b/drivers/ata/sata_via.c
@@ -73,7 +73,14 @@ enum {
SATA_EXT_PHY = (1 << 6), /* 0==use PATA, 1==ext phy */
};
+struct svia_priv {
+ bool wd_workaround;
+};
+
static int svia_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+#ifdef CONFIG_PM_SLEEP
+static int svia_pci_device_resume(struct pci_dev *pdev);
+#endif
static int svia_scr_read(struct ata_link *link, unsigned int sc_reg, u32 *val);
static int svia_scr_write(struct ata_link *link, unsigned int sc_reg, u32 val);
static int vt8251_scr_read(struct ata_link *link, unsigned int scr, u32 *val);
@@ -85,6 +92,7 @@ static void vt6420_bmdma_start(struct ata_queued_cmd *qc);
static int vt6421_pata_cable_detect(struct ata_port *ap);
static void vt6421_set_pio_mode(struct ata_port *ap, struct ata_device *adev);
static void vt6421_set_dma_mode(struct ata_port *ap, struct ata_device *adev);
+static void vt6421_error_handler(struct ata_port *ap);
static const struct pci_device_id svia_pci_tbl[] = {
{ PCI_VDEVICE(VIA, 0x5337), vt6420 },
@@ -105,7 +113,7 @@ static struct pci_driver svia_pci_driver = {
.probe = svia_init_one,
#ifdef CONFIG_PM_SLEEP
.suspend = ata_pci_device_suspend,
- .resume = ata_pci_device_resume,
+ .resume = svia_pci_device_resume,
#endif
.remove = ata_pci_remove_one,
};
@@ -137,6 +145,7 @@ static struct ata_port_operations vt6421_sata_ops = {
.inherits = &svia_base_ops,
.scr_read = svia_scr_read,
.scr_write = svia_scr_write,
+ .error_handler = vt6421_error_handler,
};
static struct ata_port_operations vt8251_ops = {
@@ -536,7 +545,36 @@ static int vt8251_prepare_host(struct pci_dev *pdev, struct ata_host **r_host)
return 0;
}
-static void svia_configure(struct pci_dev *pdev, int board_id)
+static void svia_wd_fix(struct pci_dev *pdev)
+{
+ u8 tmp8;
+
+ pci_read_config_byte(pdev, 0x52, &tmp8);
+ pci_write_config_byte(pdev, 0x52, tmp8 | BIT(2));
+}
+
+static void vt6421_error_handler(struct ata_port *ap)
+{
+ struct svia_priv *hpriv = ap->host->private_data;
+ struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+ u32 serror;
+
+ /* see svia_configure() for description */
+ if (!hpriv->wd_workaround) {
+ svia_scr_read(&ap->link, SCR_ERROR, &serror);
+ if (serror == 0x1000500) {
+ ata_port_warn(ap, "Incompatible drive: enabling workaround. This slows down transfer rate to ~60 MB/s");
+ svia_wd_fix(pdev);
+ hpriv->wd_workaround = true;
+ ap->link.eh_context.i.flags |= ATA_EHI_QUIET;
+ }
+ }
+
+ ata_sff_error_handler(ap);
+}
+
+static void svia_configure(struct pci_dev *pdev, int board_id,
+ struct svia_priv *hpriv)
{
u8 tmp8;
@@ -593,11 +631,15 @@ static void svia_configure(struct pci_dev *pdev, int board_id)
* https://bugzilla.kernel.org/show_bug.cgi?id=15173
* http://article.gmane.org/gmane.linux.ide/46352
* http://thread.gmane.org/gmane.linux.kernel/1062139
+ *
+ * As the fix slows down data transfer, apply it only if the error
+ * actually appears - see vt6421_error_handler()
+ * Apply the fix always on vt6420 as we don't know if SCR_ERROR can be
+ * read safely.
*/
- if (board_id == vt6420 || board_id == vt6421) {
- pci_read_config_byte(pdev, 0x52, &tmp8);
- tmp8 |= 1 << 2;
- pci_write_config_byte(pdev, 0x52, tmp8);
+ if (board_id == vt6420) {
+ svia_wd_fix(pdev);
+ hpriv->wd_workaround = true;
}
}
@@ -608,6 +650,7 @@ static int svia_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
struct ata_host *host = NULL;
int board_id = (int) ent->driver_data;
const unsigned *bar_sizes;
+ struct svia_priv *hpriv;
ata_print_version_once(&pdev->dev, DRV_VERSION);
@@ -647,11 +690,35 @@ static int svia_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
if (rc)
return rc;
- svia_configure(pdev, board_id);
+ hpriv = devm_kzalloc(&pdev->dev, sizeof(*hpriv), GFP_KERNEL);
+ if (!hpriv)
+ return -ENOMEM;
+ host->private_data = hpriv;
+
+ svia_configure(pdev, board_id, hpriv);
pci_set_master(pdev);
return ata_host_activate(host, pdev->irq, ata_bmdma_interrupt,
IRQF_SHARED, &svia_sht);
}
+#ifdef CONFIG_PM_SLEEP
+static int svia_pci_device_resume(struct pci_dev *pdev)
+{
+ struct ata_host *host = pci_get_drvdata(pdev);
+ struct svia_priv *hpriv = host->private_data;
+ int rc;
+
+ rc = ata_pci_device_do_resume(pdev);
+ if (rc)
+ return rc;
+
+ if (hpriv->wd_workaround)
+ svia_wd_fix(pdev);
+ ata_host_resume(host);
+
+ return 0;
+}
+#endif
+
module_pci_driver(svia_pci_driver);
--
Ondrej Zary
Powered by blists - more mailing lists