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. > + > + /* 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.