linux-kernel - Re: [Xen-devel] [PATCH V3 1/2] Drivers/PCI: Export pcie_has

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: <20171214125206.GA24958@infradead.org>
Date:   Thu, 14 Dec 2017 04:52:06 -0800
From:   Christoph Hellwig <hch@...radead.org>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     Govinda Tatti <Govinda.Tatti@...cle.COM>, jgross@...e.com,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        JBeulich@...e.com, bhelgaas@...gle.com,
        xen-devel@...ts.xenproject.org, boris.ostrovsky@...cle.COM,
        roger.pau@...rix.com, Christoph Hellwig <hch@...radead.org>
Subject: Re: [Xen-devel] [PATCH V3 1/2] Drivers/PCI: Export pcie_has_flr()
 interface

On Wed, Dec 13, 2017 at 03:24:21PM -0600, Bjorn Helgaas wrote:
> Prior to a60a2b73ba69, we had
> 
>   int pcie_flr(struct pci_dev *dev, int probe);
> 
> like all the other reset methods.  AFAICT, the addition of
> pcie_has_flr() was to optimize the path slightly because when drivers
> call pcie_flr(), they should already know that their hardware supports
> FLR.  But I don't think that optimization is worth the extra code
> complexity.  If we do need to optimize it, we can check this in the
> core during enumeration and set PCI_DEV_FLAGS_NO_FLR_RESET
> accordingly.
> 
> Christoph, chime in if I'm missing something here.

Didn't we just have that discussion in another thread a few days
ago?  I think that the pcie_has_flr was a mistake in retrospective but I
think the bool probe API was an even bigger mistake.  The only use
of it is to hide the reset attribute in sysfs.  I'd much rather always
have it and have it return EOPNOTSUPP if no reset method is supported.

I can send a patch for that if it sounds fine to you.