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: <CAPcyv4h5kCKVyCjomBUY27MJwheDZ8v87+a9K-2YCgyqRWR7eQ@mail.gmail.com>
Date:   Tue, 13 Aug 2019 15:07:17 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     Stephen Douthit <stephend@...icom-usa.com>,
        Jens Axboe <axboe@...nel.dk>,
        "linux-ide@...r.kernel.org" <linux-ide@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ata: ahci: Lookup PCS register offset based on PCI device ID

On Tue, Aug 13, 2019 at 12:31 AM Christoph Hellwig <hch@...radead.org> wrote:
>
> On Mon, Aug 12, 2019 at 12:31:35PM -0700, Dan Williams wrote:
> > It seems platforms / controllers that fail to run the option-rom
> > should be quirked by device-id, but the PCS register twiddling be
> > removed for everyone else. "Card BIOS" to me implies devices with an
> > Option-ROM BAR which I don't think modern devices have, so that might
> > be a simple way to try to phase out this quirk going forward without
> > regressing working setups that might be relying on this.
> >
> > Then again the driver is already depending on the number of enabled
> > ports to be reliable before PCS is written, and the current driver
> > does not attempt to enable ports that were not enabled previously.
> > That tells me that if the PCS quirk ever mattered it would have
> > already regressed when the driver switched from blindly writing 0xf to
> > only setting the bits that were already set in ->port_map.
>
> But how do we find that out?

We can layer another assumption on top of Tejun's assumptions from
commit 49f290903935 "ahci: update PCS programming". The kernel
community has not received any regression reports from that change
which says:

"
    port_map is determined from PORTS_IMPL PCI register which is
    implemented as write or write-once register.  If the register isn't
    programmed, ahci automatically generates it from number of ports,
    which is good enough for PCS programming.  ICH6/7M are probably the
    only ones where non-contiguous enable bits are necessary && PORTS_IMPL
    isn't programmed properly but they're proven to work reliably with 0xf
    anyway.
"

So the potential options I see are:

1/ Keep the current scheme, but limit it to cases where PORTS_IMPL is
less than 8 and assume this need to set the bits is unnecessary legacy
to carry forward

2/ Option1 + additionally use PORTS_IMPL as a gate to guess when the
PCS format might be different for values >= 8.

I think the driver does not need to consider Option2 unless / until it
encounters a platform where firmware does not "do the right thing",
and given Denverton has been in the wild with the wrong PCS twiddling
it seems to suggest nothing needs to be done there.

> A compromise to me seems that we just do the PCS quirk for all Intel
> devices explicitly listed in the PCI Ids based on new board_* values
> as long as they have the old PCS location, and assume anything new
> enough to have the new location won't need to quirk, given that it
> never properly worked.  This might miss some intel devices that were
> supported with the class based catchall, though.

I'd be more comfortable with PORT_IMPL as the deciding factor.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ