linux-kernel - Re: [PATCH v2 -next 05/11] PCI: brcmstb: Restore CRS in RootCtl after prstn

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: <a93c9757-963a-4b4f-a169-0c17ff39576b@suse.de>
Date: Tue, 17 Sep 2024 13:24:30 +0300
From: Stanimir Varbanov <svarbanov@...e.de>
To: Florian Fainelli <florian.fainelli@...adcom.com>,
 Stanimir Varbanov <svarbanov@...e.de>, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rpi-kernel@...ts.infradead.org, linux-pci@...r.kernel.org,
 Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Jim Quinlan <jim2101024@...il.com>,
 Nicolas Saenz Julienne <nsaenz@...nel.org>,
 Bjorn Helgaas <bhelgaas@...gle.com>,
 Lorenzo Pieralisi <lpieralisi@...nel.org>, kw@...ux.com,
 Philipp Zabel <p.zabel@...gutronix.de>,
 Andrea della Porta <andrea.porta@...e.com>,
 Phil Elwell <phil@...pberrypi.com>, Jonathan Bell <jonathan@...pberrypi.com>
Subject: Re: [PATCH v2 -next 05/11] PCI: brcmstb: Restore CRS in RootCtl after
 prstn_n

Hi Florian,

On 9/10/24 19:59, Florian Fainelli wrote:
> On 9/10/24 08:18, Stanimir Varbanov wrote:
>> RootCtl bits might reset by perst_n during probe, re-enable
>> CRS SVE here in pcie_start_link.
>>
>> Signed-off-by: Stanimir Varbanov <svarbanov@...e.de>
> 
> This looks like a bug fix, and we should explain what is the user
> visible effect of that, if any.

It is definitely a bugfix. Otherwise, CRS Software Visibility is
important feature from pcie1.1. Not enabling it on Root Port could lead
to infinite configuration retry cycles when enumerate endpoints which
supports CRS. For more information [1] and [2].

I spent some time debugging it and found that this is not the proper
solution.  I think the issue comes from wrongly implemented .add_bus
pci_ops. Looks like .add_bus op shouldn't call brcm_pcie_start_link()
but invoke before pci_host_probe(), then the issue will fix by itself.

What I observed is that pci_enable_crs() is setting CSR Software
Visibility Enable bit but the controller is ignoring it without error
(reading the Root Control register returns zero). This means that the
controller is not ready to accept configuration write requests at that
time, that's why I tried the following diff which seems to work:

 static struct pci_ops brcm_pcie_ops = {
        .map_bus = brcm_pcie_map_bus,
        .read = pci_generic_config_read,
        .write = pci_generic_config_write,
-       .add_bus = brcm_pcie_add_bus,
-       .remove_bus = brcm_pcie_remove_bus,
 };

 static struct pci_ops brcm7425_pcie_ops = {
@@ -1983,6 +2018,9 @@ static int brcm_pcie_probe(struct platform_device
*pdev)

        platform_set_drvdata(pdev, pcie);

+       //TODO: check for error
+       brcm_pcie_start_link(pcie);
+
        ret = pci_host_probe(bridge);
        if (!ret && !brcm_pcie_link_up(pcie))
                ret = -ENODEV;

Of course this change would work on RPi5 because there are no regulators.

I will drop the patch from the series for now and work on a proper solution.

regards,
~Stan

[1]
https://patchwork.kernel.org/project/linux-pci/patch/53FFA54D.9000907@gmail.com/
[2]
https://blog.linuxplumbersconf.org/2017/ocw/system/presentations/4732/original/crs.pdf