diff mbox series

[v5,02/14] cxl: Add callback to parse the DSLBIS subtable from CDAT

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

Commit Message

Dave Jiang May 8, 2023, 8:47 p.m. UTC 
Provide a callback to parse the Device Scoped Latency and Bandwidth
Information Structure (DSLBIS) in the CDAT structures. The DSLBIS
contains the bandwidth and latency information that's tied to a DSMAS
handle. The driver will retrieve the read and write latency and
bandwidth associated with the DSMAS which is tied to a DPA range.

Coherent Device Attribute Table 1.03 2.1 Device Scoped Latency and
Bandwidth Information Structure (DSLBIS)

Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v5:
- Remove macro for common headers. (Jonathan)
- Use acpi_table_parse_cdat().
- Remove unlikely(). (Dan)
v3:
- Added spec section in commit header. (Alison)
- Remove void * recast. (Alison)
- Rework comment. (Alison)
- Move CDAT parse to cxl_endpoint_port_probe()
- Convert to use 'struct node_hmem_attrs'

v2:
- Add size check to DSLIBIS table. (Lukas)
- Remove unnecessary entry type check. (Jonathan)
- Move data_type check to after match. (Jonathan)
- Skip unknown data type. (Jonathan)
- Add overflow check for unit multiply. (Jonathan)
- Use dev_warn() when entries parsing fail. (Jonathan)
---
 drivers/cxl/core/cdat.c |   90 ++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/cxl/cxl.h       |    2 +
 2 files changed, 90 insertions(+), 2 deletions(-)

Comments

Jonathan Cameron May 12, 2023, 2:33 p.m. UTC | #1 
On Mon, 08 May 2023 13:47:05 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Provide a callback to parse the Device Scoped Latency and Bandwidth
> Information Structure (DSLBIS) in the CDAT structures. The DSLBIS
> contains the bandwidth and latency information that's tied to a DSMAS
> handle. The driver will retrieve the read and write latency and
> bandwidth associated with the DSMAS which is tied to a DPA range.
> 
> Coherent Device Attribute Table 1.03 2.1 Device Scoped Latency and
> Bandwidth Information Structure (DSLBIS)
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
A few trivial suggestions inline. Change them if you like. I don't mind that much.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> ---
> v5:
> - Remove macro for common headers. (Jonathan)
> - Use acpi_table_parse_cdat().
> - Remove unlikely(). (Dan)
> v3:
> - Added spec section in commit header. (Alison)
> - Remove void * recast. (Alison)
> - Rework comment. (Alison)
> - Move CDAT parse to cxl_endpoint_port_probe()
> - Convert to use 'struct node_hmem_attrs'
> 
> v2:
> - Add size check to DSLIBIS table. (Lukas)
> - Remove unnecessary entry type check. (Jonathan)
> - Move data_type check to after match. (Jonathan)
> - Skip unknown data type. (Jonathan)
> - Add overflow check for unit multiply. (Jonathan)
> - Use dev_warn() when entries parsing fail. (Jonathan)
> ---
>  drivers/cxl/core/cdat.c |   90 ++++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/cxl/cxl.h       |    2 +
>  2 files changed, 90 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index 61979f0789aa..6e14d04c0453 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Copyright(c) 2023 Intel Corporation. All rights reserved. */
>  #include <linux/acpi.h>
> +#include <linux/overflow.h>
>  #include "cxlpci.h"
>  #include "cxl.h"
>  
> @@ -32,9 +33,94 @@ static int cdat_dsmas_handler(union acpi_subtable_headers *header, void *arg,
>  	return 0;
>  }
>  
> +static void cxl_access_coordinate_set(struct access_coordinate *coord,
> +				      int access, unsigned int val)
> +{
> +	switch (access) {
> +	case ACPI_HMAT_ACCESS_LATENCY:
> +		coord->read_latency = val;
> +		coord->write_latency = val;
> +		break;
> +	case ACPI_HMAT_READ_LATENCY:
> +		coord->read_latency = val;
> +		break;
> +	case ACPI_HMAT_WRITE_LATENCY:
> +		coord->write_latency = val;
> +		break;
> +	case ACPI_HMAT_ACCESS_BANDWIDTH:
> +		coord->read_bandwidth = val;
> +		coord->write_bandwidth = val;
> +		break;
> +	case ACPI_HMAT_READ_BANDWIDTH:
> +		coord->read_bandwidth = val;
> +		break;
> +	case ACPI_HMAT_WRITE_BANDWIDTH:
> +		coord->write_bandwidth = val;
> +		break;
> +	}
> +}
> +
> +static int cdat_dslbis_handler(union acpi_subtable_headers *header, void *arg,
> +			       const unsigned long end)
> +{
> +	struct acpi_cdat_dslbis *dslbis = (struct acpi_cdat_dslbis *)header;

As for DSMAS, cast from the more obviously appropriate &header->cdat

> +	struct list_head *dsmas_list = arg;
> +	struct dsmas_entry *dent;
> +	u16 len;
> +
> +	len = le16_to_cpu((__force __le16)dslbis->header.length);
> +	if (len != sizeof(*dslbis) || (unsigned long)header + len > end) {
> +		pr_warn("Malformed DSLBIS table length: (%lu:%u)\n",
> +			(unsigned long)sizeof(*dslbis), len);
> +		return -EINVAL;
> +	}
> +
> +	/* Skip unrecognized data type */
> +	if (dslbis->data_type > ACPI_HMAT_WRITE_BANDWIDTH)
> +		return 0;
> +
> +	list_for_each_entry(dent, dsmas_list, list) {
> +		u64 val;
> +		int rc;
> +
> +		if (dslbis->handle != dent->handle)
> +			continue;
> +
> +		/* Not a memory type, skip */
> +		if ((dslbis->flags & ACPI_HMAT_MEMORY_HIERARCHY) !=
> +		    ACPI_HMAT_MEMORY)
> +			return 0;
> +
> +		rc = check_mul_overflow(le64_to_cpu((__force __le64)dslbis->entry_base_unit),
> +					le16_to_cpu((__force __le16)dslbis->entry[0]), &val);
> +		if (rc)
> +			pr_warn("DSLBIS value overflowed.\n");
> +
> +		cxl_access_coordinate_set(&dent->coord, dslbis->data_type, val);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
>  int cxl_cdat_endpoint_process(struct cxl_port *port, struct list_head *list)
>  {
> -	return acpi_table_parse_cdat(ACPI_CDAT_TYPE_DSMAS, cdat_dsmas_handler,
> -				     list, port->cdat.table);
> +	int rc;
> +
> +	rc = acpi_table_parse_cdat(ACPI_CDAT_TYPE_DSMAS, cdat_dsmas_handler,
> +				   list, port->cdat.table);
> +	if (rc <= 0) {
> +		if (rc == 0)
> +			rc = -ENOENT;
> +		return rc;
> +	}

	if (rc < 0)
		return rc;
	if (rc == 0)
		return -ENOENT;

is a tiny bit simpler.

> +
> +	rc = acpi_table_parse_cdat(ACPI_CDAT_TYPE_DSLBIS,
> +				   cdat_dslbis_handler,
> +				   list, port->cdat.table);
> +	if (rc == 0)
> +		rc = -ENOENT;

I'd burn a few lines of code for readability rather than fudging the
return value.

	if (rc < 0)
		return rc;
	if (rc == 0)
		return -ENOENT;

	return 0;
> +
> +	return rc;
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_cdat_endpoint_process, CXL);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index dda7238b47f5..ca3d0d74f2e5 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -8,6 +8,7 @@
>  #include <linux/bitfield.h>
>  #include <linux/bitops.h>
>  #include <linux/list.h>
> +#include <linux/node.h>
>  #include <linux/log2.h>
>  #include <linux/io.h>
>  
> @@ -797,6 +798,7 @@ struct dsmas_entry {
>  	struct list_head list;
>  	struct range dpa_range;
>  	u8 handle;
> +	struct access_coordinate coord;
>  };
>  
>  #ifdef CONFIG_ACPI
> 
>
diff mbox series

Patch

diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index 61979f0789aa..6e14d04c0453 100644
--- a/drivers/cxl/core/cdat.c
+++ b/drivers/cxl/core/cdat.c
@@ -1,6 +1,7 @@ 
 // SPDX-License-Identifier: GPL-2.0-only
 /* Copyright(c) 2023 Intel Corporation. All rights reserved. */
 #include <linux/acpi.h>
+#include <linux/overflow.h>
 #include "cxlpci.h"
 #include "cxl.h"
 
@@ -32,9 +33,94 @@  static int cdat_dsmas_handler(union acpi_subtable_headers *header, void *arg,
 	return 0;
 }
 
+static void cxl_access_coordinate_set(struct access_coordinate *coord,
+				      int access, unsigned int val)
+{
+	switch (access) {
+	case ACPI_HMAT_ACCESS_LATENCY:
+		coord->read_latency = val;
+		coord->write_latency = val;
+		break;
+	case ACPI_HMAT_READ_LATENCY:
+		coord->read_latency = val;
+		break;
+	case ACPI_HMAT_WRITE_LATENCY:
+		coord->write_latency = val;
+		break;
+	case ACPI_HMAT_ACCESS_BANDWIDTH:
+		coord->read_bandwidth = val;
+		coord->write_bandwidth = val;
+		break;
+	case ACPI_HMAT_READ_BANDWIDTH:
+		coord->read_bandwidth = val;
+		break;
+	case ACPI_HMAT_WRITE_BANDWIDTH:
+		coord->write_bandwidth = val;
+		break;
+	}
+}
+
+static int cdat_dslbis_handler(union acpi_subtable_headers *header, void *arg,
+			       const unsigned long end)
+{
+	struct acpi_cdat_dslbis *dslbis = (struct acpi_cdat_dslbis *)header;
+	struct list_head *dsmas_list = arg;
+	struct dsmas_entry *dent;
+	u16 len;
+
+	len = le16_to_cpu((__force __le16)dslbis->header.length);
+	if (len != sizeof(*dslbis) || (unsigned long)header + len > end) {
+		pr_warn("Malformed DSLBIS table length: (%lu:%u)\n",
+			(unsigned long)sizeof(*dslbis), len);
+		return -EINVAL;
+	}
+
+	/* Skip unrecognized data type */
+	if (dslbis->data_type > ACPI_HMAT_WRITE_BANDWIDTH)
+		return 0;
+
+	list_for_each_entry(dent, dsmas_list, list) {
+		u64 val;
+		int rc;
+
+		if (dslbis->handle != dent->handle)
+			continue;
+
+		/* Not a memory type, skip */
+		if ((dslbis->flags & ACPI_HMAT_MEMORY_HIERARCHY) !=
+		    ACPI_HMAT_MEMORY)
+			return 0;
+
+		rc = check_mul_overflow(le64_to_cpu((__force __le64)dslbis->entry_base_unit),
+					le16_to_cpu((__force __le16)dslbis->entry[0]), &val);
+		if (rc)
+			pr_warn("DSLBIS value overflowed.\n");
+
+		cxl_access_coordinate_set(&dent->coord, dslbis->data_type, val);
+		break;
+	}
+
+	return 0;
+}
+
 int cxl_cdat_endpoint_process(struct cxl_port *port, struct list_head *list)
 {
-	return acpi_table_parse_cdat(ACPI_CDAT_TYPE_DSMAS, cdat_dsmas_handler,
-				     list, port->cdat.table);
+	int rc;
+
+	rc = acpi_table_parse_cdat(ACPI_CDAT_TYPE_DSMAS, cdat_dsmas_handler,
+				   list, port->cdat.table);
+	if (rc <= 0) {
+		if (rc == 0)
+			rc = -ENOENT;
+		return rc;
+	}
+
+	rc = acpi_table_parse_cdat(ACPI_CDAT_TYPE_DSLBIS,
+				   cdat_dslbis_handler,
+				   list, port->cdat.table);
+	if (rc == 0)
+		rc = -ENOENT;
+
+	return rc;
 }
 EXPORT_SYMBOL_NS_GPL(cxl_cdat_endpoint_process, CXL);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index dda7238b47f5..ca3d0d74f2e5 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -8,6 +8,7 @@ 
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/list.h>
+#include <linux/node.h>
 #include <linux/log2.h>
 #include <linux/io.h>
 
@@ -797,6 +798,7 @@  struct dsmas_entry {
 	struct list_head list;
 	struct range dpa_range;
 	u8 handle;
+	struct access_coordinate coord;
 };
 
 #ifdef CONFIG_ACPI