From: Brett Russ <russb@emc.com>
To: linux-kernel@vger.kernel.org
Cc: linux-ide@vger.kernel.org, B.Zolnierkiewicz@elka.pw.edu.pl
Subject: [PATCH] (IDE) restore access to low order LBA following error
Date: Thu, 05 Aug 2004 12:46:16 -0400 [thread overview]
Message-ID: <41126458.1050203@emc.com> (raw)
I've been playing with code which needs access to the drive's reported
error LBA following a drive detected sector error. Currently, as far as
I can tell, the only code which uses this information are the
*_dump_status routines (ide_, idedisk_, and (only in 2.4) taskfile_)
which put the error info in the syslog. Anyone else who wants to read
the LBA that the error occurred on will be accessing the drive registers
after these functions have run.
The problem is, these routines assume no one else looks at the registers
and leaves them pointed to the high 24 bits of the 48 bit address. I
think the kernel should leave the registers in a known state.
The following patches restore the drive register access to a known state
(the low order bits of the LBA) following status dumps. The patches
are based on 2.4.26 and 2.6.7 respectively and apply cleanly to
2.4.27-rc5 and 2.6.8-rc3.
I previously sent a version of this patch to the IDE list, please ignore
that one as it was rushed, incomplete, and had a mistake.
I considered several alternatives to the patch I'm submitting:
1) whether ide_read_24 should take an argument (low/high) that would
access the desired bits and clean up after itself
2) whether all readers of the LBA register should point to the desired
bits before reading the LBA
...but I think this patched implementation may be more acceptable. If
not, please advise and I will code either alternative.
I have one question on the ide-taskfile.c patch for 2.4 below: should
drive->ctl be OR'ed with 0x80 when setting the high order LBA access
bit? As you can see, it wasn't previously but this seems at risk of
losing the nIEN bit. It may be a non-issue though because I don't think
taskfile_dump_status is used.
Thank you!
Brett
2.4============================================
--- linux-2.4.26/drivers/ide/ide-disk.c Fri Nov 28 13:26:20 2003
+++ linux/drivers/ide/ide-disk.c Thu Aug 5 11:25:58 2004
@@ -857,6 +857,9 @@
low = idedisk_read_24(drive);
hwif->OUTB(drive->ctl|0x80,
IDE_CONTROL_REG);
high = idedisk_read_24(drive);
+ /* Restore access to low order LBA */
+ hwif->OUTB(drive->ctl & ~0x80,
+ IDE_CONTROL_REG);
sectors = ((__u64)high << 24) | low;
printk(", LBAsect=%llu, high=%d, low=%d",
(unsigned long long) sectors,
--- linux-2.4.26/drivers/ide/ide.c Wed Feb 18 08:36:31 2004
+++ linux/drivers/ide/ide.c Thu Aug 5 11:37:04 2004
@@ -407,8 +407,12 @@
u64 sectors = 0;
u32 high = 0;
u32 low = read_24(drive);
- hwif->OUTB(drive->ctl|0x80,
IDE_CONTROL_REG);
+ hwif->OUTB(drive->ctl | 0x80,
+ IDE_CONTROL_REG);
high = read_24(drive);
+ /* Restore access to low order
LBA */
+ hwif->OUTB(drive->ctl & ~0x80,
+ IDE_CONTROL_REG);
sectors = ((u64)high << 24) | low;
printk(", LBAsect=%llu,
high=%d, low=%d",
--- linux-2.4.26/drivers/ide/ide-taskfile.c Fri Jun 13 10:51:33 2003
+++ linux/drivers/ide/ide-taskfile.c Thu Aug 5 11:27:59 2004
@@ -308,8 +308,11 @@
u64 sectors = 0;
u32 high = 0;
u32 low = task_read_24(drive);
- hwif->OUTB(0x80, IDE_CONTROL_REG);
+ hwif->OUTB(drive->ctl | 0x80,
IDE_CONTROL_REG);
high = task_read_24(drive);
+ /* Restore access to low order LBA */
+ hwif->OUTB(drive->ctl & ~0x80,
+ IDE_CONTROL_REG);
sectors = ((u64)high << 24) | low;
printk(", LBAsect=%lld", sectors);
} else {
2.6============================================
--- linux-2.6.7/drivers/ide/ide.c Wed Jun 16 01:19:03 2004
+++ linux/drivers/ide/ide.c Thu Aug 5 11:09:25 2004
@@ -409,6 +409,9 @@
u32 low = ide_read_24(drive);
hwif->OUTB(drive->ctl|0x80,
IDE_CONTROL_REG);
high = ide_read_24(drive);
+ /* Restore access to low order
LBA */
+ hwif->OUTB(drive->ctl & ~0x80,
+ IDE_CONTROL_REG);
sectors = ((u64)high << 24) | low;
printk(", LBAsect=%llu,
high=%d, low=%d",
--- linux-2.6.7/drivers/ide/ide-disk.c Wed Jun 16 01:19:17 2004
+++ linux/drivers/ide/ide-disk.c Thu Aug 5 11:10:28 2004
@@ -675,6 +675,9 @@
low = ide_read_24(drive);
hwif->OUTB(drive->ctl|0x80,
IDE_CONTROL_REG);
high = ide_read_24(drive);
+ /* Restore access to low order LBA */
+ hwif->OUTB(drive->ctl & ~0x80,
+ IDE_CONTROL_REG);
sectors = ((__u64)high << 24) | low;
printk(", LBAsect=%llu, high=%d, low=%d",
(unsigned long long) sectors,
next reply other threads:[~2004-08-05 16:46 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-08-05 16:46 Brett Russ [this message]
2004-08-05 16:45 ` [PATCH] (IDE) restore access to low order LBA following error Alan Cox
2004-08-05 20:50 ` Brett Russ
2004-08-05 23:13 ` Jeff Garzik
2004-08-06 0:01 ` Alan Cox
2004-08-06 1:45 ` Jeff Garzik
2004-08-06 16:23 ` Doug Maxey
2004-08-06 17:06 ` Jeff Garzik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=41126458.1050203@emc.com \
--to=russb@emc.com \
--cc=B.Zolnierkiewicz@elka.pw.edu.pl \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).