linux-kernel - Re: [PATCH] ide: Fix ata_id_has_dword_io to return DWORD I/O support properly

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: <200812182142.16533.bzolnier@gmail.com>
Date:	Thu, 18 Dec 2008 21:42:16 +0100
From:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To:	Sergei Shtylyov <sshtylyov@...mvista.com>
Cc:	Jeff Garzik <jeff@...zik.org>,
	Mario Schwalbe <schwalbe@....tu-dresden.de>,
	linux-kernel@...r.kernel.org,
	Linux IDE mailing list <linux-ide@...r.kernel.org>
Subject: Re: [PATCH] ide: Fix ata_id_has_dword_io to return DWORD I/O support properly

On Thursday 18 December 2008, Sergei Shtylyov wrote:
> Hello.
> 
> Jeff Garzik wrote:
> 
> >> This patch fixes ata_id_has_dword_io to return 1 (supported) if
> >> the drive is compliant to ATA2 or newer and evaluates the config
> >> word for older drives.
> 
> >> Signed-off-by: Mario Schwalbe <schwalbe@....tu-dresden.de>
> 
> >> diff --git a/include/linux/ata.h b/include/linux/ata.h
> >> index a53318b..d0d6a9c 100644
> >> --- a/include/linux/ata.h
> >> +++ b/include/linux/ata.h
> >> @@ -700,12 +700,12 @@ static inline int ata_id_has_tpm(const u16 *id)
> 
> >>  static inline int ata_id_has_dword_io(const u16 *id)
> >>  {
> >> -    /* ATA 8 reuses this flag for "trusted" computing */
> >> -    if (ata_id_major_version(id) > 7)
> >> -        return 0;
> >> -    if (id[ATA_ID_DWORD_IO] & (1 << 0))
> >> -        return 1;
> >> -    return 0;
> >> +    /* This flag is defined up to ATA1 and deprecated since then
> >> +       (ATA 8 reuses this flag for "trusted" computing). */
> >> +    if (ata_id_major_version(id) <= 1)
> >> +        return (id[ATA_ID_DWORD_IO] & (1 << 0)) != 0;
> >> +    /* later revision drives support DWORD I/O just fine */
> >> +    return 1;
> 
> > This seems like a risky assumption...
> 
>     Not at all, I think. It's total mystery how "DWORD I/O" could've been even 
> (not) supported by a *drive* at all. Support of "DWORD I/O" is a property of 
> the PATA controller. Frankly speaking, I don't think that this function is at 
> all useful...

Indeed, this function can be probably removed altogether later...

OTOH the current patch is safe even for 2.6.28 (based on years of experience
with the check that we had in IDE subsystem) and will fix some libata drivers
(pata_legacy, pata_qdi and pata_winbond) to use dword IO on >= ATA-2 devices.

[ In reality this a regression fix for IDE -> libata conversion as it is a
  huge performance improvement for the above mentioned DMA-less drivers. ]

Jeff, I would like to merge it through IDE tree since the other patch depends
on it but if you want to go ahead and push it to Linus earlier feel free to
do it (or I can include it into the next IDE fixes pull request if you like).

Thanks,
Bart
--
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/