| Message ID | 167995347963.2857312.11781710463537827645.stgit@djiang5-mobl3 |
|---|---|
| State | New |
| Headers | show |
| Series | cxl: Add support for QTG ID retrieval for CXL subsystem | expand |
On Mon, Mar 27, 2023 at 02:44:39PM -0700, 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. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > > --- > 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 | 41 +++++++++++++++++++++++++++++++++++++++++ > drivers/cxl/cxlpci.h | 34 +++++++++++++++++++++++++++++++++- > drivers/cxl/port.c | 9 ++++++++- > 3 files changed, 82 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c > index d068609fb6f9..0e88973e9f38 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,43 @@ int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg) > return 0; > } > EXPORT_SYMBOL_NS_GPL(cxl_dsmas_parse_entry, CXL); > + > +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 = (struct list_head *)arg; cast from void ? > + 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; > + } > + > + /* Unrecognized data type, we can skip */ /* 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; > + > + /* 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)) > + pr_warn("DSLBIS value overflowed!\n"); Must this be shouted ! I have a vague recollection of being told not to use exclamation points in user visible messages. > + > + dent->qos[dslbis->data_type] = val; > + break; > + } > + > + return 0; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_dslbis_parse_entry, CXL); > diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h > index 9a2468a93d83..1429de49e0c4 100644 > --- a/drivers/cxl/cxlpci.h > +++ b/drivers/cxl/cxlpci.h > @@ -104,10 +104,21 @@ struct cdat_subtable_entry { > enum cdat_type type; > }; > > +enum { > + HMAT_SLLBIS_ACCESS_LATENCY = 0, > + HMAT_SLLBIS_READ_LATENCY, > + HMAT_SLLBIS_WRITE_LATENCY, > + HMAT_SLLBIS_ACCESS_BANDWIDTH, > + HMAT_SLLBIS_READ_BANDWIDTH, > + HMAT_SLLBIS_WRITE_BANDWIDTH, > + HMAT_SLLBIS_DATA_TYPE_MAX > +}; > + > struct dsmas_entry { > struct list_head list; > struct range dpa_range; > u8 handle; > + u64 qos[HMAT_SLLBIS_DATA_TYPE_MAX]; > }; > > /* Sub-table 0: Device Scoped Memory Affinity Structure (DSMAS) */ > @@ -120,6 +131,23 @@ struct cdat_dsmas { > __le64 dpa_length; > } __packed; > > +/* Sub-table 1: Device Scoped Latency and Bandwidth Information Structure (DSLBIS) */ > +struct cdat_dslbis { > + struct cdat_entry_header hdr; > + u8 handle; > + u8 flags; > + u8 data_type; > + u8 reserved; > + __le64 entry_base_unit; > + __le16 entry[3]; > + __le16 reserved2; > +} __packed; > + > +/* Flags for subtable above */ /* Flags for DSLBIS subtable */ > + > +#define DSLBIS_MEM_MASK GENMASK(3, 0) > +#define DSLBIS_MEM_MEMORY 0 > + > int devm_cxl_port_enumerate_dports(struct cxl_port *port); > struct cxl_dev_state; > int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm, > @@ -136,5 +164,9 @@ cdat_table_parse(dsmas); > cdat_table_parse(dslbis); > cdat_table_parse(sslbis); > > -int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg); > +#define cxl_parse_entry(x) \ > +int cxl_##x##_parse_entry(struct cdat_entry_header *header, void *arg) > + > +cxl_parse_entry(dsmas); > +cxl_parse_entry(dslbis); > #endif /* __CXL_PCI_H__ */ > diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c > index c8136797d528..6f2b327f7128 100644 > --- a/drivers/cxl/port.c > +++ b/drivers/cxl/port.c > @@ -152,8 +152,15 @@ static int cxl_port_probe(struct device *dev) > rc = cdat_table_parse_dsmas(port->cdat.table, > cxl_dsmas_parse_entry, > (void *)&dsmas_list); > - if (rc < 0) > + if (rc > 0) { > + rc = cdat_table_parse_dslbis(port->cdat.table, > + cxl_dslbis_parse_entry, > + (void *)&dsmas_list); > + if (rc <= 0) > + dev_warn(dev, "Failed to parse DSLBIS: %d\n", rc); > + } else { > dev_warn(dev, "Failed to parse DSMAS: %d\n", rc); > + } I see you touch this func more than once. Maybe some earlier nips and tucks, makes this more readable. > > dsmas_list_destroy(&dsmas_list); > } > >
On 3/28/23 5:44 PM, Alison Schofield wrote: > On Mon, Mar 27, 2023 at 02:44:39PM -0700, 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. >> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> >> --- >> 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 | 41 +++++++++++++++++++++++++++++++++++++++++ >> drivers/cxl/cxlpci.h | 34 +++++++++++++++++++++++++++++++++- >> drivers/cxl/port.c | 9 ++++++++- >> 3 files changed, 82 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c >> index d068609fb6f9..0e88973e9f38 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,43 @@ int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg) >> return 0; >> } >> EXPORT_SYMBOL_NS_GPL(cxl_dsmas_parse_entry, CXL); >> + >> +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 = (struct list_head *)arg; > > cast from void ? ok > >> + 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; >> + } >> + >> + /* Unrecognized data type, we can skip */ > > /* Skip unrecognized data type */ ok > >> + 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; >> + >> + /* 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)) >> + pr_warn("DSLBIS value overflowed!\n"); > > Must this be shouted ! > I have a vague recollection of being told not to use exclamation points > in user visible messages. ok > >> + >> + dent->qos[dslbis->data_type] = val; >> + break; >> + } >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_dslbis_parse_entry, CXL); >> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h >> index 9a2468a93d83..1429de49e0c4 100644 >> --- a/drivers/cxl/cxlpci.h >> +++ b/drivers/cxl/cxlpci.h >> @@ -104,10 +104,21 @@ struct cdat_subtable_entry { >> enum cdat_type type; >> }; >> >> +enum { >> + HMAT_SLLBIS_ACCESS_LATENCY = 0, >> + HMAT_SLLBIS_READ_LATENCY, >> + HMAT_SLLBIS_WRITE_LATENCY, >> + HMAT_SLLBIS_ACCESS_BANDWIDTH, >> + HMAT_SLLBIS_READ_BANDWIDTH, >> + HMAT_SLLBIS_WRITE_BANDWIDTH, >> + HMAT_SLLBIS_DATA_TYPE_MAX >> +}; >> + >> struct dsmas_entry { >> struct list_head list; >> struct range dpa_range; >> u8 handle; >> + u64 qos[HMAT_SLLBIS_DATA_TYPE_MAX]; >> }; >> >> /* Sub-table 0: Device Scoped Memory Affinity Structure (DSMAS) */ >> @@ -120,6 +131,23 @@ struct cdat_dsmas { >> __le64 dpa_length; >> } __packed; >> >> +/* Sub-table 1: Device Scoped Latency and Bandwidth Information Structure (DSLBIS) */ >> +struct cdat_dslbis { >> + struct cdat_entry_header hdr; >> + u8 handle; >> + u8 flags; >> + u8 data_type; >> + u8 reserved; >> + __le64 entry_base_unit; >> + __le16 entry[3]; >> + __le16 reserved2; >> +} __packed; >> + >> +/* Flags for subtable above */ > > /* Flags for DSLBIS subtable */ ok > >> + >> +#define DSLBIS_MEM_MASK GENMASK(3, 0) >> +#define DSLBIS_MEM_MEMORY 0 >> + >> int devm_cxl_port_enumerate_dports(struct cxl_port *port); >> struct cxl_dev_state; >> int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm, >> @@ -136,5 +164,9 @@ cdat_table_parse(dsmas); >> cdat_table_parse(dslbis); >> cdat_table_parse(sslbis); >> >> -int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg); >> +#define cxl_parse_entry(x) \ >> +int cxl_##x##_parse_entry(struct cdat_entry_header *header, void *arg) >> + >> +cxl_parse_entry(dsmas); >> +cxl_parse_entry(dslbis); >> #endif /* __CXL_PCI_H__ */ >> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c >> index c8136797d528..6f2b327f7128 100644 >> --- a/drivers/cxl/port.c >> +++ b/drivers/cxl/port.c >> @@ -152,8 +152,15 @@ static int cxl_port_probe(struct device *dev) >> rc = cdat_table_parse_dsmas(port->cdat.table, >> cxl_dsmas_parse_entry, >> (void *)&dsmas_list); >> - if (rc < 0) >> + if (rc > 0) { >> + rc = cdat_table_parse_dslbis(port->cdat.table, >> + cxl_dslbis_parse_entry, >> + (void *)&dsmas_list); >> + if (rc <= 0) >> + dev_warn(dev, "Failed to parse DSLBIS: %d\n", rc); >> + } else { >> dev_warn(dev, "Failed to parse DSMAS: %d\n", rc); >> + } > > I see you touch this func more than once. Maybe some earlier nips and > tucks, makes this more readable. Not sure what you mean. > >> >> dsmas_list_destroy(&dsmas_list); >> } >> >>
On Wed, Mar 29, 2023 at 01:59:12PM -0700, Dave Jiang wrote: > > > On 3/28/23 5:44 PM, Alison Schofield wrote: > > On Mon, Mar 27, 2023 at 02:44:39PM -0700, 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. > > > > > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > > > > > > --- > > > 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 | 41 +++++++++++++++++++++++++++++++++++++++++ > > > drivers/cxl/cxlpci.h | 34 +++++++++++++++++++++++++++++++++- > > > drivers/cxl/port.c | 9 ++++++++- > > > 3 files changed, 82 insertions(+), 2 deletions(-) > > > snip > > > --- a/drivers/cxl/port.c > > > +++ b/drivers/cxl/port.c > > > @@ -152,8 +152,15 @@ static int cxl_port_probe(struct device *dev) > > > rc = cdat_table_parse_dsmas(port->cdat.table, > > > cxl_dsmas_parse_entry, > > > (void *)&dsmas_list); > > > - if (rc < 0) > > > + if (rc > 0) { > > > + rc = cdat_table_parse_dslbis(port->cdat.table, > > > + cxl_dslbis_parse_entry, > > > + (void *)&dsmas_list); > > > + if (rc <= 0) > > > + dev_warn(dev, "Failed to parse DSLBIS: %d\n", rc); > > > + } else { > > > dev_warn(dev, "Failed to parse DSMAS: %d\n", rc); > > > + } > > > > I see you touch this func more than once. Maybe some earlier nips and > > tucks, makes this more readable. > > Not sure what you mean. I thought this was the same func, cxl_port_probe(), that I commented on in the previous patch, so maybe it was already getting re-aligned. > > > > > > dsmas_list_destroy(&dsmas_list); > > > } > > > > > >
diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c index d068609fb6f9..0e88973e9f38 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,43 @@ int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg) return 0; } EXPORT_SYMBOL_NS_GPL(cxl_dsmas_parse_entry, CXL); + +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 = (struct list_head *)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; + } + + /* Unrecognized data type, we can skip */ + 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; + + /* 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)) + pr_warn("DSLBIS value overflowed!\n"); + + dent->qos[dslbis->data_type] = val; + break; + } + + return 0; +} +EXPORT_SYMBOL_NS_GPL(cxl_dslbis_parse_entry, CXL); diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h index 9a2468a93d83..1429de49e0c4 100644 --- a/drivers/cxl/cxlpci.h +++ b/drivers/cxl/cxlpci.h @@ -104,10 +104,21 @@ struct cdat_subtable_entry { enum cdat_type type; }; +enum { + HMAT_SLLBIS_ACCESS_LATENCY = 0, + HMAT_SLLBIS_READ_LATENCY, + HMAT_SLLBIS_WRITE_LATENCY, + HMAT_SLLBIS_ACCESS_BANDWIDTH, + HMAT_SLLBIS_READ_BANDWIDTH, + HMAT_SLLBIS_WRITE_BANDWIDTH, + HMAT_SLLBIS_DATA_TYPE_MAX +}; + struct dsmas_entry { struct list_head list; struct range dpa_range; u8 handle; + u64 qos[HMAT_SLLBIS_DATA_TYPE_MAX]; }; /* Sub-table 0: Device Scoped Memory Affinity Structure (DSMAS) */ @@ -120,6 +131,23 @@ struct cdat_dsmas { __le64 dpa_length; } __packed; +/* Sub-table 1: Device Scoped Latency and Bandwidth Information Structure (DSLBIS) */ +struct cdat_dslbis { + struct cdat_entry_header hdr; + u8 handle; + u8 flags; + u8 data_type; + u8 reserved; + __le64 entry_base_unit; + __le16 entry[3]; + __le16 reserved2; +} __packed; + +/* Flags for subtable above */ + +#define DSLBIS_MEM_MASK GENMASK(3, 0) +#define DSLBIS_MEM_MEMORY 0 + int devm_cxl_port_enumerate_dports(struct cxl_port *port); struct cxl_dev_state; int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm, @@ -136,5 +164,9 @@ cdat_table_parse(dsmas); cdat_table_parse(dslbis); cdat_table_parse(sslbis); -int cxl_dsmas_parse_entry(struct cdat_entry_header *header, void *arg); +#define cxl_parse_entry(x) \ +int cxl_##x##_parse_entry(struct cdat_entry_header *header, void *arg) + +cxl_parse_entry(dsmas); +cxl_parse_entry(dslbis); #endif /* __CXL_PCI_H__ */ diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c index c8136797d528..6f2b327f7128 100644 --- a/drivers/cxl/port.c +++ b/drivers/cxl/port.c @@ -152,8 +152,15 @@ static int cxl_port_probe(struct device *dev) rc = cdat_table_parse_dsmas(port->cdat.table, cxl_dsmas_parse_entry, (void *)&dsmas_list); - if (rc < 0) + if (rc > 0) { + rc = cdat_table_parse_dslbis(port->cdat.table, + cxl_dslbis_parse_entry, + (void *)&dsmas_list); + if (rc <= 0) + dev_warn(dev, "Failed to parse DSLBIS: %d\n", rc); + } else { dev_warn(dev, "Failed to parse DSMAS: %d\n", rc); + } dsmas_list_destroy(&dsmas_list); }
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. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- 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 | 41 +++++++++++++++++++++++++++++++++++++++++ drivers/cxl/cxlpci.h | 34 +++++++++++++++++++++++++++++++++- drivers/cxl/port.c | 9 ++++++++- 3 files changed, 82 insertions(+), 2 deletions(-)