Linux-ACPI Archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code
@ 2023-06-01 21:31 Dave Jiang
  2023-06-01 21:31 ` [PATCH v3 1/4] acpi: Move common tables helper functions to common lib Dave Jiang
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Dave Jiang @ 2023-06-01 21:31 UTC (permalink / raw
  To: linux-acpi, linux-cxl
  Cc: Len Brown, Rafael J. Wysocki, rafael, lenb, dan.j.williams,
	ira.weiny, vishal.l.verma, alison.schofield, lukas,
	Jonathan.Cameron

v3:
- Move common code to lib/fw_table.c
v2:
- Split out with CONFIG_ACPI_TABLES_LIB to be independent
- Fixed 0-day issues
- Change CDAT releveant names to prefix with cdat/CDAT instead of
  acpi/ACPI. (Jonathan)
- Make table_header a union with cdat table header instead of
  'acpi_table_header'. (Jonathan)
- Removed ACPI_SIG_CDAT, already defined.

Hi Rafael,
Please consider ack these patches. Dan can take these through the CXL tree. After
attempting to rename the cxl_ prefixes of functions and non ACPICA data structures
to something more common, it seems that significant amount of ACPI code would be
touched for the rename. For this series I left it alone in order to have the minimal
changes to ACPI code.

I've broken out the "cxl: Add support for QTG ID retrieval for CXL subsystem" [1]
series in order to make it more manageable. Here's the first part of the ACPI
changes. These changes are added to allow reuse of ACPI tables code to parse
the CDAT tables. While CDAT is not part of ACPI, the table structures are similar
to ACPI layouts that the code can be reused with some small modifications.

However, in order to be properly utilized by CXL users, the tables code needs
to be refactored out to be independent of ACPI. For example, a PPC BE host may
have CXL and does not have ACPI support. But it will have CDAT to read from
devices and switches. I have created CONFIG_ACPI_TABLES_LIB in order to allow
the common code to be independent. 0-day seems to be happy now for all the
different configs and archs.

1/4: Split out the common code from drivers/acpi/tables.c to lib/fw_table.c
2/4: Add CDAT support
3,4/4: These two are minor patches that has ACPICA impact. Has been merged into
       the ACPICA git repo [3].

The whole series is at [2] for convenience.

[1]: https://lore.kernel.org/linux-cxl/168193556660.1178687.15477509915255912089.stgit@djiang5-mobl3/T/#t
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-qtg
[3]: https://github.com/acpica/acpica/pull/874

---

Dave Jiang (4):
      acpi: Move common tables helper functions to common lib
      lib/firmware_table: tables: Add CDAT table parsing support
      acpi: fix misnamed define for CDAT DSMAS
      acpi: Add defines for CDAT SSLBIS


 drivers/acpi/Kconfig     |   1 +
 drivers/acpi/tables.c    | 178 +----------------------------
 include/acpi/actbl1.h    |   5 +-
 include/linux/acpi.h     |  22 +---
 include/linux/fw_table.h |  52 +++++++++
 lib/Kconfig              |   3 +
 lib/Makefile             |   2 +
 lib/fw_table.c           | 236 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 302 insertions(+), 197 deletions(-)
 create mode 100644 include/linux/fw_table.h
 create mode 100644 lib/fw_table.c

--


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/4] acpi: Move common tables helper functions to common lib
  2023-06-01 21:31 [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code Dave Jiang
@ 2023-06-01 21:31 ` Dave Jiang
  2023-06-02 12:35   ` Jonathan Cameron
  2023-06-01 21:31 ` [PATCH v3 2/4] lib/firmware_table: tables: Add CDAT table parsing support Dave Jiang
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Dave Jiang @ 2023-06-01 21:31 UTC (permalink / raw
  To: linux-acpi, linux-cxl
  Cc: Rafael J. Wysocki, rafael, lenb, dan.j.williams, ira.weiny,
	vishal.l.verma, alison.schofield, lukas, Jonathan.Cameron

Some of the routines in ACPI driver/acpi/tables.c can be shared with
parsing CDAT. CDAT is a device-provided data structure that is formatted
similar to a platform provided ACPI table. CDAT is used by CXL and can
exist on platforms that do not use ACPI. Split out the common routine
from ACPI to accommodate platforms that do not support ACPI and move that
to /lib. The common routines can be built outside of ACPI if
FIRMWARE_TABLES is selected.

Link: https://lore.kernel.org/linux-cxl/CAJZ5v0jipbtTNnsA0-o5ozOk8ZgWnOg34m34a9pPenTyRLj=6A@mail.gmail.com/
Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
---
 drivers/acpi/Kconfig     |    1 
 drivers/acpi/tables.c    |  173 ------------------------------------------
 include/linux/acpi.h     |   22 -----
 include/linux/fw_table.h |   43 ++++++++++
 lib/Kconfig              |    3 +
 lib/Makefile             |    2 
 lib/fw_table.c           |  189 ++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 239 insertions(+), 194 deletions(-)
 create mode 100644 include/linux/fw_table.h
 create mode 100644 lib/fw_table.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index ccbeab9500ec..320d36b2ec78 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -12,6 +12,7 @@ menuconfig ACPI
 	select PNP
 	select NLS
 	select CRC32
+	select FIRMWARE_TABLE
 	default y if X86
 	help
 	  Advanced Configuration and Power Interface (ACPI) support for 
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 7b4680da57d7..cfc76efd8788 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -37,18 +37,6 @@ static struct acpi_table_desc initial_tables[ACPI_MAX_TABLES] __initdata;
 
 static int acpi_apic_instance __initdata_or_acpilib;
 
-enum acpi_subtable_type {
-	ACPI_SUBTABLE_COMMON,
-	ACPI_SUBTABLE_HMAT,
-	ACPI_SUBTABLE_PRMT,
-	ACPI_SUBTABLE_CEDT,
-};
-
-struct acpi_subtable_entry {
-	union acpi_subtable_headers *hdr;
-	enum acpi_subtable_type type;
-};
-
 /*
  * Disable table checksum verification for the early stage due to the size
  * limitation of the current x86 early mapping implementation.
@@ -227,167 +215,6 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header)
 	}
 }
 
-static unsigned long __init_or_acpilib
-acpi_get_entry_type(struct acpi_subtable_entry *entry)
-{
-	switch (entry->type) {
-	case ACPI_SUBTABLE_COMMON:
-		return entry->hdr->common.type;
-	case ACPI_SUBTABLE_HMAT:
-		return entry->hdr->hmat.type;
-	case ACPI_SUBTABLE_PRMT:
-		return 0;
-	case ACPI_SUBTABLE_CEDT:
-		return entry->hdr->cedt.type;
-	}
-	return 0;
-}
-
-static unsigned long __init_or_acpilib
-acpi_get_entry_length(struct acpi_subtable_entry *entry)
-{
-	switch (entry->type) {
-	case ACPI_SUBTABLE_COMMON:
-		return entry->hdr->common.length;
-	case ACPI_SUBTABLE_HMAT:
-		return entry->hdr->hmat.length;
-	case ACPI_SUBTABLE_PRMT:
-		return entry->hdr->prmt.length;
-	case ACPI_SUBTABLE_CEDT:
-		return entry->hdr->cedt.length;
-	}
-	return 0;
-}
-
-static unsigned long __init_or_acpilib
-acpi_get_subtable_header_length(struct acpi_subtable_entry *entry)
-{
-	switch (entry->type) {
-	case ACPI_SUBTABLE_COMMON:
-		return sizeof(entry->hdr->common);
-	case ACPI_SUBTABLE_HMAT:
-		return sizeof(entry->hdr->hmat);
-	case ACPI_SUBTABLE_PRMT:
-		return sizeof(entry->hdr->prmt);
-	case ACPI_SUBTABLE_CEDT:
-		return sizeof(entry->hdr->cedt);
-	}
-	return 0;
-}
-
-static enum acpi_subtable_type __init_or_acpilib
-acpi_get_subtable_type(char *id)
-{
-	if (strncmp(id, ACPI_SIG_HMAT, 4) == 0)
-		return ACPI_SUBTABLE_HMAT;
-	if (strncmp(id, ACPI_SIG_PRMT, 4) == 0)
-		return ACPI_SUBTABLE_PRMT;
-	if (strncmp(id, ACPI_SIG_CEDT, 4) == 0)
-		return ACPI_SUBTABLE_CEDT;
-	return ACPI_SUBTABLE_COMMON;
-}
-
-static __init_or_acpilib bool has_handler(struct acpi_subtable_proc *proc)
-{
-	return proc->handler || proc->handler_arg;
-}
-
-static __init_or_acpilib int call_handler(struct acpi_subtable_proc *proc,
-					  union acpi_subtable_headers *hdr,
-					  unsigned long end)
-{
-	if (proc->handler)
-		return proc->handler(hdr, end);
-	if (proc->handler_arg)
-		return proc->handler_arg(hdr, proc->arg, end);
-	return -EINVAL;
-}
-
-/**
- * acpi_parse_entries_array - for each proc_num find a suitable subtable
- *
- * @id: table id (for debugging purposes)
- * @table_size: size of the root table
- * @table_header: where does the table start?
- * @proc: array of acpi_subtable_proc struct containing entry id
- *        and associated handler with it
- * @proc_num: how big proc is?
- * @max_entries: how many entries can we process?
- *
- * For each proc_num find a subtable with proc->id and run proc->handler
- * on it. Assumption is that there's only single handler for particular
- * entry id.
- *
- * The table_size is not the size of the complete ACPI table (the length
- * field in the header struct), but only the size of the root table; i.e.,
- * the offset from the very first byte of the complete ACPI table, to the
- * first byte of the very first subtable.
- *
- * On success returns sum of all matching entries for all proc handlers.
- * Otherwise, -ENODEV or -EINVAL is returned.
- */
-static int __init_or_acpilib acpi_parse_entries_array(
-	char *id, unsigned long table_size,
-	struct acpi_table_header *table_header, struct acpi_subtable_proc *proc,
-	int proc_num, unsigned int max_entries)
-{
-	struct acpi_subtable_entry entry;
-	unsigned long table_end, subtable_len, entry_len;
-	int count = 0;
-	int errs = 0;
-	int i;
-
-	table_end = (unsigned long)table_header + table_header->length;
-
-	/* Parse all entries looking for a match. */
-
-	entry.type = acpi_get_subtable_type(id);
-	entry.hdr = (union acpi_subtable_headers *)
-	    ((unsigned long)table_header + table_size);
-	subtable_len = acpi_get_subtable_header_length(&entry);
-
-	while (((unsigned long)entry.hdr) + subtable_len  < table_end) {
-		if (max_entries && count >= max_entries)
-			break;
-
-		for (i = 0; i < proc_num; i++) {
-			if (acpi_get_entry_type(&entry) != proc[i].id)
-				continue;
-			if (!has_handler(&proc[i]) ||
-			    (!errs &&
-			     call_handler(&proc[i], entry.hdr, table_end))) {
-				errs++;
-				continue;
-			}
-
-			proc[i].count++;
-			break;
-		}
-		if (i != proc_num)
-			count++;
-
-		/*
-		 * If entry->length is 0, break from this loop to avoid
-		 * infinite loop.
-		 */
-		entry_len = acpi_get_entry_length(&entry);
-		if (entry_len == 0) {
-			pr_err("[%4.4s:0x%02x] Invalid zero length\n", id, proc->id);
-			return -EINVAL;
-		}
-
-		entry.hdr = (union acpi_subtable_headers *)
-		    ((unsigned long)entry.hdr + entry_len);
-	}
-
-	if (max_entries && count > max_entries) {
-		pr_warn("[%4.4s:0x%02x] found the maximum %i entries\n",
-			id, proc->id, count);
-	}
-
-	return errs ? -EINVAL : count;
-}
-
 int __init_or_acpilib acpi_table_parse_entries_array(
 	char *id, unsigned long table_size, struct acpi_subtable_proc *proc,
 	int proc_num, unsigned int max_entries)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 7b71dd74baeb..57004cd0a7a3 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -15,6 +15,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/property.h>
 #include <linux/uuid.h>
+#include <linux/fw_table.h>
 
 struct irq_domain;
 struct irq_domain_ops;
@@ -132,21 +133,8 @@ enum acpi_address_range_id {
 
 
 /* Table Handlers */
-union acpi_subtable_headers {
-	struct acpi_subtable_header common;
-	struct acpi_hmat_structure hmat;
-	struct acpi_prmt_module_header prmt;
-	struct acpi_cedt_header cedt;
-};
-
 typedef int (*acpi_tbl_table_handler)(struct acpi_table_header *table);
 
-typedef int (*acpi_tbl_entry_handler)(union acpi_subtable_headers *header,
-				      const unsigned long end);
-
-typedef int (*acpi_tbl_entry_handler_arg)(union acpi_subtable_headers *header,
-					  void *arg, const unsigned long end);
-
 /* Debugger support */
 
 struct acpi_debugger_ops {
@@ -220,14 +208,6 @@ static inline int acpi_debugger_notify_command_complete(void)
 		(!entry) || (unsigned long)entry + sizeof(*entry) > end ||  \
 		((struct acpi_subtable_header *)entry)->length < sizeof(*entry))
 
-struct acpi_subtable_proc {
-	int id;
-	acpi_tbl_entry_handler handler;
-	acpi_tbl_entry_handler_arg handler_arg;
-	void *arg;
-	int count;
-};
-
 void __iomem *__acpi_map_table(unsigned long phys, unsigned long size);
 void __acpi_unmap_table(void __iomem *map, unsigned long size);
 int early_acpi_boot_init(void);
diff --git a/include/linux/fw_table.h b/include/linux/fw_table.h
new file mode 100644
index 000000000000..ff8fa58d5818
--- /dev/null
+++ b/include/linux/fw_table.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *  fw_tables.h - Parsing support for ACPI and ACPI-like tables provided by
+ *                platform or device firmware
+ *
+ *  Copyright (C) 2001 Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
+ *  Copyright (C) 2023 Intel Corp.
+ */
+#ifndef _FW_TABLE_H_
+#define _FW_TABLE_H_
+
+union acpi_subtable_headers;
+
+typedef int (*acpi_tbl_entry_handler)(union acpi_subtable_headers *header,
+				      const unsigned long end);
+
+typedef int (*acpi_tbl_entry_handler_arg)(union acpi_subtable_headers *header,
+					  void *arg, const unsigned long end);
+
+struct acpi_subtable_proc {
+	int id;
+	acpi_tbl_entry_handler handler;
+	acpi_tbl_entry_handler_arg handler_arg;
+	void *arg;
+	int count;
+};
+
+#include <linux/acpi.h>
+#include <acpi/acpi.h>
+
+union acpi_subtable_headers {
+	struct acpi_subtable_header common;
+	struct acpi_hmat_structure hmat;
+	struct acpi_prmt_module_header prmt;
+	struct acpi_cedt_header cedt;
+};
+
+int acpi_parse_entries_array(char *id, unsigned long table_size,
+			     struct acpi_table_header *table_header,
+			     struct acpi_subtable_proc *proc,
+			     int proc_num, unsigned int max_entries);
+
+#endif
diff --git a/lib/Kconfig b/lib/Kconfig
index 5c2da561c516..b03714d45444 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -763,3 +763,6 @@ config ASN1_ENCODER
 
 config POLYNOMIAL
        tristate
+
+config FIRMWARE_TABLE
+	bool
diff --git a/lib/Makefile b/lib/Makefile
index 876fcdeae34e..16e238baef4b 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -397,6 +397,8 @@ obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
 
 obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
 
+obj-$(CONFIG_FIRMWARE_TABLE) += fw_table.o
+
 # FORTIFY_SOURCE compile-time behavior tests
 TEST_FORTIFY_SRCS = $(wildcard $(srctree)/$(src)/test_fortify/*-*.c)
 TEST_FORTIFY_LOGS = $(patsubst $(srctree)/$(src)/%.c, %.log, $(TEST_FORTIFY_SRCS))
diff --git a/lib/fw_table.c b/lib/fw_table.c
new file mode 100644
index 000000000000..e84bd0866e10
--- /dev/null
+++ b/lib/fw_table.c
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  fw_tables.c - Parsing support for ACPI and ACPI-like tables provided by
+ *                platform or device firmware
+ *
+ *  Copyright (C) 2001 Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
+ *  Copyright (C) 2023 Intel Corp.
+ */
+#include <linux/errno.h>
+#include <linux/fw_table.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+enum acpi_subtable_type {
+	ACPI_SUBTABLE_COMMON,
+	ACPI_SUBTABLE_HMAT,
+	ACPI_SUBTABLE_PRMT,
+	ACPI_SUBTABLE_CEDT,
+};
+
+struct acpi_subtable_entry {
+	union acpi_subtable_headers *hdr;
+	enum acpi_subtable_type type;
+};
+
+static unsigned long __init_or_acpilib
+acpi_get_entry_type(struct acpi_subtable_entry *entry)
+{
+	switch (entry->type) {
+	case ACPI_SUBTABLE_COMMON:
+		return entry->hdr->common.type;
+	case ACPI_SUBTABLE_HMAT:
+		return entry->hdr->hmat.type;
+	case ACPI_SUBTABLE_PRMT:
+		return 0;
+	case ACPI_SUBTABLE_CEDT:
+		return entry->hdr->cedt.type;
+	}
+	return 0;
+}
+
+static unsigned long __init_or_acpilib
+acpi_get_entry_length(struct acpi_subtable_entry *entry)
+{
+	switch (entry->type) {
+	case ACPI_SUBTABLE_COMMON:
+		return entry->hdr->common.length;
+	case ACPI_SUBTABLE_HMAT:
+		return entry->hdr->hmat.length;
+	case ACPI_SUBTABLE_PRMT:
+		return entry->hdr->prmt.length;
+	case ACPI_SUBTABLE_CEDT:
+		return entry->hdr->cedt.length;
+	}
+	return 0;
+}
+
+static unsigned long __init_or_acpilib
+acpi_get_subtable_header_length(struct acpi_subtable_entry *entry)
+{
+	switch (entry->type) {
+	case ACPI_SUBTABLE_COMMON:
+		return sizeof(entry->hdr->common);
+	case ACPI_SUBTABLE_HMAT:
+		return sizeof(entry->hdr->hmat);
+	case ACPI_SUBTABLE_PRMT:
+		return sizeof(entry->hdr->prmt);
+	case ACPI_SUBTABLE_CEDT:
+		return sizeof(entry->hdr->cedt);
+	}
+	return 0;
+}
+
+static enum acpi_subtable_type __init_or_acpilib
+acpi_get_subtable_type(char *id)
+{
+	if (strncmp(id, ACPI_SIG_HMAT, 4) == 0)
+		return ACPI_SUBTABLE_HMAT;
+	if (strncmp(id, ACPI_SIG_PRMT, 4) == 0)
+		return ACPI_SUBTABLE_PRMT;
+	if (strncmp(id, ACPI_SIG_CEDT, 4) == 0)
+		return ACPI_SUBTABLE_CEDT;
+	return ACPI_SUBTABLE_COMMON;
+}
+
+static __init_or_acpilib bool has_handler(struct acpi_subtable_proc *proc)
+{
+	return proc->handler || proc->handler_arg;
+}
+
+static __init_or_acpilib int call_handler(struct acpi_subtable_proc *proc,
+					  union acpi_subtable_headers *hdr,
+					  unsigned long end)
+{
+	if (proc->handler)
+		return proc->handler(hdr, end);
+	if (proc->handler_arg)
+		return proc->handler_arg(hdr, proc->arg, end);
+	return -EINVAL;
+}
+
+/**
+ * acpi_parse_entries_array - for each proc_num find a suitable subtable
+ *
+ * @id: table id (for debugging purposes)
+ * @table_size: size of the root table
+ * @table_header: where does the table start?
+ * @proc: array of acpi_subtable_proc struct containing entry id
+ *        and associated handler with it
+ * @proc_num: how big proc is?
+ * @max_entries: how many entries can we process?
+ *
+ * For each proc_num find a subtable with proc->id and run proc->handler
+ * on it. Assumption is that there's only single handler for particular
+ * entry id.
+ *
+ * The table_size is not the size of the complete ACPI table (the length
+ * field in the header struct), but only the size of the root table; i.e.,
+ * the offset from the very first byte of the complete ACPI table, to the
+ * first byte of the very first subtable.
+ *
+ * On success returns sum of all matching entries for all proc handlers.
+ * Otherwise, -ENODEV or -EINVAL is returned.
+ */
+int __init_or_acpilib
+acpi_parse_entries_array(char *id, unsigned long table_size,
+			 struct acpi_table_header *table_header,
+			 struct acpi_subtable_proc *proc,
+			 int proc_num, unsigned int max_entries)
+{
+	unsigned long table_end, subtable_len, entry_len;
+	struct acpi_subtable_entry entry;
+	int count = 0;
+	int errs = 0;
+	int i;
+
+	table_end = (unsigned long)table_header + table_header->length;
+
+	/* Parse all entries looking for a match. */
+
+	entry.type = acpi_get_subtable_type(id);
+	entry.hdr = (union acpi_subtable_headers *)
+	    ((unsigned long)table_header + table_size);
+	subtable_len = acpi_get_subtable_header_length(&entry);
+
+	while (((unsigned long)entry.hdr) + subtable_len  < table_end) {
+		if (max_entries && count >= max_entries)
+			break;
+
+		for (i = 0; i < proc_num; i++) {
+			if (acpi_get_entry_type(&entry) != proc[i].id)
+				continue;
+			if (!has_handler(&proc[i]) ||
+			    (!errs &&
+			     call_handler(&proc[i], entry.hdr, table_end))) {
+				errs++;
+				continue;
+			}
+
+			proc[i].count++;
+			break;
+		}
+		if (i != proc_num)
+			count++;
+
+		/*
+		 * If entry->length is 0, break from this loop to avoid
+		 * infinite loop.
+		 */
+		entry_len = acpi_get_entry_length(&entry);
+		if (entry_len == 0) {
+			pr_err("[%4.4s:0x%02x] Invalid zero length\n", id, proc->id);
+			return -EINVAL;
+		}
+
+		entry.hdr = (union acpi_subtable_headers *)
+		    ((unsigned long)entry.hdr + entry_len);
+	}
+
+	if (max_entries && count > max_entries) {
+		pr_warn("[%4.4s:0x%02x] found the maximum %i entries\n",
+			id, proc->id, count);
+	}
+
+	return errs ? -EINVAL : count;
+}
+EXPORT_SYMBOL_GPL(acpi_parse_entries_array);



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 2/4] lib/firmware_table: tables: Add CDAT table parsing support
  2023-06-01 21:31 [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code Dave Jiang
  2023-06-01 21:31 ` [PATCH v3 1/4] acpi: Move common tables helper functions to common lib Dave Jiang
@ 2023-06-01 21:31 ` Dave Jiang
  2023-06-02 12:36   ` Jonathan Cameron
  2023-06-01 21:32 ` [PATCH v3 3/4] acpi: fix misnamed define for CDAT DSMAS Dave Jiang
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Dave Jiang @ 2023-06-01 21:31 UTC (permalink / raw
  To: linux-acpi, linux-cxl
  Cc: Rafael J. Wysocki, Len Brown, rafael, lenb, dan.j.williams,
	ira.weiny, vishal.l.verma, alison.schofield, lukas,
	Jonathan.Cameron

The CDAT table is very similar to ACPI tables when it comes to sub-table
and entry structures. The helper functions can be also used to parse the
CDAT table. Add support to the helper functions to deal with an external
CDAT table, and also handle the endieness since CDAT can be processed by a
BE host. Export a function cdat_table_parse() for CXL driver to parse
a CDAT table.

In order to minimize ACPICA code changes, __force is being utilized to deal
with the case of a big endian (BE) host parsing a CDAT. All CDAT data
structure variables are being force casted to __leX as appropriate.

Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v3:
Rafael,
Attempted to rename acpi_ prefix to something common but that ended up
causing a significant amount of ACPI code changes. I'll leave that alone
unless you'd like me to go that direction.

v2:
- Make acpi_table_header and acpi_table_cdat a union. (Jonathan)
- Use local var to make acpi_table_get_length() more readable. (Jonathan)
- Remove ACPI_SIG_CDAT define, already defined.
---
 drivers/acpi/tables.c    |    5 +++-
 include/linux/fw_table.h |   11 +++++++++-
 lib/fw_table.c           |   53 +++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index cfc76efd8788..d4c7a76a0fce 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -241,8 +241,9 @@ int __init_or_acpilib acpi_table_parse_entries_array(
 		return -ENODEV;
 	}
 
-	count = acpi_parse_entries_array(id, table_size, table_header,
-			proc, proc_num, max_entries);
+	count = acpi_parse_entries_array(id, table_size,
+					 (union fw_table_header *)table_header,
+					 proc, proc_num, max_entries);
 
 	acpi_put_table(table_header);
 	return count;
diff --git a/include/linux/fw_table.h b/include/linux/fw_table.h
index ff8fa58d5818..a97d368df520 100644
--- a/include/linux/fw_table.h
+++ b/include/linux/fw_table.h
@@ -28,16 +28,25 @@ struct acpi_subtable_proc {
 #include <linux/acpi.h>
 #include <acpi/acpi.h>
 
+union fw_table_header {
+	struct acpi_table_header acpi;
+	struct acpi_table_cdat cdat;
+};
+
 union acpi_subtable_headers {
 	struct acpi_subtable_header common;
 	struct acpi_hmat_structure hmat;
 	struct acpi_prmt_module_header prmt;
 	struct acpi_cedt_header cedt;
+	struct acpi_cdat_header cdat;
 };
 
 int acpi_parse_entries_array(char *id, unsigned long table_size,
-			     struct acpi_table_header *table_header,
+			     union fw_table_header *table_header,
 			     struct acpi_subtable_proc *proc,
 			     int proc_num, unsigned int max_entries);
 
+int cdat_table_parse(enum acpi_cdat_type type,
+		     acpi_tbl_entry_handler_arg handler_arg, void *arg,
+		     struct acpi_table_cdat *table_header);
 #endif
diff --git a/lib/fw_table.c b/lib/fw_table.c
index e84bd0866e10..abf71bdb109a 100644
--- a/lib/fw_table.c
+++ b/lib/fw_table.c
@@ -18,6 +18,7 @@ enum acpi_subtable_type {
 	ACPI_SUBTABLE_HMAT,
 	ACPI_SUBTABLE_PRMT,
 	ACPI_SUBTABLE_CEDT,
+	CDAT_SUBTABLE,
 };
 
 struct acpi_subtable_entry {
@@ -37,6 +38,8 @@ acpi_get_entry_type(struct acpi_subtable_entry *entry)
 		return 0;
 	case ACPI_SUBTABLE_CEDT:
 		return entry->hdr->cedt.type;
+	case CDAT_SUBTABLE:
+		return entry->hdr->cdat.type;
 	}
 	return 0;
 }
@@ -53,6 +56,10 @@ acpi_get_entry_length(struct acpi_subtable_entry *entry)
 		return entry->hdr->prmt.length;
 	case ACPI_SUBTABLE_CEDT:
 		return entry->hdr->cedt.length;
+	case CDAT_SUBTABLE:
+		__le16 length = (__force __le16)entry->hdr->cdat.length;
+
+		return le16_to_cpu(length);
 	}
 	return 0;
 }
@@ -69,6 +76,8 @@ acpi_get_subtable_header_length(struct acpi_subtable_entry *entry)
 		return sizeof(entry->hdr->prmt);
 	case ACPI_SUBTABLE_CEDT:
 		return sizeof(entry->hdr->cedt);
+	case CDAT_SUBTABLE:
+		return sizeof(entry->hdr->cdat);
 	}
 	return 0;
 }
@@ -82,9 +91,24 @@ acpi_get_subtable_type(char *id)
 		return ACPI_SUBTABLE_PRMT;
 	if (strncmp(id, ACPI_SIG_CEDT, 4) == 0)
 		return ACPI_SUBTABLE_CEDT;
+	if (strncmp(id, ACPI_SIG_CDAT, 4) == 0)
+		return CDAT_SUBTABLE;
 	return ACPI_SUBTABLE_COMMON;
 }
 
+static unsigned long __init_or_acpilib
+acpi_table_get_length(enum acpi_subtable_type type,
+		      union fw_table_header *header)
+{
+	if (type == CDAT_SUBTABLE) {
+		__le32 length = (__force __le32)header->cdat.length;
+
+		return le32_to_cpu(length);
+	}
+
+	return header->acpi.length;
+}
+
 static __init_or_acpilib bool has_handler(struct acpi_subtable_proc *proc)
 {
 	return proc->handler || proc->handler_arg;
@@ -126,21 +150,24 @@ static __init_or_acpilib int call_handler(struct acpi_subtable_proc *proc,
  */
 int __init_or_acpilib
 acpi_parse_entries_array(char *id, unsigned long table_size,
-			 struct acpi_table_header *table_header,
+			 union fw_table_header *table_header,
 			 struct acpi_subtable_proc *proc,
 			 int proc_num, unsigned int max_entries)
 {
 	unsigned long table_end, subtable_len, entry_len;
 	struct acpi_subtable_entry entry;
+	enum acpi_subtable_type type;
 	int count = 0;
 	int errs = 0;
 	int i;
 
-	table_end = (unsigned long)table_header + table_header->length;
+	type = acpi_get_subtable_type(id);
+	table_end = (unsigned long)table_header +
+		    acpi_table_get_length(type, table_header);
 
 	/* Parse all entries looking for a match. */
 
-	entry.type = acpi_get_subtable_type(id);
+	entry.type = type;
 	entry.hdr = (union acpi_subtable_headers *)
 	    ((unsigned long)table_header + table_size);
 	subtable_len = acpi_get_subtable_header_length(&entry);
@@ -187,3 +214,23 @@ acpi_parse_entries_array(char *id, unsigned long table_size,
 	return errs ? -EINVAL : count;
 }
 EXPORT_SYMBOL_GPL(acpi_parse_entries_array);
+
+int cdat_table_parse(enum acpi_cdat_type type,
+		     acpi_tbl_entry_handler_arg handler_arg, void *arg,
+		     struct acpi_table_cdat *table_header)
+{
+	struct acpi_subtable_proc proc = {
+		.id		= type,
+		.handler_arg	= handler_arg,
+		.arg		= arg,
+	};
+
+	if (!table_header)
+		return -EINVAL;
+
+	return acpi_parse_entries_array(ACPI_SIG_CDAT,
+					sizeof(struct acpi_table_cdat),
+					(union fw_table_header *)table_header,
+					&proc, 1, 0);
+}
+EXPORT_SYMBOL_NS_GPL(cdat_table_parse, CXL);



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 3/4] acpi: fix misnamed define for CDAT DSMAS
  2023-06-01 21:31 [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code Dave Jiang
  2023-06-01 21:31 ` [PATCH v3 1/4] acpi: Move common tables helper functions to common lib Dave Jiang
  2023-06-01 21:31 ` [PATCH v3 2/4] lib/firmware_table: tables: Add CDAT table parsing support Dave Jiang
@ 2023-06-01 21:32 ` Dave Jiang
  2023-06-02 12:38   ` Jonathan Cameron
  2023-06-01 21:32 ` [PATCH v3 4/4] acpi: Add defines for CDAT SSLBIS Dave Jiang
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Dave Jiang @ 2023-06-01 21:32 UTC (permalink / raw
  To: linux-acpi, linux-cxl
  Cc: Rafael J. Wysocki, Len Brown, rafael, lenb, dan.j.williams,
	ira.weiny, vishal.l.verma, alison.schofield, lukas,
	Jonathan.Cameron

ACPI_CEDT_DSMAS_NON_VOLATILE should be defined as
ACPI_CDAT_DSMAS_NON_VOLATILE. Fix misspelled define.

Links: https://github.com/acpica/acpica/pull/874
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
---
 include/acpi/actbl1.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 58b0490a2ad1..8d5572ad48cb 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -402,7 +402,7 @@ struct acpi_cdat_dsmas {
 
 /* Flags for subtable above */
 
-#define ACPI_CEDT_DSMAS_NON_VOLATILE        (1 << 2)
+#define ACPI_CDAT_DSMAS_NON_VOLATILE        (1 << 2)
 
 /* Subtable 1: Device scoped Latency and Bandwidth Information Structure (DSLBIS) */
 



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 4/4] acpi: Add defines for CDAT SSLBIS
  2023-06-01 21:31 [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code Dave Jiang
                   ` (2 preceding siblings ...)
  2023-06-01 21:32 ` [PATCH v3 3/4] acpi: fix misnamed define for CDAT DSMAS Dave Jiang
@ 2023-06-01 21:32 ` Dave Jiang
  2023-06-02 12:38   ` Jonathan Cameron
  2023-06-04 16:09 ` [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code Rafael J. Wysocki
  2023-06-06  1:36 ` Hanjun Guo
  5 siblings, 1 reply; 14+ messages in thread
From: Dave Jiang @ 2023-06-01 21:32 UTC (permalink / raw
  To: linux-acpi, linux-cxl
  Cc: rafael, lenb, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, lukas, Jonathan.Cameron

Add upstream port and any port definition for SSLBIS.

Links: https://github.com/acpica/acpica/pull/874
Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
---
 include/acpi/actbl1.h |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 8d5572ad48cb..a33375e055ad 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -465,6 +465,9 @@ struct acpi_cdat_sslbe {
 	u16 reserved;
 };
 
+#define ACPI_CDAT_SSLBIS_US_PORT	0x0100
+#define ACPI_CDAT_SSLBIS_ANY_PORT	0xffff
+
 /*******************************************************************************
  *
  * CEDT - CXL Early Discovery Table



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] acpi: Move common tables helper functions to common lib
  2023-06-01 21:31 ` [PATCH v3 1/4] acpi: Move common tables helper functions to common lib Dave Jiang
@ 2023-06-02 12:35   ` Jonathan Cameron
  2023-06-12 20:47     ` Dave Jiang
  0 siblings, 1 reply; 14+ messages in thread
From: Jonathan Cameron @ 2023-06-02 12:35 UTC (permalink / raw
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, Rafael J. Wysocki, lenb, dan.j.williams,
	ira.weiny, vishal.l.verma, alison.schofield, lukas

On Thu, 01 Jun 2023 14:31:52 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Some of the routines in ACPI driver/acpi/tables.c can be shared with
> parsing CDAT. CDAT is a device-provided data structure that is formatted
> similar to a platform provided ACPI table. CDAT is used by CXL and can
> exist on platforms that do not use ACPI. Split out the common routine
> from ACPI to accommodate platforms that do not support ACPI and move that
> to /lib. The common routines can be built outside of ACPI if
> FIRMWARE_TABLES is selected.
> 
> Link: https://lore.kernel.org/linux-cxl/CAJZ5v0jipbtTNnsA0-o5ozOk8ZgWnOg34m34a9pPenTyRLj=6A@mail.gmail.com/
> Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Minor comment to fix inline. With that tidied up

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> diff --git a/include/linux/fw_table.h b/include/linux/fw_table.h
> new file mode 100644
> index 000000000000..ff8fa58d5818
> --- /dev/null
> +++ b/include/linux/fw_table.h
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + *  fw_tables.h - Parsing support for ACPI and ACPI-like tables provided by
> + *                platform or device firmware
> + *
> + *  Copyright (C) 2001 Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
> + *  Copyright (C) 2023 Intel Corp.
> + */
> +#ifndef _FW_TABLE_H_
> +#define _FW_TABLE_H_
> +
> +union acpi_subtable_headers;
> +
> +typedef int (*acpi_tbl_entry_handler)(union acpi_subtable_headers *header,
> +				      const unsigned long end);
> +
> +typedef int (*acpi_tbl_entry_handler_arg)(union acpi_subtable_headers *header,
> +					  void *arg, const unsigned long end);
> +
> +struct acpi_subtable_proc {
> +	int id;
> +	acpi_tbl_entry_handler handler;
> +	acpi_tbl_entry_handler_arg handler_arg;
> +	void *arg;
> +	int count;
> +};
> +
> +#include <linux/acpi.h>
> +#include <acpi/acpi.h>

Includes mid way down the files is not a common pattern and I can't see why
it's particularly useful to do so here.

+ linux/acpi.h includes acpi/acpi.h and I can't see that changing any time
soon...

> +
> +union acpi_subtable_headers {
> +	struct acpi_subtable_header common;
> +	struct acpi_hmat_structure hmat;
> +	struct acpi_prmt_module_header prmt;
> +	struct acpi_cedt_header cedt;
> +};
> +
> +int acpi_parse_entries_array(char *id, unsigned long table_size,
> +			     struct acpi_table_header *table_header,
> +			     struct acpi_subtable_proc *proc,
> +			     int proc_num, unsigned int max_entries);
> +
> +#endif



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] lib/firmware_table: tables: Add CDAT table parsing support
  2023-06-01 21:31 ` [PATCH v3 2/4] lib/firmware_table: tables: Add CDAT table parsing support Dave Jiang
@ 2023-06-02 12:36   ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2023-06-02 12:36 UTC (permalink / raw
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, Rafael J. Wysocki, Len Brown,
	dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	lukas

On Thu, 01 Jun 2023 14:31:58 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> The CDAT table is very similar to ACPI tables when it comes to sub-table
> and entry structures. The helper functions can be also used to parse the
> CDAT table. Add support to the helper functions to deal with an external
> CDAT table, and also handle the endieness since CDAT can be processed by a
> BE host. Export a function cdat_table_parse() for CXL driver to parse
> a CDAT table.
> 
> In order to minimize ACPICA code changes, __force is being utilized to deal
> with the case of a big endian (BE) host parsing a CDAT. All CDAT data
> structure variables are being force casted to __leX as appropriate.
> 
> Cc: Rafael J. Wysocki <rafael@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
>
LGTM

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/4] acpi: fix misnamed define for CDAT DSMAS
  2023-06-01 21:32 ` [PATCH v3 3/4] acpi: fix misnamed define for CDAT DSMAS Dave Jiang
@ 2023-06-02 12:38   ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2023-06-02 12:38 UTC (permalink / raw
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, Rafael J. Wysocki, Len Brown,
	dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	lukas

On Thu, 01 Jun 2023 14:32:05 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> ACPI_CEDT_DSMAS_NON_VOLATILE should be defined as
> ACPI_CDAT_DSMAS_NON_VOLATILE. Fix misspelled define.
> 
> Links: https://github.com/acpica/acpica/pull/874
> Cc: Rafael J. Wysocki <rafael@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Given question of acpica addressed by your link above
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> 
> ---
> ---
>  include/acpi/actbl1.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
> index 58b0490a2ad1..8d5572ad48cb 100644
> --- a/include/acpi/actbl1.h
> +++ b/include/acpi/actbl1.h
> @@ -402,7 +402,7 @@ struct acpi_cdat_dsmas {
>  
>  /* Flags for subtable above */
>  
> -#define ACPI_CEDT_DSMAS_NON_VOLATILE        (1 << 2)
> +#define ACPI_CDAT_DSMAS_NON_VOLATILE        (1 << 2)
>  
>  /* Subtable 1: Device scoped Latency and Bandwidth Information Structure (DSLBIS) */
>  
> 
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/4] acpi: Add defines for CDAT SSLBIS
  2023-06-01 21:32 ` [PATCH v3 4/4] acpi: Add defines for CDAT SSLBIS Dave Jiang
@ 2023-06-02 12:38   ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2023-06-02 12:38 UTC (permalink / raw
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, rafael, lenb, dan.j.williams, ira.weiny,
	vishal.l.verma, alison.schofield, lukas

On Thu, 01 Jun 2023 14:32:11 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Add upstream port and any port definition for SSLBIS.
> 
> Links: https://github.com/acpica/acpica/pull/874
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

FWIW given may come from acpica route anyway.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> 
> ---
> ---
>  include/acpi/actbl1.h |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
> index 8d5572ad48cb..a33375e055ad 100644
> --- a/include/acpi/actbl1.h
> +++ b/include/acpi/actbl1.h
> @@ -465,6 +465,9 @@ struct acpi_cdat_sslbe {
>  	u16 reserved;
>  };
>  
> +#define ACPI_CDAT_SSLBIS_US_PORT	0x0100
> +#define ACPI_CDAT_SSLBIS_ANY_PORT	0xffff
> +
>  /*******************************************************************************
>   *
>   * CEDT - CXL Early Discovery Table
> 
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code
  2023-06-01 21:31 [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code Dave Jiang
                   ` (3 preceding siblings ...)
  2023-06-01 21:32 ` [PATCH v3 4/4] acpi: Add defines for CDAT SSLBIS Dave Jiang
@ 2023-06-04 16:09 ` Rafael J. Wysocki
  2023-06-12 20:16   ` Dave Jiang
  2023-06-06  1:36 ` Hanjun Guo
  5 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2023-06-04 16:09 UTC (permalink / raw
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, Len Brown, Rafael J. Wysocki,
	dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	lukas, Jonathan.Cameron

On Thu, Jun 1, 2023 at 11:31 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
> v3:
> - Move common code to lib/fw_table.c
> v2:
> - Split out with CONFIG_ACPI_TABLES_LIB to be independent
> - Fixed 0-day issues
> - Change CDAT releveant names to prefix with cdat/CDAT instead of
>   acpi/ACPI. (Jonathan)
> - Make table_header a union with cdat table header instead of
>   'acpi_table_header'. (Jonathan)
> - Removed ACPI_SIG_CDAT, already defined.
>
> Hi Rafael,
> Please consider ack these patches. Dan can take these through the CXL tree. After
> attempting to rename the cxl_ prefixes of functions and non ACPICA data structures
> to something more common, it seems that significant amount of ACPI code would be
> touched for the rename. For this series I left it alone in order to have the minimal
> changes to ACPI code.
>
> I've broken out the "cxl: Add support for QTG ID retrieval for CXL subsystem" [1]
> series in order to make it more manageable. Here's the first part of the ACPI
> changes. These changes are added to allow reuse of ACPI tables code to parse
> the CDAT tables. While CDAT is not part of ACPI, the table structures are similar
> to ACPI layouts that the code can be reused with some small modifications.
>
> However, in order to be properly utilized by CXL users, the tables code needs
> to be refactored out to be independent of ACPI. For example, a PPC BE host may
> have CXL and does not have ACPI support. But it will have CDAT to read from
> devices and switches. I have created CONFIG_ACPI_TABLES_LIB in order to allow
> the common code to be independent. 0-day seems to be happy now for all the
> different configs and archs.
>
> 1/4: Split out the common code from drivers/acpi/tables.c to lib/fw_table.c
> 2/4: Add CDAT support
> 3,4/4: These two are minor patches that has ACPICA impact. Has been merged into
>        the ACPICA git repo [3].
>
> The whole series is at [2] for convenience.
>
> [1]: https://lore.kernel.org/linux-cxl/168193556660.1178687.15477509915255912089.stgit@djiang5-mobl3/T/#t
> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-qtg
> [3]: https://github.com/acpica/acpica/pull/874
>
> ---
>
> Dave Jiang (4):
>       acpi: Move common tables helper functions to common lib
>       lib/firmware_table: tables: Add CDAT table parsing support
>       acpi: fix misnamed define for CDAT DSMAS
>       acpi: Add defines for CDAT SSLBIS
>
>
>  drivers/acpi/Kconfig     |   1 +
>  drivers/acpi/tables.c    | 178 +----------------------------
>  include/acpi/actbl1.h    |   5 +-
>  include/linux/acpi.h     |  22 +---
>  include/linux/fw_table.h |  52 +++++++++
>  lib/Kconfig              |   3 +
>  lib/Makefile             |   2 +
>  lib/fw_table.c           | 236 +++++++++++++++++++++++++++++++++++++++
>  8 files changed, 302 insertions(+), 197 deletions(-)
>  create mode 100644 include/linux/fw_table.h
>  create mode 100644 lib/fw_table.c
>
> --

I think that this series can go in via the CXL tree and I'm expecting
ACPICA to make a new release including the counterparts of patches
[3-4/4] shortly.

Please feel free to add

Acled-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

to the series.

Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code
  2023-06-01 21:31 [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code Dave Jiang
                   ` (4 preceding siblings ...)
  2023-06-04 16:09 ` [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code Rafael J. Wysocki
@ 2023-06-06  1:36 ` Hanjun Guo
  2023-06-12 20:17   ` Dave Jiang
  5 siblings, 1 reply; 14+ messages in thread
From: Hanjun Guo @ 2023-06-06  1:36 UTC (permalink / raw
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: Len Brown, Rafael J. Wysocki, dan.j.williams, ira.weiny,
	vishal.l.verma, alison.schofield, lukas, Jonathan.Cameron

On 2023/6/2 5:31, Dave Jiang wrote:
> v3:
> - Move common code to lib/fw_table.c
> v2:
> - Split out with CONFIG_ACPI_TABLES_LIB to be independent
> - Fixed 0-day issues
> - Change CDAT releveant names to prefix with cdat/CDAT instead of
>    acpi/ACPI. (Jonathan)
> - Make table_header a union with cdat table header instead of
>    'acpi_table_header'. (Jonathan)
> - Removed ACPI_SIG_CDAT, already defined.
> 
> Hi Rafael,
> Please consider ack these patches. Dan can take these through the CXL tree. After
> attempting to rename the cxl_ prefixes of functions and non ACPICA data structures
> to something more common, it seems that significant amount of ACPI code would be
> touched for the rename. For this series I left it alone in order to have the minimal
> changes to ACPI code.
> 
> I've broken out the "cxl: Add support for QTG ID retrieval for CXL subsystem" [1]
> series in order to make it more manageable. Here's the first part of the ACPI
> changes. These changes are added to allow reuse of ACPI tables code to parse
> the CDAT tables. While CDAT is not part of ACPI, the table structures are similar
> to ACPI layouts that the code can be reused with some small modifications.
> 
> However, in order to be properly utilized by CXL users, the tables code needs
> to be refactored out to be independent of ACPI. For example, a PPC BE host may
> have CXL and does not have ACPI support. But it will have CDAT to read from
> devices and switches. I have created CONFIG_ACPI_TABLES_LIB in order to allow
> the common code to be independent. 0-day seems to be happy now for all the
> different configs and archs.
> 
> 1/4: Split out the common code from drivers/acpi/tables.c to lib/fw_table.c
> 2/4: Add CDAT support
> 3,4/4: These two are minor patches that has ACPICA impact. Has been merged into
>         the ACPICA git repo [3].
> 
> The whole series is at [2] for convenience.
> 
> [1]: https://lore.kernel.org/linux-cxl/168193556660.1178687.15477509915255912089.stgit@djiang5-mobl3/T/#t
> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-qtg
> [3]: https://github.com/acpica/acpica/pull/874
> 
> ---
> 
> Dave Jiang (4):
>        acpi: Move common tables helper functions to common lib
>        lib/firmware_table: tables: Add CDAT table parsing support
>        acpi: fix misnamed define for CDAT DSMAS
>        acpi: Add defines for CDAT SSLBIS
> 
> 
>   drivers/acpi/Kconfig     |   1 +
>   drivers/acpi/tables.c    | 178 +----------------------------
>   include/acpi/actbl1.h    |   5 +-
>   include/linux/acpi.h     |  22 +---
>   include/linux/fw_table.h |  52 +++++++++
>   lib/Kconfig              |   3 +
>   lib/Makefile             |   2 +
>   lib/fw_table.c           | 236 +++++++++++++++++++++++++++++++++++++++

Who will maintain this file? since it's the core function of parsing
ACPI tables, I would like the update of this file in the future will
Cc ACPI mailing list.

Thanks
Hanjun

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code
  2023-06-04 16:09 ` [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code Rafael J. Wysocki
@ 2023-06-12 20:16   ` Dave Jiang
  0 siblings, 0 replies; 14+ messages in thread
From: Dave Jiang @ 2023-06-12 20:16 UTC (permalink / raw
  To: Rafael J. Wysocki
  Cc: linux-acpi, linux-cxl, Len Brown, dan.j.williams, ira.weiny,
	vishal.l.verma, alison.schofield, lukas, Jonathan.Cameron


On 6/4/23 09:09, Rafael J. Wysocki wrote:
> On Thu, Jun 1, 2023 at 11:31 PM Dave Jiang <dave.jiang@intel.com> wrote:
>> v3:
>> - Move common code to lib/fw_table.c
>> v2:
>> - Split out with CONFIG_ACPI_TABLES_LIB to be independent
>> - Fixed 0-day issues
>> - Change CDAT releveant names to prefix with cdat/CDAT instead of
>>    acpi/ACPI. (Jonathan)
>> - Make table_header a union with cdat table header instead of
>>    'acpi_table_header'. (Jonathan)
>> - Removed ACPI_SIG_CDAT, already defined.
>>
>> Hi Rafael,
>> Please consider ack these patches. Dan can take these through the CXL tree. After
>> attempting to rename the cxl_ prefixes of functions and non ACPICA data structures
>> to something more common, it seems that significant amount of ACPI code would be
>> touched for the rename. For this series I left it alone in order to have the minimal
>> changes to ACPI code.
>>
>> I've broken out the "cxl: Add support for QTG ID retrieval for CXL subsystem" [1]
>> series in order to make it more manageable. Here's the first part of the ACPI
>> changes. These changes are added to allow reuse of ACPI tables code to parse
>> the CDAT tables. While CDAT is not part of ACPI, the table structures are similar
>> to ACPI layouts that the code can be reused with some small modifications.
>>
>> However, in order to be properly utilized by CXL users, the tables code needs
>> to be refactored out to be independent of ACPI. For example, a PPC BE host may
>> have CXL and does not have ACPI support. But it will have CDAT to read from
>> devices and switches. I have created CONFIG_ACPI_TABLES_LIB in order to allow
>> the common code to be independent. 0-day seems to be happy now for all the
>> different configs and archs.
>>
>> 1/4: Split out the common code from drivers/acpi/tables.c to lib/fw_table.c
>> 2/4: Add CDAT support
>> 3,4/4: These two are minor patches that has ACPICA impact. Has been merged into
>>         the ACPICA git repo [3].
>>
>> The whole series is at [2] for convenience.
>>
>> [1]: https://lore.kernel.org/linux-cxl/168193556660.1178687.15477509915255912089.stgit@djiang5-mobl3/T/#t
>> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-qtg
>> [3]: https://github.com/acpica/acpica/pull/874
>>
>> ---
>>
>> Dave Jiang (4):
>>        acpi: Move common tables helper functions to common lib
>>        lib/firmware_table: tables: Add CDAT table parsing support
>>        acpi: fix misnamed define for CDAT DSMAS
>>        acpi: Add defines for CDAT SSLBIS
>>
>>
>>   drivers/acpi/Kconfig     |   1 +
>>   drivers/acpi/tables.c    | 178 +----------------------------
>>   include/acpi/actbl1.h    |   5 +-
>>   include/linux/acpi.h     |  22 +---
>>   include/linux/fw_table.h |  52 +++++++++
>>   lib/Kconfig              |   3 +
>>   lib/Makefile             |   2 +
>>   lib/fw_table.c           | 236 +++++++++++++++++++++++++++++++++++++++
>>   8 files changed, 302 insertions(+), 197 deletions(-)
>>   create mode 100644 include/linux/fw_table.h
>>   create mode 100644 lib/fw_table.c
>>
>> --
> I think that this series can go in via the CXL tree and I'm expecting
> ACPICA to make a new release including the counterparts of patches
> [3-4/4] shortly.
>
> Please feel free to add
>
> Acled-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> to the series.

Thank you Rafael!


> Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code
  2023-06-06  1:36 ` Hanjun Guo
@ 2023-06-12 20:17   ` Dave Jiang
  0 siblings, 0 replies; 14+ messages in thread
From: Dave Jiang @ 2023-06-12 20:17 UTC (permalink / raw
  To: Hanjun Guo, linux-acpi, linux-cxl
  Cc: Len Brown, Rafael J. Wysocki, dan.j.williams, ira.weiny,
	vishal.l.verma, alison.schofield, lukas, Jonathan.Cameron


On 6/5/23 18:36, Hanjun Guo wrote:
> On 2023/6/2 5:31, Dave Jiang wrote:
>> v3:
>> - Move common code to lib/fw_table.c
>> v2:
>> - Split out with CONFIG_ACPI_TABLES_LIB to be independent
>> - Fixed 0-day issues
>> - Change CDAT releveant names to prefix with cdat/CDAT instead of
>>    acpi/ACPI. (Jonathan)
>> - Make table_header a union with cdat table header instead of
>>    'acpi_table_header'. (Jonathan)
>> - Removed ACPI_SIG_CDAT, already defined.
>>
>> Hi Rafael,
>> Please consider ack these patches. Dan can take these through the CXL 
>> tree. After
>> attempting to rename the cxl_ prefixes of functions and non ACPICA 
>> data structures
>> to something more common, it seems that significant amount of ACPI 
>> code would be
>> touched for the rename. For this series I left it alone in order to 
>> have the minimal
>> changes to ACPI code.
>>
>> I've broken out the "cxl: Add support for QTG ID retrieval for CXL 
>> subsystem" [1]
>> series in order to make it more manageable. Here's the first part of 
>> the ACPI
>> changes. These changes are added to allow reuse of ACPI tables code 
>> to parse
>> the CDAT tables. While CDAT is not part of ACPI, the table structures 
>> are similar
>> to ACPI layouts that the code can be reused with some small 
>> modifications.
>>
>> However, in order to be properly utilized by CXL users, the tables 
>> code needs
>> to be refactored out to be independent of ACPI. For example, a PPC BE 
>> host may
>> have CXL and does not have ACPI support. But it will have CDAT to 
>> read from
>> devices and switches. I have created CONFIG_ACPI_TABLES_LIB in order 
>> to allow
>> the common code to be independent. 0-day seems to be happy now for 
>> all the
>> different configs and archs.
>>
>> 1/4: Split out the common code from drivers/acpi/tables.c to 
>> lib/fw_table.c
>> 2/4: Add CDAT support
>> 3,4/4: These two are minor patches that has ACPICA impact. Has been 
>> merged into
>>         the ACPICA git repo [3].
>>
>> The whole series is at [2] for convenience.
>>
>> [1]: 
>> https://lore.kernel.org/linux-cxl/168193556660.1178687.15477509915255912089.stgit@djiang5-mobl3/T/#t
>> [2]: 
>> https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=cxl-qtg
>> [3]: https://github.com/acpica/acpica/pull/874
>>
>> ---
>>
>> Dave Jiang (4):
>>        acpi: Move common tables helper functions to common lib
>>        lib/firmware_table: tables: Add CDAT table parsing support
>>        acpi: fix misnamed define for CDAT DSMAS
>>        acpi: Add defines for CDAT SSLBIS
>>
>>
>>   drivers/acpi/Kconfig     |   1 +
>>   drivers/acpi/tables.c    | 178 +----------------------------
>>   include/acpi/actbl1.h    |   5 +-
>>   include/linux/acpi.h     |  22 +---
>>   include/linux/fw_table.h |  52 +++++++++
>>   lib/Kconfig              |   3 +
>>   lib/Makefile             |   2 +
>>   lib/fw_table.c           | 236 +++++++++++++++++++++++++++++++++++++++
>
> Who will maintain this file? since it's the core function of parsing
> ACPI tables, I would like the update of this file in the future will
> Cc ACPI mailing list.

I can add a MAINTAINERS entry. Since the original code is from ACPI, it 
can be maintained by ACPI subsystem.



>
> Thanks
> Hanjun

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] acpi: Move common tables helper functions to common lib
  2023-06-02 12:35   ` Jonathan Cameron
@ 2023-06-12 20:47     ` Dave Jiang
  0 siblings, 0 replies; 14+ messages in thread
From: Dave Jiang @ 2023-06-12 20:47 UTC (permalink / raw
  To: Jonathan Cameron
  Cc: linux-acpi, linux-cxl, Rafael J. Wysocki, lenb, dan.j.williams,
	ira.weiny, vishal.l.verma, alison.schofield, lukas


On 6/2/23 05:35, Jonathan Cameron wrote:
> On Thu, 01 Jun 2023 14:31:52 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
>
>> Some of the routines in ACPI driver/acpi/tables.c can be shared with
>> parsing CDAT. CDAT is a device-provided data structure that is formatted
>> similar to a platform provided ACPI table. CDAT is used by CXL and can
>> exist on platforms that do not use ACPI. Split out the common routine
>> from ACPI to accommodate platforms that do not support ACPI and move that
>> to /lib. The common routines can be built outside of ACPI if
>> FIRMWARE_TABLES is selected.
>>
>> Link: https://lore.kernel.org/linux-cxl/CAJZ5v0jipbtTNnsA0-o5ozOk8ZgWnOg34m34a9pPenTyRLj=6A@mail.gmail.com/
>> Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Minor comment to fix inline. With that tidied up
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
>> diff --git a/include/linux/fw_table.h b/include/linux/fw_table.h
>> new file mode 100644
>> index 000000000000..ff8fa58d5818
>> --- /dev/null
>> +++ b/include/linux/fw_table.h
>> @@ -0,0 +1,43 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + *  fw_tables.h - Parsing support for ACPI and ACPI-like tables provided by
>> + *                platform or device firmware
>> + *
>> + *  Copyright (C) 2001 Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
>> + *  Copyright (C) 2023 Intel Corp.
>> + */
>> +#ifndef _FW_TABLE_H_
>> +#define _FW_TABLE_H_
>> +
>> +union acpi_subtable_headers;
>> +
>> +typedef int (*acpi_tbl_entry_handler)(union acpi_subtable_headers *header,
>> +				      const unsigned long end);
>> +
>> +typedef int (*acpi_tbl_entry_handler_arg)(union acpi_subtable_headers *header,
>> +					  void *arg, const unsigned long end);
>> +
>> +struct acpi_subtable_proc {
>> +	int id;
>> +	acpi_tbl_entry_handler handler;
>> +	acpi_tbl_entry_handler_arg handler_arg;
>> +	void *arg;
>> +	int count;
>> +};
>> +
>> +#include <linux/acpi.h>
>> +#include <acpi/acpi.h>
> Includes mid way down the files is not a common pattern and I can't see why
> it's particularly useful to do so here.
>
> + linux/acpi.h includes acpi/acpi.h and I can't see that changing any time
> soon...

Unfortunately because linux/acpi.h needs defines from this header file, 
and this header file needs ACPI table definitions from acpi/acpi.h, and 
acpi/acpi.h can't be included independently without linux/acpi.h, this 
seems to be the only way to get everything to build w/o complaints from 
the compiler. This header really just needs some ACPI table struct 
definitions but there's no easy way to just include those.  Also having 
the headers in the beginning causes compiler errors. It's ugly but seems 
to be the way things will build cleanly.


>> +
>> +union acpi_subtable_headers {
>> +	struct acpi_subtable_header common;
>> +	struct acpi_hmat_structure hmat;
>> +	struct acpi_prmt_module_header prmt;
>> +	struct acpi_cedt_header cedt;
>> +};
>> +
>> +int acpi_parse_entries_array(char *id, unsigned long table_size,
>> +			     struct acpi_table_header *table_header,
>> +			     struct acpi_subtable_proc *proc,
>> +			     int proc_num, unsigned int max_entries);
>> +
>> +#endif
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-06-12 20:53 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-01 21:31 [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code Dave Jiang
2023-06-01 21:31 ` [PATCH v3 1/4] acpi: Move common tables helper functions to common lib Dave Jiang
2023-06-02 12:35   ` Jonathan Cameron
2023-06-12 20:47     ` Dave Jiang
2023-06-01 21:31 ` [PATCH v3 2/4] lib/firmware_table: tables: Add CDAT table parsing support Dave Jiang
2023-06-02 12:36   ` Jonathan Cameron
2023-06-01 21:32 ` [PATCH v3 3/4] acpi: fix misnamed define for CDAT DSMAS Dave Jiang
2023-06-02 12:38   ` Jonathan Cameron
2023-06-01 21:32 ` [PATCH v3 4/4] acpi: Add defines for CDAT SSLBIS Dave Jiang
2023-06-02 12:38   ` Jonathan Cameron
2023-06-04 16:09 ` [PATCH v3 0/4] acpi: Add CDAT parsing support to ACPI tables code Rafael J. Wysocki
2023-06-12 20:16   ` Dave Jiang
2023-06-06  1:36 ` Hanjun Guo
2023-06-12 20:17   ` Dave Jiang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).