linux-kernel - Re: [PATCH] PCI: dwc: Added link up check in map_bus of dw_child_pcie

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200918124710.GA1800067@bjorn-Precision-5520>
Date:   Fri, 18 Sep 2020 07:47:10 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     "Z.q. Hou" <zhiqiang.hou@....com>
Cc:     Rob Herring <robh@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        PCI <linux-pci@...r.kernel.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
        Michael Walle <michael@...le.cc>,
        Ard Biesheuvel <ardb@...nel.org>
Subject: Re: [PATCH] PCI: dwc: Added link up check in map_bus of
 dw_child_pcie_ops

On Fri, Sep 18, 2020 at 11:02:07AM +0000, Z.q. Hou wrote:

> But now the SError is exactly caused by the first access of the
> non-existent function, I dug into the kernel enumeration code and
> found it will fire a 4Byte CFG read transaction to read the Vendor
> ID and Device ID together, so I suspect the root cause is access the
> Device ID of a non-existent function triggers SError.
> 
> So the alternative solution seems to correct the PCIe enumeration, I
> will submit a patch to let the first access only read the Vendor ID.

If it is incorrect for the first access to be a 32-bit read of both
the Vendor and the Device ID, please cite the relevant section of the
spec in your patch.

I don't like to make changes to generic code to accommodate specific
pieces of hardware because then we restrict future changes based on
some device that will soon be obsolete and forgotten.

I'm pretty sure the spec language about CRS handling is careful to
talk about "reads that *include* Vendor ID", not just "reads of Vendor
ID", so the implication is that it covers 32-bit reads as well as
16-bit reads.

Bjorn