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: <20171215002430.GS30595@bhelgaas-glaptop.roam.corp.google.com>
Date:   Thu, 14 Dec 2017 18:24:30 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Christoph Hellwig <hch@...radead.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
Subject: Re: [Xen-devel] [PATCH V3 1/2] Drivers/PCI: Export pcie_has_flr()
 interface

On Thu, Dec 14, 2017 at 04:52:06AM -0800, Christoph Hellwig wrote:
> 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?

Probably, but I didn't have a clear picture of what you were suggesting.

> 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.

If you can get rid of the whole probe infrastructure, that sounds
good to me.

Bjorn