diff mbox series

[v5,06/12] cxl: Split out host bridge access coordinates

Message ID 20240206222951.1833098-7-dave.jiang@intel.com
State Superseded
Headers show
Series cxl: Add support to report region access coordinates to numa nodes | expand

Commit Message

Dave Jiang Feb. 6, 2024, 10:28 p.m. UTC
The difference between access class 0 and access class 1 for 'struct
access_coordinate', if any, is that class 0 is for the distance from
the target to the closest initiator and that class 1 is for the distance
from the target to the cloest CPU. For CXL memory, the nearest initiator
may not necessarily be a CPU node. The performance path from the CXL
endpoint to the host bridge should remain the same. However, the numbers
extracted and stored from HMAT is the difference for the two access
classes. Split out the performance numbers for the host bridge (generic
target) from the calculation of the entire path in order to allow
calculation of both access classes for a CXL region.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/cdat.c | 28 ++++++++++++++++++++++------
 drivers/cxl/core/port.c | 34 +++++++++++++++++++++++++++++++---
 drivers/cxl/cxl.h       |  2 ++
 3 files changed, 55 insertions(+), 9 deletions(-)

Comments

Jonathan Cameron Feb. 15, 2024, 4:56 p.m. UTC | #1
On Tue, 6 Feb 2024 15:28:34 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> The difference between access class 0 and access class 1 for 'struct
> access_coordinate', if any, is that class 0 is for the distance from
> the target to the closest initiator and that class 1 is for the distance
> from the target to the cloest CPU. For CXL memory, the nearest initiator

spell check.

> may not necessarily be a CPU node. The performance path from the CXL
> endpoint to the host bridge should remain the same. However, the numbers
> extracted and stored from HMAT is the difference for the two access
> classes. Split out the performance numbers for the host bridge (generic
> target) from the calculation of the entire path in order to allow
> calculation of both access classes for a CXL region.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

one suggestion on function comment.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/core/cdat.c | 28 ++++++++++++++++++++++------
>  drivers/cxl/core/port.c | 34 +++++++++++++++++++++++++++++++---
>  drivers/cxl/cxl.h       |  2 ++
>  3 files changed, 55 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index 096320390ad9..79844874a34b 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -162,15 +162,22 @@ static int cxl_cdat_endpoint_process(struct cxl_port *port,
>  static int cxl_port_perf_data_calculate(struct cxl_port *port,
>  					struct xarray *dsmas_xa)
>  {
> -	struct access_coordinate c;
> +	struct access_coordinate ep_c;
> +	struct access_coordinate coord[ACCESS_COORDINATE_MAX];
>  	struct dsmas_entry *dent;
>  	int valid_entries = 0;
>  	unsigned long index;
>  	int rc;
>  
> -	rc = cxl_endpoint_get_perf_coordinates(port, &c);
> +	rc = cxl_endpoint_get_perf_coordinates(port, &ep_c);
>  	if (rc) {
> -		dev_dbg(&port->dev, "Failed to retrieve perf coordinates.\n");
> +		dev_dbg(&port->dev, "Failed to retrieve ep perf coordinates.\n");
> +		return rc;
> +	}
> +
> +	rc = cxl_hb_get_perf_coordinates(port, coord);
> +	if (rc)  {
> +		dev_dbg(&port->dev, "Failed to retrieve hb perf coordinates.\n");
>  		return rc;
>  	}
>  
> @@ -185,10 +192,19 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port,
>  	xa_for_each(dsmas_xa, index, dent) {
>  		int qos_class;
>  
> -		cxl_coordinates_combine(&dent->coord, &dent->coord, &c);
> +		cxl_coordinates_combine(&dent->coord, &dent->coord, &ep_c);
> +		/*
> +		 * Keeping the host bridge coordinates separate from the dsmas
> +		 * coordinates in order to allow calculation of access class
> +		 * 0 and 1 for region later.
> +		 */
> +		cxl_coordinates_combine(&coord[ACCESS_COORDINATE_LOCAL],
> +					&coord[ACCESS_COORDINATE_LOCAL],
> +					&dent->coord);
>  		dent->entries = 1;
> -		rc = cxl_root->ops->qos_class(cxl_root, &dent->coord, 1,
> -					      &qos_class);
> +		rc = cxl_root->ops->qos_class(cxl_root,
> +					      &coord[ACCESS_COORDINATE_LOCAL],
> +					      1, &qos_class);
>  		if (rc != 1)
>  			continue;
>  
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index af9458b2678c..e8029170b8c6 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -2096,6 +2096,37 @@ bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd)
>  }
>  EXPORT_SYMBOL_NS_GPL(schedule_cxl_memdev_detach, CXL);
>  
> +/**
> + * cxl_hb_get_perf_coordinates - Retrieve performance numbers from host bridge

This description confused me.  What numbers does the host bridge have?

between initiator and host bridge
perhaps?

> + *
> + * @port: endpoint cxl_port
> + * @coord: output access coordinates
> + *
> + * Return: errno on failure, 0 on success.
> + */
> +int cxl_hb_get_perf_coordinates(struct cxl_port *port,
> +				struct access_coordinate *coord)
> +{
> +	struct cxl_port *iter = port;
> +	struct cxl_dport *dport;
> +
> +	if (!is_cxl_endpoint(port))
> +		return -EINVAL;
> +
> +	dport = iter->parent_dport;
> +	while (iter && !is_cxl_root(to_cxl_port(iter->dev.parent))) {
> +		iter = to_cxl_port(iter->dev.parent);
> +		dport = iter->parent_dport;
> +	}
> +
> +	coord[ACCESS_COORDINATE_LOCAL] =
> +		dport->hb_coord[ACCESS_COORDINATE_LOCAL];
> +	coord[ACCESS_COORDINATE_CPU] =
> +		dport->hb_coord[ACCESS_COORDINATE_CPU];
> +
> +	return 0;
> +}
> +
>  /**
>   * cxl_endpoint_get_perf_coordinates - Retrieve performance numbers stored in dports
>   *				   of CXL path
> @@ -2137,9 +2168,6 @@ int cxl_endpoint_get_perf_coordinates(struct cxl_port *port,
>  		dport = iter->parent_dport;
>  	}
>  
> -	/* Augment with the generic port (host bridge) perf data */
> -	cxl_coordinates_combine(&c, &c, &dport->hb_coord[ACCESS_COORDINATE_LOCAL]);
> -
>  	/* Get the calculated PCI paths bandwidth */
>  	pdev = to_pci_dev(port->uport_dev->parent);
>  	bw = pcie_bandwidth_available(pdev, NULL, NULL, NULL);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index fab2da4b1f04..de477eb7f5d5 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -879,6 +879,8 @@ void cxl_switch_parse_cdat(struct cxl_port *port);
>  
>  int cxl_endpoint_get_perf_coordinates(struct cxl_port *port,
>  				      struct access_coordinate *coord);
> +int cxl_hb_get_perf_coordinates(struct cxl_port *port,
> +				struct access_coordinate *coord);
>  
>  void cxl_memdev_update_perf(struct cxl_memdev *cxlmd);
>
diff mbox series

Patch

diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index 096320390ad9..79844874a34b 100644
--- a/drivers/cxl/core/cdat.c
+++ b/drivers/cxl/core/cdat.c
@@ -162,15 +162,22 @@  static int cxl_cdat_endpoint_process(struct cxl_port *port,
 static int cxl_port_perf_data_calculate(struct cxl_port *port,
 					struct xarray *dsmas_xa)
 {
-	struct access_coordinate c;
+	struct access_coordinate ep_c;
+	struct access_coordinate coord[ACCESS_COORDINATE_MAX];
 	struct dsmas_entry *dent;
 	int valid_entries = 0;
 	unsigned long index;
 	int rc;
 
-	rc = cxl_endpoint_get_perf_coordinates(port, &c);
+	rc = cxl_endpoint_get_perf_coordinates(port, &ep_c);
 	if (rc) {
-		dev_dbg(&port->dev, "Failed to retrieve perf coordinates.\n");
+		dev_dbg(&port->dev, "Failed to retrieve ep perf coordinates.\n");
+		return rc;
+	}
+
+	rc = cxl_hb_get_perf_coordinates(port, coord);
+	if (rc)  {
+		dev_dbg(&port->dev, "Failed to retrieve hb perf coordinates.\n");
 		return rc;
 	}
 
@@ -185,10 +192,19 @@  static int cxl_port_perf_data_calculate(struct cxl_port *port,
 	xa_for_each(dsmas_xa, index, dent) {
 		int qos_class;
 
-		cxl_coordinates_combine(&dent->coord, &dent->coord, &c);
+		cxl_coordinates_combine(&dent->coord, &dent->coord, &ep_c);
+		/*
+		 * Keeping the host bridge coordinates separate from the dsmas
+		 * coordinates in order to allow calculation of access class
+		 * 0 and 1 for region later.
+		 */
+		cxl_coordinates_combine(&coord[ACCESS_COORDINATE_LOCAL],
+					&coord[ACCESS_COORDINATE_LOCAL],
+					&dent->coord);
 		dent->entries = 1;
-		rc = cxl_root->ops->qos_class(cxl_root, &dent->coord, 1,
-					      &qos_class);
+		rc = cxl_root->ops->qos_class(cxl_root,
+					      &coord[ACCESS_COORDINATE_LOCAL],
+					      1, &qos_class);
 		if (rc != 1)
 			continue;
 
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index af9458b2678c..e8029170b8c6 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -2096,6 +2096,37 @@  bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd)
 }
 EXPORT_SYMBOL_NS_GPL(schedule_cxl_memdev_detach, CXL);
 
+/**
+ * cxl_hb_get_perf_coordinates - Retrieve performance numbers from host bridge
+ *
+ * @port: endpoint cxl_port
+ * @coord: output access coordinates
+ *
+ * Return: errno on failure, 0 on success.
+ */
+int cxl_hb_get_perf_coordinates(struct cxl_port *port,
+				struct access_coordinate *coord)
+{
+	struct cxl_port *iter = port;
+	struct cxl_dport *dport;
+
+	if (!is_cxl_endpoint(port))
+		return -EINVAL;
+
+	dport = iter->parent_dport;
+	while (iter && !is_cxl_root(to_cxl_port(iter->dev.parent))) {
+		iter = to_cxl_port(iter->dev.parent);
+		dport = iter->parent_dport;
+	}
+
+	coord[ACCESS_COORDINATE_LOCAL] =
+		dport->hb_coord[ACCESS_COORDINATE_LOCAL];
+	coord[ACCESS_COORDINATE_CPU] =
+		dport->hb_coord[ACCESS_COORDINATE_CPU];
+
+	return 0;
+}
+
 /**
  * cxl_endpoint_get_perf_coordinates - Retrieve performance numbers stored in dports
  *				   of CXL path
@@ -2137,9 +2168,6 @@  int cxl_endpoint_get_perf_coordinates(struct cxl_port *port,
 		dport = iter->parent_dport;
 	}
 
-	/* Augment with the generic port (host bridge) perf data */
-	cxl_coordinates_combine(&c, &c, &dport->hb_coord[ACCESS_COORDINATE_LOCAL]);
-
 	/* Get the calculated PCI paths bandwidth */
 	pdev = to_pci_dev(port->uport_dev->parent);
 	bw = pcie_bandwidth_available(pdev, NULL, NULL, NULL);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index fab2da4b1f04..de477eb7f5d5 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -879,6 +879,8 @@  void cxl_switch_parse_cdat(struct cxl_port *port);
 
 int cxl_endpoint_get_perf_coordinates(struct cxl_port *port,
 				      struct access_coordinate *coord);
+int cxl_hb_get_perf_coordinates(struct cxl_port *port,
+				struct access_coordinate *coord);
 
 void cxl_memdev_update_perf(struct cxl_memdev *cxlmd);