On Tue, 6 Feb 2024 15:28:34 -0700
Dave Jiang <dave.jiang@xxxxxxxxx> 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@xxxxxxxxx>
one suggestion on function comment.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> ---
> 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);
>
