stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	patches@lists.linux.dev, Sheng Bi <windy.bi.enflame@gmail.com>,
	Ravi Kishore Koppuravuri <ravi.kishore.koppuravuri@intel.com>,
	Lukas Wunner <lukas@wunner.de>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Kuppuswamy Sathyanarayanan 
	<sathyanarayanan.kuppuswamy@linux.intel.com>
Subject: [PATCH 5.4 58/60] PCI: Unify delay handling for reset and resume
Date: Mon, 20 Mar 2023 15:55:07 +0100	[thread overview]
Message-ID: <20230320145433.342750665@linuxfoundation.org> (raw)
In-Reply-To: <20230320145430.861072439@linuxfoundation.org>

From: Lukas Wunner <lukas@wunner.de>

commit ac91e6980563ed53afadd925fa6585ffd2bc4a2c upstream.

Sheng Bi reports that pci_bridge_secondary_bus_reset() may fail to wait
for devices on the secondary bus to become accessible after reset:

Although it does call pci_dev_wait(), it erroneously passes the bridge's
pci_dev rather than that of a child.  The bridge of course is always
accessible while its secondary bus is reset, so pci_dev_wait() returns
immediately.

Sheng Bi proposes introducing a new pci_bridge_secondary_bus_wait()
function which is called from pci_bridge_secondary_bus_reset():

https://lore.kernel.org/linux-pci/20220523171517.32407-1-windy.bi.enflame@gmail.com/

However we already have pci_bridge_wait_for_secondary_bus() which does
almost exactly what we need.  So far it's only called on resume from
D3cold (which implies a Fundamental Reset per PCIe r6.0 sec 5.8).
Re-using it for Secondary Bus Resets is a leaner and more rational
approach than introducing a new function.

That only requires a few minor tweaks:

- Amend pci_bridge_wait_for_secondary_bus() to await accessibility of
  the first device on the secondary bus by calling pci_dev_wait() after
  performing the prescribed delays.  pci_dev_wait() needs two parameters,
  a reset reason and a timeout, which callers must now pass to
  pci_bridge_wait_for_secondary_bus().  The timeout is 1 sec for resume
  (PCIe r6.0 sec 6.6.1) and 60 sec for reset (commit 821cdad5c46c ("PCI:
  Wait up to 60 seconds for device to become ready after FLR")).
  Introduce a PCI_RESET_WAIT macro for the 1 sec timeout.

- Amend pci_bridge_wait_for_secondary_bus() to return 0 on success or
  -ENOTTY on error for consumption by pci_bridge_secondary_bus_reset().

- Drop an unnecessary 1 sec delay from pci_reset_secondary_bus() which
  is now performed by pci_bridge_wait_for_secondary_bus().  A static
  delay this long is only necessary for Conventional PCI, so modern
  PCIe systems benefit from shorter reset times as a side effect.

Fixes: 6b2f1351af56 ("PCI: Wait for device to become ready after secondary bus reset")
Link: https://lore.kernel.org/r/da77c92796b99ec568bd070cbe4725074a117038.1673769517.git.lukas@wunner.de
Reported-by: Sheng Bi <windy.bi.enflame@gmail.com>
Tested-by: Ravi Kishore Koppuravuri <ravi.kishore.koppuravuri@intel.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: stable@vger.kernel.org # v4.17+
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/pci/pci-driver.c |    4 +--
 drivers/pci/pci.c        |   54 ++++++++++++++++++++---------------------------
 drivers/pci/pci.h        |   10 +++++++-
 3 files changed, 35 insertions(+), 33 deletions(-)

--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -946,7 +946,7 @@ static int pci_pm_resume_noirq(struct de
 	pcie_pme_root_status_cleanup(pci_dev);
 
 	if (!skip_bus_pm && prev_state == PCI_D3cold)
-		pci_bridge_wait_for_secondary_bus(pci_dev);
+		pci_bridge_wait_for_secondary_bus(pci_dev, "resume", PCI_RESET_WAIT);
 
 	if (pci_has_legacy_pm_support(pci_dev))
 		return pci_legacy_resume_early(dev);
@@ -1355,7 +1355,7 @@ static int pci_pm_runtime_resume(struct
 	pci_fixup_device(pci_fixup_resume, pci_dev);
 
 	if (prev_state == PCI_D3cold)
-		pci_bridge_wait_for_secondary_bus(pci_dev);
+		pci_bridge_wait_for_secondary_bus(pci_dev, "resume", PCI_RESET_WAIT);
 
 	if (pm && pm->runtime_resume)
 		rc = pm->runtime_resume(dev);
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4483,7 +4483,7 @@ static int pci_dev_wait(struct pci_dev *
 			return -ENOTTY;
 		}
 
-		if (delay > 1000)
+		if (delay > PCI_RESET_WAIT)
 			pci_info(dev, "not ready %dms after %s; waiting\n",
 				 delay - 1, reset_type);
 
@@ -4492,7 +4492,7 @@ static int pci_dev_wait(struct pci_dev *
 		pci_read_config_dword(dev, PCI_COMMAND, &id);
 	}
 
-	if (delay > 1000)
+	if (delay > PCI_RESET_WAIT)
 		pci_info(dev, "ready %dms after %s\n", delay - 1,
 			 reset_type);
 
@@ -4727,24 +4727,31 @@ static int pci_bus_max_d3cold_delay(cons
 /**
  * pci_bridge_wait_for_secondary_bus - Wait for secondary bus to be accessible
  * @dev: PCI bridge
+ * @reset_type: reset type in human-readable form
+ * @timeout: maximum time to wait for devices on secondary bus (milliseconds)
  *
  * Handle necessary delays before access to the devices on the secondary
- * side of the bridge are permitted after D3cold to D0 transition.
+ * side of the bridge are permitted after D3cold to D0 transition
+ * or Conventional Reset.
  *
  * For PCIe this means the delays in PCIe 5.0 section 6.6.1. For
  * conventional PCI it means Tpvrh + Trhfa specified in PCI 3.0 section
  * 4.3.2.
+ *
+ * Return 0 on success or -ENOTTY if the first device on the secondary bus
+ * failed to become accessible.
  */
-void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev)
+int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type,
+				      int timeout)
 {
 	struct pci_dev *child;
 	int delay;
 
 	if (pci_dev_is_disconnected(dev))
-		return;
+		return 0;
 
 	if (!pci_is_bridge(dev))
-		return;
+		return 0;
 
 	down_read(&pci_bus_sem);
 
@@ -4756,14 +4763,14 @@ void pci_bridge_wait_for_secondary_bus(s
 	 */
 	if (!dev->subordinate || list_empty(&dev->subordinate->devices)) {
 		up_read(&pci_bus_sem);
-		return;
+		return 0;
 	}
 
 	/* Take d3cold_delay requirements into account */
 	delay = pci_bus_max_d3cold_delay(dev->subordinate);
 	if (!delay) {
 		up_read(&pci_bus_sem);
-		return;
+		return 0;
 	}
 
 	child = list_first_entry(&dev->subordinate->devices, struct pci_dev,
@@ -4772,14 +4779,12 @@ void pci_bridge_wait_for_secondary_bus(s
 
 	/*
 	 * Conventional PCI and PCI-X we need to wait Tpvrh + Trhfa before
-	 * accessing the device after reset (that is 1000 ms + 100 ms). In
-	 * practice this should not be needed because we don't do power
-	 * management for them (see pci_bridge_d3_possible()).
+	 * accessing the device after reset (that is 1000 ms + 100 ms).
 	 */
 	if (!pci_is_pcie(dev)) {
 		pci_dbg(dev, "waiting %d ms for secondary bus\n", 1000 + delay);
 		msleep(1000 + delay);
-		return;
+		return 0;
 	}
 
 	/*
@@ -4796,11 +4801,11 @@ void pci_bridge_wait_for_secondary_bus(s
 	 * configuration requests if we only wait for 100 ms (see
 	 * https://bugzilla.kernel.org/show_bug.cgi?id=203885).
 	 *
-	 * Therefore we wait for 100 ms and check for the device presence.
-	 * If it is still not present give it an additional 100 ms.
+	 * Therefore we wait for 100 ms and check for the device presence
+	 * until the timeout expires.
 	 */
 	if (!pcie_downstream_port(dev))
-		return;
+		return 0;
 
 	if (pcie_get_speed_cap(dev) <= PCIE_SPEED_5_0GT) {
 		pci_dbg(dev, "waiting %d ms for downstream link\n", delay);
@@ -4810,14 +4815,11 @@ void pci_bridge_wait_for_secondary_bus(s
 			delay);
 		if (!pcie_wait_for_link_delay(dev, true, delay)) {
 			/* Did not train, no need to wait any further */
-			return;
+			return -ENOTTY;
 		}
 	}
 
-	if (!pci_device_is_present(child)) {
-		pci_dbg(child, "waiting additional %d ms to become accessible\n", delay);
-		msleep(delay);
-	}
+	return pci_dev_wait(child, reset_type, timeout - delay);
 }
 
 void pci_reset_secondary_bus(struct pci_dev *dev)
@@ -4836,15 +4838,6 @@ void pci_reset_secondary_bus(struct pci_
 
 	ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET;
 	pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
-
-	/*
-	 * Trhfa for conventional PCI is 2^25 clock cycles.
-	 * Assuming a minimum 33MHz clock this results in a 1s
-	 * delay before we can consider subordinate devices to
-	 * be re-initialized.  PCIe has some ways to shorten this,
-	 * but we don't make use of them yet.
-	 */
-	ssleep(1);
 }
 
 void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
@@ -4863,7 +4856,8 @@ int pci_bridge_secondary_bus_reset(struc
 {
 	pcibios_reset_secondary_bus(dev);
 
-	return pci_dev_wait(dev, "bus reset", PCIE_RESET_READY_POLL_MS);
+	return pci_bridge_wait_for_secondary_bus(dev, "bus reset",
+						 PCIE_RESET_READY_POLL_MS);
 }
 EXPORT_SYMBOL_GPL(pci_bridge_secondary_bus_reset);
 
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -47,6 +47,13 @@ int pci_bus_error_reset(struct pci_dev *
 #define PCI_PM_D3COLD_WAIT      100
 #define PCI_PM_BUS_WAIT         50
 
+/*
+ * Following exit from Conventional Reset, devices must be ready within 1 sec
+ * (PCIe r6.0 sec 6.6.1).  A D3cold to D0 transition implies a Conventional
+ * Reset (PCIe r6.0 sec 5.8).
+ */
+#define PCI_RESET_WAIT		1000	/* msec */
+
 /**
  * struct pci_platform_pm_ops - Firmware PM callbacks
  *
@@ -107,7 +114,8 @@ void pci_allocate_cap_save_buffers(struc
 void pci_free_cap_save_buffers(struct pci_dev *dev);
 bool pci_bridge_d3_possible(struct pci_dev *dev);
 void pci_bridge_d3_update(struct pci_dev *dev);
-void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev);
+int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type,
+				      int timeout);
 
 static inline void pci_wakeup_event(struct pci_dev *dev)
 {



  parent reply	other threads:[~2023-03-20 15:08 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-20 14:54 [PATCH 5.4 00/60] 5.4.238-rc1 review Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 01/60] ext4: fix cgroup writeback accounting with fs-layer encryption Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 02/60] xfrm: Allow transport-mode states with AF_UNSPEC selector Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 03/60] drm/panfrost: Dont sync rpm suspension after mmu flushing Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 04/60] cifs: Move the in_send statistic to __smb_send_rqst() Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 05/60] drm/meson: fix 1px pink line on GXM when scaling video overlay Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 06/60] clk: HI655X: select REGMAP instead of depending on it Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 07/60] docs: Correct missing "d_" prefix for dentry_operations member d_weak_revalidate Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 08/60] scsi: mpt3sas: Fix NULL pointer access in mpt3sas_transport_port_add() Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 09/60] ALSA: hda - add Intel DG1 PCI and HDMI ids Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 10/60] ALSA: hda - controller is in GPU on the DG1 Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 11/60] ALSA: hda: Add Alderlake-S PCI ID and HDMI codec vid Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 12/60] ALSA: hda: Add Intel DG2 " Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 13/60] ALSA: hda: Match only Intel devices with CONTROLLER_IN_GPU() Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 14/60] netfilter: nft_redir: correct value of inet type `.maxattrs` Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 15/60] scsi: core: Fix a comment in function scsi_host_dev_release() Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 16/60] scsi: core: Fix a procfs host directory removal regression Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 17/60] tcp: tcp_make_synack() can be called from process context Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 18/60] nfc: pn533: initialize struct pn533_out_arg properly Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 19/60] ipvlan: Make skb->skb_iif track skb->dev for l3s mode Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 20/60] i40e: Fix kernel crash during reboot when adapter is in recovery mode Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 21/60] qed/qed_dev: guard against a possible division by zero Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 22/60] net: tunnels: annotate lockless accesses to dev->needed_headroom Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 23/60] net: phy: smsc: bail out in lan87xx_read_status if genphy_read_status fails Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 24/60] nfc: st-nci: Fix use after free bug in ndlc_remove due to race condition Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 25/60] net: usb: smsc75xx: Limit packet length to skb->len Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 26/60] nvmet: avoid potential UAF in nvmet_req_complete() Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 27/60] block: sunvdc: add check for mdesc_grab() returning NULL Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 28/60] ipv4: Fix incorrect table ID in IOCTL path Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 29/60] net: usb: smsc75xx: Move packet length check to prevent kernel panic in skb_pull Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 30/60] net/iucv: Fix size of interrupt data Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 31/60] ethernet: sun: add check for the mdesc_grab() Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 32/60] hwmon: (adt7475) Display smoothing attributes in correct order Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 33/60] hwmon: (adt7475) Fix masking of hysteresis registers Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 34/60] hwmon: (xgene) Fix use after free bug in xgene_hwmon_remove due to race condition Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 35/60] hwmon: (ina3221) return prober error code Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 36/60] media: m5mols: fix off-by-one loop termination error Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 37/60] mmc: atmel-mci: fix race between stop command and start of next command Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 38/60] jffs2: correct logic when creating a hole in jffs2_write_begin Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 39/60] rust: arch/um: Disable FP/SIMD instruction to match x86 Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 40/60] ext4: fail ext4_iget if special inode unallocated Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 41/60] ext4: fix task hung in ext4_xattr_delete_inode Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 42/60] drm/amdkfd: Fix an illegal memory access Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 43/60] sh: intc: Avoid spurious sizeof-pointer-div warning Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 44/60] ext4: fix possible double unlock when moving a directory Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 45/60] tty: serial: fsl_lpuart: skip waiting for transmission complete when UARTCTRL_SBK is asserted Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 46/60] interconnect: fix mem leak when freeing nodes Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 47/60] tracing: Check field value in hist_field_name() Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 48/60] tracing: Make tracepoint lockdep check actually test something Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 49/60] KVM: nVMX: add missing consistency checks for CR0 and CR4 Greg Kroah-Hartman
2023-03-20 14:54 ` [PATCH 5.4 50/60] ftrace: Fix invalid address access in lookup_rec() when index is 0 Greg Kroah-Hartman
2023-03-20 14:55 ` [PATCH 5.4 51/60] fbdev: stifb: Provide valid pixelclock and add fb_check_var() checks Greg Kroah-Hartman
2023-03-20 14:55 ` [PATCH 5.4 52/60] x86/mm: Fix use of uninitialized buffer in sme_enable() Greg Kroah-Hartman
2023-03-20 14:55 ` [PATCH 5.4 53/60] Revert "treewide: Replace DECLARE_TASKLET() with DECLARE_TASKLET_OLD()" Greg Kroah-Hartman
2023-03-20 14:55 ` [PATCH 5.4 54/60] treewide: Replace DECLARE_TASKLET() with DECLARE_TASKLET_OLD() Greg Kroah-Hartman
2023-03-20 17:40   ` Tom Saeger
2023-03-20 14:55 ` [PATCH 5.4 55/60] drm/i915: Dont use stolen memory for ring buffers with LLC Greg Kroah-Hartman
2023-03-20 14:55 ` [PATCH 5.4 56/60] serial: 8250_em: Fix UART port type Greg Kroah-Hartman
2023-03-20 14:55 ` [PATCH 5.4 57/60] s390/ipl: add missing intersection check to ipl_report handling Greg Kroah-Hartman
2023-03-20 14:55 ` Greg Kroah-Hartman [this message]
2023-03-20 14:55 ` [PATCH 5.4 59/60] HID: core: Provide new max_buffer_size attribute to over-ride the default Greg Kroah-Hartman
2023-03-20 14:55 ` [PATCH 5.4 60/60] HID: uhid: Over-ride the default maximum data buffer value with our own Greg Kroah-Hartman
2023-03-20 17:19 ` [PATCH 5.4 00/60] 5.4.238-rc1 review Chris Paterson
2023-03-20 18:46 ` Florian Fainelli
2023-03-20 20:36 ` Naresh Kamboju
2023-03-20 23:14 ` Shuah Khan
2023-03-20 23:37 ` Shuah Khan
2023-03-21  5:07 ` Harshit Mogalapalli
2023-03-21 11:54 ` Jon Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230320145433.342750665@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=bhelgaas@google.com \
    --cc=lukas@wunner.de \
    --cc=mika.westerberg@linux.intel.com \
    --cc=patches@lists.linux.dev \
    --cc=ravi.kishore.koppuravuri@intel.com \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=stable@vger.kernel.org \
    --cc=windy.bi.enflame@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).