RE: [PATCH v4 11/23] cxl: Add helper function that calculates QoS values for switches

[Dan Williams <dan.j.williams@intel.com>]

Dave Jiang wrote:
> The CDAT information from the switch, Switch Scoped Latency and Bandwidth
> Information Strucutre (SSLBIS), is parsed and stored in an xarray under the
> cxl_port. The QoS data are indexed by the downstream port id.  Walk the CXL
> ports from endpoint to root and retrieve the relevant QoS information
> (bandwidth and latency) that are from the switch CDAT. If read or write QoS
> values are not available, then use the access QoS value.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> ---
> v3:
> - Move to use 'struct node_hmem_attrs'
> ---
>  drivers/cxl/core/port.c |   81 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/cxl/cxl.h       |    2 +
>  2 files changed, 83 insertions(+)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 3fedbabac1af..770b540d5325 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1921,6 +1921,87 @@ bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd)
>  }
>  EXPORT_SYMBOL_NS_GPL(schedule_cxl_memdev_detach, CXL);
>  
> +/**
> + * cxl_port_get_switch_qos - retrieve QoS data for CXL switches
> + * @port: endpoint cxl_port
> + * @rd_bw: writeback value for min read bandwidth
> + * @rd_lat: writeback value for total read latency
> + * @wr_bw: writeback value for min write bandwidth
> + * @wr_lat: writeback value for total write latency
> + *
> + * Return: Errno on failure, 0 on success. -ENOENT if no switch device
> + */
> +int cxl_port_get_switch_qos(struct cxl_port *port, u64 *rd_bw, u64 *rd_lat,
> +			    u64 *wr_bw, u64 *wr_lat)
> +{
> +	u64 min_rd_bw = ULONG_MAX;
> +	u64 min_wr_bw = ULONG_MAX;
> +	struct cxl_dport *dport;
> +	struct cxl_port *nport;
> +	u64 total_rd_lat = 0;
> +	u64 total_wr_lat = 0;
> +	struct device *next;
> +	int switches = 0;
> +	int rc = 0;
> +
> +	if (!is_cxl_endpoint(port))
> +		return -EINVAL;
> +
> +	/* Skip the endpoint */
> +	next = port->dev.parent;
> +	nport = to_cxl_port(next);
> +	dport = port->parent_dport;
> +
> +	do {
> +		struct node_hmem_attrs *hmem_attrs;
> +		u64 lat, bw;
> +
> +		if (!nport->cdat.table)
> +			break;
> +
> +		if (!dev_is_pci(dport->dport))
> +			break;
> +
> +		hmem_attrs = xa_load(&nport->cdat.sslbis_xa, dport->port_id);
> +		if (xa_is_err(hmem_attrs))
> +			return xa_err(hmem_attrs);
> +
> +		if (!hmem_attrs) {
> +			hmem_attrs = xa_load(&nport->cdat.sslbis_xa, SSLBIS_ANY_PORT);
> +			if (xa_is_err(hmem_attrs))
> +				return xa_err(hmem_attrs);
> +			if (!hmem_attrs)
> +				return -ENXIO;
> +		}

Yeah, I think my comment from a few patches back stands. There appears
to be no need to maintain the xarray if each dport just resolves and
caches its relative access coordinate at init time.