| 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
| ||
|
Message-Id: <200703240053.42845.bzolnier@gmail.com>
Date: Sat, 24 Mar 2007 00:53:42 +0100
From: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To: Suleiman Souhlal <ssouhlal@...ebsd.org>
Cc: linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org,
Alan Cox <alan@...hat.com>
Subject: Re: [PATCH 3/3] Use correct IDE error recovery
Hi,
On Thursday 08 March 2007, Suleiman Souhlal wrote:
>
> On Mar 7, 2007, at 1:16 PM, Bartlomiej Zolnierkiewicz wrote:
> > Please respin the patch so I could merge it.
>
> Ok.
Since I think that it's worth to have it in 2.6.21-final and respin didn't
happen I did the required changes myself (it also turned out that I missed
few things during initial review), then applied the patch...
Please let my know whether you are fine with my changes, thanks.
[PATCH] ide: use correct IDE error recovery
From: Suleiman Souhlal <suleiman@...gle.com>
IDE error recovery is using IDLE IMMEDIATE if the drive is busy or has DRQ set.
This violates the ATA spec (can only send IDLE IMMEDIATE when drive is not
busy) and really hoses up some drives (modern drives will not be able to
recover using this error handling). The correct thing to do is issue a SRST
followed by a SET FEATURES command. This is what Western Digital recommends
for error recovery and what Western Digital says Windows does. It also does
not violate the ATA spec as far as I can tell.
Bart:
* port the patch over the current tree
* undo the recalibration code removal
* send SET FEATURES command after checking for good drive status
* don't check whether the current request is of REQ_TYPE_ATA_{CMD,TASK}
type because we need to send SET FEATURES before handling any requests
* some pre-ATA4 drives require INITIALIZE DEVICE PARAMETERS command before
other commands (except IDENTIFY) so send SET FEATURES only if there are
no pending drive->special requests
* update comments and patch description
* any bugs introduced by this patch are mine and not Suleiman's :-)
Signed-off-by: Suleiman Souhlal <suleiman@...gle.com>
Acked-by: Alan Cox <alan@...hat.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
---
drivers/ide/ide-io.c | 32 +++++++++++++++++++++-----------
drivers/ide/ide-iops.c | 3 +++
include/linux/ide.h | 1 +
3 files changed, 25 insertions(+), 11 deletions(-)
Index: b/drivers/ide/ide-io.c
===================================================================
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -519,21 +519,24 @@ static ide_startstop_t ide_ata_error(ide
if ((stat & DRQ_STAT) && rq_data_dir(rq) == READ && hwif->err_stops_fifo == 0)
try_to_flush_leftover_data(drive);
+ if (rq->errors >= ERROR_MAX || blk_noretry_request(rq)) {
+ ide_kill_rq(drive, rq);
+ return ide_stopped;
+ }
+
if (hwif->INB(IDE_STATUS_REG) & (BUSY_STAT|DRQ_STAT))
- /* force an abort */
- hwif->OUTB(WIN_IDLEIMMEDIATE, IDE_COMMAND_REG);
+ rq->errors |= ERROR_RESET;
- if (rq->errors >= ERROR_MAX || blk_noretry_request(rq))
- ide_kill_rq(drive, rq);
- else {
- if ((rq->errors & ERROR_RESET) == ERROR_RESET) {
- ++rq->errors;
- return ide_do_reset(drive);
- }
- if ((rq->errors & ERROR_RECAL) == ERROR_RECAL)
- drive->special.b.recalibrate = 1;
+ if ((rq->errors & ERROR_RESET) == ERROR_RESET) {
++rq->errors;
+ return ide_do_reset(drive);
}
+
+ if ((rq->errors & ERROR_RECAL) == ERROR_RECAL)
+ drive->special.b.recalibrate = 1;
+
+ ++rq->errors;
+
return ide_stopped;
}
@@ -1025,6 +1028,13 @@ static ide_startstop_t start_request (id
if (!drive->special.all) {
ide_driver_t *drv;
+ /*
+ * We reset the drive so we need to issue a SETFEATURES.
+ * Do it _after_ do_special() restored device parameters.
+ */
+ if (drive->current_speed == 0xff)
+ ide_config_drive_speed(drive, drive->desired_speed);
+
if (rq->cmd_type == REQ_TYPE_ATA_CMD ||
rq->cmd_type == REQ_TYPE_ATA_TASK ||
rq->cmd_type == REQ_TYPE_ATA_TASKFILE)
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -1094,6 +1094,9 @@ static void pre_reset(ide_drive_t *drive
if (HWIF(drive)->pre_reset != NULL)
HWIF(drive)->pre_reset(drive);
+ if (drive->current_speed != 0xff)
+ drive->desired_speed = drive->current_speed;
+ drive->current_speed = 0xff;
}
/*
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -615,6 +615,7 @@ typedef struct ide_drive_s {
u8 init_speed; /* transfer rate set at boot */
u8 pio_speed; /* unused by core, used by some drivers for fallback from DMA */
u8 current_speed; /* current transfer rate set */
+ u8 desired_speed; /* desired transfer rate set */
u8 dn; /* now wide spread use */
u8 wcache; /* status of write cache */
u8 acoustic; /* acoustic management */
-
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/
Powered by blists - more mailing lists