rpms/kernel/F-9 linux-2.6-libata-fix-a-large-collection-of-DMA-mode-mismatches.patch, NONE, 1.1 linux-2.6-libata-lba-28-48-off-by-one-in-ata.h.patch, NONE, 1.1 kernel.spec, 1.765, 1.766
Chuck Ebbert
cebbert at fedoraproject.org
Sun Sep 14 01:57:56 UTC 2008
Author: cebbert
Update of /cvs/pkgs/rpms/kernel/F-9
In directory cvs1.fedora.phx.redhat.com:/tmp/cvs-serv15271
Modified Files:
kernel.spec
Added Files:
linux-2.6-libata-fix-a-large-collection-of-DMA-mode-mismatches.patch
linux-2.6-libata-lba-28-48-off-by-one-in-ata.h.patch
Log Message:
libata: fix DMA mode mistmatches
libata: interpret the LBA28 spec properly
linux-2.6-libata-fix-a-large-collection-of-DMA-mode-mismatches.patch:
--- NEW FILE linux-2.6-libata-fix-a-large-collection-of-DMA-mode-mismatches.patch ---
From: Alan Cox <alan at redhat.com>
Date: Fri, 1 Aug 2008 08:18:34 +0000 (+0100)
Subject: libata: Fix a large collection of DMA mode mismatches
X-Git-Tag: v2.6.27-rc5~51^2~1
X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=commitdiff_plain;h=b15b3ebae102f89c25ccbcae0b2099af312f2e82
libata: Fix a large collection of DMA mode mismatches
Dave Müller sent a diff for the pata_oldpiix that highlighted a problem
where a lot of the ATA drivers assume dma_mode == 0 means "no DMA" while
the core code uses 0xFF.
This turns out to have other consequences such as code doing >= XFER_UDMA_0
also catching 0xFF as UDMAlots. Fortunately it doesn't generally affect
set_dma_mode, although some drivers call back into their own set mode code
from other points.
Having been through the drivers I've added helpers for using_udma/using_mwdma
dma_enabled so that people don't open code ranges that may change (eg if UDMA8
appears somewhere)
Thanks to David for the initial bits
[and added fix for pata_oldpiix from and signed-off-by Dave Mueller
<dave.mueller at gmx.ch> -jg]
Signed-off-by: Alan Cox <alan at redhat.com>
Signed-off-by: Jeff Garzik <jgarzik at redhat.com>
---
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 5f8f57a..79e3a8e 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3288,7 +3288,7 @@ int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
dev->dma_mode = ata_xfer_mask2mode(dma_mask);
found = 1;
- if (dev->dma_mode != 0xff)
+ if (ata_dma_enabled(dev))
used_dma = 1;
}
if (!found)
@@ -3313,7 +3313,7 @@ int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
/* step 3: set host DMA timings */
ata_link_for_each_dev(dev, link) {
- if (!ata_dev_enabled(dev) || dev->dma_mode == 0xff)
+ if (!ata_dev_enabled(dev) || !ata_dma_enabled(dev))
continue;
dev->xfer_mode = dev->dma_mode;
diff --git a/drivers/ata/pata_acpi.c b/drivers/ata/pata_acpi.c
index fbe6057..eb919c1 100644
--- a/drivers/ata/pata_acpi.c
+++ b/drivers/ata/pata_acpi.c
@@ -181,7 +181,7 @@ static unsigned int pacpi_qc_issue(struct ata_queued_cmd *qc)
if (adev != acpi->last) {
pacpi_set_piomode(ap, adev);
- if (adev->dma_mode)
+ if (ata_dma_enabled(adev))
pacpi_set_dmamode(ap, adev);
acpi->last = adev;
}
diff --git a/drivers/ata/pata_atiixp.c b/drivers/ata/pata_atiixp.c
index d7de7ba..e8a0d99 100644
--- a/drivers/ata/pata_atiixp.c
+++ b/drivers/ata/pata_atiixp.c
@@ -183,7 +183,7 @@ static void atiixp_bmdma_start(struct ata_queued_cmd *qc)
u16 tmp16;
pci_read_config_word(pdev, ATIIXP_IDE_UDMA_CONTROL, &tmp16);
- if (adev->dma_mode >= XFER_UDMA_0)
+ if (ata_using_udma(adev))
tmp16 |= (1 << dn);
else
tmp16 &= ~(1 << dn);
diff --git a/drivers/ata/pata_cs5530.c b/drivers/ata/pata_cs5530.c
index 744beeb..0c4b271 100644
--- a/drivers/ata/pata_cs5530.c
+++ b/drivers/ata/pata_cs5530.c
@@ -149,10 +149,10 @@ static unsigned int cs5530_qc_issue(struct ata_queued_cmd *qc)
struct ata_device *prev = ap->private_data;
/* See if the DMA settings could be wrong */
- if (adev->dma_mode != 0 && adev != prev && prev != NULL) {
+ if (ata_dma_enabled(adev) && adev != prev && prev != NULL) {
/* Maybe, but do the channels match MWDMA/UDMA ? */
- if ((adev->dma_mode >= XFER_UDMA_0 && prev->dma_mode < XFER_UDMA_0) ||
- (adev->dma_mode < XFER_UDMA_0 && prev->dma_mode >= XFER_UDMA_0))
+ if ((ata_using_udma(adev) && !ata_using_udma(prev)) ||
+ (ata_using_udma(prev) && !ata_using_udma(adev)))
/* Switch the mode bits */
cs5530_set_dmamode(ap, adev);
}
diff --git a/drivers/ata/pata_oldpiix.c b/drivers/ata/pata_oldpiix.c
index e678af3..df64f24 100644
--- a/drivers/ata/pata_oldpiix.c
+++ b/drivers/ata/pata_oldpiix.c
@@ -198,7 +198,7 @@ static unsigned int oldpiix_qc_issue(struct ata_queued_cmd *qc)
if (adev != ap->private_data) {
oldpiix_set_piomode(ap, adev);
- if (adev->dma_mode)
+ if (ata_dma_enabled(adev))
oldpiix_set_dmamode(ap, adev);
}
return ata_sff_qc_issue(qc);
diff --git a/drivers/ata/pata_sc1200.c b/drivers/ata/pata_sc1200.c
index cbab397..0278fd2 100644
--- a/drivers/ata/pata_sc1200.c
+++ b/drivers/ata/pata_sc1200.c
@@ -167,10 +167,10 @@ static unsigned int sc1200_qc_issue(struct ata_queued_cmd *qc)
struct ata_device *prev = ap->private_data;
/* See if the DMA settings could be wrong */
- if (adev->dma_mode != 0 && adev != prev && prev != NULL) {
+ if (ata_dma_enabled(adev) && adev != prev && prev != NULL) {
/* Maybe, but do the channels match MWDMA/UDMA ? */
- if ((adev->dma_mode >= XFER_UDMA_0 && prev->dma_mode < XFER_UDMA_0) ||
- (adev->dma_mode < XFER_UDMA_0 && prev->dma_mode >= XFER_UDMA_0))
+ if ((ata_using_udma(adev) && !ata_using_udma(prev)) ||
+ (ata_using_udma(prev) && !ata_using_udma(adev)))
/* Switch the mode bits */
sc1200_set_dmamode(ap, adev);
}
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 80233fd..225bfc5 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1429,6 +1429,28 @@ static inline unsigned long ata_deadline(unsigned long from_jiffies,
return from_jiffies + msecs_to_jiffies(timeout_msecs);
}
+/* Don't open code these in drivers as there are traps. Firstly the range may
+ change in future hardware and specs, secondly 0xFF means 'no DMA' but is
+ > UDMA_0. Dyma ddreigiau */
+
+static inline int ata_using_mwdma(struct ata_device *adev)
+{
+ if (adev->dma_mode >= XFER_MW_DMA_0 && adev->dma_mode <= XFER_MW_DMA_4)
+ return 1;
+ return 0;
+}
+
+static inline int ata_using_udma(struct ata_device *adev)
+{
+ if (adev->dma_mode >= XFER_UDMA_0 && adev->dma_mode <= XFER_UDMA_7)
+ return 1;
+ return 0;
+}
+
+static inline int ata_dma_enabled(struct ata_device *adev)
+{
+ return (adev->dma_mode == 0xFF ? 0 : 1);
+}
/**************************************************************************
* PMP - drivers/ata/libata-pmp.c
linux-2.6-libata-lba-28-48-off-by-one-in-ata.h.patch:
--- NEW FILE linux-2.6-libata-lba-28-48-off-by-one-in-ata.h.patch ---
From: Taisuke Yamada <tai at rakugaki.org>
Date: Sat, 13 Sep 2008 20:46:15 +0000 (-0400)
Subject: [libata] LBA28/LBA48 off-by-one bug in ata.h
X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=commitdiff_plain;h=97b697a11b07e2ebfa69c488132596cc5eb24119
[libata] LBA28/LBA48 off-by-one bug in ata.h
I recently bought 3 HGST P7K500-series 500GB SATA drives and
had trouble accessing the block right on the LBA28-LBA48 border.
Here's how it fails (same for all 3 drives):
# dd if=/dev/sdc bs=512 count=1 skip=268435455 > /dev/null
dd: reading `/dev/sdc': Input/output error
0+0 records in
0+0 records out
0 bytes (0 B) copied, 0.288033 seconds, 0.0 kB/s
# dmesg
ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
ata1.00: BMDMA stat 0x25
ata1.00: cmd c8/00:08:f8:ff:ff/00:00:00:00:00/ef tag 0 dma 4096 in
res 51/04:08:f8:ff:ff/00:00:00:00:00/ef Emask 0x1 (device error)
ata1.00: status: { DRDY ERR }
ata1.00: error: { ABRT }
ata1.00: configured for UDMA/33
ata1: EH complete
...
After some investigations, it turned out this seems to be caused
by misinterpretation of the ATA specification on LBA28 access.
Following part is the code in question:
=== include/linux/ata.h ===
static inline int lba_28_ok(u64 block, u32 n_block)
{
/* check the ending block number */
return ((block + n_block - 1) < ((u64)1 << 28)) && (n_block <= 256);
}
HGST drive (sometimes) fails with LBA28 access of {block = 0xfffffff,
n_block = 1}, and this behavior seems to be comformant. Other drives,
including other HGST drives are not that strict, through.
>From the ATA specification:
(http://www.t13.org/Documents/UploadedDocuments/project/d1410r3b-ATA-ATAPI-6.pdf)
8.15.29 Word (61:60): Total number of user addressable sectors
This field contains a value that is one greater than the total number
of user addressable sectors (see 6.2). The maximum value that shall
be placed in this field is 0FFFFFFFh.
So the driver shouldn't use the value of 0xfffffff for LBA28 request
as this exceeds maximum user addressable sector. The logical maximum
value for LBA28 is 0xffffffe.
The obvious fix is to cut "- 1" part, and the patch attached just do
that. I've been using the patched kernel for about a month now, and
the same fix is also floating on the net for some time. So I believe
this fix works reliably.
Just FYI, many Windows/Intel platform users also seems to be struck
by this, and HGST has issued a note pointing to Intel ICH8/9 driver.
"28-bit LBA command is being used to access LBAs 29-bits in length"
http://www.hitachigst.com/hddt/knowtree.nsf/cffe836ed7c12018862565b000530c74/b531b8bce8745fb78825740f00580e23
Also, *BSDs seems to have similar fix included sometime around ~2004,
through I have not checked out exact portion of the code.
Signed-off-by: Taisuke Yamada <tai at rakugaki.org>
Signed-off-by: Jeff Garzik <jgarzik at redhat.com>
---
diff --git a/include/linux/ata.h b/include/linux/ata.h
index 1ce19c1..8a12d71 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -745,7 +745,7 @@ static inline int ata_ok(u8 status)
static inline int lba_28_ok(u64 block, u32 n_block)
{
/* check the ending block number */
- return ((block + n_block - 1) < ((u64)1 << 28)) && (n_block <= 256);
+ return ((block + n_block) < ((u64)1 << 28)) && (n_block <= 256);
}
static inline int lba_48_ok(u64 block, u32 n_block)
Index: kernel.spec
===================================================================
RCS file: /cvs/pkgs/rpms/kernel/F-9/kernel.spec,v
retrieving revision 1.765
retrieving revision 1.766
diff -u -r1.765 -r1.766
--- kernel.spec 14 Sep 2008 01:43:33 -0000 1.765
+++ kernel.spec 14 Sep 2008 01:57:25 -0000 1.766
@@ -640,6 +640,8 @@
Patch671: linux-2.6-libata-pata_it821x-driver-updates-and-reworking.patch
Patch674: linux-2.6-sata-eeepc-faster.patch
Patch675: linux-2.6-libata-pata_marvell-play-nice-with-ahci.patch
+Patch676: linux-2.6-libata-fix-a-large-collection-of-DMA-mode-mismatches.patch
+Patch677: linux-2.6-libata-lba-28-48-off-by-one-in-ata.h.patch
Patch680: linux-2.6-wireless.patch
Patch681: linux-2.6-wireless-pending.patch
@@ -1203,6 +1205,10 @@
ApplyPatch linux-2.6-sata-eeepc-faster.patch
# don't use ahci for pata_marvell adapters
ApplyPatch linux-2.6-libata-pata_marvell-play-nice-with-ahci.patch
+# fix drivers making wrong assumptions about what dma values mean
+ApplyPatch linux-2.6-libata-fix-a-large-collection-of-DMA-mode-mismatches.patch
+# libata breaks lba28 rules
+ApplyPatch linux-2.6-libata-lba-28-48-off-by-one-in-ata.h.patch
# wireless patches headed for 2.6.26
#ApplyPatch linux-2.6-wireless.patch
@@ -1856,6 +1862,10 @@
%kernel_variant_files -a /%{image_install_path}/xen*-%{KVERREL}.xen -e /etc/ld.so.conf.d/kernelcap-%{KVERREL}.xen.conf %{with_xen} xen
%changelog
+* Sat Sep 13 2008 Chuck Ebbert <cebbert at redhat.com> 2.6.26.5-39
+- libata: fix DMA mode mistmatches
+- libata: interpret the LBA28 spec properly
+
* Sat Sep 13 2008 Chuck Ebbert <cebbert at redhat.com> 2.6.26.5-38
- Add two patches scheduled for 2.6.26-stable:
cifs: fix plaintext authentication
More information about the fedora-extras-commits
mailing list