[PATCH] PCI: Add pcie_get_link_endpoints() helper

[PATCH] PCI: Add pcie_get_link_endpoints() helper

[Priyank Rathod]

In PCIe topologies, physical links are point-to-point connections
between an Upstream Component (Downstream Port, such as a Root Port
or Switch Downstream Port) and a Downstream Component (Upstream Port,
such as an Endpoint or Switch Upstream Port), per PCIe Base Specification
r7.0/r6.0 sec 1.3.1.

Currently, various drivers across drivers/pci/ independently infer the
two ends of a PCIe link using varied ad-hoc methods:
  - ASPM (drivers/pci/pcie/aspm.c): pcie_aspm_get_link(),
    alloc_pcie_link_state(), and pci_configure_ltr() resolve parent
    bridges and subordinate function 0.
  - Precision Time Measurement (drivers/pci/pcie/ptm.c):
    pci_upstream_ptm() traverses pci_upstream_bridge() to validate
    upstream link partner capabilities.
  - PCIe Link Retrain (drivers/pci/pci.c): pcie_retrain_link() and
    pcie_update_link_speed() coordinate link retraining from the
    Downstream Port across the subordinate bus.
  - AER/DPC Recovery (drivers/pci/pcie/err.c): pcie_do_recovery()
    identifies the parent bridge via pci_upstream_bridge() to trigger
    secondary bus reset and broadcast driver recovery callbacks.
  - Lane Margining at Receiver (LMR): margin_enable_write() requires
    both link endpoints to establish hierarchical locking and runtime PM
    pinning.

Ad-hoc subordinate bus iteration risks races with concurrent device
removal or hotplug if pci_bus_sem is omitted.

Introduce pcie_get_link_endpoints() and pcie_put_link_endpoints() in the
PCI core to provide a standardized, symmetric, and race-safe helper:
  - For Endpoints: resolves the parent Downstream Port via
    pci_upstream_bridge(pdev).
  - For Downstream Ports: safely inspects the subordinate bus under
    down_read(&pci_bus_sem) and acquires a reference via pci_dev_get()
    on the child device.

Callers release the acquired reference using pcie_put_link_endpoints().

Signed-off-by: Priyank Rathod <rathodpriyank@google.com>
---
Hi Bjorn, Ilpo, and PCI maintainers,

During the review of the PCIe Lane Margining at Receiver (LMR) patch series
(v7: https://lore.kernel.org/linux-pci/20260828-pcie-lmt-v7-1-6012e9e0940a@google.com/),
Ilpo Järvinen pointed out that feature drivers (such as LMR) resolving link
partners duplicate link traversal logic that is already present in drivers such
as ASPM:

  "This feels like duplicating similar functionality with the aspm driver
   that also wants to infer ends of the link when giving a pci_dev in.
   The aspm driver currently does that within, but it kind of duplicating
   pci_bus. It would be nice to avoid the duplication and have something
   similar for this in PCI core."

In PCIe topologies, physical links are point-to-point interconnects
between an Upstream Component (Downstream Port, such as a Root Port or Switch
Downstream Port) and a Downstream Component (Upstream Port, such as an Endpoint
or Switch Upstream Port), per PCIe Base Specification Revision 7.0 / 6.0
Section 1.3.1.

Several subsystems across drivers/pci/ independently infer and coordinate both
ends of a PCIe link:
  1. ASPM (drivers/pci/pcie/aspm.c): pcie_aspm_get_link(),
     alloc_pcie_link_state(), and pci_configure_ltr() resolve parent bridges
     and subordinate function 0 to configure ASPMC and L1SS.
  2. Precision Time Measurement (drivers/pci/pcie/ptm.c): pci_upstream_ptm()
     traverses pci_upstream_bridge() to validate upstream link partner
     capabilities.
  3. PCIe Link Retrain (drivers/pci/pci.c): pcie_retrain_link() and
     pcie_update_link_speed() coordinate link retraining from the Downstream
     Port across the subordinate bus.
  4. AER & DPC Recovery (drivers/pci/pcie/err.c): pcie_do_recovery() identifies
     the parent bridge via pci_upstream_bridge() to trigger secondary bus
     resets and broadcast driver error callbacks.
  5. Lane Margining at Receiver (LMR): margin_enable_write() requires both
     link endpoints to establish hierarchical locking (pci_dev_lock) and
     runtime PM pinning.

Currently, these drivers independently implement ad-hoc traversals via
pci_upstream_bridge() or subordinate bus device iteration. Ad-hoc subordinate
bus iteration is error-prone and risks races with concurrent hot-unplug or
device removal if pci_bus_sem is omitted.

This patch introduces pcie_get_link_endpoints() and pcie_put_link_endpoints()
in the PCI core (drivers/pci/pci.c and include/linux/pci.h) as a standalone
helper:
  - For Endpoints: resolves the parent Downstream Port via
    pci_upstream_bridge(pdev).
  - For Downstream Ports: safely inspects the subordinate bus under
    down_read(&pci_bus_sem) and acquires a reference via pci_dev_get() on the
    child device.

Callers release the acquired reference with pcie_put_link_endpoints().

Validation:
  - Compiled clean on x86_64 defconfig (0 warnings, 0 errors).
  - Multi-architecture build validated for ARM64 and x86_64 targets.
  - Passed checkpatch.pl (0 warnings, 0 errors).
  - Passed Sashiko pre-commit test runner (ID: 46b5f8a3da1ca0056407b3db6c82b001264f86b7).
---
 drivers/pci/pci.c   | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |  6 ++++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 77b17b13ee61..7bdfe7e5ab3a 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4618,6 +4618,56 @@ int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
 	return rc;
 }
 
+/**
+ * pcie_get_link_endpoints - Identify Upstream and Downstream ends of a PCIe link
+ * @pdev: Any PCIe device on the link (Downstream Port or Endpoint)
+ * @downstream_port: Output pointer to Downstream Port (Upstream Component)
+ * @upstream_port: Output pointer to Upstream Port (Downstream Component)
+ *
+ * Identifies both ends of a point-to-point PCIe link. Increments reference count
+ * on @upstream_port if dynamically discovered on a downstream port. Callers must
+ * release with pcie_put_link_endpoints().
+ *
+ * Return: 0 on success, or -EINVAL if @pdev is NULL or not PCIe.
+ */
+int pcie_get_link_endpoints(struct pci_dev *pdev,
+			    struct pci_dev **downstream_port,
+			    struct pci_dev **upstream_port)
+{
+	if (!pdev || !pci_is_pcie(pdev))
+		return -EINVAL;
+
+	if (pcie_downstream_port(pdev)) {
+		*downstream_port = pdev;
+		down_read(&pci_bus_sem);
+		*upstream_port = pdev->subordinate ?
+			pci_dev_get(list_first_entry_or_null(&pdev->subordinate->devices,
+							     struct pci_dev, bus_list)) : NULL;
+		up_read(&pci_bus_sem);
+	} else {
+		*downstream_port = pci_upstream_bridge(pdev);
+		*upstream_port = pdev;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pcie_get_link_endpoints);
+
+/**
+ * pcie_put_link_endpoints - Release references acquired by pcie_get_link_endpoints
+ * @pdev: Device passed to pcie_get_link_endpoints()
+ * @downstream_port: Downstream Port pointer
+ * @upstream_port: Upstream Port pointer
+ */
+void pcie_put_link_endpoints(struct pci_dev *pdev,
+			     struct pci_dev *downstream_port,
+			     struct pci_dev *upstream_port)
+{
+	if (pdev && pcie_downstream_port(pdev) && upstream_port)
+		pci_dev_put(upstream_port);
+}
+EXPORT_SYMBOL_GPL(pcie_put_link_endpoints);
+
 /**
  * pcie_wait_for_link_delay - Wait until link is active or inactive
  * @pdev: Bridge device
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 64b308b6e61c..d30bf056a7ce 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1264,6 +1264,12 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
 u8 pci_swizzle_interrupt_pin(const struct pci_dev *dev, u8 pin);
 int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge);
 u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp);
+int pcie_get_link_endpoints(struct pci_dev *pdev,
+			    struct pci_dev **downstream_port,
+			    struct pci_dev **upstream_port);
+void pcie_put_link_endpoints(struct pci_dev *pdev,
+			     struct pci_dev *downstream_port,
+			     struct pci_dev *upstream_port);
 struct pci_dev *pci_dev_get(struct pci_dev *dev);
 void pci_dev_put(struct pci_dev *dev);
 DEFINE_FREE(pci_dev_put, struct pci_dev *, if (_T) pci_dev_put(_T))

---
base-commit: 0f23d56f17fdfc7db69d51f64c8b91bbab947aa9
change-id: 20260831-pcie-link-endpoints-978e100d5d06

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

Re: [PATCH] PCI: Add pcie_get_link_endpoints() helper

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] A race condition between `pcie_get_link_endpoints()` and `pci_remove_bus_device()` can lead to a Use-After-Free when accessing `pdev->subordinate`.
- [High] `pcie_get_link_endpoints()` returns success (0) even when it fails to find both ends of the PCIe link, leaving output pointers as NULL.
- [Medium] `pcie_get_link_endpoints()` does not guarantee returning Function 0 for multi-function devices, and resolves Virtual Functions improperly.
--

commit f96df0f9857c01cf8b72af2e93119c328134d9e0
Author: Priyank Rathod <rathodpriyank@google.com>

PCI: Add pcie_get_link_endpoints() helper

In PCIe topologies, physical links are point-to-point connections
between an upstream component and a downstream component. This patch
introduces pcie_get_link_endpoints() in the PCI core to provide a
standardized, symmetric helper to identify both ends of a PCIe link.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 77b17b13ee615..7bdfe7e5ab3a4 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c

[ ... ]

> @@ -4618,6 +4618,56 @@ int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
>  	return rc;
>  }
>  
> +/**
> + * pcie_get_link_endpoints - Identify Upstream and Downstream ends of a PCIe link
> + * @pdev: Any PCIe device on the link (Downstream Port or Endpoint)
> + * @downstream_port: Output pointer to Downstream Port (Upstream Component)
> + * @upstream_port: Output pointer to Upstream Port (Downstream Component)
> + *
> + * Identifies both ends of a point-to-point PCIe link. Increments reference count
> + * on @upstream_port if dynamically discovered on a downstream port. Callers must
> + * release with pcie_put_link_endpoints().
> + *
> + * Return: 0 on success, or -EINVAL if @pdev is NULL or not PCIe.
> + */
> +int pcie_get_link_endpoints(struct pci_dev *pdev,
> +			    struct pci_dev **downstream_port,
> +			    struct pci_dev **upstream_port)
> +{
> +	if (!pdev || !pci_is_pcie(pdev))
> +		return -EINVAL;
> +
> +	if (pcie_downstream_port(pdev)) {
> +		*downstream_port = pdev;
> +		down_read(&pci_bus_sem);
> +		*upstream_port = pdev->subordinate ?
> +			pci_dev_get(list_first_entry_or_null(&pdev->subordinate->devices,
> +							     struct pci_dev, bus_list)) : NULL;

[Severity: High]
Can this result in a use-after-free regression if a concurrent hot-unplug
occurs?

In pci_remove_bus_device(), the subordinate bus is removed by calling
pci_remove_bus(bus). This takes pci_bus_sem for write, removes the bus
from lists, drops the lock, and calls device_unregister(&bus->dev) which
frees the struct pci_bus memory.

Crucially, dev->subordinate is cleared only after pci_remove_bus() returns,
completely outside of pci_bus_sem:

drivers/pci/remove.c:pci_remove_bus_device() {
    ...
    pci_remove_bus(bus);
    dev->subordinate = NULL;
    ...
}

If pcie_get_link_endpoints() acquires pci_bus_sem in this narrow window,
will it observe a dangling pdev->subordinate pointer and cause a
use-after-free read when dereferencing &pdev->subordinate->devices?

[Severity: Medium]
Will this grab the wrong function if function 0 of a multi-function device
has been removed?

The commit message mentions standardizing aspm logic, which explicitly
resolves subordinate function 0. By using list_first_entry_or_null() here,
if function 0 is hot-removed, function 1 or later becomes the first entry.

Since the PCIe base specification requires link capabilities and control
registers to be implemented in function 0 (while others may hardwire them
to 0), could returning a non-zero function cause feature drivers to fail
when configuring the link?

> +		up_read(&pci_bus_sem);
> +	} else {
> +		*downstream_port = pci_upstream_bridge(pdev);
> +		*upstream_port = pdev;

[Severity: Medium]
Does this improperly set the link endpoint if pdev is a virtual function?

If this is called on a virtual function, it returns the device directly
without resolving its physical function. Like non-zero functions, virtual
functions may hardwire link capabilities to 0, which could break drivers
relying on this helper to read valid link states.

> +	}
> +
> +	return 0;

[Severity: High]
Will callers encounter a NULL pointer dereference regression when this
returns success but fails to resolve the endpoints?

If this helper is called on an empty downstream port,
list_first_entry_or_null() returns NULL, leaving *upstream_port = NULL.

Similarly, if called on a root complex integrated endpoint,
pci_upstream_bridge(pdev) returns NULL, leaving *downstream_port = NULL.

In both scenarios, the function unconditionally returns 0 (success). Since
callers expect valid pointers on success, won't they blindly dereference
these NULL pointers? Should this return an error code like -ENODEV if
either endpoint cannot be resolved?

> +}
> +EXPORT_SYMBOL_GPL(pcie_get_link_endpoints);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-pcie-link-endpoints-v1-1-32c2fd893e9e@google.com?part=1

Re: [PATCH] PCI: Add pcie_get_link_endpoints() helper

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 8320 bytes --]

On Mon, 31 Aug 2026, Priyank Rathod wrote:

> In PCIe topologies, physical links are point-to-point connections
> between an Upstream Component (Downstream Port, such as a Root Port
> or Switch Downstream Port) and a Downstream Component (Upstream Port,
> such as an Endpoint or Switch Upstream Port), per PCIe Base Specification
> r7.0/r6.0 sec 1.3.1.
> 
> Currently, various drivers across drivers/pci/ independently infer the
> two ends of a PCIe link using varied ad-hoc methods:
>   - ASPM (drivers/pci/pcie/aspm.c): pcie_aspm_get_link(),
>     alloc_pcie_link_state(), and pci_configure_ltr() resolve parent
>     bridges and subordinate function 0.

This is not the full picture. The aspm driver has to write the same config 
to all functions (except L1SS that is only in func 0).

>   - Precision Time Measurement (drivers/pci/pcie/ptm.c):
>     pci_upstream_ptm() traverses pci_upstream_bridge() to validate
>     upstream link partner capabilities.

This looks something entirely different, and it's pure get (a certain) 
device upstream, not the get-my-link-partner query.

>   - PCIe Link Retrain (drivers/pci/pci.c): pcie_retrain_link() and
>     pcie_update_link_speed() coordinate link retraining from the
>     Downstream Port across the subordinate bus.

I don't entirely agree with this assessment. pcie_retrain_link() is given 
a downstream port, mere looking into ->subordinate doesn't constitute as a 
need for having access to the struct of the other end of the link.

>   - AER/DPC Recovery (drivers/pci/pcie/err.c): pcie_do_recovery()
>     identifies the parent bridge via pci_upstream_bridge() to trigger
>     secondary bus reset and broadcast driver recovery callbacks.

This seems to have some validity, though it must cover RCiEPs.

>   - Lane Margining at Receiver (LMR): margin_enable_write() requires
>     both link endpoints to establish hierarchical locking and runtime PM
>     pinning.
> 
> Ad-hoc subordinate bus iteration risks races with concurrent device
> removal or hotplug if pci_bus_sem is omitted.
> 
> Introduce pcie_get_link_endpoints() and pcie_put_link_endpoints() in the
> PCI core to provide a standardized, symmetric, and race-safe helper:
>   - For Endpoints: resolves the parent Downstream Port via
>     pci_upstream_bridge(pdev).
>   - For Downstream Ports: safely inspects the subordinate bus under
>     down_read(&pci_bus_sem) and acquires a reference via pci_dev_get()
>     on the child device.
> 
> Callers release the acquired reference using pcie_put_link_endpoints().
> 
> Signed-off-by: Priyank Rathod <rathodpriyank@google.com>
> ---
> Hi Bjorn, Ilpo, and PCI maintainers,
> 
> During the review of the PCIe Lane Margining at Receiver (LMR) patch series
> (v7: https://lore.kernel.org/linux-pci/20260828-pcie-lmt-v7-1-6012e9e0940a@google.com/),
> Ilpo Järvinen pointed out that feature drivers (such as LMR) resolving link
> partners duplicate link traversal logic that is already present in drivers such
> as ASPM:
> 
>   "This feels like duplicating similar functionality with the aspm driver
>    that also wants to infer ends of the link when giving a pci_dev in.
>    The aspm driver currently does that within, but it kind of duplicating
>    pci_bus. It would be nice to avoid the duplication and have something
>    similar for this in PCI core."
> 
> In PCIe topologies, physical links are point-to-point interconnects
> between an Upstream Component (Downstream Port, such as a Root Port or Switch
> Downstream Port) and a Downstream Component (Upstream Port, such as an Endpoint
> or Switch Upstream Port), per PCIe Base Specification Revision 7.0 / 6.0
> Section 1.3.1.
> 
> Several subsystems across drivers/pci/ independently infer and coordinate both
> ends of a PCIe link:
>   1. ASPM (drivers/pci/pcie/aspm.c): pcie_aspm_get_link(),
>      alloc_pcie_link_state(), and pci_configure_ltr() resolve parent bridges
>      and subordinate function 0 to configure ASPMC and L1SS.
>   2. Precision Time Measurement (drivers/pci/pcie/ptm.c): pci_upstream_ptm()
>      traverses pci_upstream_bridge() to validate upstream link partner
>      capabilities.
>   3. PCIe Link Retrain (drivers/pci/pci.c): pcie_retrain_link() and
>      pcie_update_link_speed() coordinate link retraining from the Downstream
>      Port across the subordinate bus.
>   4. AER & DPC Recovery (drivers/pci/pcie/err.c): pcie_do_recovery() identifies
>      the parent bridge via pci_upstream_bridge() to trigger secondary bus
>      resets and broadcast driver error callbacks.
>   5. Lane Margining at Receiver (LMR): margin_enable_write() requires both
>      link endpoints to establish hierarchical locking (pci_dev_lock) and
>      runtime PM pinning.
> 
> Currently, these drivers independently implement ad-hoc traversals via
> pci_upstream_bridge() or subordinate bus device iteration. Ad-hoc subordinate
> bus iteration is error-prone and risks races with concurrent hot-unplug or
> device removal if pci_bus_sem is omitted.
> 
> This patch introduces pcie_get_link_endpoints() and pcie_put_link_endpoints()
> in the PCI core (drivers/pci/pci.c and include/linux/pci.h) as a standalone
> helper:
>   - For Endpoints: resolves the parent Downstream Port via
>     pci_upstream_bridge(pdev).
>   - For Downstream Ports: safely inspects the subordinate bus under
>     down_read(&pci_bus_sem) and acquires a reference via pci_dev_get() on the
>     child device.
> 
> Callers release the acquired reference with pcie_put_link_endpoints().
> 
> Validation:
>   - Compiled clean on x86_64 defconfig (0 warnings, 0 errors).
>   - Multi-architecture build validated for ARM64 and x86_64 targets.
>   - Passed checkpatch.pl (0 warnings, 0 errors).
>   - Passed Sashiko pre-commit test runner (ID: 46b5f8a3da1ca0056407b3db6c82b001264f86b7).
> ---
>  drivers/pci/pci.c   | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h |  6 ++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 77b17b13ee61..7bdfe7e5ab3a 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4618,6 +4618,56 @@ int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
>  	return rc;
>  }
>  
> +/**
> + * pcie_get_link_endpoints - Identify Upstream and Downstream ends of a PCIe link
> + * @pdev: Any PCIe device on the link (Downstream Port or Endpoint)
> + * @downstream_port: Output pointer to Downstream Port (Upstream Component)
> + * @upstream_port: Output pointer to Upstream Port (Downstream Component)
> + *
> + * Identifies both ends of a point-to-point PCIe link. Increments reference count
> + * on @upstream_port if dynamically discovered on a downstream port. Callers must
> + * release with pcie_put_link_endpoints().
> + *
> + * Return: 0 on success, or -EINVAL if @pdev is NULL or not PCIe.
> + */
> +int pcie_get_link_endpoints(struct pci_dev *pdev,
> +			    struct pci_dev **downstream_port,
> +			    struct pci_dev **upstream_port)
> +{
> +	if (!pdev || !pci_is_pcie(pdev))
> +		return -EINVAL;
> +
> +	if (pcie_downstream_port(pdev)) {
> +		*downstream_port = pdev;
> +		down_read(&pci_bus_sem);
> +		*upstream_port = pdev->subordinate ?
> +			pci_dev_get(list_first_entry_or_null(&pdev->subordinate->devices,
> +							     struct pci_dev, bus_list)) : NULL;

Too much is crammed into one statement.

> +		up_read(&pci_bus_sem);
> +	} else {
> +		*downstream_port = pci_upstream_bridge(pdev);
> +		*upstream_port = pdev;

Why complicate things with the unbalance?

> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(pcie_get_link_endpoints);
> +
> +/**
> + * pcie_put_link_endpoints - Release references acquired by pcie_get_link_endpoints
> + * @pdev: Device passed to pcie_get_link_endpoints()
> + * @downstream_port: Downstream Port pointer
> + * @upstream_port: Upstream Port pointer
> + */
> +void pcie_put_link_endpoints(struct pci_dev *pdev,
> +			     struct pci_dev *downstream_port,
> +			     struct pci_dev *upstream_port)
> +{
> +	if (pdev && pcie_downstream_port(pdev) && upstream_port)
> +		pci_dev_put(upstream_port);
> +}
> +EXPORT_SYMBOL_GPL(pcie_put_link_endpoints);

--
 i.