Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "windy.bi.enflame" <windy.bi.enflame@gmail.com>
To:     bhelgaas@google.com
Cc:     linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
        "windy.bi.enflame" <windy.bi.enflame@gmail.com>
Subject: [PATCH] drivers/pci: wait downstream hierarchy ready instead of slot itself ready, after secondary bus reset
Date:   Tue, 17 May 2022 01:30:47 +0800
Message-Id: <20220516173047.123317-1-windy.bi.enflame@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

While I do reset test of a PCIe endpoint device on a server, I find that
the EP device always been removed and re-inserted again by hotplug module,
 after secondary bus reset.

After checking I find:
1> "pciehp_reset_slot()" always disable slot's DLLSC interrupt before
   doing reset and restore after reset, to try to filter the hotplug
   event happened during reset.
2> "pci_bridge_secondary_bus_reset()" sleep 1 seconad and "pci_dev_wait()"
   until device ready with "PCIE_RESET_READY_POLL_MS" timeout.
3> There is a PCIe switch between CPU and the EP devicem the topology as:
   CPU <-> Switch <-> EP.
4> While trigger sbr reset at the switch's downstream port, it needs 1.5
   seconds for internal enumeration.

About why 1.5 seconds ready time is not filtered by "pci_dev_wait()" with
"PCIE_RESET_READY_POLL_MS" timeout, I find it is because in
"pci_bridge_secondary_bus_reset()", the function is operating slot's
config space to trigger sbr and also wait slot itself ready by input same
"dev" parameter. Different from other resets like FLR which is triggered
by operating the config space of EP device itself, sbr is triggered by
up slot but need to wait downstream devices' ready, so I think function
"pci_dev_wait()" works for resets like FLR but not for sbr.

In this proposed patch, I'm changing the waiting function used in sbr to
"pci_bridge_secondary_bus_wait()" which will wait all the downstream
hierarchy ready with the same timeout setting "PCIE_RESET_READY_POLL_MS".
In "pci_bridge_secondary_bus_wait()" the "subordinate" and
"subordinate->devices" will be checked firstly, and then downstream
devices' present state.

Signed-off-by: windy.bi.enflame <windy.bi.enflame@gmail.com>
---
 drivers/pci/pci.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 9ecce435fb3f..d7ec3859268b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5002,6 +5002,29 @@ void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev)
 	}
 }
 
+int pci_bridge_secondary_bus_wait(struct pci_dev *bridge, int timeout)
+{
+	struct pci_dev *dev;
+	int delay = 1;
+
+	if (!bridge->subordinate || list_empty(&bridge->subordinate->devices))
+		return 0;
+
+	list_for_each_entry(dev, &bridge->subordinate->devices, bus_list) {
+		while (!pci_device_is_present(dev)) {
+			if (delay > timeout) {
+				pci_warn(dev, "secondary bus not ready after %dms\n", delay);
+				return -ENOTTY;
+			}
+
+			msleep(delay);
+			delay *= 2;
+		}
+	}
+
+	return 0;
+}
+
 void pci_reset_secondary_bus(struct pci_dev *dev)
 {
 	u16 ctrl;
@@ -5045,7 +5068,7 @@ int pci_bridge_secondary_bus_reset(struct pci_dev *dev)
 {
 	pcibios_reset_secondary_bus(dev);
 
-	return pci_dev_wait(dev, "bus reset", PCIE_RESET_READY_POLL_MS);
+	return pci_bridge_secondary_bus_wait(dev, PCIE_RESET_READY_POLL_MS);
 }
 EXPORT_SYMBOL_GPL(pci_bridge_secondary_bus_reset);
 
-- 
2.36.1