linux-kernel - Re: [PATCH v4 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link

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: <20200218173238.GA214360@google.com>
Date:   Tue, 18 Feb 2020 11:32:38 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     Stuart Hayes <stuart.w.hayes@...il.com>,
        Austin Bolen <austin_bolen@...l.com>, keith.busch@...el.com,
        Alexandru Gagniuc <mr.nuke.me@...il.com>,
        "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        "Gustavo A . R . Silva" <gustavo@...eddedor.com>,
        Sinan Kaya <okaya@...nel.org>,
        Oza Pawandeep <poza@...eaurora.org>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org, Libor Pechacek <lpechacek@...e.cz>
Subject: Re: [PATCH v4 0/3] PCI: pciehp: Do not turn off slot if presence
 comes up after link

On Tue, Feb 11, 2020 at 03:32:02PM +0100, Lukas Wunner wrote:
> On Tue, Feb 11, 2020 at 08:14:44AM -0600, Bjorn Helgaas wrote:
> > Feels like sort of a
> > double-negative situation, too.  Obviously the hardware bit has to be
> > "1 means disabled" to be compatible with previous spec versions, but
> > the code is usually easier to read if we test for something being
> > *enabled*.
> 
> It's a similar situation with the "DisINTx" bit in the Command
> register, which, if disabled, is shown as "DisINTx-" in lspci even
> though the more intuitive notion is that INTx is *enabled*.  I think
> you did the right thing by showing it as "IbPresDis-" because it's
> consistent with how it's done elsewhere for similar bits.

Everything else we decode is *capability* bits and IBPD is another
one.  So by the principle of least surprise, I propose this:

+       ctrl_info(ctrl, "Slot #%d AttnBtn%c PwrCtrl%c MRL%c AttnInd%c PwrInd%c HotPlug%c Surprise%c Interlock%c NoCompl%c IbPresDis%c LLActRep%c%s\n",
+               FLAG(slot_cap2, PCI_EXP_SLTCAP2_IBPD),

That works out to be the same as printing

  inbound_presence_disabled ? '+' : '-'

because we always set inbound_presence_disabled when
PCI_EXP_SLTCAP2_IBPD is supported.