linux-kernel - Re: [PATCH 1/1] PCI: X-Gene: Disable Configuration Request Retry Status for X-Gene v1 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAErSpo5Oc_8ca4h7xYSkGPFE1vxWg1sy1rJRc0B=u9LrdOLY7g@mail.gmail.com>
Date:	Fri, 12 Jun 2015 18:08:01 -0500
From:	Bjorn Helgaas <bhelgaas@...gle.com>
To:	Duc Dang <dhdang@....com>
Cc:	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	linux-arm <linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Ian Campbell <ian.campbell@...rix.com>,
	Marcin Juszkiewicz <mjuszkiewicz@...hat.com>,
	Tanmay Inamdar <tinamdar@....com>, patches <patches@....com>
Subject: Re: [PATCH 1/1] PCI: X-Gene: Disable Configuration Request Retry
 Status for X-Gene v1 PCIe

On Fri, Jun 12, 2015 at 5:10 PM, Duc Dang <dhdang@....com> wrote:
> Hi Bjorn,
>
> On Fri, Jun 12, 2015 at 2:59 PM, Bjorn Helgaas <bhelgaas@...gle.com> wrote:
>> Hi Duc,
>>
>> On Thu, Jun 11, 2015 at 01:08:14PM -0700, Duc Dang wrote:
>>> X-Gene v1 PCIe controller has a bug in Configuration Request Retry
>>> Status (CRS) logic:
>>>   When CPU tries to read Vendor ID and Device ID of not-existed
>>>   remote device, the controller returns 0xFFFF0001 instead of
>>>   0xFFFFFFFF; this will add significant delay in boot time as
>>>   pci_bus_read_dev_vendor_id will wait for 60 seconds before
>>>   giving up.
>>
>> OK, help me understand how this works.  I think this is related to the
>> problem I reported where if the slot is empty, "lspci" doesn't show
>> anything, not even the Root Port leading to the slot.
>>
>> I think this happens because when we try to read the Root Port's config
>> space,
>>
>>   - the slot below the Root Port is empty
>>   - the Root Port's link is down
>>   - xgene_pcie_map_bus() returns NULL because !port->link_up
>>   - pci_generic_config_read32() returns PCIBIOS_DEVICE_NOT_FOUND
>>
>> so it looks like the Root Port itself doesn't exist.
>>
>> I proposed to change xgene_pcie_map_bus() so it didn't check whether the
>> link was up.  That change makes reads of the Root Port's config space work.
>>
>> After we learn the Root Port exists, the PCI core enumerates devices below
>> the Root Port, e.g., on bus 01.  X-Gene advertises that it supports CRS, so
>> we enable it.  When we try to read the Vendor ID of 01:00.0, there's no
>> response from the device (because the slot is empty), and the Root Complex
>> should complete the read by fabricating data of all ones, i.e., 0xFFFFFFFF.
>> But apparently X-Gene supplies 0xFFFF0001 instead, which means "there's a
>> device here, but it's not ready yet," so the PCI core retries the read for
>> 60 seconds before timing out.
>>
>> This patch is basically a quirk that keeps X-Gene from advertising CRS
>> support, so the PCI core won't enable CRS.  In the example above, I guess
>> that means the Root Complex will supply 0xFFFFFFFF and the core will see
>> that the slot is empty.
>>
>> But this patch leaves the "!port->link_up" test in xgene_pcie_map_bus().
>> Doesn't that mean the core will still not discover the Root Port when the
>> slot is empty?
>>
>> It seems to me that you would want both the xgene_pcie_map_bus() change and
>> this patch.  The first would fix the problem that we don't enumerate Root
>> Ports leading to empty slots, and the second would fix the problem that we
>> enable CRS and timeout when enumerating below those Root Ports.
>>
> Yes, you are right. I plan to send a follow up patch after this one to
> remove the port->link_up check in xgene_pcie_map_bus and make root
> port discoverable even when the slot is empty. I can combine this
> follow up patch with this one if you feel OK with it.

Two patches would be great.  I just didn't know about your plans for
the follow-up one.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/