linux-kernel - Re: [PATCH v2] PCI: dwc: do not ignore link errors

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]

Date:   Wed, 21 Jul 2021 12:11:42 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Christian Gmeiner <christian.gmeiner@...il.com>
Cc:     Jingoo Han <jingoohan1@...il.com>,
        Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] PCI: dwc: do not ignore link errors

On Wed, Jul 21, 2021 at 05:28:21PM +0200, Christian Gmeiner wrote:
> This fixes long boot delays of about 10 seconds.
> 
> I am working on a device powered by an TI am65 SoC where
> we have a PCIe expansion slot. If there is no PCIe device
> connected I see boot delays caused by pci_host_probe(..).

I agree this is problematic.  Surely the controller can generate an
interrupt when the link comes up?  If so, can we make an interrupt
handler for that "Link Up" interrupt that calls or schedules
pci_host_probe(), and get rid of dw_pcie_wait_for_link() altogether?

I doubt we want to just return failure here if the link doesn't come
up right away, because then we can never hot-add a device later.

886a9c134755 ("PCI: dwc: Move link handling into common code")
addresses this in the commit log:

  The behavior for a link down was inconsistent as some drivers would
  fail probe in that case while others succeed. Let's standardize this
  to succeed as there are usecases where devices (and the link) appear
  later even without hotplug. For example, a reconfigured FPGA device.

> Signed-off-by: Christian Gmeiner <christian.gmeiner@...il.com>
> ---
> 
> V2: fix compile
> 
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index a608ae1fad57..ce7d3a6f9a03 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -408,8 +408,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  			goto err_free_msi;
>  	}
>  
> -	/* Ignore errors, the link may come up later */
> -	dw_pcie_wait_for_link(pci);
> +	ret = dw_pcie_wait_for_link(pci);
> +	if (ret)
> +		goto err_free_msi;
>  
>  	bridge->sysdata = pp;
>  
> -- 
> 2.31.1
>