Re: [PATCH 2/3] cxl: Account for link width in latency calculation

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, Oct 09, 2026 at 01:54:51PM +0900, Koichiro Den wrote:
> cxl_pci_get_latency() derives bandwidth from the per-lane speed returned
> by pcie_link_speed_mbps(), ignoring the negotiated link width. This
> overestimates the FlitLatency contribution for multi-lane links.
> Sections 2.11.3 and 2.11.4 of the Intel CXL Memory Device Software Guide
> describe the link bandwidth as the product of the negotiated speed and
> width.
> 
> Replace pcie_link_speed_mbps() with pcie_link_bandwidth_mbps() and
> migrate both CXL callers (the only users of the API). The new helper
> obtains speed and width from a single Link Status read. The CXL
> bandwidth calculation no longer needs a separate width read and
> multiplication.
> 
> While at it, make error handling more robust by converting PCI config
> read errors to negative errno values and checking for zero bandwidth
> before calculating latency.
> 
> Fixes: 4d07a05397c8 ("cxl: Calculate and store PCI link latency for the downstream ports")
> Cc: stable@vger.kernel.org
> Signed-off-by: Koichiro Den <den@valinux.co.jp>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>	# pci/pci.c

> ---
>  drivers/cxl/core/pci.c | 22 ++++++++--------------
>  drivers/pci/pci.c      | 23 ++++++++++++++++++-----
>  include/linux/pci.h    |  2 +-
>  3 files changed, 27 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 43b9b7afff29..8b52621ffea1 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -660,8 +660,8 @@ long cxl_pci_get_latency(struct pci_dev *pdev)
>  {
>  	long bw;
>  
> -	bw = pcie_link_speed_mbps(pdev);
> -	if (bw < 0)
> +	bw = pcie_link_bandwidth_mbps(pdev);
> +	if (bw <= 0)
>  		return 0;
>  	bw /= BITS_PER_BYTE;
>  
> @@ -765,18 +765,12 @@ EXPORT_SYMBOL_NS_GPL(cxl_pci_setup_regs, "CXL");
>  
>  int cxl_pci_get_bandwidth(struct pci_dev *pdev, struct access_coordinate *c)
>  {
> -	int speed, bw;
> -	u16 lnksta;
> -	u32 width;
> -
> -	speed = pcie_link_speed_mbps(pdev);
> -	if (speed < 0)
> -		return speed;
> -	speed /= BITS_PER_BYTE;
> -
> -	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
> -	width = FIELD_GET(PCI_EXP_LNKSTA_NLW, lnksta);
> -	bw = speed * width;
> +	int bw;
> +
> +	bw = pcie_link_bandwidth_mbps(pdev);
> +	if (bw < 0)
> +		return bw;
> +	bw /= BITS_PER_BYTE;
>  
>  	for (int i = 0; i < ACCESS_COORDINATE_MAX; i++) {
>  		c[i].read_bandwidth = bw;
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b2879a6be5f8..aa3d180de56e 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5980,18 +5980,31 @@ static enum pci_bus_speed to_pcie_link_speed(u16 lnksta)
>  	return pcie_link_speed[FIELD_GET(PCI_EXP_LNKSTA_CLS, lnksta)];
>  }
>  
> -int pcie_link_speed_mbps(struct pci_dev *pdev)
> +/**
> + * pcie_link_bandwidth_mbps - get the current bandwidth of a single PCIe link
> + * @pdev: PCI device to query
> + *
> + * The bandwidth includes all negotiated lanes and does not account for
> + * encoding overhead. A negotiated width of zero gives zero bandwidth.
> + *
> + * Return: Bandwidth in Mb/s, or a negative errno on failure.
> + */
> +int pcie_link_bandwidth_mbps(struct pci_dev *pdev)
>  {
>  	u16 lnksta;
> -	int err;
> +	int speed, err;
>  
>  	err = pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
>  	if (err)
> -		return err;
> +		return pcibios_err_to_errno(err);
> +
> +	speed = pcie_dev_speed_mbps(to_pcie_link_speed(lnksta));
> +	if (speed < 0)
> +		return speed;
>  
> -	return pcie_dev_speed_mbps(to_pcie_link_speed(lnksta));
> +	return speed * FIELD_GET(PCI_EXP_LNKSTA_NLW, lnksta);
>  }
> -EXPORT_SYMBOL(pcie_link_speed_mbps);
> +EXPORT_SYMBOL(pcie_link_bandwidth_mbps);
>  
>  /**
>   * pcie_bandwidth_available - determine minimum link settings of a PCIe
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d31a8d107b1e..bbfb0b98ff46 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1477,7 +1477,7 @@ int pcie_set_mps(struct pci_dev *dev, int mps);
>  u32 pcie_bandwidth_available(struct pci_dev *dev, struct pci_dev **limiting_dev,
>  			     enum pci_bus_speed *speed,
>  			     enum pcie_link_width *width);
> -int pcie_link_speed_mbps(struct pci_dev *pdev);
> +int pcie_link_bandwidth_mbps(struct pci_dev *pdev);
>  void pcie_print_link_status(struct pci_dev *dev);
>  int pcie_reset_flr(struct pci_dev *dev, bool probe);
>  int pcie_flr(struct pci_dev *dev);
> -- 
> 2.51.0
>