Re: [PATCH v4 06/23] cxl: Add callback to parse the DSLBIS subtable from CDAT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/24/23 3:46 PM, Dan Williams wrote:
Dave Jiang 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@xxxxxxxxx>

---
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 |   68 +++++++++++++++++++++++++++++++++++++++++++++++
  drivers/cxl/cxlpci.h    |   34 +++++++++++++++++++++++-
  drivers/cxl/port.c      |   11 +++++++-
  3 files changed, 111 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index 6f20af83a3ed..e8b9bb99dfdf 100644
--- a/drivers/cxl/core/cdat.c
+++ b/drivers/cxl/core/cdat.c
@@ -1,5 +1,6 @@
  // SPDX-License-Identifier: GPL-2.0-only
  /* Copyright(c) 2023 Intel Corporation. All rights reserved. */
+#include <linux/overflow.h>
  #include "cxlpci.h"
  #include "cxl.h"
@@ -124,3 +125,70 @@ int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg)
  	return 0;
  }
  EXPORT_SYMBOL_NS_GPL(cxl_dsmas_parse_entry, CXL);
+
+static void cxl_hmem_attrs_set(struct node_hmem_attrs *attrs,
+			       int access, unsigned int val)
+{
+	switch (access) {
+	case HMAT_SLLBIS_ACCESS_LATENCY:
+		attrs->read_latency = val;
+		attrs->write_latency = val;
+		break;
+	case HMAT_SLLBIS_READ_LATENCY:
+		attrs->read_latency = val;
+		break;
+	case HMAT_SLLBIS_WRITE_LATENCY:
+		attrs->write_latency = val;
+		break;
+	case HMAT_SLLBIS_ACCESS_BANDWIDTH:
+		attrs->read_bandwidth = val;
+		attrs->write_bandwidth = val;
+		break;
+	case HMAT_SLLBIS_READ_BANDWIDTH:
+		attrs->read_bandwidth = val;
+		break;
+	case HMAT_SLLBIS_WRITE_BANDWIDTH:
+		attrs->write_bandwidth = val;
+		break;
+	}
+}
+
+int cxl_dslbis_parse_entry(struct cdat_entry_header *header, void *arg)
+{
+	struct cdat_dslbis *dslbis = (struct cdat_dslbis *)header;
+	struct list_head *dsmas_list = arg;
+	struct dsmas_entry *dent;
+
+	if (dslbis->hdr.length != sizeof(*dslbis)) {
+		pr_warn("Malformed DSLBIS table length: (%lu:%u)\n",
+			(unsigned long)sizeof(*dslbis), dslbis->hdr.length);
+		return -EINVAL;
+	}
+
+	/* Skip unrecognized data type */
+	if (dslbis->data_type >= HMAT_SLLBIS_DATA_TYPE_MAX)
+		return 0;
+
+	list_for_each_entry(dent, dsmas_list, list) {
+		u64 val;
+		int rc;
+
+		if (dslbis->handle != dent->handle)
+			continue;

Oh, now I see why the list is needed. Update the changelog of the
previous patch to indicate that the entries are cached to a list so they
can be cross referenced during dslbis parsing. At least that would have
saved me from picking on it.

Jonathan had the same comment. It'll be updated for the next rev to make the connection.



+
+		/* Not a memory type, skip */
+		if ((dslbis->flags & DSLBIS_MEM_MASK) != DSLBIS_MEM_MEMORY)
+			return 0;
+
+		rc = check_mul_overflow(le64_to_cpu(dslbis->entry_base_unit),
+					le16_to_cpu(dslbis->entry[0]), &val);
+		if (unlikely(rc))

Don't use likely() / unlikely() without performance numbers. The
compiler generally does a better job and this is not a hot path.



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux