linux-kernel - [PATCH v3] PCI: keystone: Fix race condition when initializing PHYs

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-next>] [day] [month] [year] [list]

Message-ID: <20230927041845.1222080-1-s-vadapalli@ti.com>
Date:   Wed, 27 Sep 2023 09:48:45 +0530
From:   Siddharth Vadapalli <s-vadapalli@...com>
To:     <lpieralisi@...nel.org>, <robh@...nel.org>, <kw@...ux.com>,
        <bhelgaas@...gle.com>
CC:     <linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <ilpo.jarvinen@...ux.intel.com>, <vigneshr@...com>,
        <r-gunasekaran@...com>, <srk@...com>, <s-vadapalli@...com>
Subject: [PATCH v3] PCI: keystone: Fix race condition when initializing PHYs

The PCI driver invokes the PHY APIs using the ks_pcie_enable_phy()
function. The PHY in this case is the Serdes. It is possible that the
PCI instance is configured for 2 lane operation across two different
Serdes instances, using 1 lane of each Serdes. In such a configuration,
if the reference clock for one Serdes is provided by the other Serdes,
it results in a race condition. After the Serdes providing the reference
clock is initialized by the PCI driver by invoking its PHY APIs, it is
not guaranteed that this Serdes remains powered on long enough for the
PHY APIs based initialization of the dependent Serdes. In such cases,
the PLL of the dependent Serdes fails to lock due to the absence of the
reference clock from the former Serdes which has been powered off by the
PM Core.

Fix this by obtaining reference to the PHYs before invoking the PHY
initialization APIs and releasing reference after the initialization is
complete.

Fixes: 49229238ab47 ("PCI: keystone: Cleanup PHY handling")
Signed-off-by: Siddharth Vadapalli <s-vadapalli@...com>
---

NOTE: This patch is based on linux-next tagged next-20230927.

v2:
https://lore.kernel.org/r/20230926063638.1005124-1-s-vadapalli@ti.com/

Changes since v2:
- Implement suggestion by Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
  moving the phy_pm_runtime_put_sync() For-Loop section before the
  return value of ks_pcie_enable_phy(ks_pcie) is checked, thereby
  preventing duplication of the For-Loop.
- Rebase patch on next-20230927.

v1:
https://lore.kernel.org/r/20230926054200.963803-1-s-vadapalli@ti.com/

Changes since v1:
- Add code to release reference(s) to the phy(s) when
  ks_pcie_enable_phy(ks_pcie) fails.

Regards,
Siddharth.

 drivers/pci/controller/dwc/pci-keystone.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 49aea6ce3e87..0ec6720cc2df 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -1218,7 +1218,16 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 		goto err_link;
 	}

+	/* Obtain reference(s) to the phy(s) */
+	for (i = 0; i < num_lanes; i++)
+		phy_pm_runtime_get_sync(ks_pcie->phy[i]);
+
 	ret = ks_pcie_enable_phy(ks_pcie);
+
+	/* Release reference(s) to the phy(s) */
+	for (i = 0; i < num_lanes; i++)
+		phy_pm_runtime_put_sync(ks_pcie->phy[i]);
+
 	if (ret) {
 		dev_err(dev, "failed to enable phy\n");
 		goto err_link;
-- 
2.34.1