Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH v5] PCI/pcie: Add PCIe Lane Margining at Receiver (LMR) support
@ 2026-08-20 21:38 Priyank Rathod
  2026-08-20 21:51 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Priyank Rathod @ 2026-08-20 21:38 UTC (permalink / raw)
  To: Bjorn Helgaas, Priyank Rathod, Shuah Khan, Kees Cook,
	Gustavo A. R. Silva
  Cc: linux-kernel, linux-pci, linux-kselftest, linux-hardening,
	Ilpo Järvinen

Per PCIe Base Specification r6.0, sec 8.4.4 ("Lane Margining at
Receiver"), PCIe devices operating at 16.0 GT/s (Gen 4) or higher data
rates support the Lane Margining at Receiver Extended Capability
(ID 0x27), and it is mandatory for receivers operating at 64.0 GT/s
(Gen 6) or higher data rates. Lane Margining allows software to
evaluate high-speed link margins by measuring timing and voltage steps
for each individual physical lane and receiver.

Add driver and debugfs support for PCIe Lane Margining at Receiver:

  - Add Lane Margining at Receiver Extended Capability register
    definitions (PCI_EXT_CAP_ID_LMR, PCI_LMR_PORT_CAP, PCI_LMR_PORT_STS,
    PCI_LMR_LANE_CTRL, PCI_LMR_LANE_STS) to <uapi/linux/pci_regs.h>.
  - Add Kconfig option CONFIG_PCIE_LMR (under drivers/pci/pcie/Kconfig)
    dependent on DEBUG_FS.
  - Implement drivers/pci/pcie/margin.c to probe the capability on Gen4+
    links and expose per-device debugfs entries under:
      /sys/kernel/debug/pci/pcie_lmr_<pci_dev_name>/
    providing control over margining enablement, receiver selection, and
    execution of timing/voltage margin step commands. Distinguish
    between missing mandatory LMR capability on Gen6+ vs optional on
    Gen4/Gen5.
  - Hook pci_lmr_init() into pci_init_capabilities() during device probe
    in drivers/pci/probe.c and pci_lmr_exit() into drivers/pci/remove.c.
  - Add kselftest script under tools/testing/selftests/pcie_lmt/pcie_lmt.sh
    to test debugfs capability reads, enablement, and stepping.
  - Add MAINTAINERS entry for PCIe Lane Margining at Receiver (LMR).

Signed-off-by: Priyank Rathod <rathodpriyank@google.com>
---
Per PCIe Base Specification r6.0, section 8.4.4 ("Lane Margining at Receiver"),
PCIe devices operating at 16.0 GT/s (Gen 4) or higher data rates support the
Lane Margining at Receiver Extended Capability (ID 0x27), and it is mandatory
for receivers operating at 64.0 GT/s (Gen 6) or higher data rates.

Lane Margining allows system software to evaluate high-speed link signal
integrity and margins by measuring timing and voltage steps for each physical
lane and receiver independently.

This series introduces kernel driver support, debugfs controls, and a
kselftest automation script for PCIe Lane Margining at Receiver (LMR/LMT).

==============================================================================
1. How to Enable & Configure
==============================================================================
Enable the Kconfig option under PCI support:
  CONFIG_PCIE_LMR=y (or =m)
  (Depends on CONFIG_PCI and CONFIG_DEBUG_FS)

Upon boot or device hotplug on Gen4+ links (>= 16.0 GT/s), the driver probes
Extended Capability ID 0x27 and exposes per-device debugfs interfaces:
  /sys/kernel/debug/pci/pcie_lmr_<domain>:<bus>:<dev>.<func>/

==============================================================================
2. How to Use the Debugfs Interface (Manual Margining)
==============================================================================
Inspect device-wide margining capabilities and port status:
  # Inspect root device LMR capabilities & status
  cat /sys/kernel/debug/pci/pcie_lmr_0000:01:00.0/capabilities
  cat /sys/kernel/debug/pci/pcie_lmr_0000:01:00.0/port_status

Enable active Lane Margining on the device:
  # Enable Lane Margining state machine
  echo 1 > /sys/kernel/debug/pci/pcie_lmr_0000:01:00.0/enable

Inspect and step individual lanes (e.g. lane0):
  # Select target receiver (0 = local receiver, 1..6 = retimers/link partners)
  echo 0 > /sys/kernel/debug/pci/pcie_lmr_0000:01:00.0/lane0/receiver

  # Check available timing and voltage steps for this receiver
  cat /sys/kernel/debug/pci/pcie_lmr_0000:01:00.0/lane0/caps
  cat /sys/kernel/debug/pci/pcie_lmr_0000:01:00.0/lane0/num_timing_steps
  cat /sys/kernel/debug/pci/pcie_lmr_0000:01:00.0/lane0/num_voltage_steps

  # Step timing margin or voltage margin offset
  echo 2 > /sys/kernel/debug/pci/pcie_lmr_0000:01:00.0/lane0/margin_timing
  echo 1 > /sys/kernel/debug/pci/pcie_lmr_0000:01:00.0/lane0/margin_voltage

  # Reset margin offset back to nominal (0)
  echo 0 > /sys/kernel/debug/pci/pcie_lmr_0000:01:00.0/lane0/margin_timing
  echo 0 > /sys/kernel/debug/pci/pcie_lmr_0000:01:00.0/lane0/margin_voltage

Disable Lane Margining when finished:
  echo 0 > /sys/kernel/debug/pci/pcie_lmr_0000:01:00.0/enable

==============================================================================
3. How to Run Automated Kselftests Using the Test Script
==============================================================================
An automated kselftest script is included to test capability reads, receiver
selection, and margining commands across all enumerated LMR devices:

  # Run directly as root
  sudo ./tools/testing/selftests/pcie_lmt/pcie_lmt.sh

Or run via the kselftest Makefile harness:
  make -C tools/testing/selftests TARGETS=pcie_lmt run_tests

Sample script output on an LMR-capable device:
  pcie_lmt: testing PCIe LMR debugfs entries
  pcie_lmt: probing device pcie_lmr_0000:01:00.0
    pcie_lmr_0000:01:00.0: capabilities read OK
    pcie_lmr_0000:01:00.0: port_status read OK
    pcie_lmr_0000:01:00.0: margining enabled OK
    pcie_lmr_0000:01:00.0: testing lane0
    pcie_lmr_0000:01:00.0: testing lane1
    pcie_lmr_0000:01:00.0: margining disabled OK
  pcie_lmt [PASS]

To: Bjorn Helgaas <bhelgaas@google.com>
To: Shuah Khan <shuah@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-kselftest@vger.kernel.org
Cc: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Changes in v5:
  - Sorted #include directives alphabetically and added missing includes for bits.h, bitfield.h, cleanup.h, overflow.h, and slab.h (Ilpo Järvinen).
  - Converted bitmasks to GENMASK() and BIT() macros and used FIELD_PREP() and FIELD_GET() instead of manual bit shifts (Ilpo Järvinen).
  - Added pci_lmr_sts_payload() helper to cleanly extract the status payload byte before applying step and capability masks (Ilpo Järvinen).
  - Replaced manual mutex locking sequences with guard(mutex)(&mdev->lock) across show and write callbacks to simplify control flow (Ilpo Järvinen).
  - Documented mutex lock protection scope in kerneldoc for struct pci_margin_dev (Ilpo Järvinen).
  - Used standard PCI_POSSIBLE_ERROR(), str_yes_no(), and scnprintf() helpers throughout the driver (Ilpo Järvinen).
  - Clarified receiver range (0..6 per PCIe r6.0 sec 8.4.4; 7 reserved) in comments and validation checks (Ilpo Järvinen).
  - Deduplicated timing and voltage show/write handlers using margin_lane_steps_show() and margin_lane_step_write() (Ilpo Järvinen).
  - Placed speed check immediately following pcie_get_speed_cap() and handled PCI_SPEED_UNKNOWN (Ilpo Järvinen).
  - Converted lanes in struct pci_margin_dev to a flexible array member with __counted_by(num_lanes) allocated via struct_size() (Ilpo Järvinen).

Changes in v4:
  - Added Sample Multiple Receivers (Bit 5) concurrency verification in margin_lane_timing_write() and margin_lane_voltage_write() per PCIe r6.0 sec 8.4.4, returning -EBUSY if another lane on the same receiver is already margined when simultaneous lane margining is not supported.
  - Added active operating link speed verification (PCI_EXP_LNKSTA_CLS >= 16.0 GT/s) in margin_enable_write() before enabling LMR, as LMR commands are physically undefined on links operating at Gen1/Gen2/Gen3 speeds.
  - Added fast-path hardware NAK detection in pci_lmr_run_cmd() to return -EOPNOTSUPP immediately if a receiver echoes MTYPE == NO_CMD (0x7) after command issuance rather than waiting 150ms for a timeout.
  - Added pci_reset_lmr() hooked into __pci_reset_function_locked() to synchronize software state and demargin on FLR or Secondary Bus Reset.
  - Comprehensive NULL pointer checks and array/lane/receiver bounds checks added across all internal helpers and debugfs write handlers.
  - Added MAINTAINERS entry for PCIe Lane Margining at Receiver (LMR).

Changes in v2:
  - Fixed NO_CMD (0x7) clearing in pci_lmr_run_cmd() before issuing new commands per PCIe r6.0 sec 8.4.4.
  - Protected plane->rx updates with mdev->lock in margin_lane_receiver_write().
  - Corrected Margining Port Capabilities bit definition to PCI_LMR_PORT_CAP_USES_SW_READY (0x0001) in <uapi/linux/pci_regs.h>.
  - Updated kselftest script (pcie_lmt.sh) to locate LMR debugfs entries.
  - Validated integer bounds against LMR_MAX_TIMING_STEP / LMR_MAX_VOLTAGE_STEP before narrowing u8 cast.
  - Moved mdev->enabled checks inside mutex_lock(&mdev->lock) to eliminate TOCTOU races.
  - Checked return values of all pci_read_config_word() calls, propagating -EIO on failure.
  - Eliminated dead store of cap in margin_enable_write().
  - Explicitly checked speed == PCIE_SPEED_64_0GT in pci_lmr_init() to avoid misidentifying PCI_SPEED_UNKNOWN (0xFF) as Gen6.
---
 MAINTAINERS                                  |   7 +
 drivers/pci/pci-driver.c                     |   1 +
 drivers/pci/pci.c                            |   4 +-
 drivers/pci/pci.h                            |  12 +
 drivers/pci/pcie/Kconfig                     |  12 +
 drivers/pci/pcie/Makefile                    |   1 +
 drivers/pci/pcie/margin.c                    | 970 +++++++++++++++++++++++++++
 drivers/pci/probe.c                          |   1 +
 drivers/pci/remove.c                         |   1 +
 include/linux/pci.h                          |   6 +
 include/uapi/linux/pci_regs.h                |  18 +
 tools/testing/selftests/Makefile             |   1 +
 tools/testing/selftests/pcie_lmt/Makefile    |   3 +
 tools/testing/selftests/pcie_lmt/pcie_lmt.sh | 105 +++
 14 files changed, 1141 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index b7094a616afd..2935b624ac67 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21059,6 +21059,13 @@ F:	Documentation/devicetree/bindings/pci/qcom,sa8255p-pcie-ep.yaml
 F:	drivers/pci/controller/dwc/pcie-qcom-common.c
 F:	drivers/pci/controller/dwc/pcie-qcom-ep.c
 
+PCIE LANE MARGINING AT RECEIVER (LMR)
+M:	Priyank Rathod <rathodpriyank@google.com>
+L:	linux-pci@vger.kernel.org
+S:	Maintained
+F:	drivers/pci/pcie/margin.c
+F:	tools/testing/selftests/pcie_lmt/
+
 PCMCIA SUBSYSTEM
 M:	Dominik Brodowski <linux@dominikbrodowski.net>
 S:	Odd Fixes
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index f36778e62ac1..17544a7023fc 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -821,6 +821,7 @@ static int pci_pm_suspend(struct device *dev)
 	 * since Coffee Lake, to enter a lower-power PM state.
 	 */
 	pci_suspend_ptm(pci_dev);
+	pci_suspend_lmr(pci_dev);
 
 	if (pci_has_legacy_pm_support(pci_dev))
 		return pci_legacy_suspend(dev, PMSG_SUSPEND);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 77b17b13ee61..dc9724cb7b4d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5145,8 +5145,10 @@ int __pci_reset_function_locked(struct pci_dev *dev)
 		method = &pci_reset_fn_methods[m];
 		pci_dbg(dev, "reset via %s\n", method->name);
 		rc = method->reset_fn(dev, PCI_RESET_DO_RESET);
-		if (!rc)
+		if (!rc) {
+			pci_reset_lmr(dev);
 			return 0;
+		}
 
 		pci_dbg(dev, "%s failed with %d\n", method->name, rc);
 		if (rc != -ENOTTY)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4469e1a77f3c..6322a81f9e50 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1023,6 +1023,18 @@ static inline void pci_no_tph(void) { }
 static inline void pci_tph_init(struct pci_dev *dev) { }
 #endif
 
+#ifdef CONFIG_PCIE_LMR
+void pci_lmr_init(struct pci_dev *dev);
+void pci_lmr_exit(struct pci_dev *dev);
+void pci_suspend_lmr(struct pci_dev *dev);
+void pci_reset_lmr(struct pci_dev *dev);
+#else
+static inline void pci_lmr_init(struct pci_dev *dev) { }
+static inline void pci_lmr_exit(struct pci_dev *dev) { }
+static inline void pci_suspend_lmr(struct pci_dev *dev) { }
+static inline void pci_reset_lmr(struct pci_dev *dev) { }
+#endif
+
 #ifdef CONFIG_PCIE_PTM
 void pci_ptm_init(struct pci_dev *dev);
 void pci_save_ptm_state(struct pci_dev *dev);
diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
index 207c2deae35f..3b021ca2fe84 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -137,6 +137,18 @@ config PCIE_PTM
 	  This is only useful if you have devices that support PTM, but it
 	  is safe to enable even if you don't.
 
+config PCIE_LMR
+	bool "PCI Express Lane Margining at Receiver Support"
+	depends on DEBUG_FS
+	help
+	  This enables the PCI Express Lane Margining at Receiver support.
+	  Lane Margining allows software to determine the voltage and
+	  timing margin of each lane on a PCIe link (16.0 GT/s and above).
+	  The margining data is exposed via debugfs.
+
+	  This is only useful if you have devices that support lane
+	  margining, but it is safe to enable even if you don't.
+
 config PCIE_EDR
 	bool "PCI Express Error Disconnect Recover support"
 	depends on PCIE_DPC && ACPI
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index b0b43a18c304..aac45ae0402e 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -13,4 +13,5 @@ obj-$(CONFIG_PCIEAER_INJECT)	+= aer_inject.o
 obj-$(CONFIG_PCIE_PME)		+= pme.o
 obj-$(CONFIG_PCIE_DPC)		+= dpc.o
 obj-$(CONFIG_PCIE_PTM)		+= ptm.o
+obj-$(CONFIG_PCIE_LMR)		+= margin.o
 obj-$(CONFIG_PCIE_EDR)		+= edr.o
diff --git a/drivers/pci/pcie/margin.c b/drivers/pci/pcie/margin.c
new file mode 100644
index 000000000000..59649ed7dc4f
--- /dev/null
+++ b/drivers/pci/pcie/margin.c
@@ -0,0 +1,970 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCI Express Lane Margining at Receiver
+ *
+ * Copyright (C) 2026 Google LLC
+ * Author: Priyank Rathod <rathodpriyank@google.com>
+ *
+ * Lane Margining at Receiver (PCIe Base Specification r6.0, sec 8.4.4)
+ * allows system software to determine the voltage and timing margins of
+ * each physical lane on a PCIe link. The Extended Capability (ID 0x27)
+ * is available for receivers operating at 16.0 GT/s (Gen4) or higher data
+ * rates, and is mandatory for receivers operating at 64.0 GT/s (Gen6) or
+ * higher data rates.
+ *
+ * This driver implements:
+ *   - Probing Extended Capability ID 0x27 and Margining Port Capabilities.
+ *   - Managing ASPM L0s/L1 link states during active margining with restoration.
+ *   - PCIe r6.0 NO_CMD (0x7) clearing handshake per receiver and lane.
+ *   - Caching receiver capabilities & step counts to avoid DEMARGIN side-effects.
+ *   - Handling Symmetric vs Independent Left/Right & Up/Down margin steps.
+ *   - Runtime PM protection (D0 enforcement) during active margining.
+ *   - Exposing per-device debugfs interfaces under /sys/kernel/debug/pci/.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/jiffies.h>
+#include <linux/kstrtox.h>
+#include <linux/minmax.h>
+#include <linux/mutex.h>
+#include <linux/overflow.h>
+#include <linux/pci.h>
+#include <linux/pm_runtime.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+#include <linux/sprintf.h>
+#include <linux/string_choices.h>
+#include <linux/types.h>
+
+#include "../pci.h"
+
+/* Margin type encodings per PCIe Base Spec r6.0 sec 8.4.4 */
+#define LMR_TYPE_DEMARGIN               0x0
+#define LMR_TYPE_REPORT_CAPS            0x1
+#define LMR_TYPE_REPORT_VOLTAGE_STEPS   0x2
+#define LMR_TYPE_REPORT_TIMING_STEPS    0x3
+#define LMR_TYPE_TIMING                 0x4
+#define LMR_TYPE_VOLTAGE                0x5
+#define LMR_TYPE_NO_CMD                 0x7
+
+/* LMR command timing parameters */
+#define LMR_CMD_TIMEOUT_MS              150
+#define LMR_CMD_SLEEP_MIN_US            100
+#define LMR_CMD_SLEEP_MAX_US            250
+#define LMR_ENABLE_TIMEOUT_MS           150
+#define LMR_ENABLE_SLEEP_MIN_US         1000
+#define LMR_ENABLE_SLEEP_MAX_US         2000
+
+/*
+ * LMR limits:
+ * Valid receiver numbers are 0 (local receiver) to 6 (up to 3 retimers)
+ * per PCIe Base Specification r6.0 sec 8.4.4. Receiver number 7 is reserved.
+ */
+#define LMR_MAX_LANES                   32
+#define LMR_MAX_RX_NUM                  6
+#define LMR_MAX_TIMING_STEP             63
+#define LMR_MAX_VOLTAGE_STEP            127
+
+/* LMR PCIe generation numbers and helper */
+#define LMR_GEN6                        6
+#define LMR_GEN5                        5
+#define LMR_GEN4                        4
+
+#define LMR_SPEED_TO_GEN(speed) \
+	((speed) >= PCIE_SPEED_64_0GT ? LMR_GEN6 : \
+	 (speed) >= PCIE_SPEED_32_0GT ? LMR_GEN5 : \
+	 LMR_GEN4)
+
+/* LMR lane register stride */
+#define LMR_LANE_REG_STRIDE             4
+
+/* LMR receivers and directions */
+#define LMR_RX_LOCAL                    0
+#define LMR_STEP_DIR_INCREASE           1
+#define LMR_STEP_DIR_DECREASE           0
+
+/* LMR payload field masks per PCIe Base Spec r6.0 sec 8.4.4 */
+#define LMR_STEPS_MASK			GENMASK(6, 0)
+#define LMR_TIMING_STEP_MASK		GENMASK(5, 0)
+#define LMR_TIMING_DIR_MASK		BIT(6)
+#define LMR_VOLTAGE_STEP_MASK		GENMASK(6, 0)
+#define LMR_VOLTAGE_DIR_MASK		BIT(7)
+
+/* LMR capabilities report bit fields */
+#define LMR_CAP_MARGIN_HV		BIT(0)
+#define LMR_CAP_MARGIN_EV_IV		BIT(1)
+#define LMR_CAP_IND_LEFT_RIGHT_TIMING	BIT(2)
+#define LMR_CAP_IND_UP_DOWN_VOLTAGE	BIT(3)
+#define LMR_CAP_ERROR_SAMPLER		BIT(4)
+#define LMR_CAP_SAMPLE_MULTIPLE_RX	BIT(5)
+
+/**
+ * struct pci_margin_rx_info - Cached Lane Margining receiver capabilities
+ * @caps_cached: True if receiver capabilities and step limits are cached
+ * @caps: Margining capabilities byte reported by receiver
+ * @num_timing_steps: Maximum timing margin steps supported by receiver
+ * @num_voltage_steps: Maximum voltage margin steps supported by receiver
+ */
+struct pci_margin_rx_info {
+	bool caps_cached;
+	u8 caps;
+	u8 num_timing_steps;
+	u8 num_voltage_steps;
+};
+
+/**
+ * struct pci_margin_lane - Per-lane margining state
+ * @mdev: Parent LMR margin device
+ * @lane: Physical lane index (0..num_lanes - 1)
+ * @rx: Selected target receiver number (0 = local, 1..6 = retimers)
+ * @timing_val: Current applied timing margin step offset (+/-)
+ * @voltage_val: Current applied voltage margin step offset (+/-)
+ * @rx_info: Cached receiver capabilities per receiver number
+ */
+struct pci_margin_lane {
+	struct pci_margin_dev *mdev;
+	int lane;
+	u8 rx;
+	int timing_val;
+	int voltage_val;
+	struct pci_margin_rx_info rx_info[LMR_MAX_RX_NUM + 1];
+};
+
+/**
+ * struct pci_margin_dev - PCIe Lane Margining device instance
+ * @dev: Underlying PCI device
+ * @cap: Extended capability offset (PCI_EXT_CAP_ID_LMR)
+ * @debugfs: Root debugfs dentry for this device
+ * @lock: Mutex protecting LMR hardware access, active margining enablement,
+ *        target receiver selection, lane margining steps, and ASPM state
+ * @enabled: True if Lane Margining is currently enabled
+ * @aspm_saved: True if original ASPM configuration has been saved
+ * @saved_aspm: Saved ASPM control register bits for the device
+ * @saved_parent_aspm: Saved ASPM control register bits for parent bridge
+ * @num_lanes: Number of lanes on the link
+ * @lanes: Flexible array of per-lane state structures
+ */
+struct pci_margin_dev {
+	struct pci_dev *dev;
+	u16 cap;
+	struct dentry *debugfs;
+	struct mutex lock;
+	bool enabled;
+	bool aspm_saved;
+	u16 saved_aspm;
+	u16 saved_parent_aspm;
+	int num_lanes;
+	struct pci_margin_lane lanes[] __counted_by(num_lanes);
+};
+
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+static DEFINE_MUTEX(pci_debugfs_root_lock);
+static struct dentry *pci_debugfs_root_dir;
+
+static struct dentry *get_pci_debugfs_root(void)
+{
+	mutex_lock(&pci_debugfs_root_lock);
+	if (!pci_debugfs_root_dir)
+		pci_debugfs_root_dir = debugfs_lookup("pci", NULL);
+	if (!pci_debugfs_root_dir)
+		pci_debugfs_root_dir = debugfs_create_dir("pci", NULL);
+	mutex_unlock(&pci_debugfs_root_lock);
+	return pci_debugfs_root_dir;
+}
+#endif
+
+/*
+ * pci_lmr_disable_aspm() - Temporarily disable ASPM L0s/L1 during active
+ * margining per PCIe Base Spec r6.0 sec 8.4.4, saving original ASPMC bits.
+ */
+static void pci_lmr_disable_aspm(struct pci_margin_dev *mdev)
+{
+	struct pci_dev *dev = mdev->dev;
+	struct pci_dev *parent = pci_upstream_bridge(dev);
+	u16 ctl;
+
+	if (mdev->aspm_saved)
+		return;
+
+	if (!pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &ctl)) {
+		mdev->saved_aspm = ctl & PCI_EXP_LNKCTL_ASPMC;
+		pcie_capability_clear_word(dev, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_ASPMC);
+	}
+
+	if (parent && pci_is_pcie(parent)) {
+		if (!pcie_capability_read_word(parent, PCI_EXP_LNKCTL, &ctl)) {
+			mdev->saved_parent_aspm = ctl & PCI_EXP_LNKCTL_ASPMC;
+			pcie_capability_clear_word(parent, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_ASPMC);
+		}
+	}
+	mdev->aspm_saved = true;
+}
+
+/*
+ * pci_lmr_restore_aspm() - Restore original ASPM L0s/L1 state when margining
+ * is disabled or torn down.
+ */
+static void pci_lmr_restore_aspm(struct pci_margin_dev *mdev)
+{
+	struct pci_dev *dev = mdev->dev;
+	struct pci_dev *parent = pci_upstream_bridge(dev);
+
+	if (!mdev->aspm_saved)
+		return;
+
+	pcie_capability_clear_and_set_word(dev, PCI_EXP_LNKCTL,
+					   PCI_EXP_LNKCTL_ASPMC,
+					   mdev->saved_aspm);
+	if (parent && pci_is_pcie(parent))
+		pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL,
+						   PCI_EXP_LNKCTL_ASPMC,
+						   mdev->saved_parent_aspm);
+	mdev->aspm_saved = false;
+}
+
+static inline u8 pci_lmr_sts_payload(u16 sts)
+{
+	return FIELD_GET(PCI_LMR_LANE_STS_PAYLOAD, sts);
+}
+
+/*
+ * pci_lmr_run_cmd() - Issue LMR command to Lane Control and wait for Status.
+ * Must be called with mdev->lock held.
+ */
+static int pci_lmr_run_cmd(struct pci_dev *dev, int lane, u8 rx, u8 type,
+			   u8 usage, u8 payload, u16 *status_val)
+{
+	u16 lmr, ctrl_offset, sts_offset;
+	u16 ctrl, sts;
+	unsigned long timeout;
+
+	if (!dev || !dev->lmr || lane < 0 || lane >= dev->lmr->num_lanes ||
+	    rx > LMR_MAX_RX_NUM)
+		return -EINVAL;
+
+	lmr = dev->lmr_cap;
+	ctrl_offset = lmr + PCI_LMR_LANE_CTRL + LMR_LANE_REG_STRIDE * lane;
+	sts_offset = lmr + PCI_LMR_LANE_STS + LMR_LANE_REG_STRIDE * lane;
+
+	/*
+	 * Per PCIe Base Spec r6.0 sec 8.4.4, software must issue NO_CMD (0x7)
+	 * targeting the specific receiver (rx) to clear MTYPE in Lane Status
+	 * before issuing a subsequent command.
+	 */
+	if (type != LMR_TYPE_NO_CMD) {
+		ctrl = FIELD_PREP(PCI_LMR_LANE_CTRL_RX_NUM, rx) |
+		       FIELD_PREP(PCI_LMR_LANE_CTRL_MTYPE, LMR_TYPE_NO_CMD) |
+		       FIELD_PREP(PCI_LMR_LANE_CTRL_USAGE, 0) |
+		       FIELD_PREP(PCI_LMR_LANE_CTRL_PAYLOAD, 0);
+
+		pci_write_config_word(dev, ctrl_offset, ctrl);
+
+		timeout = jiffies + msecs_to_jiffies(LMR_CMD_TIMEOUT_MS);
+		while (1) {
+			if (pci_read_config_word(dev, sts_offset, &sts))
+				return -EIO;
+			if (PCI_POSSIBLE_ERROR(sts))
+				return -ENODEV;
+			if (FIELD_GET(PCI_LMR_LANE_STS_MTYPE, sts) == LMR_TYPE_NO_CMD &&
+			    FIELD_GET(PCI_LMR_LANE_STS_RX_NUM, sts) == rx)
+				break;
+			if (time_after(jiffies, timeout))
+				return -ETIMEDOUT;
+			usleep_range(LMR_CMD_SLEEP_MIN_US, LMR_CMD_SLEEP_MAX_US);
+		}
+	}
+
+	ctrl = FIELD_PREP(PCI_LMR_LANE_CTRL_RX_NUM, rx) |
+	       FIELD_PREP(PCI_LMR_LANE_CTRL_MTYPE, type) |
+	       FIELD_PREP(PCI_LMR_LANE_CTRL_USAGE, usage) |
+	       FIELD_PREP(PCI_LMR_LANE_CTRL_PAYLOAD, payload);
+
+	pci_write_config_word(dev, ctrl_offset, ctrl);
+
+	timeout = jiffies + msecs_to_jiffies(LMR_CMD_TIMEOUT_MS);
+	while (1) {
+		if (pci_read_config_word(dev, sts_offset, &sts))
+			return -EIO;
+		if (PCI_POSSIBLE_ERROR(sts))
+			return -ENODEV;
+
+		if (FIELD_GET(PCI_LMR_LANE_STS_MTYPE, sts) == type &&
+		    FIELD_GET(PCI_LMR_LANE_STS_RX_NUM, sts) == rx) {
+			if (status_val)
+				*status_val = sts;
+			return 0;
+		}
+
+		/*
+		 * Per PCIe Base Spec r6.0 sec 8.4.4, if receiver echoes NO_CMD (0x7)
+		 * after a command was sent, it indicates command rejection/NAK.
+		 */
+		if (FIELD_GET(PCI_LMR_LANE_STS_MTYPE, sts) == LMR_TYPE_NO_CMD &&
+		    FIELD_GET(PCI_LMR_LANE_STS_RX_NUM, sts) == rx)
+			return -EOPNOTSUPP;
+
+		if (time_after(jiffies, timeout))
+			break;
+
+		usleep_range(LMR_CMD_SLEEP_MIN_US, LMR_CMD_SLEEP_MAX_US);
+	}
+
+	return -ETIMEDOUT;
+}
+
+static int pci_lmr_demargin_lane(struct pci_margin_lane *plane)
+{
+	u16 sts;
+	int ret;
+
+	if (!plane || !plane->mdev)
+		return -EINVAL;
+
+	if (plane->timing_val == 0 && plane->voltage_val == 0)
+		return 0;
+
+	ret = pci_lmr_run_cmd(plane->mdev->dev, plane->lane, plane->rx,
+			      LMR_TYPE_DEMARGIN, 0, 0, &sts);
+	if (!ret) {
+		plane->timing_val = 0;
+		plane->voltage_val = 0;
+	}
+	return ret;
+}
+
+static int pci_lmr_cache_rx_info(struct pci_margin_lane *plane, u8 rx)
+{
+	struct pci_margin_rx_info *info;
+	u16 sts;
+	int ret;
+
+	if (!plane || rx > LMR_MAX_RX_NUM)
+		return -EINVAL;
+
+	info = &plane->rx_info[rx];
+
+	if (info->caps_cached)
+		return 0;
+
+	/* Issuing REPORT_CAPS aborts any active margin per PCIe spec */
+	ret = pci_lmr_demargin_lane(plane);
+	if (ret)
+		return ret;
+
+	ret = pci_lmr_run_cmd(plane->mdev->dev, plane->lane, rx,
+			      LMR_TYPE_REPORT_CAPS, 0, 0, &sts);
+	if (ret)
+		return ret;
+	info->caps = pci_lmr_sts_payload(sts);
+
+	ret = pci_lmr_run_cmd(plane->mdev->dev, plane->lane, rx,
+			      LMR_TYPE_REPORT_TIMING_STEPS, 0, 0, &sts);
+	if (ret)
+		return ret;
+	info->num_timing_steps = FIELD_GET(LMR_STEPS_MASK, pci_lmr_sts_payload(sts));
+
+	ret = pci_lmr_run_cmd(plane->mdev->dev, plane->lane, rx,
+			      LMR_TYPE_REPORT_VOLTAGE_STEPS, 0, 0, &sts);
+	if (ret)
+		return ret;
+	info->num_voltage_steps = FIELD_GET(LMR_STEPS_MASK, pci_lmr_sts_payload(sts));
+
+	info->caps_cached = true;
+	return 0;
+}
+
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+
+static int margin_caps_show(struct seq_file *s, void *v)
+{
+	struct pci_margin_dev *mdev = s->private;
+	struct pci_dev *dev = mdev->dev;
+	u16 cap;
+
+	if (pci_read_config_word(dev, mdev->cap + PCI_LMR_PORT_CAP, &cap))
+		return -EIO;
+	seq_printf(s, "Port Capabilities: %#06x\n", cap);
+	seq_printf(s, "  Uses SW Ready: %s\n",
+		   str_yes_no(cap & PCI_LMR_PORT_CAP_USES_SW_READY));
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(margin_caps);
+
+static int margin_port_status_show(struct seq_file *s, void *v)
+{
+	struct pci_margin_dev *mdev = s->private;
+	struct pci_dev *dev = mdev->dev;
+	u16 sts;
+
+	if (pci_read_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, &sts))
+		return -EIO;
+	seq_printf(s, "Port Status: %#06x\n", sts);
+	seq_printf(s, "  Margining Ready: %s\n",
+		   str_yes_no(sts & PCI_LMR_PORT_STS_MARGIN_READY));
+	seq_printf(s, "  SW Ready: %s\n",
+		   str_yes_no(sts & PCI_LMR_PORT_STS_SW_READY));
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(margin_port_status);
+
+static int margin_enable_show(struct seq_file *s, void *v)
+{
+	struct pci_margin_dev *mdev = s->private;
+
+	guard(mutex)(&mdev->lock);
+	seq_printf(s, "%d\n", mdev->enabled);
+	return 0;
+}
+
+static void __pci_suspend_lmr_locked(struct pci_dev *dev)
+{
+	struct pci_margin_dev *mdev = dev->lmr;
+	int i;
+	u16 sts;
+
+	if (!mdev || !mdev->enabled)
+		return;
+
+	for (i = 0; i < mdev->num_lanes; i++)
+		pci_lmr_demargin_lane(&mdev->lanes[i]);
+
+	if (pci_read_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, &sts) == 0) {
+		sts &= ~PCI_LMR_PORT_STS_SW_READY;
+		pci_write_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, sts);
+	}
+	pci_lmr_restore_aspm(mdev);
+	pm_runtime_put(&dev->dev);
+	mdev->enabled = false;
+}
+
+static ssize_t margin_enable_write(struct file *file, const char __user *user_buf,
+			       size_t count, loff_t *ppos)
+{
+	struct seq_file *s = file->private_data;
+	struct pci_margin_dev *mdev = s->private;
+	struct pci_dev *dev = mdev->dev;
+	unsigned long timeout;
+	u16 sts, cap, lnksta;
+	bool enable;
+	int ret, i;
+
+	ret = kstrtobool_from_user(user_buf, count, &enable);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&mdev->lock);
+
+	if (mdev->enabled == enable)
+		return count;
+
+	if (!enable) {
+		__pci_suspend_lmr_locked(dev);
+		return count;
+	}
+
+	/* PCIe r6.0 sec 8.4.4: LMR is physically undefined below 16.0 GT/s (Gen4) */
+	pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &lnksta);
+	if ((lnksta & PCI_EXP_LNKSTA_CLS) < PCI_EXP_LNKSTA_CLS_16_0GB)
+		return -EOPNOTSUPP;
+
+	if (pci_read_config_word(dev, mdev->cap + PCI_LMR_PORT_CAP, &cap))
+		return -EIO;
+
+	/* Prevent D3hot runtime PM transition during active margining */
+	ret = pm_runtime_resume_and_get(&dev->dev);
+	if (ret < 0)
+		return ret;
+
+	/* Disable ASPM L0s/L1 during margining with restoration path */
+	pci_lmr_disable_aspm(mdev);
+
+	/* Ensure link is settled in L0 mode per PCIe r6.0 sec 8.4.4 */
+	usleep_range(2000, 3000);
+
+	if (cap & PCI_LMR_PORT_CAP_USES_SW_READY) {
+		if (pci_read_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, &sts)) {
+			ret = -EIO;
+			goto err_enable;
+		}
+		sts |= PCI_LMR_PORT_STS_SW_READY;
+		pci_write_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, sts);
+	}
+
+	timeout = jiffies + msecs_to_jiffies(LMR_ENABLE_TIMEOUT_MS);
+	while (1) {
+		if (pci_read_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, &sts)) {
+			ret = -EIO;
+			goto err_enable;
+		}
+		if (PCI_POSSIBLE_ERROR(sts)) {
+			ret = -ENODEV;
+			goto err_enable;
+		}
+		if (sts & PCI_LMR_PORT_STS_MARGIN_READY)
+			break;
+		if (time_after(jiffies, timeout)) {
+			ret = -ETIMEDOUT;
+			goto err_enable;
+		}
+		usleep_range(LMR_ENABLE_SLEEP_MIN_US, LMR_ENABLE_SLEEP_MAX_US);
+	}
+
+	/* Cache capabilities for configured receiver on all lanes */
+	for (i = 0; i < mdev->num_lanes; i++) {
+		ret = pci_lmr_cache_rx_info(&mdev->lanes[i],
+					    mdev->lanes[i].rx);
+		if (ret)
+			goto err_enable;
+	}
+	mdev->enabled = true;
+	return count;
+
+err_enable:
+	if (cap & PCI_LMR_PORT_CAP_USES_SW_READY) {
+		if (pci_read_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, &sts) == 0) {
+			sts &= ~PCI_LMR_PORT_STS_SW_READY;
+			pci_write_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, sts);
+		}
+	}
+	pci_lmr_restore_aspm(mdev);
+	pm_runtime_put(&dev->dev);
+	return ret;
+}
+
+static int margin_enable_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, margin_enable_show, inode->i_private);
+}
+
+static const struct file_operations margin_enable_fops = {
+	.open = margin_enable_open,
+	.read = seq_read,
+	.write = margin_enable_write,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static int margin_lane_receiver_show(struct seq_file *s, void *v)
+{
+	struct pci_margin_lane *plane = s->private;
+
+	guard(mutex)(&plane->mdev->lock);
+	seq_printf(s, "%d\n", plane->rx);
+	return 0;
+}
+
+static ssize_t margin_lane_receiver_write(struct file *file, const char __user *user_buf,
+				      size_t count, loff_t *ppos)
+{
+	struct seq_file *s = file->private_data;
+	struct pci_margin_lane *plane = s->private;
+	struct pci_margin_dev *mdev = plane->mdev;
+	int ret;
+	u8 rx;
+
+	ret = kstrtou8_from_user(user_buf, count, 0, &rx);
+	if (ret)
+		return ret;
+
+	/* Valid receiver numbers are 0..6 per PCIe r6.0 sec 8.4.4; 7 is reserved */
+	if (rx > LMR_MAX_RX_NUM)
+		return -EINVAL;
+
+	guard(mutex)(&mdev->lock);
+	if (plane->rx == rx)
+		return count;
+
+	if (mdev->enabled) {
+		/* Demargin previous receiver per single-receiver spec rule */
+		ret = pci_lmr_demargin_lane(plane);
+		if (ret)
+			return ret;
+		ret = pci_lmr_cache_rx_info(plane, rx);
+		if (ret)
+			return ret;
+	}
+
+	plane->rx = rx;
+	return count;
+}
+
+static int margin_lane_receiver_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, margin_lane_receiver_show, inode->i_private);
+}
+
+static const struct file_operations margin_lane_receiver_fops = {
+	.open = margin_lane_receiver_open,
+	.read = seq_read,
+	.write = margin_lane_receiver_write,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static int margin_lane_caps_show(struct seq_file *s, void *v)
+{
+	struct pci_margin_lane *plane = s->private;
+	struct pci_margin_dev *mdev = plane->mdev;
+	struct pci_margin_rx_info *info;
+	int ret;
+	u8 val;
+
+	guard(mutex)(&mdev->lock);
+	if (!mdev->enabled)
+		return -EACCES;
+
+	ret = pci_lmr_cache_rx_info(plane, plane->rx);
+	if (ret)
+		return ret;
+
+	info = &plane->rx_info[plane->rx];
+	val = info->caps;
+	seq_printf(s, "Lane %d Rx %d Capabilities: %#02x\n", plane->lane, plane->rx, val);
+	seq_printf(s, "  Margining H/V: %s\n", (val & LMR_CAP_MARGIN_HV) ? "both" : "either");
+	seq_printf(s, "  Margining eV/iV: %s\n",
+		   (val & LMR_CAP_MARGIN_EV_IV) ? "both (close & open)" : "one (close only)");
+	seq_printf(s, "  Left/Right: %s\n", (val & LMR_CAP_IND_LEFT_RIGHT_TIMING) ? "both" : "one");
+	seq_printf(s, "  Up/Down: %s\n", (val & LMR_CAP_IND_UP_DOWN_VOLTAGE) ? "both" : "one");
+	seq_printf(s, "  Error Sampler: %s\n",
+		   (val & LMR_CAP_ERROR_SAMPLER) ? "yes" : "no (main sampler)");
+	seq_printf(s, "  Sample Multiple Receivers: %s\n",
+		   str_yes_no(val & LMR_CAP_SAMPLE_MULTIPLE_RX));
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(margin_lane_caps);
+
+static int margin_lane_steps_show(struct seq_file *s, bool is_voltage)
+{
+	struct pci_margin_lane *plane = s->private;
+	struct pci_margin_dev *mdev = plane->mdev;
+	struct pci_margin_rx_info *info;
+	int ret;
+
+	guard(mutex)(&mdev->lock);
+	if (!mdev->enabled)
+		return -EACCES;
+
+	ret = pci_lmr_cache_rx_info(plane, plane->rx);
+	if (ret)
+		return ret;
+
+	info = &plane->rx_info[plane->rx];
+	seq_printf(s, "%d\n", is_voltage ? info->num_voltage_steps : info->num_timing_steps);
+	return 0;
+}
+
+static int margin_lane_timing_steps_show(struct seq_file *s, void *v)
+{
+	return margin_lane_steps_show(s, false);
+}
+DEFINE_SHOW_ATTRIBUTE(margin_lane_timing_steps);
+
+static int margin_lane_voltage_steps_show(struct seq_file *s, void *v)
+{
+	return margin_lane_steps_show(s, true);
+}
+DEFINE_SHOW_ATTRIBUTE(margin_lane_voltage_steps);
+
+static bool pci_lmr_check_sample_multiple_rx(struct pci_margin_dev *mdev,
+					     struct pci_margin_lane *plane)
+{
+	struct pci_margin_rx_info *info = &plane->rx_info[plane->rx];
+	int i;
+
+	if (info->caps & LMR_CAP_SAMPLE_MULTIPLE_RX)
+		return true;
+
+	for (i = 0; i < mdev->num_lanes; i++) {
+		struct pci_margin_lane *other = &mdev->lanes[i];
+
+		if (i == plane->lane)
+			continue;
+		if (other->rx == plane->rx &&
+		    (other->timing_val != 0 || other->voltage_val != 0))
+			return false;
+	}
+	return true;
+}
+
+static ssize_t margin_lane_step_write(struct file *file, const char __user *user_buf,
+				      size_t count, bool is_voltage)
+{
+	struct seq_file *s = file->private_data;
+	struct pci_margin_lane *plane = s->private;
+	struct pci_margin_dev *mdev = plane->mdev;
+	struct pci_margin_rx_info *info;
+	u8 step, dir, payload;
+	int max_step, val, ret;
+	u16 sts;
+	u8 caps;
+
+	ret = kstrtoint_from_user(user_buf, count, 0, &val);
+	if (ret)
+		return ret;
+
+	max_step = is_voltage ? LMR_MAX_VOLTAGE_STEP : LMR_MAX_TIMING_STEP;
+	if (val > max_step || val < -max_step)
+		return -EINVAL;
+
+	guard(mutex)(&mdev->lock);
+	if (!mdev->enabled)
+		return -EACCES;
+
+	if (val == 0)
+		return pci_lmr_demargin_lane(plane) ?: count;
+
+	ret = pci_lmr_cache_rx_info(plane, plane->rx);
+	if (ret)
+		return ret;
+
+	if (!pci_lmr_check_sample_multiple_rx(mdev, plane))
+		return -EBUSY;
+
+	info = &plane->rx_info[plane->rx];
+	caps = info->caps;
+
+	if (val < 0) {
+		step = -val;
+		if (is_voltage) {
+			/* Symmetric voltage margining requires dir=1 per spec */
+			if (!(caps & LMR_CAP_IND_UP_DOWN_VOLTAGE))
+				return -EINVAL;
+			dir = 0;
+		} else {
+			/* Symmetric timing margining requires dir=1 per spec */
+			if (!(caps & LMR_CAP_IND_LEFT_RIGHT_TIMING))
+				return -EINVAL;
+			dir = LMR_STEP_DIR_DECREASE;
+		}
+	} else {
+		step = val;
+		dir = is_voltage ? 1 : LMR_STEP_DIR_INCREASE;
+	}
+
+	max_step = is_voltage ? info->num_voltage_steps : info->num_timing_steps;
+	if (step > max_step)
+		return -EINVAL;
+
+	if (is_voltage) {
+		payload = FIELD_PREP(LMR_VOLTAGE_DIR_MASK, dir) |
+			  FIELD_PREP(LMR_VOLTAGE_STEP_MASK, step);
+		ret = pci_lmr_run_cmd(mdev->dev, plane->lane, plane->rx,
+				      LMR_TYPE_VOLTAGE, 0, payload, &sts);
+		if (ret)
+			return ret;
+		step = FIELD_GET(LMR_VOLTAGE_STEP_MASK, pci_lmr_sts_payload(sts));
+		plane->voltage_val = (dir == 0) ? -step : step;
+	} else {
+		payload = FIELD_PREP(LMR_TIMING_DIR_MASK, dir) |
+			  FIELD_PREP(LMR_TIMING_STEP_MASK, step);
+		ret = pci_lmr_run_cmd(mdev->dev, plane->lane, plane->rx,
+				      LMR_TYPE_TIMING, 0, payload, &sts);
+		if (ret)
+			return ret;
+		step = FIELD_GET(LMR_TIMING_STEP_MASK, pci_lmr_sts_payload(sts));
+		plane->timing_val = (dir == LMR_STEP_DIR_DECREASE) ? -step : step;
+	}
+
+	return count;
+}
+
+static ssize_t margin_lane_timing_write(struct file *file, const char __user *user_buf,
+					size_t count, loff_t *ppos)
+{
+	return margin_lane_step_write(file, user_buf, count, false);
+}
+
+static int margin_lane_step_show(struct seq_file *s, bool is_voltage)
+{
+	struct pci_margin_lane *plane = s->private;
+
+	guard(mutex)(&plane->mdev->lock);
+	seq_printf(s, "%d\n", is_voltage ? plane->voltage_val : plane->timing_val);
+	return 0;
+}
+
+static int margin_lane_timing_show(struct seq_file *s, void *v)
+{
+	return margin_lane_step_show(s, false);
+}
+
+static int margin_lane_timing_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, margin_lane_timing_show, inode->i_private);
+}
+
+static const struct file_operations margin_lane_timing_fops = {
+	.open = margin_lane_timing_open,
+	.read = seq_read,
+	.write = margin_lane_timing_write,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static ssize_t margin_lane_voltage_write(struct file *file, const char __user *user_buf,
+					 size_t count, loff_t *ppos)
+{
+	return margin_lane_step_write(file, user_buf, count, true);
+}
+
+static int margin_lane_voltage_show(struct seq_file *s, void *v)
+{
+	return margin_lane_step_show(s, true);
+}
+
+static int margin_lane_voltage_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, margin_lane_voltage_show, inode->i_private);
+}
+
+static const struct file_operations margin_lane_voltage_fops = {
+	.open = margin_lane_voltage_open,
+	.read = seq_read,
+	.write = margin_lane_voltage_write,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static void pci_margin_debugfs_init(struct pci_margin_dev *mdev)
+{
+	struct pci_dev *dev = mdev->dev;
+	struct dentry *parent;
+	char dirname[64];
+	int i;
+
+	parent = get_pci_debugfs_root();
+	scnprintf(dirname, sizeof(dirname), "pcie_lmr_%s", dev_name(&dev->dev));
+	mdev->debugfs = debugfs_create_dir(dirname, parent);
+	if (IS_ERR_OR_NULL(mdev->debugfs))
+		return;
+
+	debugfs_create_file("capabilities", 0444, mdev->debugfs, mdev, &margin_caps_fops);
+	debugfs_create_file("port_status", 0444, mdev->debugfs, mdev, &margin_port_status_fops);
+	debugfs_create_file("enable", 0644, mdev->debugfs, mdev, &margin_enable_fops);
+
+	for (i = 0; i < mdev->num_lanes; i++) {
+		struct pci_margin_lane *plane = &mdev->lanes[i];
+		struct dentry *lane_dir;
+		char lane_name[16];
+
+		scnprintf(lane_name, sizeof(lane_name), "lane%d", i);
+		lane_dir = debugfs_create_dir(lane_name, mdev->debugfs);
+		if (IS_ERR_OR_NULL(lane_dir))
+			continue;
+
+		debugfs_create_file("receiver", 0644, lane_dir, plane, &margin_lane_receiver_fops);
+		debugfs_create_file("caps", 0444, lane_dir, plane, &margin_lane_caps_fops);
+		debugfs_create_file("num_timing_steps", 0444, lane_dir, plane,
+				    &margin_lane_timing_steps_fops);
+		debugfs_create_file("num_voltage_steps", 0444, lane_dir, plane,
+				    &margin_lane_voltage_steps_fops);
+		debugfs_create_file("margin_timing", 0644, lane_dir, plane,
+				    &margin_lane_timing_fops);
+		debugfs_create_file("margin_voltage", 0644, lane_dir, plane,
+				    &margin_lane_voltage_fops);
+	}
+}
+
+static void pci_margin_debugfs_remove(struct pci_margin_dev *mdev)
+{
+	debugfs_remove_recursive(mdev->debugfs);
+}
+
+#else
+static inline void pci_margin_debugfs_init(struct pci_margin_dev *mdev) { }
+static inline void pci_margin_debugfs_remove(struct pci_margin_dev *mdev) { }
+#endif
+
+void pci_lmr_init(struct pci_dev *dev)
+{
+	struct pci_margin_dev *mdev;
+	enum pci_bus_speed speed;
+	u16 lmr, lnksta;
+	int num_lanes, i;
+
+	if (!dev || !pci_is_pcie(dev))
+		return;
+
+	speed = pcie_get_speed_cap(dev);
+	if (speed < PCIE_SPEED_16_0GT || speed == PCI_SPEED_UNKNOWN)
+		return;
+
+	lmr = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LMR);
+	if (!lmr) {
+		if (speed >= PCIE_SPEED_64_0GT)
+			pci_warn(dev,
+				 "Missing Lane Margining at Receiver Capability (mandatory for Gen6+)\n");
+		else
+			pci_dbg(dev,
+				"Optional Lane Margining at Receiver Capability not found\n");
+		return;
+	}
+
+	pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &lnksta);
+	num_lanes = FIELD_GET(PCI_EXP_LNKSTA_NLW, lnksta);
+	if (num_lanes == 0 || num_lanes > LMR_MAX_LANES) {
+		pci_warn(dev, "Invalid link width %d for LMR\n", num_lanes);
+		return;
+	}
+
+	dev->lmr_cap = lmr;
+
+	mdev = kzalloc(struct_size(mdev, lanes, num_lanes), GFP_KERNEL);
+	if (!mdev)
+		return;
+
+	mdev->num_lanes = num_lanes;
+	mdev->dev = dev;
+	mdev->cap = lmr;
+	mutex_init(&mdev->lock);
+
+	for (i = 0; i < num_lanes; i++) {
+		mdev->lanes[i].mdev = mdev;
+		mdev->lanes[i].lane = i;
+		mdev->lanes[i].rx = LMR_RX_LOCAL;
+	}
+
+	pci_margin_debugfs_init(mdev);
+
+	dev->lmr = mdev;
+
+	pci_info(dev, "Lane Margining at Receiver (Gen%u) Capability detected\n",
+		 LMR_SPEED_TO_GEN(speed));
+}
+
+void pci_lmr_exit(struct pci_dev *dev)
+{
+	struct pci_margin_dev *mdev = dev->lmr;
+
+	if (!dev || !mdev)
+		return;
+
+	pci_suspend_lmr(dev);
+
+	pci_margin_debugfs_remove(mdev);
+	mutex_destroy(&mdev->lock);
+	kfree(mdev);
+	dev->lmr = NULL;
+}
+
+void pci_suspend_lmr(struct pci_dev *dev)
+{
+	struct pci_margin_dev *mdev = dev->lmr;
+
+	if (!dev || !mdev)
+		return;
+
+	guard(mutex)(&mdev->lock);
+	__pci_suspend_lmr_locked(dev);
+}
+
+void pci_reset_lmr(struct pci_dev *dev)
+{
+	pci_suspend_lmr(dev);
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index dd0abbc63e18..352b95568ebf 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2666,6 +2666,7 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	pci_pasid_init(dev);		/* Process Address Space ID */
 	pci_acs_init(dev);		/* Access Control Services */
 	pci_ptm_init(dev);		/* Precision Time Measurement */
+	pci_lmr_init(dev);		/* Lane Margining at Receiver */
 	pci_aer_init(dev);		/* Advanced Error Reporting */
 	pci_dpc_init(dev);		/* Downstream Port Containment */
 	pci_rcec_init(dev);		/* Root Complex Event Collector */
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index d8bffa21498a..6fba29040e44 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -36,6 +36,7 @@ static void pci_destroy_dev(struct pci_dev *dev)
 
 	pci_doe_sysfs_teardown(dev);
 	pci_npem_remove(dev);
+	pci_lmr_exit(dev);
 
 	/*
 	 * While device is in D0 drop the device from TSM link operations
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 64b308b6e61c..ef1275f4c5b6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -349,6 +349,8 @@ struct rcec_ea;
  *			number resources to allow for hierarchy expansion.
  * @is_pciehp:		PCIe Hot-Plug Capable bridge.
  */
+struct pci_margin_dev;
+
 struct pci_dev {
 	struct list_head bus_list;	/* Node in per-bus list */
 	struct pci_bus	*bus;		/* Bus this device is on */
@@ -528,6 +530,10 @@ struct pci_dev {
 	atomic_t	ptm_enable_cnt;
 	u8		ptm_granularity;
 #endif
+#ifdef CONFIG_PCIE_LMR
+	u16			lmr_cap;	/* Lane Margining Capability */
+	struct pci_margin_dev	*lmr;
+#endif
 #ifdef CONFIG_PCI_MSI
 	void __iomem	*msix_base;
 	raw_spinlock_t	msi_lock;
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index facaa324bd86..90cbe310e62f 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -757,6 +757,7 @@
 #define PCI_EXT_CAP_ID_VF_REBAR 0x24	/* VF Resizable BAR */
 #define PCI_EXT_CAP_ID_DLF	0x25	/* Data Link Feature */
 #define PCI_EXT_CAP_ID_PL_16GT	0x26	/* Physical Layer 16.0 GT/s */
+#define PCI_EXT_CAP_ID_LMR	0x27	/* Lane Margining at Receiver */
 #define PCI_EXT_CAP_ID_NPEM	0x29	/* Native PCIe Enclosure Management */
 #define PCI_EXT_CAP_ID_PL_32GT  0x2A    /* Physical Layer 32.0 GT/s */
 #define PCI_EXT_CAP_ID_DOE	0x2E	/* Data Object Exchange */
@@ -1181,6 +1182,23 @@
 #define  PCI_PL_16GT_LE_CTRL_USP_TX_PRESET_MASK		0x000000F0
 #define  PCI_PL_16GT_LE_CTRL_USP_TX_PRESET_SHIFT	4
 
+/* Lane Margining at Receiver */
+#define PCI_LMR_PORT_CAP		0x04	/* Margining Port Capabilities */
+#define  PCI_LMR_PORT_CAP_USES_SW_READY	0x0001	/* Margining Uses Software Ready */
+#define PCI_LMR_PORT_STS		0x06	/* Margining Port Status */
+#define  PCI_LMR_PORT_STS_MARGIN_READY	0x0001	/* Margining Ready */
+#define  PCI_LMR_PORT_STS_SW_READY	0x0002	/* Margining SW Ready */
+#define PCI_LMR_LANE_CTRL		0x08	/* Margining Lane Control */
+#define  PCI_LMR_LANE_CTRL_RX_NUM	0x0007	/* Receiver Number */
+#define  PCI_LMR_LANE_CTRL_MTYPE	0x0038	/* Margining Type */
+#define  PCI_LMR_LANE_CTRL_USAGE	0x0040	/* Margining Usage Model */
+#define  PCI_LMR_LANE_CTRL_PAYLOAD	0xFF00	/* Margining Payload */
+#define PCI_LMR_LANE_STS		0x0A	/* Margining Lane Status */
+#define  PCI_LMR_LANE_STS_RX_NUM	0x0007	/* Receiver Number */
+#define  PCI_LMR_LANE_STS_MTYPE		0x0038	/* Margining Type */
+#define  PCI_LMR_LANE_STS_USAGE		0x0040	/* Margining Usage Model */
+#define  PCI_LMR_LANE_STS_PAYLOAD	0xFF00	/* Margining Payload */
+
 /* Physical Layer 32.0 GT/s */
 #define PCI_PL_32GT_LE_CTRL	0x20	/* Lane Equalization Control Register */
 
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 8a4b6ddc68df..6990d999388a 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -91,6 +91,7 @@ TARGETS += net/tcp_ao
 TARGETS += nolibc
 TARGETS += pci_endpoint
 TARGETS += pcie_bwctrl
+TARGETS += pcie_lmt
 TARGETS += perf_events
 TARGETS += pidfd
 TARGETS += pid_namespace
diff --git a/tools/testing/selftests/pcie_lmt/Makefile b/tools/testing/selftests/pcie_lmt/Makefile
new file mode 100644
index 000000000000..36ac85937d78
--- /dev/null
+++ b/tools/testing/selftests/pcie_lmt/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+TEST_PROGS = pcie_lmt.sh
+include ../lib.mk
diff --git a/tools/testing/selftests/pcie_lmt/pcie_lmt.sh b/tools/testing/selftests/pcie_lmt/pcie_lmt.sh
new file mode 100755
index 000000000000..22c00c2b8956
--- /dev/null
+++ b/tools/testing/selftests/pcie_lmt/pcie_lmt.sh
@@ -0,0 +1,105 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) 2026 Google LLC
+# Author: Priyank Rathod <rathodpriyank@google.com>
+#
+# Kselftest for PCIe Lane Margining at Receiver (LMR / LMT)
+# Tests the debugfs interface exposed by drivers/pci/pcie/margin.c
+# (/sys/kernel/debug/pci/pcie_lmr_<pci_dev_name>/)
+
+set -e
+
+TESTNAME="pcie_lmt"
+
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+retval=0
+skipmsg="skip all tests:"
+
+if [ $UID != 0 ]; then
+	echo "$skipmsg must be run as root" >&2
+	exit $ksft_skip
+fi
+
+DEBUGFS=$(mount -t debugfs | head -1 | awk '{ print $3 }')
+if [ -z "$DEBUGFS" ]; then
+	if [ -d "/sys/kernel/debug" ]; then
+		DEBUGFS="/sys/kernel/debug"
+	else
+		echo "$skipmsg debugfs is not mounted" >&2
+		exit $ksft_skip
+	fi
+fi
+
+if [ ! -d "$DEBUGFS/pci" ]; then
+	# Allow searching debugfs root or pci directory
+	:
+fi
+
+LMR_DEVS=$(ls -d $DEBUGFS/pci/pcie_lmr_* $DEBUGFS/pcie_lmr_* 2>/dev/null || true)
+if [ -z "$LMR_DEVS" ]; then
+	echo "$skipmsg no PCIe LMR devices found in $DEBUGFS/" >&2
+	exit $ksft_skip
+fi
+
+cleanup_dev()
+{
+	local dev="$1"
+	echo 0 > "$dev/enable" 2>/dev/null || true
+}
+
+echo "$TESTNAME: testing PCIe LMR debugfs entries"
+
+for dev in $LMR_DEVS; do
+	dev_name=$(basename "$dev")
+	echo "$TESTNAME: probing device $dev_name"
+
+	if [ ! -r "$dev/capabilities" ] || [ ! -r "$dev/port_status" ] ||
+	   [ ! -r "$dev/enable" ] || [ ! -w "$dev/enable" ]; then
+		echo "$TESTNAME: $dev_name missing mandatory root attributes"
+		retval=1
+		continue
+	fi
+
+	caps=$(cat "$dev/capabilities")
+	status=$(cat "$dev/port_status")
+	echo "  $dev_name: capabilities read OK"
+	echo "  $dev_name: port_status read OK"
+
+	trap 'cleanup_dev "$dev"' EXIT
+
+	if ! echo 1 > "$dev/enable" 2>/dev/null; then
+		echo "  $dev_name: margining not ready by hardware (skipping active lanes)"
+		continue
+	fi
+
+	echo "  $dev_name: margining enabled OK"
+
+	for lane_dir in $(ls -d "$dev"/lane* 2>/dev/null || true); do
+		lane=$(basename "$lane_dir")
+		echo "  $dev_name: testing $lane"
+
+		# Test setting receiver (Rx 0 is always local receiver)
+		echo 0 > "$lane_dir/receiver"
+		cat "$lane_dir/caps" > /dev/null
+		cat "$lane_dir/num_timing_steps" > /dev/null
+		cat "$lane_dir/num_voltage_steps" > /dev/null
+
+		# Test resetting timing and voltage margin
+		echo 0 > "$lane_dir/margin_timing"
+		echo 0 > "$lane_dir/margin_voltage"
+	done
+
+	echo 0 > "$dev/enable"
+	trap - EXIT
+	echo "  $dev_name: margining disabled OK"
+done
+
+if [ $retval -eq 0 ]; then
+	echo "$TESTNAME [PASS]"
+else
+	echo "$TESTNAME [FAIL]"
+fi
+
+exit $retval

---
base-commit: 0f23d56f17fdfc7db69d51f64c8b91bbab947aa9
change-id: 20260818-pcie-lmt-3044d586aaec

Best regards,
-- 
Priyank Rathod <rathodpriyank@google.com>


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v5] PCI/pcie: Add PCIe Lane Margining at Receiver (LMR) support
  2026-08-20 21:38 [PATCH v5] PCI/pcie: Add PCIe Lane Margining at Receiver (LMR) support Priyank Rathod
@ 2026-08-20 21:51 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-08-20 21:51 UTC (permalink / raw)
  To: Priyank Rathod; +Cc: linux-pci

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Asynchronous pm_runtime_put() is used immediately before device_del(), causing race conditions on device teardown.
- [High] Spurious command failure (-EOPNOTSUPP) in pci_lmr_run_cmd() due to a fast-path NAK check racing with hardware state updates.
- [High] Graceful LMR suspend on reset device causes timeout delays and state corruption.
- [High] Direct modification of ASPM Link Control registers bypasses the PCI core ASPM subsystem.
- [High] Runtime PM violations with unprotected config space reads.
- [Medium] Mixing guard() lock acquisition with goto-based error handling in margin_enable_write().
- [Medium] Incorrect bitmask used when extracting the maximum number of timing steps.
--

--- Patch [1]: [PATCH v5] PCI/pcie: Add PCIe Lane Margining at Receiver (LMR) support ---
commit 067656b09520551d8f1bf1f71cfd2550c4c5e0ba
Author: Priyank Rathod <rathodpriyank@google.com>

PCI/pcie: Add PCIe Lane Margining at Receiver (LMR) support

This commit introduces driver and debugfs support for PCIe Lane Margining at
Receiver (LMR). It exposes per-device interfaces to evaluate high-speed link
margins by controlling receiver selection and margin steps.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5145,8 +5145,10 @@ int __pci_reset_function_locked(struct pci_dev *dev)
>  		method = &pci_reset_fn_methods[m];
>  		pci_dbg(dev, "reset via %s\n", method->name);
>  		rc = method->reset_fn(dev, PCI_RESET_DO_RESET);
> -		if (!rc)
> +		if (!rc) {
> +			pci_reset_lmr(dev);
> +			return 0;
> +		}

[Severity: High]
Does calling pci_reset_lmr() here attempt to communicate with hardware that has
just been reset?

If the hardware state is already cleared by reset, delegating to the graceful
suspend routine might time out when trying to issue demargin commands to
inactive hardware.

> diff --git a/drivers/pci/pcie/margin.c b/drivers/pci/pcie/margin.c
> --- a/drivers/pci/pcie/margin.c
> +++ b/drivers/pci/pcie/margin.c
[ ... ]
> @@ -183,6 +183,23 @@ static struct dentry *get_pci_debugfs_root(void)
>  
>  /*
>   * pci_lmr_disable_aspm() - Temporarily disable ASPM L0s/L1 during active
>   * margining per PCIe Base Spec r6.0 sec 8.4.4, saving original ASPMC bits.
>   */
>  static void pci_lmr_disable_aspm(struct pci_margin_dev *mdev)
>  {
>  	struct pci_dev *dev = mdev->dev;
>  	struct pci_dev *parent = pci_upstream_bridge(dev);
>  	u16 ctl;
>  
>  	if (mdev->aspm_saved)
>  		return;
>  
>  	if (!pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &ctl)) {
>  		mdev->saved_aspm = ctl & PCI_EXP_LNKCTL_ASPMC;
>  		pcie_capability_clear_word(dev, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_ASPMC);
>  	}
>  
>  	if (parent && pci_is_pcie(parent)) {
>  		if (!pcie_capability_read_word(parent, PCI_EXP_LNKCTL, &ctl)) {
>  			mdev->saved_parent_aspm = ctl & PCI_EXP_LNKCTL_ASPMC;
>  			pcie_capability_clear_word(parent, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_ASPMC);
>  		}

[Severity: High]
Does directly modifying the ASPM Link Control registers here bypass the PCI
core ASPM subsystem's refcounting and synchronization?

If multiple child devices on the same bridge enable LMR concurrently, they
might race when manipulating the shared parent upstream bridge config space.
When one child restores its saved state, it might inadvertently re-enable
ASPM while a sibling is still actively margining.

[ ... ]
> @@ -240,6 +240,55 @@ static int pci_lmr_run_cmd(struct pci_dev *dev, int lane, u8 rx, u8 type,
[ ... ]
>  	timeout = jiffies + msecs_to_jiffies(LMR_CMD_TIMEOUT_MS);
>  	while (1) {
>  		if (pci_read_config_word(dev, sts_offset, &sts))
>  			return -EIO;
>  		if (PCI_POSSIBLE_ERROR(sts))
>  			return -ENODEV;
>  
>  		if (FIELD_GET(PCI_LMR_LANE_STS_MTYPE, sts) == type &&
>  		    FIELD_GET(PCI_LMR_LANE_STS_RX_NUM, sts) == rx) {
>  			if (status_val)
>  				*status_val = sts;
>  			return 0;
>  		}
>  
>  		/*
>  		 * Per PCIe Base Spec r6.0 sec 8.4.4, if receiver echoes NO_CMD (0x7)
>  		 * after a command was sent, it indicates command rejection/NAK.
>  		 */
>  		if (FIELD_GET(PCI_LMR_LANE_STS_MTYPE, sts) == LMR_TYPE_NO_CMD &&
>  		    FIELD_GET(PCI_LMR_LANE_STS_RX_NUM, sts) == rx)
>  			return -EOPNOTSUPP;

[Severity: High]
Is there a risk that this fast-path NAK check will read a stale NO_CMD status
from the previous step?

Hardware state machine updates are typically not instantaneous. If the status
register is polled immediately after writing the new command, it may still
contain the NO_CMD state from the prior handshake.

This might cause valid margining commands to spuriously fail with -EOPNOTSUPP
before they have a chance to complete.

[ ... ]
> @@ -279,6 +279,11 @@ static int pci_lmr_cache_rx_info(struct pci_margin_lane *plane, u8 rx)
>  	if (ret)
>  		return ret;
>  	info->caps = pci_lmr_sts_payload(sts);
>  
>  	ret = pci_lmr_run_cmd(plane->mdev->dev, plane->lane, rx,
>  			      LMR_TYPE_REPORT_TIMING_STEPS, 0, 0, &sts);
>  	if (ret)
>  		return ret;
>  	info->num_timing_steps = FIELD_GET(LMR_STEPS_MASK, pci_lmr_sts_payload(sts));

[Severity: Medium]
Should this use LMR_TIMING_STEP_MASK instead of LMR_STEPS_MASK?

The PCIe Base Specification defines the Max Timing Margin payload as 6 bits.
Since LMR_STEPS_MASK uses a 7-bit mask (GENMASK(6,0)), the driver might parse
an incorrectly large step limit if the device sets bit 6.

[ ... ]
> @@ -406,6 +406,17 @@ static void __pci_suspend_lmr_locked(struct pci_dev *dev)
>  	if (pci_read_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, &sts) == 0) {
>  		sts &= ~PCI_LMR_PORT_STS_SW_READY;
>  		pci_write_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, sts);
>  	}
>  	pci_lmr_restore_aspm(mdev);
>  	pm_runtime_put(&dev->dev);
>  	mdev->enabled = false;
>  }

[Severity: High]
Does this asynchronous pm_runtime_put() create a race condition during device
teardown?

In pci_destroy_dev(), device_del() is called immediately after pci_lmr_exit():

drivers/pci/remove.c:pci_destroy_dev() {
    ...
	pci_lmr_exit(dev);
	/* ... */
	pci_tsm_destroy(dev);
	device_del(&dev->dev);
    ...
}

Using pm_runtime_put_sync() instead might ensure the device reaches the proper
power state and that pending idle/suspend work is completed before the device
structures are removed.

[ ... ]
> @@ -428,6 +428,74 @@ static ssize_t margin_enable_write(struct file *file, const char __user *user_buf,
>  			       size_t count, loff_t *ppos)
>  {
[ ... ]
>  	guard(mutex)(&mdev->lock);
>  
>  	if (mdev->enabled == enable)
>  		return count;
>  
>  	if (!enable) {
>  		__pci_suspend_lmr_locked(dev);
>  		return count;
>  	}
>  
>  	/* PCIe r6.0 sec 8.4.4: LMR is physically undefined below 16.0 GT/s (Gen4) */
>  	pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &lnksta);
>  	if ((lnksta & PCI_EXP_LNKSTA_CLS) < PCI_EXP_LNKSTA_CLS_16_0GB)
>  		return -EOPNOTSUPP;
>  
>  	if (pci_read_config_word(dev, mdev->cap + PCI_LMR_PORT_CAP, &cap))
>  		return -EIO;
>  
>  	/* Prevent D3hot runtime PM transition during active margining */
>  	ret = pm_runtime_resume_and_get(&dev->dev);

[Severity: High]
Can these configuration space reads return invalid data if the device is
currently in the D3hot power state?

Reading PCI_EXP_LNKSTA and PCI_LMR_PORT_CAP before calling
pm_runtime_resume_and_get() could return 0xFFFF or 0. This might cause the
link speed check to fail incorrectly, or cause the driver to erroneously see
USES_SW_READY as true.

>  	if (ret < 0)
>  		return ret;
[ ... ]
>  	if (cap & PCI_LMR_PORT_CAP_USES_SW_READY) {
>  		if (pci_read_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, &sts)) {
>  			ret = -EIO;
>  			goto err_enable;
>  		}
>  		sts |= PCI_LMR_PORT_STS_SW_READY;
>  		pci_write_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, sts);
>  	}
>  
>  	timeout = jiffies + msecs_to_jiffies(LMR_ENABLE_TIMEOUT_MS);
>  	while (1) {
>  		if (pci_read_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, &sts)) {
>  			ret = -EIO;
>  			goto err_enable;
>  		}
[ ... ]
>  err_enable:
>  	if (cap & PCI_LMR_PORT_CAP_USES_SW_READY) {
>  		if (pci_read_config_word(dev, mdev->cap + PCI_LMR_PORT_STS, &sts) == 0) {

[Severity: Medium]
Is it safe to mix goto-based error handling with scope-based lock acquisition
in the same function?

While the label is at the end of the scope here, mixing the goto pattern
with guard(mutex)(&mdev->lock) makes ownership semantics less clear and might
increase the risk of resource leaks or locking issues during future updates.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-pcie-lmt-v5-1-943b3b0e18bf@google.com?part=1

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-20 21:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20 21:38 [PATCH v5] PCI/pcie: Add PCIe Lane Margining at Receiver (LMR) support Priyank Rathod
2026-08-20 21:51 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox