linux-kernel - Re: 2.6.29-rc libata sff 32bit PIO regression

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.0902012050190.16351@blonde.anvils>
Date:	Sun, 1 Feb 2009 21:13:08 +0000 (GMT)
From:	Hugh Dickins <hugh@...itas.com>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
cc:	Jeff Garzik <jgarzik@...hat.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Sergei Shtylyov <sshtylyov@...mvista.com>,
	Larry Finger <Larry.Finger@...inger.net>,
	Mikael Pettersson <mikpe@...uu.se>, linux-ide@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: 2.6.29-rc libata sff 32bit PIO regression

On Mon, 26 Jan 2009, Alan Cox wrote:
> > [PATCH] libata sff: 32bit PIO use 16bit on slop
> > 
> > 871af1210f13966ab911ed2166e4ab2ce775b99d libata: Add 32bit PIO support
> > causes errors on a four-year-old ata_piix Dell Precision 670.  Using
> > 16bit PIO instead of 32bit PIO on the odd 1, 2 or 3 chars fixes that.
> > 
> > Signed-off-by: Hugh Dickins <hugh@...itas.com>
> 
> For the 3 bytes of slop it should use a single iowrite32 but otherwise
> that seems ok. We do need to handle the FIFO setup on the AMD differently
> if we do this ...

Sorry, I believe you were waiting on me for this, to accompany your
AMD and VLB patches.  I'm afraid I don't have any such AMD devices
to test this along with yours, and the only non-0 slop that I've seen
in testing has been 2 (about 25% of ops, so I removed the "unlikely").
But this patch works as well for me as the patch I posted before
(though much more verbose: please simplify if you see a better way).


[PATCH] libata sff: 32bit PIO use 16bit on slop

871af1210f13966ab911ed2166e4ab2ce775b99d libata: Add 32bit PIO support
causes errors on a four-year-old ata_piix Dell Precision 670.  Using
16bit PIO instead of 32bit PIO on the odd 1 or 2 chars fixes that,
but Alan Cox indicates that we should still use 32bit for 3 chars.

Signed-off-by: Hugh Dickins <hugh@...itas.com>
---

 drivers/ata/libata-sff.c |   33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

--- 2.6.29-rc3/drivers/ata/libata-sff.c	2009-01-29 12:33:28.000000000 +0000
+++ linux/drivers/ata/libata-sff.c	2009-02-01 20:21:13.000000000 +0000
@@ -773,18 +773,33 @@ unsigned int ata_sff_data_xfer32(struct
 	else
 		iowrite32_rep(data_addr, buf, words);
 
-	if (unlikely(slop)) {
-		__le32 pad;
-		if (rw == READ) {
-			pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr));
-			memcpy(buf + buflen - slop, &pad, slop);
+	if (slop) {
+		unsigned char *trailing_buf = buf + buflen - slop;
+
+		if (slop <= 2) {
+			__le16 slop_word;
+			if (rw == READ) {
+				slop_word = cpu_to_le16(ioread16(data_addr));
+				memcpy(trailing_buf, &slop_word, slop);
+			} else {
+				slop_word = 0;
+				memcpy(&slop_word, trailing_buf, slop);
+				iowrite16(le16_to_cpu(slop_word), data_addr);
+			}
 		} else {
-			memcpy(&pad, buf + buflen - slop, slop);
-			iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr);
+			__le32 slop_word;
+			if (rw == READ) {
+				slop_word = cpu_to_le32(ioread32(data_addr));
+				memcpy(trailing_buf, &slop_word, slop);
+			} else {
+				slop_word = 0;
+				memcpy(&slop_word, trailing_buf, slop);
+				iowrite32(le32_to_cpu(slop_word), data_addr);
+			}
 		}
-		words++;
 	}
-	return words << 2;
+
+	return buflen + (buflen & 1);
 }
 EXPORT_SYMBOL_GPL(ata_sff_data_xfer32);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/