| Message ID | 168193568543.1178687.3067575213689202382.stgit@djiang5-mobl3 |
|---|---|
| State | New |
| Headers | show |
| Series | cxl: Add support for QTG ID retrieval for CXL subsystem | expand |
Dave Jiang wrote: > Add helper functions to parse the CDAT table and provide a callback to > parse the sub-table. Helpers are provided for DSMAS and DSLBIS sub-table > parsing. The code is patterned after the ACPI table parsing helpers. It seems a shame that CDAT is so ACPI-like, but can't reuse the ACPI table parsing infrastructure. Can this not be achieved by modifying some of the helpers helpers in drivers/acpi/tables.c to take a passed in @table_header?
On 4/24/23 3:33 PM, Dan Williams wrote: > Dave Jiang wrote: >> Add helper functions to parse the CDAT table and provide a callback to >> parse the sub-table. Helpers are provided for DSMAS and DSLBIS sub-table >> parsing. The code is patterned after the ACPI table parsing helpers. > > It seems a shame that CDAT is so ACPI-like, but can't reuse the ACPI > table parsing infrastructure. Can this not be achieved by modifying some > of the helpers helpers in drivers/acpi/tables.c to take a passed in > @table_header? Rafael, Do you have any issues with adding some endieness support in drivers/acpi/tables.c in order to support CDAT parsing by BE hosts? To start off with something like below? diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index 7b4680da57d7..e63e2daf151d 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -287,6 +287,12 @@ acpi_get_subtable_type(char *id) return ACPI_SUBTABLE_COMMON; } +static unsigned long __init_or_acpilib +acpi_table_get_length(struct acpi_table_header *hdr) +{ + return le32_to_cpu((__force __le32)hdr->length); +} + static __init_or_acpilib bool has_handler(struct acpi_subtable_proc *proc) { return proc->handler || proc->handler_arg; @@ -337,7 +343,8 @@ static int __init_or_acpilib acpi_parse_entries_array( int errs = 0; int i; - table_end = (unsigned long)table_header + table_header->length; + table_end = (unsigned long)table_header + + acpi_table_get_length(table_header); /* Parse all entries looking for a match. */
Dave Jiang wrote: > > > On 4/24/23 3:33 PM, Dan Williams wrote: > > Dave Jiang wrote: > >> Add helper functions to parse the CDAT table and provide a callback to > >> parse the sub-table. Helpers are provided for DSMAS and DSLBIS sub-table > >> parsing. The code is patterned after the ACPI table parsing helpers. > > > > It seems a shame that CDAT is so ACPI-like, but can't reuse the ACPI > > table parsing infrastructure. Can this not be achieved by modifying some > > of the helpers helpers in drivers/acpi/tables.c to take a passed in > > @table_header? > > Rafael, > Do you have any issues with adding some endieness support in > drivers/acpi/tables.c in order to support CDAT parsing by BE hosts? To > start off with something like below? Some additional background, recall that CDAT is an ACPI-like data structure that lives on endpoint CXL devices to describe the access characteristics of the device's memory similar to SRAT+HMAT for host-memory. Unlike ACPI that is guaranteed to be deployed on a little-endian host-cpu, a big-endian host might also encounter CXL endpoints. This reuse ends up at ~50 lines and duplication ends up at ~100 lines. Not a huge win, but a win nonetheless. > > diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c > index 7b4680da57d7..e63e2daf151d 100644 > --- a/drivers/acpi/tables.c > +++ b/drivers/acpi/tables.c > @@ -287,6 +287,12 @@ acpi_get_subtable_type(char *id) > return ACPI_SUBTABLE_COMMON; > } > > +static unsigned long __init_or_acpilib > +acpi_table_get_length(struct acpi_table_header *hdr) > +{ > + return le32_to_cpu((__force __le32)hdr->length); > +} > + > static __init_or_acpilib bool has_handler(struct acpi_subtable_proc *proc) > { > return proc->handler || proc->handler_arg; > @@ -337,7 +343,8 @@ static int __init_or_acpilib acpi_parse_entries_array( > int errs = 0; > int i; > > - table_end = (unsigned long)table_header + table_header->length; > + table_end = (unsigned long)table_header + > + acpi_table_get_length(table_header); > > /* Parse all entries looking for a match. */ >
diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile index ca4ae31d8f57..867a8014b462 100644 --- a/drivers/cxl/core/Makefile +++ b/drivers/cxl/core/Makefile @@ -12,5 +12,6 @@ cxl_core-y += memdev.o cxl_core-y += mbox.o cxl_core-y += pci.o cxl_core-y += hdm.o +cxl_core-y += cdat.o cxl_core-$(CONFIG_TRACING) += trace.o cxl_core-$(CONFIG_CXL_REGION) += region.o diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c new file mode 100644 index 000000000000..210f4499bddb --- /dev/null +++ b/drivers/cxl/core/cdat.c @@ -0,0 +1,100 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright(c) 2023 Intel Corporation. All rights reserved. */ +#include "cxlpci.h" +#include "cxl.h" + +static bool has_handler(struct cdat_subtable_proc *proc) +{ + return proc->handler; +} + +static int call_handler(struct cdat_subtable_proc *proc, + struct cdat_subtable_entry *ent) +{ + if (has_handler(proc)) + return proc->handler(ent->hdr, proc->arg); + return -EINVAL; +} + +static bool cdat_is_subtable_match(struct cdat_subtable_entry *ent) +{ + return ent->hdr->type == ent->type; +} + +static int cdat_table_parse_entries(enum cdat_type type, + struct cdat_header *table_header, + struct cdat_subtable_proc *proc) +{ + unsigned long table_end, entry_len; + struct cdat_subtable_entry entry; + int count = 0; + int rc; + + if (!has_handler(proc)) + return -EINVAL; + + table_end = (unsigned long)table_header + table_header->length; + + if (type >= CDAT_TYPE_RESERVED) + return -EINVAL; + + entry.type = type; + entry.hdr = (struct cdat_entry_header *)(table_header + 1); + + while ((unsigned long)entry.hdr < table_end) { + entry_len = le16_to_cpu(entry.hdr->length); + + if ((unsigned long)entry.hdr + entry_len > table_end) + return -EINVAL; + + if (entry_len == 0) + return -EINVAL; + + if (cdat_is_subtable_match(&entry)) { + rc = call_handler(proc, &entry); + if (rc) + return rc; + count++; + } + + entry.hdr = (struct cdat_entry_header *)((unsigned long)entry.hdr + entry_len); + } + + return count; +} + +int cdat_table_parse_dsmas(struct cdat_header *table, + cdat_tbl_entry_handler handler, void *arg) +{ + struct cdat_subtable_proc proc = { + .handler = handler, + .arg = arg, + }; + + return cdat_table_parse_entries(CDAT_TYPE_DSMAS, table, &proc); +} +EXPORT_SYMBOL_NS_GPL(cdat_table_parse_dsmas, CXL); + +int cdat_table_parse_dslbis(struct cdat_header *table, + cdat_tbl_entry_handler handler, void *arg) +{ + struct cdat_subtable_proc proc = { + .handler = handler, + .arg = arg, + }; + + return cdat_table_parse_entries(CDAT_TYPE_DSLBIS, table, &proc); +} +EXPORT_SYMBOL_NS_GPL(cdat_table_parse_dslbis, CXL); + +int cdat_table_parse_sslbis(struct cdat_header *table, + cdat_tbl_entry_handler handler, void *arg) +{ + struct cdat_subtable_proc proc = { + .handler = handler, + .arg = arg, + }; + + return cdat_table_parse_entries(CDAT_TYPE_SSLBIS, table, &proc); +} +EXPORT_SYMBOL_NS_GPL(cdat_table_parse_sslbis, CXL); diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h index 0465ef963cd6..45e2f2bf5ef8 100644 --- a/drivers/cxl/cxlpci.h +++ b/drivers/cxl/cxlpci.h @@ -76,12 +76,34 @@ struct cdat_header { __le32 sequence; } __packed; +enum cdat_type { + CDAT_TYPE_DSMAS = 0, + CDAT_TYPE_DSLBIS, + CDAT_TYPE_DSMSCIS, + CDAT_TYPE_DSIS, + CDAT_TYPE_DSEMTS, + CDAT_TYPE_SSLBIS, + CDAT_TYPE_RESERVED +}; + struct cdat_entry_header { u8 type; u8 reserved; __le16 length; } __packed; +typedef int (*cdat_tbl_entry_handler)(struct cdat_entry_header *header, void *arg); + +struct cdat_subtable_proc { + cdat_tbl_entry_handler handler; + void *arg; +}; + +struct cdat_subtable_entry { + struct cdat_entry_header *hdr; + enum cdat_type type; +}; + 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, @@ -90,4 +112,11 @@ void read_cdat_data(struct cxl_port *port); void cxl_cor_error_detected(struct pci_dev *pdev); pci_ers_result_t cxl_error_detected(struct pci_dev *pdev, pci_channel_state_t state); + +#define cdat_table_parse(x) \ +int cdat_table_parse_##x(struct cdat_header *table, \ + cdat_tbl_entry_handler handler, void *arg) +cdat_table_parse(dsmas); +cdat_table_parse(dslbis); +cdat_table_parse(sslbis); #endif /* __CXL_PCI_H__ */
Add helper functions to parse the CDAT table and provide a callback to parse the sub-table. Helpers are provided for DSMAS and DSLBIS sub-table parsing. The code is patterned after the ACPI table parsing helpers. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- v2: - Use local headers to handle LE instead of ACPI header - Reduce complexity of parser function. (Jonathan) - Directly access header type. (Jonathan) - Simplify header ptr math. (Jonathan) - Move parsed counter to the correct location. (Jonathan) - Add LE to host conversion for entry length --- drivers/cxl/core/Makefile | 1 drivers/cxl/core/cdat.c | 100 +++++++++++++++++++++++++++++++++++++++++++++ drivers/cxl/cxlpci.h | 29 +++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 drivers/cxl/core/cdat.c