diff mbox series

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

Message ID 168193573330.1178687.2727689734216767954.stgit@djiang5-mobl3
State New
Headers show
Series cxl: Add support for QTG ID retrieval for CXL subsystem | expand

Commit Message

Dave Jiang April 19, 2023, 8:22 p.m. UTC
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>
---
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(+)

Comments

Jonathan Cameron April 20, 2023, 12:32 p.m. UTC | #1
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
> 
>
Dan Williams April 25, 2023, 12:45 a.m. UTC | #2
Dave Jiang 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>
> ---
> 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;

The prototype for this function makes it seem like it can apply to
either switch downstream ports or endpoint ports. Given that this is
retrieving link speed can it not just retrieve the speed from the switch
downstream port status rather than the endpoint status?

> +	/* 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;
> +
> +	/*
> +	 * 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.
> +	 */

If performance data is cached in 'struct cxl_dport' at init then the PCI
device topology need not be walked later. All of the dev_is_pci() calls
can be removed from cxl_port_get_downstream_qos().

> +	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
> 
>
diff mbox series

Patch

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;
+
+	/*
+	 * 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