Re: [PATCH v4 12/23] cxl: Add helper function that calculate QoS values for PCI path

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Wed, 19 Apr 2023 13:22:13 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Calculate the link bandwidth and latency for the PCIe path from the device
> to the CXL Host Bridge. This does not include the CDAT data from the device
> or the switch(es) in the path.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Same comment on _qos naming and one trivial comment inline.


> ---
> v4:
> - 0-day fix, remove unused var. Fix checking < 0 for unsigned var.
> - Rework port hierachy walk to calculate the latencies correctly
> ---
>  drivers/cxl/core/port.c |   83 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/cxl/cxl.h       |    2 +
>  2 files changed, 85 insertions(+)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 770b540d5325..8da437e038b9 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -2002,6 +2002,89 @@ int cxl_port_get_switch_qos(struct cxl_port *port, u64 *rd_bw, u64 *rd_lat,
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_port_get_switch_qos, CXL);
>  
> +/**
> + * cxl_port_get_downstream_qos - retrieve QoS data for PCIE downstream path
> + * @port: endpoint cxl_port
> + * @bandwidth: writeback value for min bandwidth
> + * @latency: writeback value for total latency
> + *
> + * Return: Errno on failure, 0 on success.
> + */
> +int cxl_port_get_downstream_qos(struct cxl_port *port, u64 *bandwidth,
> +				u64 *latency)
> +{
> +	u64 min_bw = ULONG_MAX;
> +	struct pci_dev *pdev;
> +	struct cxl_port *p;
> +	struct device *dev;
> +	u64 total_lat = 0;
> +	long lat;
> +
> +	*bandwidth = 0;
> +	*latency = 0;
> +
> +	/* Grab the device that is the PCI device for CXL memdev */
> +	dev = port->uport->parent;
> +	/* Skip if it's not PCI, most likely a cxl_test device */
> +	if (!dev_is_pci(dev))
> +		return 0;
> +
> +	pdev = to_pci_dev(dev);
> +	min_bw = pcie_bandwidth_available(pdev, NULL, NULL, NULL);
> +	if (min_bw == 0)
> +		return -ENXIO;
> +
> +	/* convert to MB/s from Mb/s */
> +	min_bw >>= 3;

/ BITS_PER_BYTE; (well MEGABITS_PER_MEGABYTE but still better than >>= 3;)

> +
> +	/*
> +	 * Walk the cxl_port hierachy to retrieve the link latencies for
> +	 * each of the PCIe segments. The loop will obtain the link latency
> +	 * via each of the switch downstream port.
> +	 */
> +	p = port;
> +	do {
> +		struct cxl_dport *dport = p->parent_dport;
> +		struct device *dport_dev, *uport_dev;
> +		struct pci_dev *dport_pdev;
> +
> +		if (!dport)
> +			break;
> +
> +		dport_dev = dport->dport;
> +		if (!dev_is_pci(dport_dev))
> +			break;
> +
> +		p = dport->port;
> +		uport_dev = p->uport;
> +		if (!dev_is_pci(uport_dev))
> +			break;
> +
> +		dport_pdev = to_pci_dev(dport_dev);
> +		pdev = to_pci_dev(uport_dev);
> +		lat = cxl_pci_get_latency(dport_pdev);
> +		if (lat < 0)
> +			return lat;
> +
> +		total_lat += lat;
> +	} while (1);
> +
> +	/*
> +	 * pdev would be either the cxl device if there are no switches, or the
> +	 * upstream port of the last switch.
> +	 */
> +	lat = cxl_pci_get_latency(pdev);
> +	if (lat < 0)
> +		return lat;
> +
> +	total_lat += lat;
> +	*bandwidth = min_bw;
> +	*latency = total_lat;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_port_get_downstream_qos, CXL);
> +
>  /* for user tooling to ensure port disable work has completed */
>  static ssize_t flush_store(struct bus_type *bus, const char *buf, size_t count)
>  {
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 76ccc815134f..6a6387a545db 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -811,6 +811,8 @@ struct qtg_dsm_output *cxl_acpi_evaluate_qtg_dsm(acpi_handle handle,
>  acpi_handle cxl_acpi_get_rootdev_handle(struct device *dev);
>  int cxl_port_get_switch_qos(struct cxl_port *port, u64 *rd_bw, u64 *rd_lat,
>  			    u64 *wr_bw, u64 *wr_lat);
> +int cxl_port_get_downstream_qos(struct cxl_port *port, u64 *bandwidth,
> +				u64 *latency);
>  
>  /*
>   * Unit test builds overrides this to __weak, find the 'strong' version
> 
>