[PATCH v14 0/4] cxl, EINJ: Update EINJ for CXL error types

[PATCH v14 0/4] cxl, EINJ: Update EINJ for CXL error types

[Ben Cheatham]

v14 Changes:
	- Remove einj-cxl module and instead compile as part of EINJ module
	  (Dan)
	- Change CONFIG_ACPI_APEI_EINJ_CXL from tristate to bool (Dan)
	- Fix CONFIG_ACPI_APEI_EINJ_CXL/CONFIG_CXL_BUS dependencies
	- Remove EINJ function exports (Dan)
	- Organizational changes for CXL content in EINJ kernel
	  documentation (Dan)
	- Demote "EINJ table not found." print to pr_debug() from pr_info()
	  (Dan)

v13 Changes:
	- Create new einj-cxl module for EINJ CXL error type functionality
	- Rename CONFIG_CXL_EINJ to CONFIG_ACPI_APEI_EINJ_CXL
	- Move CONFIG_ACPI_APEI_CXL to be under CONFIG_ACPI_APEI_EINJ (due to
	  new CONFIG_CXL_BUS dependency)
	- Add an optional dependency to CONFIG_CXL_BUS on
	  CONFIG_ACPI_APEI_EINJ_CXL 
	- Change pr_warn("EINJ table not found.") to a pr_info() for when/if
	  EINJ probe fails (Tony)
	- Add a clarification that a CXL port needs to be present for CXL 
	  EINJ error types to einj.rst (Davidlohr)

The new CXL error types will use the Memory Address field in the
SET_ERROR_TYPE_WITH_ADDRESS structure in order to target a CXL 1.1
compliant memory-mapped downstream port. The value of the memory address
will be in the port's MMIO range, and it will not represent physical
(normal or persistent) memory.

Add the functionality for injecting CXL 1.1 errors to the EINJ module,
but not through the EINJ legacy interface under /sys/kernel/debug/apei/einj.
Instead, make the error types available under /sys/kernel/debug/cxl.
This allows for validating the MMIO address for a CXL 1.1 error type
while also not making the user responsible for finding it.

Ben Cheatham (4):
  EINJ: Migrate to a platform driver
  EINJ: Add CXL error type support
  cxl/core: Add CXL EINJ debugfs files
  EINJ, Documentation: Update EINJ kernel doc

 Documentation/ABI/testing/debugfs-cxl         |  30 +++++
 .../firmware-guide/acpi/apei/einj.rst         |  34 +++++
 MAINTAINERS                                   |   1 +
 drivers/acpi/apei/Kconfig                     |  12 ++
 drivers/acpi/apei/Makefile                    |   2 +
 drivers/acpi/apei/apei-internal.h             |  16 +++
 drivers/acpi/apei/{einj.c => einj-core.c}     | 124 ++++++++++++++----
 drivers/acpi/apei/einj-cxl.c                  | 120 +++++++++++++++++
 drivers/cxl/core/port.c                       |  41 ++++++
 include/linux/einj-cxl.h                      |  40 ++++++
 10 files changed, 392 insertions(+), 28 deletions(-)
 rename drivers/acpi/apei/{einj.c => einj-core.c} (91%)
 create mode 100644 drivers/acpi/apei/einj-cxl.c
 create mode 100644 include/linux/einj-cxl.h

-- 
2.34.1

[PATCH v14 1/4] EINJ: Migrate to a platform driver

[Ben Cheatham]

Change the EINJ module to install a platform device/driver on module
init and move the module init() and exit() functions to driver probe and
remove. This change allows the EINJ module to load regardless of whether
setting up EINJ succeeds, which allows dependent modules to still load
(i.e. the CXL core).

Since EINJ may no longer be initialized when the module loads, any
functions that are called from dependent/external modules should check
the einj_initialized variable before calling any EINJ functions.

Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
---
 drivers/acpi/apei/einj.c | 48 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
index 89fb9331c611..937c69844dac 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -21,6 +21,7 @@
 #include <linux/nmi.h>
 #include <linux/delay.h>
 #include <linux/mm.h>
+#include <linux/platform_device.h>
 #include <asm/unaligned.h>
 
 #include "apei-internal.h"
@@ -137,6 +138,11 @@ static struct apei_exec_ins_type einj_ins_type[] = {
  */
 static DEFINE_MUTEX(einj_mutex);
 
+/*
+ * Exported APIs use this flag to exit early if einj_probe() failed.
+ */
+static bool einj_initialized __ro_after_init;
+
 static void *einj_param;
 
 static void einj_exec_ctx_init(struct apei_exec_context *ctx)
@@ -703,21 +709,21 @@ static int einj_check_table(struct acpi_table_einj *einj_tab)
 	return 0;
 }
 
-static int __init einj_init(void)
+static int __init einj_probe(struct platform_device *pdev)
 {
 	int rc;
 	acpi_status status;
 	struct apei_exec_context ctx;
 
 	if (acpi_disabled) {
-		pr_info("ACPI disabled.\n");
+		pr_debug("ACPI disabled.\n");
 		return -ENODEV;
 	}
 
 	status = acpi_get_table(ACPI_SIG_EINJ, 0,
 				(struct acpi_table_header **)&einj_tab);
 	if (status == AE_NOT_FOUND) {
-		pr_warn("EINJ table not found.\n");
+		pr_debug("EINJ table not found.\n");
 		return -ENODEV;
 	} else if (ACPI_FAILURE(status)) {
 		pr_err("Failed to get EINJ table: %s\n",
@@ -805,7 +811,7 @@ static int __init einj_init(void)
 	return rc;
 }
 
-static void __exit einj_exit(void)
+static void __exit einj_remove(struct platform_device *pdev)
 {
 	struct apei_exec_context ctx;
 
@@ -826,6 +832,40 @@ static void __exit einj_exit(void)
 	acpi_put_table((struct acpi_table_header *)einj_tab);
 }
 
+static struct platform_device *einj_dev;
+static struct platform_driver einj_driver = {
+	.remove_new = einj_remove,
+	.driver = {
+		.name = "acpi-einj",
+	},
+};
+
+static int __init einj_init(void)
+{
+	struct platform_device_info einj_dev_info = {
+		.name = "acpi-einj",
+		.id = -1,
+	};
+	int rc;
+
+	einj_dev = platform_device_register_full(&einj_dev_info);
+	if (IS_ERR(einj_dev))
+		return PTR_ERR(einj_dev);
+
+	rc = platform_driver_probe(&einj_driver, einj_probe);
+	einj_initialized = rc == 0;
+
+	return 0;
+}
+
+static void __exit einj_exit(void)
+{
+	if (einj_initialized)
+		platform_driver_unregister(&einj_driver);
+
+	platform_device_del(einj_dev);
+}
+
 module_init(einj_init);
 module_exit(einj_exit);
 
-- 
2.34.1

Re: [PATCH v14 1/4] EINJ: Migrate to a platform driver

[Dan Williams]

Ben Cheatham wrote:
> Change the EINJ module to install a platform device/driver on module
> init and move the module init() and exit() functions to driver probe and
> remove. This change allows the EINJ module to load regardless of whether
> setting up EINJ succeeds, which allows dependent modules to still load
> (i.e. the CXL core).
> 
> Since EINJ may no longer be initialized when the module loads, any
> functions that are called from dependent/external modules should check
> the einj_initialized variable before calling any EINJ functions.

Is this statement true given that no debugfs files are registered when
einj_initialized() is false?

It would be nice to remove that from the functions and just rely on
debugfs files not being published for safety.

With that fixed up you can add:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Re: [PATCH v14 1/4] EINJ: Migrate to a platform driver

[Ben Cheatham]

On 2/27/24 11:47 PM, Dan Williams wrote:
> Ben Cheatham wrote:
>> Change the EINJ module to install a platform device/driver on module
>> init and move the module init() and exit() functions to driver probe and
>> remove. This change allows the EINJ module to load regardless of whether
>> setting up EINJ succeeds, which allows dependent modules to still load
>> (i.e. the CXL core).
>>
>> Since EINJ may no longer be initialized when the module loads, any
>> functions that are called from dependent/external modules should check
>> the einj_initialized variable before calling any EINJ functions.
> 
> Is this statement true given that no debugfs files are registered when
> einj_initialized() is false?
> 

That's true, I guess this would just be for redundancy/extra safety.

> It would be nice to remove that from the functions and just rely on
> debugfs files not being published for safety.
> 

I'll go ahead and remove those checks.

> With that fixed up you can add:
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Re: [PATCH v14 1/4] EINJ: Migrate to a platform driver

[Jonathan Cameron]

On Mon, 26 Feb 2024 16:27:01 -0600
Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:

> Change the EINJ module to install a platform device/driver on module
> init and move the module init() and exit() functions to driver probe and
> remove. This change allows the EINJ module to load regardless of whether
> setting up EINJ succeeds, which allows dependent modules to still load
> (i.e. the CXL core).
> 
> Since EINJ may no longer be initialized when the module loads, any
> functions that are called from dependent/external modules should check
> the einj_initialized variable before calling any EINJ functions.
> 
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
Looks good to me.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

I did wonder if a debug message was needed on no acpi support given
that should be really really obvious on a system, but I guess with
late probing of this from a module, maybe that is still useful.

Jonathan

[PATCH v14 2/4] EINJ: Add CXL error type support

[Ben Cheatham]

Remove CXL protocol error types from the EINJ module and move them to
a new einj_cxl module. The einj_cxl module implements the necessary
handling for CXL protocol error injection and exposes an API for the
CXL core to use said functionality. Because the CXL error types
require special handling, only allow them to be injected through the
einj_cxl module and return an error when attempting to inject through
"regular" EINJ.

Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
---
 MAINTAINERS                               |   1 +
 drivers/acpi/apei/Kconfig                 |  12 +++
 drivers/acpi/apei/Makefile                |   2 +
 drivers/acpi/apei/apei-internal.h         |  16 +++
 drivers/acpi/apei/{einj.c => einj-core.c} |  78 +++++++++-----
 drivers/acpi/apei/einj-cxl.c              | 120 ++++++++++++++++++++++
 include/linux/einj-cxl.h                  |  40 ++++++++
 7 files changed, 244 insertions(+), 25 deletions(-)
 rename drivers/acpi/apei/{einj.c => einj-core.c} (94%)
 create mode 100644 drivers/acpi/apei/einj-cxl.c
 create mode 100644 include/linux/einj-cxl.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 2ecaaec6a6bf..90cf8403dd17 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5289,6 +5289,7 @@ M:	Dan Williams <dan.j.williams@intel.com>
 L:	linux-cxl@vger.kernel.org
 S:	Maintained
 F:	drivers/cxl/
+F:	include/linux/cxl-einj.h
 F:	include/linux/cxl-event.h
 F:	include/uapi/linux/cxl_mem.h
 F:	tools/testing/cxl/
diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
index 6b18f8bc7be3..f01afa2805be 100644
--- a/drivers/acpi/apei/Kconfig
+++ b/drivers/acpi/apei/Kconfig
@@ -60,6 +60,18 @@ config ACPI_APEI_EINJ
 	  mainly used for debugging and testing the other parts of
 	  APEI and some other RAS features.
 
+config ACPI_APEI_EINJ_CXL
+	bool "CXL Error INJection Support"
+	default ACPI_APEI_EINJ
+	depends on ACPI_APEI_EINJ && CXL_BUS <= ACPI_APEI_EINJ
+	help
+	  Support for CXL protocol Error INJection through debugfs/cxl.
+	  Availability and which errors are supported is dependent on
+	  the host platform. Look to ACPI v6.5 section 18.6.4 and kernel
+	  EINJ documentation for more information.
+
+	  If unsure say 'n'
+
 config ACPI_APEI_ERST_DEBUG
 	tristate "APEI Error Record Serialization Table (ERST) Debug Support"
 	depends on ACPI_APEI
diff --git a/drivers/acpi/apei/Makefile b/drivers/acpi/apei/Makefile
index 4dfac2128737..2c474e6477e1 100644
--- a/drivers/acpi/apei/Makefile
+++ b/drivers/acpi/apei/Makefile
@@ -2,6 +2,8 @@
 obj-$(CONFIG_ACPI_APEI)		+= apei.o
 obj-$(CONFIG_ACPI_APEI_GHES)	+= ghes.o
 obj-$(CONFIG_ACPI_APEI_EINJ)	+= einj.o
+einj-y				:= einj-core.o
+einj-$(CONFIG_ACPI_APEI_EINJ_CXL) += einj-cxl.o
 obj-$(CONFIG_ACPI_APEI_ERST_DEBUG) += erst-dbg.o
 
 apei-y := apei-base.o hest.o erst.o bert.o
diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
index 67c2c3b959e1..ab039d24cd22 100644
--- a/drivers/acpi/apei/apei-internal.h
+++ b/drivers/acpi/apei/apei-internal.h
@@ -130,4 +130,20 @@ static inline u32 cper_estatus_len(struct acpi_hest_generic_status *estatus)
 }
 
 int apei_osc_setup(void);
+
+int einj_get_available_error_type(u32 *type);
+int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
+		      u64 param4);
+bool einj_is_cxl_error_type(u64 type);
+int einj_validate_error_type(u64 type);
+
+#ifndef ACPI_EINJ_CXL_CACHE_CORRECTABLE
+#define ACPI_EINJ_CXL_CACHE_CORRECTABLE     BIT(12)
+#define ACPI_EINJ_CXL_CACHE_UNCORRECTABLE   BIT(13)
+#define ACPI_EINJ_CXL_CACHE_FATAL           BIT(14)
+#define ACPI_EINJ_CXL_MEM_CORRECTABLE       BIT(15)
+#define ACPI_EINJ_CXL_MEM_UNCORRECTABLE     BIT(16)
+#define ACPI_EINJ_CXL_MEM_FATAL             BIT(17)
+#endif
+
 #endif
diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj-core.c
similarity index 94%
rename from drivers/acpi/apei/einj.c
rename to drivers/acpi/apei/einj-core.c
index 937c69844dac..1a5f53d81d09 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -37,6 +37,12 @@
 #define MEM_ERROR_MASK		(ACPI_EINJ_MEMORY_CORRECTABLE | \
 				ACPI_EINJ_MEMORY_UNCORRECTABLE | \
 				ACPI_EINJ_MEMORY_FATAL)
+#define CXL_ERROR_MASK		(ACPI_EINJ_CXL_CACHE_CORRECTABLE | \
+				ACPI_EINJ_CXL_CACHE_UNCORRECTABLE | \
+				ACPI_EINJ_CXL_CACHE_FATAL | \
+				ACPI_EINJ_CXL_MEM_CORRECTABLE | \
+				ACPI_EINJ_CXL_MEM_UNCORRECTABLE | \
+				ACPI_EINJ_CXL_MEM_FATAL)
 
 /*
  * ACPI version 5 provides a SET_ERROR_TYPE_WITH_ADDRESS action.
@@ -141,7 +147,7 @@ static DEFINE_MUTEX(einj_mutex);
 /*
  * Exported APIs use this flag to exit early if einj_probe() failed.
  */
-static bool einj_initialized __ro_after_init;
+bool einj_initialized __ro_after_init;
 
 static void *einj_param;
 
@@ -166,7 +172,7 @@ static int __einj_get_available_error_type(u32 *type)
 }
 
 /* Get error injection capabilities of the platform */
-static int einj_get_available_error_type(u32 *type)
+int einj_get_available_error_type(u32 *type)
 {
 	int rc;
 
@@ -536,8 +542,8 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 }
 
 /* Inject the specified hardware error */
-static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
-			     u64 param3, u64 param4)
+int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
+		      u64 param4)
 {
 	int rc;
 	u64 base_addr, size;
@@ -560,8 +566,16 @@ static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 	if (type & ACPI5_VENDOR_BIT) {
 		if (vendor_flags != SETWA_FLAGS_MEM)
 			goto inject;
-	} else if (!(type & MEM_ERROR_MASK) && !(flags & SETWA_FLAGS_MEM))
+	} else if (!(type & MEM_ERROR_MASK) && !(flags & SETWA_FLAGS_MEM)) {
 		goto inject;
+	/*
+	 * If the error type is for a CXL 1.0/1.1 port then skip address
+	 * checking here. The einj-cxl module should have already verified the
+	 * supplied MMIO address is correct.
+	 */
+	} else if ((type & CXL_ERROR_MASK) && (flags & SETWA_FLAGS_MEM)) {
+		goto inject;
+	}
 
 	/*
 	 * Disallow crazy address masks that give BIOS leeway to pick
@@ -613,12 +627,6 @@ static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
 	{ BIT(9), "Platform Correctable" },
 	{ BIT(10), "Platform Uncorrectable non-fatal" },
 	{ BIT(11), "Platform Uncorrectable fatal"},
-	{ BIT(12), "CXL.cache Protocol Correctable" },
-	{ BIT(13), "CXL.cache Protocol Uncorrectable non-fatal" },
-	{ BIT(14), "CXL.cache Protocol Uncorrectable fatal" },
-	{ BIT(15), "CXL.mem Protocol Correctable" },
-	{ BIT(16), "CXL.mem Protocol Uncorrectable non-fatal" },
-	{ BIT(17), "CXL.mem Protocol Uncorrectable fatal" },
 	{ BIT(31), "Vendor Defined Error Types" },
 };
 
@@ -640,29 +648,21 @@ static int available_error_type_show(struct seq_file *m, void *v)
 
 DEFINE_SHOW_ATTRIBUTE(available_error_type);
 
-static int error_type_get(void *data, u64 *val)
-{
-	*val = error_type;
-
-	return 0;
-}
-
-static int error_type_set(void *data, u64 val)
+int einj_validate_error_type(u64 type)
 {
+	u32 tval, vendor, available_error_type = 0;
 	int rc;
-	u32 available_error_type = 0;
-	u32 tval, vendor;
 
 	/* Only low 32 bits for error type are valid */
-	if (val & GENMASK_ULL(63, 32))
+	if (type & GENMASK_ULL(63, 32))
 		return -EINVAL;
 
 	/*
 	 * Vendor defined types have 0x80000000 bit set, and
 	 * are not enumerated by ACPI_EINJ_GET_ERROR_TYPE
 	 */
-	vendor = val & ACPI5_VENDOR_BIT;
-	tval = val & 0x7fffffff;
+	vendor = type & ACPI5_VENDOR_BIT;
+	tval = type & GENMASK(30, 0);
 
 	/* Only one error type can be specified */
 	if (tval & (tval - 1))
@@ -671,9 +671,37 @@ static int error_type_set(void *data, u64 val)
 		rc = einj_get_available_error_type(&available_error_type);
 		if (rc)
 			return rc;
-		if (!(val & available_error_type))
+		if (!(type & available_error_type))
 			return -EINVAL;
 	}
+
+	return 0;
+}
+
+bool einj_is_cxl_error_type(u64 type)
+{
+	return (type & CXL_ERROR_MASK) && (!(type & ACPI5_VENDOR_BIT));
+}
+
+static int error_type_get(void *data, u64 *val)
+{
+	*val = error_type;
+
+	return 0;
+}
+
+static int error_type_set(void *data, u64 val)
+{
+	int rc;
+
+	/* CXL error types have to be injected from cxl debugfs */
+	if (einj_is_cxl_error_type(val))
+		return -EINVAL;
+
+	rc = einj_validate_error_type(val);
+	if (rc)
+		return rc;
+
 	error_type = val;
 
 	return 0;
diff --git a/drivers/acpi/apei/einj-cxl.c b/drivers/acpi/apei/einj-cxl.c
new file mode 100644
index 000000000000..34badc6a801e
--- /dev/null
+++ b/drivers/acpi/apei/einj-cxl.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * CXL Error INJection support. Used by CXL core to inject
+ * protocol errors into CXL ports.
+ *
+ * Copyright (C) 2023 Advanced Micro Devices, Inc.
+ *
+ * Author: Ben Cheatham <benjamin.cheatham@amd.com>
+ */
+#include <linux/einj-cxl.h>
+#include <linux/debugfs.h>
+
+#include "apei-internal.h"
+
+/* Defined in einj-core.c */
+extern bool einj_initialized;
+
+static struct { u32 mask; const char *str; } const einj_cxl_error_type_string[] = {
+	{ BIT(12), "CXL.cache Protocol Correctable" },
+	{ BIT(13), "CXL.cache Protocol Uncorrectable non-fatal" },
+	{ BIT(14), "CXL.cache Protocol Uncorrectable fatal" },
+	{ BIT(15), "CXL.mem Protocol Correctable" },
+	{ BIT(16), "CXL.mem Protocol Uncorrectable non-fatal" },
+	{ BIT(17), "CXL.mem Protocol Uncorrectable fatal" },
+};
+
+int einj_cxl_available_error_type_show(struct seq_file *m, void *v)
+{
+	int cxl_err, rc;
+	u32 available_error_type = 0;
+
+	if (!einj_initialized)
+		return -ENXIO;
+
+	rc = einj_get_available_error_type(&available_error_type);
+	if (rc)
+		return rc;
+
+	for (int pos = 0; pos < ARRAY_SIZE(einj_cxl_error_type_string); pos++) {
+		cxl_err = ACPI_EINJ_CXL_CACHE_CORRECTABLE << pos;
+
+		if (available_error_type & cxl_err)
+			seq_printf(m, "0x%08x\t%s\n",
+				   einj_cxl_error_type_string[pos].mask,
+				   einj_cxl_error_type_string[pos].str);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(einj_cxl_available_error_type_show, CXL);
+
+static int cxl_dport_get_sbdf(struct pci_dev *dport_dev, u64 *sbdf)
+{
+	struct pci_bus *pbus;
+	struct pci_host_bridge *bridge;
+	u64 seg = 0, bus;
+
+	pbus = dport_dev->bus;
+	bridge = pci_find_host_bridge(pbus);
+
+	if (!bridge)
+		return -ENODEV;
+
+	if (bridge->domain_nr != PCI_DOMAIN_NR_NOT_SET)
+		seg = bridge->domain_nr;
+
+	bus = pbus->number;
+	*sbdf = (seg << 24) | (bus << 16) | dport_dev->devfn;
+
+	return 0;
+}
+
+int einj_cxl_inject_rch_error(u64 rcrb, u64 type)
+{
+	int rc;
+
+	if (!einj_initialized)
+		return -ENXIO;
+
+	/* Only CXL error types can be specified */
+	if (!einj_is_cxl_error_type(type))
+		return -EINVAL;
+
+	rc = einj_validate_error_type(type);
+	if (rc)
+		return rc;
+
+	return einj_error_inject(type, 0x2, rcrb, GENMASK_ULL(63, 12), 0, 0);
+}
+EXPORT_SYMBOL_NS_GPL(einj_cxl_inject_rch_error, CXL);
+
+int einj_cxl_inject_error(struct pci_dev *dport, u64 type)
+{
+	u64 param4 = 0;
+	int rc;
+
+	if (!einj_initialized)
+		return -ENXIO;
+
+	/* Only CXL error types can be specified */
+	if (!einj_is_cxl_error_type(type))
+		return -EINVAL;
+
+	rc = einj_validate_error_type(type);
+	if (rc)
+		return rc;
+
+	rc = cxl_dport_get_sbdf(dport, &param4);
+	if (rc)
+		return rc;
+
+	return einj_error_inject(type, 0x4, 0, 0, 0, param4);
+}
+EXPORT_SYMBOL_NS_GPL(einj_cxl_inject_error, CXL);
+
+bool einj_cxl_is_initialized(void)
+{
+	return einj_initialized;
+}
+EXPORT_SYMBOL_NS_GPL(einj_cxl_is_initialized, CXL);
diff --git a/include/linux/einj-cxl.h b/include/linux/einj-cxl.h
new file mode 100644
index 000000000000..4a1f4600539a
--- /dev/null
+++ b/include/linux/einj-cxl.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * CXL protocol Error INJection support.
+ *
+ * Copyright (c) 2023 Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Author: Ben Cheatham <benjamin.cheatham@amd.com>
+ */
+#ifndef EINJ_CXL_H
+#define EINJ_CXL_H
+
+#include <linux/pci.h>
+
+#if IS_ENABLED(CONFIG_ACPI_APEI_EINJ_CXL)
+int einj_cxl_available_error_type_show(struct seq_file *m, void *v);
+int einj_cxl_inject_error(struct pci_dev *dport_dev, u64 type);
+int einj_cxl_inject_rch_error(u64 rcrb, u64 type);
+bool einj_cxl_is_initialized(void);
+#else /* !IS_ENABLED(CONFIG_ACPI_APEI_EINJ_CXL) */
+static inline int einj_cxl_available_error_type_show(struct seq_file *m,
+						     void *v)
+{
+	return -ENXIO;
+}
+
+static inline int einj_cxl_inject_error(struct pci_dev *dport_dev, u64 type)
+{
+	return -ENXIO;
+}
+
+static inline int einj_cxl_inject_rch_error(u64 rcrb, u64 type)
+{
+	return -ENXIO;
+}
+
+static inline bool einj_cxl_is_initialized(void) { return false; }
+#endif /* CONFIG_ACPI_APEI_EINJ_CXL */
+
+#endif /* EINJ_CXL_H */
-- 
2.34.1

RE: [PATCH v14 2/4] EINJ: Add CXL error type support

[Luck, Tony]

> Remove CXL protocol error types from the EINJ module and move them to
> a new einj_cxl module. The einj_cxl module implements the necessary

> +config ACPI_APEI_EINJ_CXL
> +	bool "CXL Error INJection Support"

It's not really a module anymore. Need to update commit messages.

-Tony

Re: [PATCH v14 2/4] EINJ: Add CXL error type support

[Ben Cheatham]

On 2/26/24 4:47 PM, Luck, Tony wrote:
>> Remove CXL protocol error types from the EINJ module and move them to
>> a new einj_cxl module. The einj_cxl module implements the necessary
> 
>> +config ACPI_APEI_EINJ_CXL
>> +	bool "CXL Error INJection Support"
> 
> It's not really a module anymore. Need to update commit messages.
> 
> -Tony

You're 100% correct. I was in a bit of a rush getting these out yesterday and forgot
this message (and the next one) need to be updated. I'll send out replies to those
patches with the updated messages.

Thanks,
Ben

Re: [PATCH v14 2/4] EINJ: Add CXL error type support

[Ben Cheatham]

This patch had an outdated commit message, so here's the patch with an updated description.

I also realized that I was wrong about letting CXL 2.0+ error types (discussed a revision
or two ago) and I wasn't actually letting them through. I've went ahead and added
the ability to inject CXL 2.0+ error through the legacy interface. This pretty
much amounts to returning an error for CXL 1.0/1.1 injection types in einj_error_inject()
and instead routing them through a new einj_cxl_rch_error_inject() function called
in einj-cxl.c

If this change is too big I can send in another revision, I just wanted to avoid
spamming the list(s).

From eea1cf991dc2a551f6db2e3bb9510ed43c86762d Mon Sep 17 00:00:00 2001
From: Ben Cheatham <Benjamin.Cheatham@amd.com>
Date: Fri, 16 Feb 2024 11:12:51 -0600
Subject: [PATCH v14 2/4] EINJ: Add CXL error type support

Move CXL protocol error types from einj.c (now einj-core.c) to einj-cxl.c.
einj-cxl.c implements the necessary handling for CXL protocol error
injection and exposes an API for the CXL core to use said functionality,
while also allowing the EINJ module to be built without CXL support.
Because CXL error types targeting CXL 1.0/1.1 ports require special
handling, only allow them to be injected through the new cxl debugfs
interface (next commit) and return an error when attempting to inject
through the legacy interface.

Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
---
 MAINTAINERS                               |   1 +
 drivers/acpi/apei/Kconfig                 |  12 +++
 drivers/acpi/apei/Makefile                |   2 +
 drivers/acpi/apei/apei-internal.h         |  18 ++++
 drivers/acpi/apei/{einj.c => einj-core.c} |  85 +++++++++++----
 drivers/acpi/apei/einj-cxl.c              | 121 ++++++++++++++++++++++
 include/linux/einj-cxl.h                  |  40 +++++++
 7 files changed, 257 insertions(+), 22 deletions(-)
 rename drivers/acpi/apei/{einj.c => einj-core.c} (94%)
 create mode 100644 drivers/acpi/apei/einj-cxl.c
 create mode 100644 include/linux/einj-cxl.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 2ecaaec6a6bf..90cf8403dd17 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5289,6 +5289,7 @@ M:	Dan Williams <dan.j.williams@intel.com>
 L:	linux-cxl@vger.kernel.org
 S:	Maintained
 F:	drivers/cxl/
+F:	include/linux/cxl-einj.h
 F:	include/linux/cxl-event.h
 F:	include/uapi/linux/cxl_mem.h
 F:	tools/testing/cxl/
diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
index 6b18f8bc7be3..f01afa2805be 100644
--- a/drivers/acpi/apei/Kconfig
+++ b/drivers/acpi/apei/Kconfig
@@ -60,6 +60,18 @@ config ACPI_APEI_EINJ
 	  mainly used for debugging and testing the other parts of
 	  APEI and some other RAS features.
 
+config ACPI_APEI_EINJ_CXL
+	bool "CXL Error INJection Support"
+	default ACPI_APEI_EINJ
+	depends on ACPI_APEI_EINJ && CXL_BUS <= ACPI_APEI_EINJ
+	help
+	  Support for CXL protocol Error INJection through debugfs/cxl.
+	  Availability and which errors are supported is dependent on
+	  the host platform. Look to ACPI v6.5 section 18.6.4 and kernel
+	  EINJ documentation for more information.
+
+	  If unsure say 'n'
+
 config ACPI_APEI_ERST_DEBUG
 	tristate "APEI Error Record Serialization Table (ERST) Debug Support"
 	depends on ACPI_APEI
diff --git a/drivers/acpi/apei/Makefile b/drivers/acpi/apei/Makefile
index 4dfac2128737..2c474e6477e1 100644
--- a/drivers/acpi/apei/Makefile
+++ b/drivers/acpi/apei/Makefile
@@ -2,6 +2,8 @@
 obj-$(CONFIG_ACPI_APEI)		+= apei.o
 obj-$(CONFIG_ACPI_APEI_GHES)	+= ghes.o
 obj-$(CONFIG_ACPI_APEI_EINJ)	+= einj.o
+einj-y				:= einj-core.o
+einj-$(CONFIG_ACPI_APEI_EINJ_CXL) += einj-cxl.o
 obj-$(CONFIG_ACPI_APEI_ERST_DEBUG) += erst-dbg.o
 
 apei-y := apei-base.o hest.o erst.o bert.o
diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
index 67c2c3b959e1..cd2766c69d78 100644
--- a/drivers/acpi/apei/apei-internal.h
+++ b/drivers/acpi/apei/apei-internal.h
@@ -130,4 +130,22 @@ static inline u32 cper_estatus_len(struct acpi_hest_generic_status *estatus)
 }
 
 int apei_osc_setup(void);
+
+int einj_get_available_error_type(u32 *type);
+int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
+		      u64 param4);
+int einj_cxl_rch_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
+			      u64 param3, u64 param4);
+bool einj_is_cxl_error_type(u64 type);
+int einj_validate_error_type(u64 type);
+
+#ifndef ACPI_EINJ_CXL_CACHE_CORRECTABLE
+#define ACPI_EINJ_CXL_CACHE_CORRECTABLE     BIT(12)
+#define ACPI_EINJ_CXL_CACHE_UNCORRECTABLE   BIT(13)
+#define ACPI_EINJ_CXL_CACHE_FATAL           BIT(14)
+#define ACPI_EINJ_CXL_MEM_CORRECTABLE       BIT(15)
+#define ACPI_EINJ_CXL_MEM_UNCORRECTABLE     BIT(16)
+#define ACPI_EINJ_CXL_MEM_FATAL             BIT(17)
+#endif
+
 #endif
diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj-core.c
similarity index 94%
rename from drivers/acpi/apei/einj.c
rename to drivers/acpi/apei/einj-core.c
index 937c69844dac..437c13949be7 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -37,6 +37,12 @@
 #define MEM_ERROR_MASK		(ACPI_EINJ_MEMORY_CORRECTABLE | \
 				ACPI_EINJ_MEMORY_UNCORRECTABLE | \
 				ACPI_EINJ_MEMORY_FATAL)
+#define CXL_ERROR_MASK		(ACPI_EINJ_CXL_CACHE_CORRECTABLE | \
+				ACPI_EINJ_CXL_CACHE_UNCORRECTABLE | \
+				ACPI_EINJ_CXL_CACHE_FATAL | \
+				ACPI_EINJ_CXL_MEM_CORRECTABLE | \
+				ACPI_EINJ_CXL_MEM_UNCORRECTABLE | \
+				ACPI_EINJ_CXL_MEM_FATAL)
 
 /*
  * ACPI version 5 provides a SET_ERROR_TYPE_WITH_ADDRESS action.
@@ -141,7 +147,7 @@ static DEFINE_MUTEX(einj_mutex);
 /*
  * Exported APIs use this flag to exit early if einj_probe() failed.
  */
-static bool einj_initialized __ro_after_init;
+bool einj_initialized __ro_after_init;
 
 static void *einj_param;
 
@@ -166,7 +172,7 @@ static int __einj_get_available_error_type(u32 *type)
 }
 
 /* Get error injection capabilities of the platform */
-static int einj_get_available_error_type(u32 *type)
+int einj_get_available_error_type(u32 *type)
 {
 	int rc;
 
@@ -536,8 +542,8 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 }
 
 /* Inject the specified hardware error */
-static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
-			     u64 param3, u64 param4)
+int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
+		      u64 param4)
 {
 	int rc;
 	u64 base_addr, size;
@@ -560,8 +566,18 @@ static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 	if (type & ACPI5_VENDOR_BIT) {
 		if (vendor_flags != SETWA_FLAGS_MEM)
 			goto inject;
-	} else if (!(type & MEM_ERROR_MASK) && !(flags & SETWA_FLAGS_MEM))
+	} else if (!(type & MEM_ERROR_MASK) && !(flags & SETWA_FLAGS_MEM)) {
 		goto inject;
+	}
+
+	/*
+	 * Injections targeting a CXL 1.0/1.1 port have to be injected
+	 * from the CXL debugfs interface so that we can guarantee a
+	 * correct MMIO address.
+	 */
+	if (einj_is_cxl_error_type(type) && (flags & SETWA_FLAGS_MEM)) {
+		return -EINVAL;
+	}
 
 	/*
 	 * Disallow crazy address masks that give BIOS leeway to pick
@@ -593,6 +609,21 @@ static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 	return rc;
 }
 
+int einj_cxl_rch_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
+			      u64 param3, u64 param4)
+{
+	int rc;
+
+	if (!(einj_is_cxl_error_type(type) && (flags & SETWA_FLAGS_MEM)))
+		return -EINVAL;
+
+	mutex_lock(&einj_mutex);
+	rc = __einj_error_inject(type, flags, param1, param2, param3, param4);
+	mutex_unlock(&einj_mutex);
+
+	return rc;
+}
+
 static u32 error_type;
 static u32 error_flags;
 static u64 error_param1;
@@ -613,12 +644,6 @@ static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
 	{ BIT(9), "Platform Correctable" },
 	{ BIT(10), "Platform Uncorrectable non-fatal" },
 	{ BIT(11), "Platform Uncorrectable fatal"},
-	{ BIT(12), "CXL.cache Protocol Correctable" },
-	{ BIT(13), "CXL.cache Protocol Uncorrectable non-fatal" },
-	{ BIT(14), "CXL.cache Protocol Uncorrectable fatal" },
-	{ BIT(15), "CXL.mem Protocol Correctable" },
-	{ BIT(16), "CXL.mem Protocol Uncorrectable non-fatal" },
-	{ BIT(17), "CXL.mem Protocol Uncorrectable fatal" },
 	{ BIT(31), "Vendor Defined Error Types" },
 };
 
@@ -640,29 +665,26 @@ static int available_error_type_show(struct seq_file *m, void *v)
 
 DEFINE_SHOW_ATTRIBUTE(available_error_type);
 
-static int error_type_get(void *data, u64 *val)
+bool einj_is_cxl_error_type(u64 type)
 {
-	*val = error_type;
-
-	return 0;
+	return (type & CXL_ERROR_MASK) && (!(type & ACPI5_VENDOR_BIT));
 }
 
-static int error_type_set(void *data, u64 val)
+int einj_validate_error_type(u64 type)
 {
+	u32 tval, vendor, available_error_type = 0;
 	int rc;
-	u32 available_error_type = 0;
-	u32 tval, vendor;
 
 	/* Only low 32 bits for error type are valid */
-	if (val & GENMASK_ULL(63, 32))
+	if (type & GENMASK_ULL(63, 32))
 		return -EINVAL;
 
 	/*
 	 * Vendor defined types have 0x80000000 bit set, and
 	 * are not enumerated by ACPI_EINJ_GET_ERROR_TYPE
 	 */
-	vendor = val & ACPI5_VENDOR_BIT;
-	tval = val & 0x7fffffff;
+	vendor = type & ACPI5_VENDOR_BIT;
+	tval = type & GENMASK(30, 0);
 
 	/* Only one error type can be specified */
 	if (tval & (tval - 1))
@@ -671,9 +693,28 @@ static int error_type_set(void *data, u64 val)
 		rc = einj_get_available_error_type(&available_error_type);
 		if (rc)
 			return rc;
-		if (!(val & available_error_type))
+		if (!(type & available_error_type))
 			return -EINVAL;
 	}
+
+	return 0;
+}
+
+static int error_type_get(void *data, u64 *val)
+{
+	*val = error_type;
+
+	return 0;
+}
+
+static int error_type_set(void *data, u64 val)
+{
+	int rc;
+
+	rc = einj_validate_error_type(val);
+	if (rc)
+		return rc;
+
 	error_type = val;
 
 	return 0;
diff --git a/drivers/acpi/apei/einj-cxl.c b/drivers/acpi/apei/einj-cxl.c
new file mode 100644
index 000000000000..9d79c48b2dce
--- /dev/null
+++ b/drivers/acpi/apei/einj-cxl.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * CXL Error INJection support. Used by CXL core to inject
+ * protocol errors into CXL ports.
+ *
+ * Copyright (C) 2023 Advanced Micro Devices, Inc.
+ *
+ * Author: Ben Cheatham <benjamin.cheatham@amd.com>
+ */
+#include <linux/einj-cxl.h>
+#include <linux/debugfs.h>
+
+#include "apei-internal.h"
+
+/* Defined in einj-core.c */
+extern bool einj_initialized;
+
+static struct { u32 mask; const char *str; } const einj_cxl_error_type_string[] = {
+	{ BIT(12), "CXL.cache Protocol Correctable" },
+	{ BIT(13), "CXL.cache Protocol Uncorrectable non-fatal" },
+	{ BIT(14), "CXL.cache Protocol Uncorrectable fatal" },
+	{ BIT(15), "CXL.mem Protocol Correctable" },
+	{ BIT(16), "CXL.mem Protocol Uncorrectable non-fatal" },
+	{ BIT(17), "CXL.mem Protocol Uncorrectable fatal" },
+};
+
+int einj_cxl_available_error_type_show(struct seq_file *m, void *v)
+{
+	int cxl_err, rc;
+	u32 available_error_type = 0;
+
+	if (!einj_initialized)
+		return -ENXIO;
+
+	rc = einj_get_available_error_type(&available_error_type);
+	if (rc)
+		return rc;
+
+	for (int pos = 0; pos < ARRAY_SIZE(einj_cxl_error_type_string); pos++) {
+		cxl_err = ACPI_EINJ_CXL_CACHE_CORRECTABLE << pos;
+
+		if (available_error_type & cxl_err)
+			seq_printf(m, "0x%08x\t%s\n",
+				   einj_cxl_error_type_string[pos].mask,
+				   einj_cxl_error_type_string[pos].str);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(einj_cxl_available_error_type_show, CXL);
+
+static int cxl_dport_get_sbdf(struct pci_dev *dport_dev, u64 *sbdf)
+{
+	struct pci_bus *pbus;
+	struct pci_host_bridge *bridge;
+	u64 seg = 0, bus;
+
+	pbus = dport_dev->bus;
+	bridge = pci_find_host_bridge(pbus);
+
+	if (!bridge)
+		return -ENODEV;
+
+	if (bridge->domain_nr != PCI_DOMAIN_NR_NOT_SET)
+		seg = bridge->domain_nr;
+
+	bus = pbus->number;
+	*sbdf = (seg << 24) | (bus << 16) | dport_dev->devfn;
+
+	return 0;
+}
+
+int einj_cxl_inject_rch_error(u64 rcrb, u64 type)
+{
+	int rc;
+
+	if (!einj_initialized)
+		return -ENXIO;
+
+	/* Only CXL error types can be specified */
+	if (!einj_is_cxl_error_type(type))
+		return -EINVAL;
+
+	rc = einj_validate_error_type(type);
+	if (rc)
+		return rc;
+
+	return einj_cxl_rch_error_inject(type, 0x2, rcrb, GENMASK_ULL(63, 0),
+					 0, 0);
+}
+EXPORT_SYMBOL_NS_GPL(einj_cxl_inject_rch_error, CXL);
+
+int einj_cxl_inject_error(struct pci_dev *dport, u64 type)
+{
+	u64 param4 = 0;
+	int rc;
+
+	if (!einj_initialized)
+		return -ENXIO;
+
+	/* Only CXL error types can be specified */
+	if (!einj_is_cxl_error_type(type))
+		return -EINVAL;
+
+	rc = einj_validate_error_type(type);
+	if (rc)
+		return rc;
+
+	rc = cxl_dport_get_sbdf(dport, &param4);
+	if (rc)
+		return rc;
+
+	return einj_error_inject(type, 0x4, 0, 0, 0, param4);
+}
+EXPORT_SYMBOL_NS_GPL(einj_cxl_inject_error, CXL);
+
+bool einj_cxl_is_initialized(void)
+{
+	return einj_initialized;
+}
+EXPORT_SYMBOL_NS_GPL(einj_cxl_is_initialized, CXL);
diff --git a/include/linux/einj-cxl.h b/include/linux/einj-cxl.h
new file mode 100644
index 000000000000..4a1f4600539a
--- /dev/null
+++ b/include/linux/einj-cxl.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * CXL protocol Error INJection support.
+ *
+ * Copyright (c) 2023 Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Author: Ben Cheatham <benjamin.cheatham@amd.com>
+ */
+#ifndef EINJ_CXL_H
+#define EINJ_CXL_H
+
+#include <linux/pci.h>
+
+#if IS_ENABLED(CONFIG_ACPI_APEI_EINJ_CXL)
+int einj_cxl_available_error_type_show(struct seq_file *m, void *v);
+int einj_cxl_inject_error(struct pci_dev *dport_dev, u64 type);
+int einj_cxl_inject_rch_error(u64 rcrb, u64 type);
+bool einj_cxl_is_initialized(void);
+#else /* !IS_ENABLED(CONFIG_ACPI_APEI_EINJ_CXL) */
+static inline int einj_cxl_available_error_type_show(struct seq_file *m,
+						     void *v)
+{
+	return -ENXIO;
+}
+
+static inline int einj_cxl_inject_error(struct pci_dev *dport_dev, u64 type)
+{
+	return -ENXIO;
+}
+
+static inline int einj_cxl_inject_rch_error(u64 rcrb, u64 type)
+{
+	return -ENXIO;
+}
+
+static inline bool einj_cxl_is_initialized(void) { return false; }
+#endif /* CONFIG_ACPI_APEI_EINJ_CXL */
+
+#endif /* EINJ_CXL_H */
-- 
2.34.1

Re: [PATCH v14 2/4] EINJ: Add CXL error type support

[Dan Williams]

Ben Cheatham wrote:
> This patch had an outdated commit message, so here's the patch with an updated description.
> 
> I also realized that I was wrong about letting CXL 2.0+ error types (discussed a revision
> or two ago) and I wasn't actually letting them through. I've went ahead and added
> the ability to inject CXL 2.0+ error through the legacy interface. This pretty
> much amounts to returning an error for CXL 1.0/1.1 injection types in einj_error_inject()
> and instead routing them through a new einj_cxl_rch_error_inject() function called
> in einj-cxl.c
> 
> If this change is too big I can send in another revision, I just wanted to avoid
> spamming the list(s).
> 
> From eea1cf991dc2a551f6db2e3bb9510ed43c86762d Mon Sep 17 00:00:00 2001
> From: Ben Cheatham <Benjamin.Cheatham@amd.com>
> Date: Fri, 16 Feb 2024 11:12:51 -0600
> Subject: [PATCH v14 2/4] EINJ: Add CXL error type support
> 
> Move CXL protocol error types from einj.c (now einj-core.c) to einj-cxl.c.
> einj-cxl.c implements the necessary handling for CXL protocol error
> injection and exposes an API for the CXL core to use said functionality,
> while also allowing the EINJ module to be built without CXL support.
> Because CXL error types targeting CXL 1.0/1.1 ports require special
> handling, only allow them to be injected through the new cxl debugfs
> interface (next commit) and return an error when attempting to inject
> through the legacy interface.
> 
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
> ---
>  MAINTAINERS                               |   1 +
>  drivers/acpi/apei/Kconfig                 |  12 +++
>  drivers/acpi/apei/Makefile                |   2 +
>  drivers/acpi/apei/apei-internal.h         |  18 ++++
>  drivers/acpi/apei/{einj.c => einj-core.c} |  85 +++++++++++----
>  drivers/acpi/apei/einj-cxl.c              | 121 ++++++++++++++++++++++
>  include/linux/einj-cxl.h                  |  40 +++++++
>  7 files changed, 257 insertions(+), 22 deletions(-)
>  rename drivers/acpi/apei/{einj.c => einj-core.c} (94%)
>  create mode 100644 drivers/acpi/apei/einj-cxl.c
>  create mode 100644 include/linux/einj-cxl.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2ecaaec6a6bf..90cf8403dd17 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5289,6 +5289,7 @@ M:	Dan Williams <dan.j.williams@intel.com>
>  L:	linux-cxl@vger.kernel.org
>  S:	Maintained
>  F:	drivers/cxl/
> +F:	include/linux/cxl-einj.h
>  F:	include/linux/cxl-event.h
>  F:	include/uapi/linux/cxl_mem.h
>  F:	tools/testing/cxl/
> diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
> index 6b18f8bc7be3..f01afa2805be 100644
> --- a/drivers/acpi/apei/Kconfig
> +++ b/drivers/acpi/apei/Kconfig
> @@ -60,6 +60,18 @@ config ACPI_APEI_EINJ
>  	  mainly used for debugging and testing the other parts of
>  	  APEI and some other RAS features.
>  
> +config ACPI_APEI_EINJ_CXL
> +	bool "CXL Error INJection Support"
> +	default ACPI_APEI_EINJ
> +	depends on ACPI_APEI_EINJ && CXL_BUS <= ACPI_APEI_EINJ
> +	help
> +	  Support for CXL protocol Error INJection through debugfs/cxl.
> +	  Availability and which errors are supported is dependent on
> +	  the host platform. Look to ACPI v6.5 section 18.6.4 and kernel
> +	  EINJ documentation for more information.
> +
> +	  If unsure say 'n'
> +
>  config ACPI_APEI_ERST_DEBUG
>  	tristate "APEI Error Record Serialization Table (ERST) Debug Support"
>  	depends on ACPI_APEI
> diff --git a/drivers/acpi/apei/Makefile b/drivers/acpi/apei/Makefile
> index 4dfac2128737..2c474e6477e1 100644
> --- a/drivers/acpi/apei/Makefile
> +++ b/drivers/acpi/apei/Makefile
> @@ -2,6 +2,8 @@
>  obj-$(CONFIG_ACPI_APEI)		+= apei.o
>  obj-$(CONFIG_ACPI_APEI_GHES)	+= ghes.o
>  obj-$(CONFIG_ACPI_APEI_EINJ)	+= einj.o
> +einj-y				:= einj-core.o
> +einj-$(CONFIG_ACPI_APEI_EINJ_CXL) += einj-cxl.o
>  obj-$(CONFIG_ACPI_APEI_ERST_DEBUG) += erst-dbg.o
>  
>  apei-y := apei-base.o hest.o erst.o bert.o
> diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
> index 67c2c3b959e1..cd2766c69d78 100644
> --- a/drivers/acpi/apei/apei-internal.h
> +++ b/drivers/acpi/apei/apei-internal.h
> @@ -130,4 +130,22 @@ static inline u32 cper_estatus_len(struct acpi_hest_generic_status *estatus)
>  }
>  
>  int apei_osc_setup(void);
> +
> +int einj_get_available_error_type(u32 *type);
> +int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
> +		      u64 param4);
> +int einj_cxl_rch_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> +			      u64 param3, u64 param4);
> +bool einj_is_cxl_error_type(u64 type);
> +int einj_validate_error_type(u64 type);
> +
> +#ifndef ACPI_EINJ_CXL_CACHE_CORRECTABLE
> +#define ACPI_EINJ_CXL_CACHE_CORRECTABLE     BIT(12)
> +#define ACPI_EINJ_CXL_CACHE_UNCORRECTABLE   BIT(13)
> +#define ACPI_EINJ_CXL_CACHE_FATAL           BIT(14)
> +#define ACPI_EINJ_CXL_MEM_CORRECTABLE       BIT(15)
> +#define ACPI_EINJ_CXL_MEM_UNCORRECTABLE     BIT(16)
> +#define ACPI_EINJ_CXL_MEM_FATAL             BIT(17)
> +#endif
> +
>  #endif
> diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj-core.c
> similarity index 94%
> rename from drivers/acpi/apei/einj.c
> rename to drivers/acpi/apei/einj-core.c
> index 937c69844dac..437c13949be7 100644
> --- a/drivers/acpi/apei/einj.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -37,6 +37,12 @@
>  #define MEM_ERROR_MASK		(ACPI_EINJ_MEMORY_CORRECTABLE | \
>  				ACPI_EINJ_MEMORY_UNCORRECTABLE | \
>  				ACPI_EINJ_MEMORY_FATAL)
> +#define CXL_ERROR_MASK		(ACPI_EINJ_CXL_CACHE_CORRECTABLE | \
> +				ACPI_EINJ_CXL_CACHE_UNCORRECTABLE | \
> +				ACPI_EINJ_CXL_CACHE_FATAL | \
> +				ACPI_EINJ_CXL_MEM_CORRECTABLE | \
> +				ACPI_EINJ_CXL_MEM_UNCORRECTABLE | \
> +				ACPI_EINJ_CXL_MEM_FATAL)
>  
>  /*
>   * ACPI version 5 provides a SET_ERROR_TYPE_WITH_ADDRESS action.
> @@ -141,7 +147,7 @@ static DEFINE_MUTEX(einj_mutex);
>  /*
>   * Exported APIs use this flag to exit early if einj_probe() failed.
>   */
> -static bool einj_initialized __ro_after_init;
> +bool einj_initialized __ro_after_init;
>  
>  static void *einj_param;
>  
> @@ -166,7 +172,7 @@ static int __einj_get_available_error_type(u32 *type)
>  }
>  
>  /* Get error injection capabilities of the platform */
> -static int einj_get_available_error_type(u32 *type)
> +int einj_get_available_error_type(u32 *type)
>  {
>  	int rc;
>  
> @@ -536,8 +542,8 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>  }
>  
>  /* Inject the specified hardware error */
> -static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> -			     u64 param3, u64 param4)
> +int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
> +		      u64 param4)
>  {
>  	int rc;
>  	u64 base_addr, size;
> @@ -560,8 +566,18 @@ static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>  	if (type & ACPI5_VENDOR_BIT) {
>  		if (vendor_flags != SETWA_FLAGS_MEM)
>  			goto inject;
> -	} else if (!(type & MEM_ERROR_MASK) && !(flags & SETWA_FLAGS_MEM))
> +	} else if (!(type & MEM_ERROR_MASK) && !(flags & SETWA_FLAGS_MEM)) {
>  		goto inject;
> +	}
> +
> +	/*
> +	 * Injections targeting a CXL 1.0/1.1 port have to be injected
> +	 * from the CXL debugfs interface so that we can guarantee a
> +	 * correct MMIO address.
> +	 */

Given that the CXL debugfs is not present in this file I would update
this comment to give a hints using local references. Something like:

	/*
	 * Injections targeting a CXL 1.0/1.1 port have to be injected
	 * via the einj_cxl_rch_error_inject() path as that does proper
	 * input validation that passed address is an RCRB base address.
	 */

...that said, how does this work for the CXL 2.0 path? Does not
einj_cxl_inject_error() need to use __einj_error_inject() rather than
einj_error_inject() to get by this check?

Re: [PATCH v14 2/4] EINJ: Add CXL error type support

[Ben Cheatham]

On 2/28/24 12:00 AM, Dan Williams wrote:
> Ben Cheatham wrote:
>> This patch had an outdated commit message, so here's the patch with an updated description.
>>
>> I also realized that I was wrong about letting CXL 2.0+ error types (discussed a revision
>> or two ago) and I wasn't actually letting them through. I've went ahead and added
>> the ability to inject CXL 2.0+ error through the legacy interface. This pretty
>> much amounts to returning an error for CXL 1.0/1.1 injection types in einj_error_inject()
>> and instead routing them through a new einj_cxl_rch_error_inject() function called
>> in einj-cxl.c
>>
>> If this change is too big I can send in another revision, I just wanted to avoid
>> spamming the list(s).
>>
>> From eea1cf991dc2a551f6db2e3bb9510ed43c86762d Mon Sep 17 00:00:00 2001
>> From: Ben Cheatham <Benjamin.Cheatham@amd.com>
>> Date: Fri, 16 Feb 2024 11:12:51 -0600
>> Subject: [PATCH v14 2/4] EINJ: Add CXL error type support
>>
>> Move CXL protocol error types from einj.c (now einj-core.c) to einj-cxl.c.
>> einj-cxl.c implements the necessary handling for CXL protocol error
>> injection and exposes an API for the CXL core to use said functionality,
>> while also allowing the EINJ module to be built without CXL support.
>> Because CXL error types targeting CXL 1.0/1.1 ports require special
>> handling, only allow them to be injected through the new cxl debugfs
>> interface (next commit) and return an error when attempting to inject
>> through the legacy interface.
>>
>> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
>> ---
>>  MAINTAINERS                               |   1 +
>>  drivers/acpi/apei/Kconfig                 |  12 +++
>>  drivers/acpi/apei/Makefile                |   2 +
>>  drivers/acpi/apei/apei-internal.h         |  18 ++++
>>  drivers/acpi/apei/{einj.c => einj-core.c} |  85 +++++++++++----
>>  drivers/acpi/apei/einj-cxl.c              | 121 ++++++++++++++++++++++
>>  include/linux/einj-cxl.h                  |  40 +++++++
>>  7 files changed, 257 insertions(+), 22 deletions(-)
>>  rename drivers/acpi/apei/{einj.c => einj-core.c} (94%)
>>  create mode 100644 drivers/acpi/apei/einj-cxl.c
>>  create mode 100644 include/linux/einj-cxl.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 2ecaaec6a6bf..90cf8403dd17 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -5289,6 +5289,7 @@ M:	Dan Williams <dan.j.williams@intel.com>
>>  L:	linux-cxl@vger.kernel.org
>>  S:	Maintained
>>  F:	drivers/cxl/
>> +F:	include/linux/cxl-einj.h
>>  F:	include/linux/cxl-event.h
>>  F:	include/uapi/linux/cxl_mem.h
>>  F:	tools/testing/cxl/
>> diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
>> index 6b18f8bc7be3..f01afa2805be 100644
>> --- a/drivers/acpi/apei/Kconfig
>> +++ b/drivers/acpi/apei/Kconfig
>> @@ -60,6 +60,18 @@ config ACPI_APEI_EINJ
>>  	  mainly used for debugging and testing the other parts of
>>  	  APEI and some other RAS features.
>>  
>> +config ACPI_APEI_EINJ_CXL
>> +	bool "CXL Error INJection Support"
>> +	default ACPI_APEI_EINJ
>> +	depends on ACPI_APEI_EINJ && CXL_BUS <= ACPI_APEI_EINJ
>> +	help
>> +	  Support for CXL protocol Error INJection through debugfs/cxl.
>> +	  Availability and which errors are supported is dependent on
>> +	  the host platform. Look to ACPI v6.5 section 18.6.4 and kernel
>> +	  EINJ documentation for more information.
>> +
>> +	  If unsure say 'n'
>> +
>>  config ACPI_APEI_ERST_DEBUG
>>  	tristate "APEI Error Record Serialization Table (ERST) Debug Support"
>>  	depends on ACPI_APEI
>> diff --git a/drivers/acpi/apei/Makefile b/drivers/acpi/apei/Makefile
>> index 4dfac2128737..2c474e6477e1 100644
>> --- a/drivers/acpi/apei/Makefile
>> +++ b/drivers/acpi/apei/Makefile
>> @@ -2,6 +2,8 @@
>>  obj-$(CONFIG_ACPI_APEI)		+= apei.o
>>  obj-$(CONFIG_ACPI_APEI_GHES)	+= ghes.o
>>  obj-$(CONFIG_ACPI_APEI_EINJ)	+= einj.o
>> +einj-y				:= einj-core.o
>> +einj-$(CONFIG_ACPI_APEI_EINJ_CXL) += einj-cxl.o
>>  obj-$(CONFIG_ACPI_APEI_ERST_DEBUG) += erst-dbg.o
>>  
>>  apei-y := apei-base.o hest.o erst.o bert.o
>> diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
>> index 67c2c3b959e1..cd2766c69d78 100644
>> --- a/drivers/acpi/apei/apei-internal.h
>> +++ b/drivers/acpi/apei/apei-internal.h
>> @@ -130,4 +130,22 @@ static inline u32 cper_estatus_len(struct acpi_hest_generic_status *estatus)
>>  }
>>  
>>  int apei_osc_setup(void);
>> +
>> +int einj_get_available_error_type(u32 *type);
>> +int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
>> +		      u64 param4);
>> +int einj_cxl_rch_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>> +			      u64 param3, u64 param4);
>> +bool einj_is_cxl_error_type(u64 type);
>> +int einj_validate_error_type(u64 type);
>> +
>> +#ifndef ACPI_EINJ_CXL_CACHE_CORRECTABLE
>> +#define ACPI_EINJ_CXL_CACHE_CORRECTABLE     BIT(12)
>> +#define ACPI_EINJ_CXL_CACHE_UNCORRECTABLE   BIT(13)
>> +#define ACPI_EINJ_CXL_CACHE_FATAL           BIT(14)
>> +#define ACPI_EINJ_CXL_MEM_CORRECTABLE       BIT(15)
>> +#define ACPI_EINJ_CXL_MEM_UNCORRECTABLE     BIT(16)
>> +#define ACPI_EINJ_CXL_MEM_FATAL             BIT(17)
>> +#endif
>> +
>>  #endif
>> diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj-core.c
>> similarity index 94%
>> rename from drivers/acpi/apei/einj.c
>> rename to drivers/acpi/apei/einj-core.c
>> index 937c69844dac..437c13949be7 100644
>> --- a/drivers/acpi/apei/einj.c
>> +++ b/drivers/acpi/apei/einj-core.c
>> @@ -37,6 +37,12 @@
>>  #define MEM_ERROR_MASK		(ACPI_EINJ_MEMORY_CORRECTABLE | \
>>  				ACPI_EINJ_MEMORY_UNCORRECTABLE | \
>>  				ACPI_EINJ_MEMORY_FATAL)
>> +#define CXL_ERROR_MASK		(ACPI_EINJ_CXL_CACHE_CORRECTABLE | \
>> +				ACPI_EINJ_CXL_CACHE_UNCORRECTABLE | \
>> +				ACPI_EINJ_CXL_CACHE_FATAL | \
>> +				ACPI_EINJ_CXL_MEM_CORRECTABLE | \
>> +				ACPI_EINJ_CXL_MEM_UNCORRECTABLE | \
>> +				ACPI_EINJ_CXL_MEM_FATAL)
>>  
>>  /*
>>   * ACPI version 5 provides a SET_ERROR_TYPE_WITH_ADDRESS action.
>> @@ -141,7 +147,7 @@ static DEFINE_MUTEX(einj_mutex);
>>  /*
>>   * Exported APIs use this flag to exit early if einj_probe() failed.
>>   */
>> -static bool einj_initialized __ro_after_init;
>> +bool einj_initialized __ro_after_init;
>>  
>>  static void *einj_param;
>>  
>> @@ -166,7 +172,7 @@ static int __einj_get_available_error_type(u32 *type)
>>  }
>>  
>>  /* Get error injection capabilities of the platform */
>> -static int einj_get_available_error_type(u32 *type)
>> +int einj_get_available_error_type(u32 *type)
>>  {
>>  	int rc;
>>  
>> @@ -536,8 +542,8 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>>  }
>>  
>>  /* Inject the specified hardware error */
>> -static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>> -			     u64 param3, u64 param4)
>> +int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
>> +		      u64 param4)
>>  {
>>  	int rc;
>>  	u64 base_addr, size;
>> @@ -560,8 +566,18 @@ static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>>  	if (type & ACPI5_VENDOR_BIT) {
>>  		if (vendor_flags != SETWA_FLAGS_MEM)
>>  			goto inject;
>> -	} else if (!(type & MEM_ERROR_MASK) && !(flags & SETWA_FLAGS_MEM))
>> +	} else if (!(type & MEM_ERROR_MASK) && !(flags & SETWA_FLAGS_MEM)) {
>>  		goto inject;
>> +	}
>> +
>> +	/*
>> +	 * Injections targeting a CXL 1.0/1.1 port have to be injected
>> +	 * from the CXL debugfs interface so that we can guarantee a
>> +	 * correct MMIO address.
>> +	 */
> 
> Given that the CXL debugfs is not present in this file I would update
> this comment to give a hints using local references. Something like:
> 
> 	/*
> 	 * Injections targeting a CXL 1.0/1.1 port have to be injected
> 	 * via the einj_cxl_rch_error_inject() path as that does proper
> 	 * input validation that passed address is an RCRB base address.
> 	 */
> 

I agree, I'll change it to something along those lines.

> ...that said, how does this work for the CXL 2.0 path? Does not
> einj_cxl_inject_error() need to use __einj_error_inject() rather than
> einj_error_inject() to get by this check?
> 

The way that the target port type (1.0 vs. 2.0+) is differentiated is whether
we are targeting a memory address, which is what SETWA_FLAGS_MEM indicates.
So for the CXL 2.0+ path, the check will pass. Although the error type
is a CXL error type, the flags variable will be set to SETWA_FLAGS_SBDF
(I'm 90% sure that's what it's called, but it's something along those lines).
Let me know if that doesn't answer your question!

Re: [PATCH v14 2/4] EINJ: Add CXL error type support

[Dan Williams]

Ben Cheatham wrote:
> This patch had an outdated commit message, so here's the patch with an updated description.
> 
> I also realized that I was wrong about letting CXL 2.0+ error types (discussed a revision
> or two ago) and I wasn't actually letting them through. I've went ahead and added
> the ability to inject CXL 2.0+ error through the legacy interface. This pretty
> much amounts to returning an error for CXL 1.0/1.1 injection types in einj_error_inject()
> and instead routing them through a new einj_cxl_rch_error_inject() function called
> in einj-cxl.c
> 
> If this change is too big I can send in another revision, I just wanted to avoid
> spamming the list(s).

The thing to avoid is sending too many versions too quickly, and making
sure that upstream can reassemble the set with minimal effort. In this
case now that I have a few review comments and the fact that b4 likely
has no chance at assembling this correctly I would go ahead and fixup
those comments for a v15.

Re: [PATCH v14 2/4] EINJ: Add CXL error type support

[Ben Cheatham]

On 2/28/24 12:04 AM, Dan Williams wrote:
> Ben Cheatham wrote:
>> This patch had an outdated commit message, so here's the patch with an updated description.
>>
>> I also realized that I was wrong about letting CXL 2.0+ error types (discussed a revision
>> or two ago) and I wasn't actually letting them through. I've went ahead and added
>> the ability to inject CXL 2.0+ error through the legacy interface. This pretty
>> much amounts to returning an error for CXL 1.0/1.1 injection types in einj_error_inject()
>> and instead routing them through a new einj_cxl_rch_error_inject() function called
>> in einj-cxl.c
>>
>> If this change is too big I can send in another revision, I just wanted to avoid
>> spamming the list(s).
> 
> The thing to avoid is sending too many versions too quickly, and making
> sure that upstream can reassemble the set with minimal effort. In this
> case now that I have a few review comments and the fact that b4 likely
> has no chance at assembling this correctly I would go ahead and fixup
> those comments for a v15.

Alright that makes sense. I'll put together a v15 and send it out later this week/early
next week.

Thanks,
Ben

Re: [PATCH v14 2/4] EINJ: Add CXL error type support

[Jonathan Cameron]

On Mon, 26 Feb 2024 16:27:02 -0600
Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:

> Remove CXL protocol error types from the EINJ module and move them to
> a new einj_cxl module. The einj_cxl module implements the necessary
> handling for CXL protocol error injection and exposes an API for the
> CXL core to use said functionality. Because the CXL error types
> require special handling, only allow them to be injected through the
> einj_cxl module and return an error when attempting to inject through
> "regular" EINJ.
> 
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
Hi Ben,

Some minor comments inline given you are doing a v15 (yikes!)

Jonathan

> diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj-core.c
> similarity index 94%
> rename from drivers/acpi/apei/einj.c
> rename to drivers/acpi/apei/einj-core.c
> index 937c69844dac..1a5f53d81d09 100644
> --- a/drivers/acpi/apei/einj.c
> +++ b/drivers/acpi/apei/einj-core.c

...

> @@ -640,29 +648,21 @@ static int available_error_type_show(struct seq_file *m, void *v)
>  
>  DEFINE_SHOW_ATTRIBUTE(available_error_type);
>  
> -static int error_type_get(void *data, u64 *val)
> -{
> -	*val = error_type;
> -
> -	return 0;
> -}
> -
> -static int error_type_set(void *data, u64 val)
> +int einj_validate_error_type(u64 type)
>  {
> +	u32 tval, vendor, available_error_type = 0;
>  	int rc;
> -	u32 available_error_type = 0;
> -	u32 tval, vendor;
>  
>  	/* Only low 32 bits for error type are valid */
> -	if (val & GENMASK_ULL(63, 32))
> +	if (type & GENMASK_ULL(63, 32))
>  		return -EINVAL;
>  
>  	/*
>  	 * Vendor defined types have 0x80000000 bit set, and
>  	 * are not enumerated by ACPI_EINJ_GET_ERROR_TYPE
>  	 */
> -	vendor = val & ACPI5_VENDOR_BIT;
> -	tval = val & 0x7fffffff;
> +	vendor = type & ACPI5_VENDOR_BIT;
> +	tval = type & GENMASK(30, 0);
>  
>  	/* Only one error type can be specified */
>  	if (tval & (tval - 1))
> @@ -671,9 +671,37 @@ static int error_type_set(void *data, u64 val)
>  		rc = einj_get_available_error_type(&available_error_type);
>  		if (rc)
>  			return rc;
> -		if (!(val & available_error_type))
> +		if (!(type & available_error_type))
>  			return -EINVAL;
>  	}
> +
> +	return 0;
> +}
> +
> +bool einj_is_cxl_error_type(u64 type)
> +{
> +	return (type & CXL_ERROR_MASK) && (!(type & ACPI5_VENDOR_BIT));
> +}
> +
> +static int error_type_get(void *data, u64 *val)
This is reordered, but fairly sure no need to do so and it will
make patch cleaner to leave it above the validation code.

> +{
> +	*val = error_type;
> +
> +	return 0;
> +}
> +
> +static int error_type_set(void *data, u64 val)
> +{
> +	int rc;
> +
> +	/* CXL error types have to be injected from cxl debugfs */
> +	if (einj_is_cxl_error_type(val))
> +		return -EINVAL;
> +
> +	rc = einj_validate_error_type(val);

Trivial but I'd have preferred this factoring out in a precursor patch
just to make reviewing this a tiny bit easier.


> +	if (rc)
> +		return rc;
> +
>  	error_type = val;
>  
>  	return 0;
> diff --git a/drivers/acpi/apei/einj-cxl.c b/drivers/acpi/apei/einj-cxl.c
> new file mode 100644
> index 000000000000..34badc6a801e
> --- /dev/null
> +++ b/drivers/acpi/apei/einj-cxl.c
> @@ -0,0 +1,120 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * CXL Error INJection support. Used by CXL core to inject
> + * protocol errors into CXL ports.
> + *
> + * Copyright (C) 2023 Advanced Micro Devices, Inc.
> + *
> + * Author: Ben Cheatham <benjamin.cheatham@amd.com>
> + */
> +#include <linux/einj-cxl.h>
> +#include <linux/debugfs.h>
Follow include what you use principles a little closer
(there are always exceptions in kernel world...).
The following seems resonable to me.

#include <linux/array_size.h>
#include <linux/seq_file.h>


> +
> +#include "apei-internal.h"
> +
> +/* Defined in einj-core.c */
> +extern bool einj_initialized;
> +
> +static struct { u32 mask; const char *str; } const einj_cxl_error_type_string[] = {
> +	{ BIT(12), "CXL.cache Protocol Correctable" },

Use the defines for the bits? Not sure why the original code didn't do so other
than maybe long line lengths?



> +	{ BIT(13), "CXL.cache Protocol Uncorrectable non-fatal" },
> +	{ BIT(14), "CXL.cache Protocol Uncorrectable fatal" },
> +	{ BIT(15), "CXL.mem Protocol Correctable" },
> +	{ BIT(16), "CXL.mem Protocol Uncorrectable non-fatal" },
> +	{ BIT(17), "CXL.mem Protocol Uncorrectable fatal" },
> +};
> +
> +int einj_cxl_available_error_type_show(struct seq_file *m, void *v)
> +{
> +	int cxl_err, rc;
> +	u32 available_error_type = 0;
> +
> +	if (!einj_initialized)
> +		return -ENXIO;
> +
> +	rc = einj_get_available_error_type(&available_error_type);
> +	if (rc)
> +		return rc;
> +
> +	for (int pos = 0; pos < ARRAY_SIZE(einj_cxl_error_type_string); pos++) {
> +		cxl_err = ACPI_EINJ_CXL_CACHE_CORRECTABLE << pos;
Hmm. This is a little ugly.
Could do something like the following bit it's of similar level of ugly
so up to you.

	int bit_pos = ACPI_EINJ_CXL_CACHE_CORRECTABLE;
	for_each_bit_set_bit_from(bit_pos, &available_error_type,
			     ARRAY_SIZE(einj_cxl_error_type_string)) {
		int pos = bit_pos - ACPI_EINJ_CXL_CACHE_CORRECTABLE;
	
	

> +
> +		if (available_error_type & cxl_err)
> +			seq_printf(m, "0x%08x\t%s\n",
> +				   einj_cxl_error_type_string[pos].mask,
> +				   einj_cxl_error_type_string[pos].str);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(einj_cxl_available_error_type_show, CXL);


> diff --git a/include/linux/einj-cxl.h b/include/linux/einj-cxl.h
> new file mode 100644
> index 000000000000..4a1f4600539a
> --- /dev/null
> +++ b/include/linux/einj-cxl.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * CXL protocol Error INJection support.
> + *
> + * Copyright (c) 2023 Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Author: Ben Cheatham <benjamin.cheatham@amd.com>
> + */
> +#ifndef EINJ_CXL_H
> +#define EINJ_CXL_H
> +
> +#include <linux/pci.h>
Use a forwards def

struct pci_dev;

and drop the include. Also need

struct seq_file;


> +
> +#if IS_ENABLED(CONFIG_ACPI_APEI_EINJ_CXL)
> +int einj_cxl_available_error_type_show(struct seq_file *m, void *v);
> +int einj_cxl_inject_error(struct pci_dev *dport_dev, u64 type);
> +int einj_cxl_inject_rch_error(u64 rcrb, u64 type);
> +bool einj_cxl_is_initialized(void);
> +#else /* !IS_ENABLED(CONFIG_ACPI_APEI_EINJ_CXL) */
> +static inline int einj_cxl_available_error_type_show(struct seq_file *m,
> +						     void *v)
> +{
> +	return -ENXIO;
> +}
> +
> +static inline int einj_cxl_inject_error(struct pci_dev *dport_dev, u64 type)
> +{
> +	return -ENXIO;
> +}
> +
> +static inline int einj_cxl_inject_rch_error(u64 rcrb, u64 type)
> +{
> +	return -ENXIO;
> +}
> +
> +static inline bool einj_cxl_is_initialized(void) { return false; }
> +#endif /* CONFIG_ACPI_APEI_EINJ_CXL */
> +
> +#endif /* EINJ_CXL_H */

Re: [PATCH v14 2/4] EINJ: Add CXL error type support

[Ben Cheatham]

Hey Jonathan, thanks for taking a look!

On 3/7/24 6:09 AM, Jonathan Cameron wrote:
> On Mon, 26 Feb 2024 16:27:02 -0600
> Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:
> 
>> Remove CXL protocol error types from the EINJ module and move them to
>> a new einj_cxl module. The einj_cxl module implements the necessary
>> handling for CXL protocol error injection and exposes an API for the
>> CXL core to use said functionality. Because the CXL error types
>> require special handling, only allow them to be injected through the
>> einj_cxl module and return an error when attempting to inject through
>> "regular" EINJ.
>>
>> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
> Hi Ben,
> 
> Some minor comments inline given you are doing a v15 (yikes!)
> 

Yeah I know :(.

> Jonathan
> 
>> diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj-core.c
>> similarity index 94%
>> rename from drivers/acpi/apei/einj.c
>> rename to drivers/acpi/apei/einj-core.c
>> index 937c69844dac..1a5f53d81d09 100644
>> --- a/drivers/acpi/apei/einj.c
>> +++ b/drivers/acpi/apei/einj-core.c
> 
> ...
> 
>> @@ -640,29 +648,21 @@ static int available_error_type_show(struct seq_file *m, void *v)
>>  
>>  DEFINE_SHOW_ATTRIBUTE(available_error_type);
>>  
>> -static int error_type_get(void *data, u64 *val)
>> -{
>> -	*val = error_type;
>> -
>> -	return 0;
>> -}
>> -
>> -static int error_type_set(void *data, u64 val)
>> +int einj_validate_error_type(u64 type)
>>  {
>> +	u32 tval, vendor, available_error_type = 0;
>>  	int rc;
>> -	u32 available_error_type = 0;
>> -	u32 tval, vendor;
>>  
>>  	/* Only low 32 bits for error type are valid */
>> -	if (val & GENMASK_ULL(63, 32))
>> +	if (type & GENMASK_ULL(63, 32))
>>  		return -EINVAL;
>>  
>>  	/*
>>  	 * Vendor defined types have 0x80000000 bit set, and
>>  	 * are not enumerated by ACPI_EINJ_GET_ERROR_TYPE
>>  	 */
>> -	vendor = val & ACPI5_VENDOR_BIT;
>> -	tval = val & 0x7fffffff;
>> +	vendor = type & ACPI5_VENDOR_BIT;
>> +	tval = type & GENMASK(30, 0);
>>  
>>  	/* Only one error type can be specified */
>>  	if (tval & (tval - 1))
>> @@ -671,9 +671,37 @@ static int error_type_set(void *data, u64 val)
>>  		rc = einj_get_available_error_type(&available_error_type);
>>  		if (rc)
>>  			return rc;
>> -		if (!(val & available_error_type))
>> +		if (!(type & available_error_type))
>>  			return -EINVAL;
>>  	}
>> +
>> +	return 0;
>> +}
>> +
>> +bool einj_is_cxl_error_type(u64 type)
>> +{
>> +	return (type & CXL_ERROR_MASK) && (!(type & ACPI5_VENDOR_BIT));
>> +}
>> +
>> +static int error_type_get(void *data, u64 *val)
> This is reordered, but fairly sure no need to do so and it will
> make patch cleaner to leave it above the validation code.
> 

Sorry it's been a bit, but I think I moved it to go with the other error_type
functions. I don't mind leaving it where it was though.

>> +{
>> +	*val = error_type;
>> +
>> +	return 0;
>> +}
>> +
>> +static int error_type_set(void *data, u64 val)
>> +{
>> +	int rc;
>> +
>> +	/* CXL error types have to be injected from cxl debugfs */
>> +	if (einj_is_cxl_error_type(val))
>> +		return -EINVAL;
>> +
>> +	rc = einj_validate_error_type(val);
> 
> Trivial but I'd have preferred this factoring out in a precursor patch
> just to make reviewing this a tiny bit easier.
> 

It would probably make sense, but at this point I just want to get this
across the finish line :).

> 
>> +	if (rc)
>> +		return rc;
>> +
>>  	error_type = val;
>>  
>>  	return 0;
>> diff --git a/drivers/acpi/apei/einj-cxl.c b/drivers/acpi/apei/einj-cxl.c
>> new file mode 100644
>> index 000000000000..34badc6a801e
>> --- /dev/null
>> +++ b/drivers/acpi/apei/einj-cxl.c
>> @@ -0,0 +1,120 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * CXL Error INJection support. Used by CXL core to inject
>> + * protocol errors into CXL ports.
>> + *
>> + * Copyright (C) 2023 Advanced Micro Devices, Inc.
>> + *
>> + * Author: Ben Cheatham <benjamin.cheatham@amd.com>
>> + */
>> +#include <linux/einj-cxl.h>
>> +#include <linux/debugfs.h>
> Follow include what you use principles a little closer
> (there are always exceptions in kernel world...).
> The following seems resonable to me.
> 
> #include <linux/array_size.h>
> #include <linux/seq_file.h>
> 

Will do!

> 
>> +
>> +#include "apei-internal.h"
>> +
>> +/* Defined in einj-core.c */
>> +extern bool einj_initialized;
>> +
>> +static struct { u32 mask; const char *str; } const einj_cxl_error_type_string[] = {
>> +	{ BIT(12), "CXL.cache Protocol Correctable" },
> 
> Use the defines for the bits? Not sure why the original code didn't do so other
> than maybe long line lengths?
> 

I agree, this was merged from some ACPI tree changes in rc4 and I don't think
they have the defines in that tree yet.

> 
> 
>> +	{ BIT(13), "CXL.cache Protocol Uncorrectable non-fatal" },
>> +	{ BIT(14), "CXL.cache Protocol Uncorrectable fatal" },
>> +	{ BIT(15), "CXL.mem Protocol Correctable" },
>> +	{ BIT(16), "CXL.mem Protocol Uncorrectable non-fatal" },
>> +	{ BIT(17), "CXL.mem Protocol Uncorrectable fatal" },
>> +};
>> +
>> +int einj_cxl_available_error_type_show(struct seq_file *m, void *v)
>> +{
>> +	int cxl_err, rc;
>> +	u32 available_error_type = 0;
>> +
>> +	if (!einj_initialized)
>> +		return -ENXIO;
>> +
>> +	rc = einj_get_available_error_type(&available_error_type);
>> +	if (rc)
>> +		return rc;
>> +
>> +	for (int pos = 0; pos < ARRAY_SIZE(einj_cxl_error_type_string); pos++) {
>> +		cxl_err = ACPI_EINJ_CXL_CACHE_CORRECTABLE << pos;
> Hmm. This is a little ugly.
> Could do something like the following bit it's of similar level of ugly
> so up to you.
> 
> 	int bit_pos = ACPI_EINJ_CXL_CACHE_CORRECTABLE;
> 	for_each_bit_set_bit_from(bit_pos, &available_error_type,
> 			     ARRAY_SIZE(einj_cxl_error_type_string)) {
> 		int pos = bit_pos - ACPI_EINJ_CXL_CACHE_CORRECTABLE;
> 	

I agree it's ugly. I think this version has the added benfit of parity
with einj_available_error_type_show() in einj-core.c, so I think it's
better to keep it this way if it's the same to you.

> 	
> 
>> +
>> +		if (available_error_type & cxl_err)
>> +			seq_printf(m, "0x%08x\t%s\n",
>> +				   einj_cxl_error_type_string[pos].mask,
>> +				   einj_cxl_error_type_string[pos].str);
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(einj_cxl_available_error_type_show, CXL);
> 
> 
>> diff --git a/include/linux/einj-cxl.h b/include/linux/einj-cxl.h
>> new file mode 100644
>> index 000000000000..4a1f4600539a
>> --- /dev/null
>> +++ b/include/linux/einj-cxl.h
>> @@ -0,0 +1,40 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * CXL protocol Error INJection support.
>> + *
>> + * Copyright (c) 2023 Advanced Micro Devices, Inc.
>> + * All Rights Reserved.
>> + *
>> + * Author: Ben Cheatham <benjamin.cheatham@amd.com>
>> + */
>> +#ifndef EINJ_CXL_H
>> +#define EINJ_CXL_H
>> +
>> +#include <linux/pci.h>
> Use a forwards def
> 
> struct pci_dev;
> 
> and drop the include. Also need
> 
> struct seq_file;
> 

Will do.

> 
>> +
>> +#if IS_ENABLED(CONFIG_ACPI_APEI_EINJ_CXL)
>> +int einj_cxl_available_error_type_show(struct seq_file *m, void *v);
>> +int einj_cxl_inject_error(struct pci_dev *dport_dev, u64 type);
>> +int einj_cxl_inject_rch_error(u64 rcrb, u64 type);
>> +bool einj_cxl_is_initialized(void);
>> +#else /* !IS_ENABLED(CONFIG_ACPI_APEI_EINJ_CXL) */
>> +static inline int einj_cxl_available_error_type_show(struct seq_file *m,
>> +						     void *v)
>> +{
>> +	return -ENXIO;
>> +}
>> +
>> +static inline int einj_cxl_inject_error(struct pci_dev *dport_dev, u64 type)
>> +{
>> +	return -ENXIO;
>> +}
>> +
>> +static inline int einj_cxl_inject_rch_error(u64 rcrb, u64 type)
>> +{
>> +	return -ENXIO;
>> +}
>> +
>> +static inline bool einj_cxl_is_initialized(void) { return false; }
>> +#endif /* CONFIG_ACPI_APEI_EINJ_CXL */
>> +
>> +#endif /* EINJ_CXL_H */
> 

Re: [PATCH v14 2/4] EINJ: Add CXL error type support

[Jonathan Cameron]

On Thu, 7 Mar 2024 08:46:49 -0600
Ben Cheatham <benjamin.cheatham@amd.com> wrote:

> Hey Jonathan, thanks for taking a look!
> 
> On 3/7/24 6:09 AM, Jonathan Cameron wrote:
> > On Mon, 26 Feb 2024 16:27:02 -0600
> > Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:
> >   
> >> Remove CXL protocol error types from the EINJ module and move them to
> >> a new einj_cxl module. The einj_cxl module implements the necessary
> >> handling for CXL protocol error injection and exposes an API for the
> >> CXL core to use said functionality. Because the CXL error types
> >> require special handling, only allow them to be injected through the
> >> einj_cxl module and return an error when attempting to inject through
> >> "regular" EINJ.
> >>
> >> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>  
> > Hi Ben,
> > 
> > Some minor comments inline given you are doing a v15 (yikes!)
> >   
> 
> Yeah I know :(.
> 
With headers tidied up.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> > 
> >   
> >> +	{ BIT(13), "CXL.cache Protocol Uncorrectable non-fatal" },
> >> +	{ BIT(14), "CXL.cache Protocol Uncorrectable fatal" },
> >> +	{ BIT(15), "CXL.mem Protocol Correctable" },
> >> +	{ BIT(16), "CXL.mem Protocol Uncorrectable non-fatal" },
> >> +	{ BIT(17), "CXL.mem Protocol Uncorrectable fatal" },
> >> +};
> >> +
> >> +int einj_cxl_available_error_type_show(struct seq_file *m, void *v)
> >> +{
> >> +	int cxl_err, rc;
> >> +	u32 available_error_type = 0;
> >> +
> >> +	if (!einj_initialized)
> >> +		return -ENXIO;
> >> +
> >> +	rc = einj_get_available_error_type(&available_error_type);
> >> +	if (rc)
> >> +		return rc;
> >> +
> >> +	for (int pos = 0; pos < ARRAY_SIZE(einj_cxl_error_type_string); pos++) {
> >> +		cxl_err = ACPI_EINJ_CXL_CACHE_CORRECTABLE << pos;  
> > Hmm. This is a little ugly.
> > Could do something like the following bit it's of similar level of ugly
> > so up to you.
> > 
> > 	int bit_pos = ACPI_EINJ_CXL_CACHE_CORRECTABLE;
> > 	for_each_bit_set_bit_from(bit_pos, &available_error_type,
> > 			     ARRAY_SIZE(einj_cxl_error_type_string)) {
> > 		int pos = bit_pos - ACPI_EINJ_CXL_CACHE_CORRECTABLE;
> > 	  
> 
> I agree it's ugly. I think this version has the added benfit of parity
> with einj_available_error_type_show() in einj-core.c, so I think it's
> better to keep it this way if it's the same to you.
> 
Sure. We can always (maybe) tidy them both up later :)

J

[PATCH v14 3/4] cxl/core: Add CXL EINJ debugfs files

[Ben Cheatham]

Export CXL helper functions in the einj_cxl module for getting/injecting
available CXL protocol error types to sysfs under kernel/debug/cxl.

The kernel/debug/cxl/einj_types file will print the available CXL
protocol errors in the same format as the available_error_types
file provided by the einj_cxl module. The
kernel/debug/cxl/$dport_dev/einj_inject is functionally the same as the
error_type and error_inject files provided by the EINJ module, i.e.:
writing an error type into $dport_dev/einj_inject will inject said error
type into the CXL dport represented by $dport_dev.

Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
---
 Documentation/ABI/testing/debugfs-cxl | 30 ++++++++++++++++++++
 drivers/cxl/core/port.c               | 41 +++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/Documentation/ABI/testing/debugfs-cxl b/Documentation/ABI/testing/debugfs-cxl
index fe61d372e3fa..4c0f62f881ca 100644
--- a/Documentation/ABI/testing/debugfs-cxl
+++ b/Documentation/ABI/testing/debugfs-cxl
@@ -33,3 +33,33 @@ Description:
 		device cannot clear poison from the address, -ENXIO is returned.
 		The clear_poison attribute is only visible for devices
 		supporting the capability.
+
+What:		/sys/kernel/debug/cxl/einj_types
+Date:		January, 2024
+KernelVersion:	v6.9
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) Prints the CXL protocol error types made available by
+		the platform in the format "0x<error number>	<error type>".
+		The possible error types are (as of ACPI v6.5):
+			0x1000	CXL.cache Protocol Correctable
+			0x2000	CXL.cache Protocol Uncorrectable non-fatal
+			0x4000	CXL.cache Protocol Uncorrectable fatal
+			0x8000	CXL.mem Protocol Correctable
+			0x10000	CXL.mem Protocol Uncorrectable non-fatal
+			0x20000	CXL.mem Protocol Uncorrectable fatal
+
+		The <error number> can be written to einj_inject to inject
+		<error type> into a chosen dport.
+
+What:		/sys/kernel/debug/cxl/$dport_dev/einj_inject
+Date:		January, 2024
+KernelVersion:	v6.9
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(WO) Writing an integer to this file injects the corresponding
+		CXL protocol error into $dport_dev ($dport_dev will be a device
+		name from /sys/bus/pci/devices). The integer to type mapping for
+		injection can be found by reading from einj_types. If the dport
+		was enumerated in RCH mode, a CXL 1.1 error is injected, otherwise
+		a CXL 2.0 error is injected.
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index e59d9d37aa65..3b579eef4d23 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -3,6 +3,7 @@
 #include <linux/platform_device.h>
 #include <linux/memregion.h>
 #include <linux/workqueue.h>
+#include <linux/einj-cxl.h>
 #include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/module.h>
@@ -793,6 +794,39 @@ static int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport,
 	return rc;
 }
 
+DEFINE_SHOW_ATTRIBUTE(einj_cxl_available_error_type);
+
+static int cxl_einj_inject(void *data, u64 type)
+{
+	struct cxl_dport *dport = data;
+
+	if (dport->rch)
+		return einj_cxl_inject_rch_error(dport->rcrb.base, type);
+
+	return einj_cxl_inject_error(to_pci_dev(dport->dport_dev), type);
+}
+DEFINE_DEBUGFS_ATTRIBUTE(cxl_einj_inject_fops, NULL, cxl_einj_inject, "%llx\n");
+
+static void cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
+{
+	struct dentry *dir;
+
+	if (!einj_cxl_is_initialized())
+		return;
+
+	/*
+	 * dport_dev needs to be a PCIe port for CXL 2.0+ ports because
+	 * EINJ expects a dport SBDF to be specified for 2.0 error injection.
+	 */
+	if (!dport->rch && !dev_is_pci(dport->dport_dev))
+		return;
+
+	dir = cxl_debugfs_create_dir(dev_name(dport->dport_dev));
+
+	debugfs_create_file("einj_inject", 0200, dir, dport,
+			    &cxl_einj_inject_fops);
+}
+
 static struct cxl_port *__devm_cxl_add_port(struct device *host,
 					    struct device *uport_dev,
 					    resource_size_t component_reg_phys,
@@ -1149,6 +1183,8 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
 	if (dev_is_pci(dport_dev))
 		dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
 
+	cxl_debugfs_create_dport_dir(dport);
+
 	return dport;
 }
 
@@ -2221,6 +2257,11 @@ static __init int cxl_core_init(void)
 
 	cxl_debugfs = debugfs_create_dir("cxl", NULL);
 
+	if (einj_cxl_is_initialized()) {
+		debugfs_create_file("einj_types", 0400, cxl_debugfs, NULL,
+				    &einj_cxl_available_error_type_fops);
+	}
+
 	cxl_mbox_init();
 
 	rc = cxl_memdev_init();
-- 
2.34.1

Re: [PATCH v14 3/4] cxl/core: Add CXL EINJ debugfs files

[Ben Cheatham]

This patch also had an outdated commit message (still referenced the einj-cxl module).
The patch with the updated commit message is below. I also made a tiny change to
the format specifier of the einj_inject file to "0x%llx\n" from "%llx\n".

Thanks,
Ben

From 321129893da9129473c447772a461c1a4e9e0e9d Mon Sep 17 00:00:00 2001
From: Ben Cheatham <Benjamin.Cheatham@amd.com>
Date: Fri, 16 Feb 2024 11:17:01 -0600
Subject: [PATCH v14 3/4] cxl/core: Add CXL EINJ debugfs files

Export CXL helper functions in einj-cxl.c for getting/injecting
available CXL protocol error types to sysfs under kernel/debug/cxl.

The kernel/debug/cxl/einj_types file will print the available CXL
protocol errors in the same format as the available_error_types
file provided by the einj module. The
kernel/debug/cxl/$dport_dev/einj_inject file is functionally the same
as the error_type and error_inject files provided by the EINJ module,
i.e.: writing an error type into $dport_dev/einj_inject will inject
said error type into the CXL dport represented by $dport_dev.

Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
---
 Documentation/ABI/testing/debugfs-cxl | 30 +++++++++++++++++++
 drivers/cxl/core/port.c               | 42 +++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/Documentation/ABI/testing/debugfs-cxl b/Documentation/ABI/testing/debugfs-cxl
index fe61d372e3fa..4c0f62f881ca 100644
--- a/Documentation/ABI/testing/debugfs-cxl
+++ b/Documentation/ABI/testing/debugfs-cxl
@@ -33,3 +33,33 @@ Description:
 		device cannot clear poison from the address, -ENXIO is returned.
 		The clear_poison attribute is only visible for devices
 		supporting the capability.
+
+What:		/sys/kernel/debug/cxl/einj_types
+Date:		January, 2024
+KernelVersion:	v6.9
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) Prints the CXL protocol error types made available by
+		the platform in the format "0x<error number>	<error type>".
+		The possible error types are (as of ACPI v6.5):
+			0x1000	CXL.cache Protocol Correctable
+			0x2000	CXL.cache Protocol Uncorrectable non-fatal
+			0x4000	CXL.cache Protocol Uncorrectable fatal
+			0x8000	CXL.mem Protocol Correctable
+			0x10000	CXL.mem Protocol Uncorrectable non-fatal
+			0x20000	CXL.mem Protocol Uncorrectable fatal
+
+		The <error number> can be written to einj_inject to inject
+		<error type> into a chosen dport.
+
+What:		/sys/kernel/debug/cxl/$dport_dev/einj_inject
+Date:		January, 2024
+KernelVersion:	v6.9
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(WO) Writing an integer to this file injects the corresponding
+		CXL protocol error into $dport_dev ($dport_dev will be a device
+		name from /sys/bus/pci/devices). The integer to type mapping for
+		injection can be found by reading from einj_types. If the dport
+		was enumerated in RCH mode, a CXL 1.1 error is injected, otherwise
+		a CXL 2.0 error is injected.
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index e59d9d37aa65..eeeb6e53fdc4 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -3,6 +3,7 @@
 #include <linux/platform_device.h>
 #include <linux/memregion.h>
 #include <linux/workqueue.h>
+#include <linux/einj-cxl.h>
 #include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/module.h>
@@ -793,6 +794,40 @@ static int cxl_dport_setup_regs(struct device *host, struct cxl_dport *dport,
 	return rc;
 }
 
+DEFINE_SHOW_ATTRIBUTE(einj_cxl_available_error_type);
+
+static int cxl_einj_inject(void *data, u64 type)
+{
+	struct cxl_dport *dport = data;
+
+	if (dport->rch)
+		return einj_cxl_inject_rch_error(dport->rcrb.base, type);
+
+	return einj_cxl_inject_error(to_pci_dev(dport->dport_dev), type);
+}
+DEFINE_DEBUGFS_ATTRIBUTE(cxl_einj_inject_fops, NULL, cxl_einj_inject,
+			 "0x%llx\n");
+
+static void cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
+{
+	struct dentry *dir;
+
+	if (!einj_cxl_is_initialized())
+		return;
+
+	/*
+	 * dport_dev needs to be a PCIe port for CXL 2.0+ ports because
+	 * EINJ expects a dport SBDF to be specified for 2.0 error injection.
+	 */
+	if (!dport->rch && !dev_is_pci(dport->dport_dev))
+		return;
+
+	dir = cxl_debugfs_create_dir(dev_name(dport->dport_dev));
+
+	debugfs_create_file("einj_inject", 0200, dir, dport,
+			    &cxl_einj_inject_fops);
+}
+
 static struct cxl_port *__devm_cxl_add_port(struct device *host,
 					    struct device *uport_dev,
 					    resource_size_t component_reg_phys,
@@ -1149,6 +1184,8 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
 	if (dev_is_pci(dport_dev))
 		dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
 
+	cxl_debugfs_create_dport_dir(dport);
+
 	return dport;
 }
 
@@ -2221,6 +2258,11 @@ static __init int cxl_core_init(void)
 
 	cxl_debugfs = debugfs_create_dir("cxl", NULL);
 
+	if (einj_cxl_is_initialized()) {
+		debugfs_create_file("einj_types", 0400, cxl_debugfs, NULL,
+				    &einj_cxl_available_error_type_fops);
+	}
+
 	cxl_mbox_init();
 
 	rc = cxl_memdev_init();
-- 
2.34.1

Re: [PATCH v14 3/4] cxl/core: Add CXL EINJ debugfs files

[Jonathan Cameron]

On Tue, 27 Feb 2024 14:14:35 -0600
Ben Cheatham <benjamin.cheatham@amd.com> wrote:

> This patch also had an outdated commit message (still referenced the einj-cxl module).
> The patch with the updated commit message is below. I also made a tiny change to
> the format specifier of the einj_inject file to "0x%llx\n" from "%llx\n".
> 
> Thanks,
> Ben
> 
> From 321129893da9129473c447772a461c1a4e9e0e9d Mon Sep 17 00:00:00 2001
> From: Ben Cheatham <Benjamin.Cheatham@amd.com>
> Date: Fri, 16 Feb 2024 11:17:01 -0600
> Subject: [PATCH v14 3/4] cxl/core: Add CXL EINJ debugfs files
> 
> Export CXL helper functions in einj-cxl.c for getting/injecting
> available CXL protocol error types to sysfs under kernel/debug/cxl.
> 
> The kernel/debug/cxl/einj_types file will print the available CXL
> protocol errors in the same format as the available_error_types
> file provided by the einj module. The
> kernel/debug/cxl/$dport_dev/einj_inject file is functionally the same
> as the error_type and error_inject files provided by the EINJ module,
> i.e.: writing an error type into $dport_dev/einj_inject will inject
> said error type into the CXL dport represented by $dport_dev.
> 
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
LGTM other than not sending patches like this as tooling won't pick them up!
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Re: [PATCH v14 3/4] cxl/core: Add CXL EINJ debugfs files

[Ben Cheatham]

On 3/7/24 6:10 AM, Jonathan Cameron wrote:
> On Tue, 27 Feb 2024 14:14:35 -0600
> Ben Cheatham <benjamin.cheatham@amd.com> wrote:
> 
>> This patch also had an outdated commit message (still referenced the einj-cxl module).
>> The patch with the updated commit message is below. I also made a tiny change to
>> the format specifier of the einj_inject file to "0x%llx\n" from "%llx\n".
>>
>> Thanks,
>> Ben
>>
>> From 321129893da9129473c447772a461c1a4e9e0e9d Mon Sep 17 00:00:00 2001
>> From: Ben Cheatham <Benjamin.Cheatham@amd.com>
>> Date: Fri, 16 Feb 2024 11:17:01 -0600
>> Subject: [PATCH v14 3/4] cxl/core: Add CXL EINJ debugfs files
>>
>> Export CXL helper functions in einj-cxl.c for getting/injecting
>> available CXL protocol error types to sysfs under kernel/debug/cxl.
>>
>> The kernel/debug/cxl/einj_types file will print the available CXL
>> protocol errors in the same format as the available_error_types
>> file provided by the einj module. The
>> kernel/debug/cxl/$dport_dev/einj_inject file is functionally the same
>> as the error_type and error_inject files provided by the EINJ module,
>> i.e.: writing an error type into $dport_dev/einj_inject will inject
>> said error type into the CXL dport represented by $dport_dev.
>>
>> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
> LGTM other than not sending patches like this as tooling won't pick them up!
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 

Yeah I normally wouldn't send it in this way, but I caught what needed changes
before most people started to review and didn't want to make a whole new version
for them (especially since the version number is quite high already). I probably
should've just waited a day or two and double checked before sending it out in
hindsight.

Thanks,
Ben

[PATCH v14 4/4] EINJ, Documentation: Update EINJ kernel doc

[Ben Cheatham]

Update EINJ kernel document to include how to inject CXL protocol error
types, build the kernel to include CXL error types, and give an example
injection.

Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
---
 .../firmware-guide/acpi/apei/einj.rst         | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/Documentation/firmware-guide/acpi/apei/einj.rst b/Documentation/firmware-guide/acpi/apei/einj.rst
index d6b61d22f525..c52b9da08fa9 100644
--- a/Documentation/firmware-guide/acpi/apei/einj.rst
+++ b/Documentation/firmware-guide/acpi/apei/einj.rst
@@ -32,6 +32,10 @@ configuration::
   CONFIG_ACPI_APEI
   CONFIG_ACPI_APEI_EINJ
 
+...and to (optionally) enable CXL protocol error injection set::
+
+  CONFIG_ACPI_APEI_EINJ_CXL
+
 The EINJ user interface is in <debugfs mount point>/apei/einj.
 
 The following files belong to it:
@@ -118,6 +122,24 @@ The following files belong to it:
   this actually works depends on what operations the BIOS actually
   includes in the trigger phase.
 
+CXL error types are supported from ACPI 6.5 onwards (given a CXL port
+is present). The EINJ user interface for CXL error types is at
+<debugfs mount point>/cxl. The following files belong to it:
+
+- einj_types:
+
+  Provides the same functionality as available_error_types above, but
+  for CXL error types
+
+- $dport_dev/einj_inject:
+
+  Injects a CXL error type into the CXL port represented by $dport_dev,
+  where $dport_dev is the name of the CXL port (usually a PCIe device name).
+  Error injections targeting a CXL 2.0+ port can use the legacy interface
+  under <debugfs mount point>/apei/einj, while CXL 1.1/1.0 port injections
+  must use this file.
+
+
 BIOS versions based on the ACPI 4.0 specification have limited options
 in controlling where the errors are injected. Your BIOS may support an
 extension (enabled with the param_extension=1 module parameter, or boot
@@ -181,6 +203,18 @@ You should see something like this in dmesg::
   [22715.834759] EDAC sbridge MC3: PROCESSOR 0:306e7 TIME 1422553404 SOCKET 0 APIC 0
   [22716.616173] EDAC MC3: 1 CE memory read error on CPU_SrcID#0_Channel#0_DIMM#0 (channel:0 slot:0 page:0x12345 offset:0x0 grain:32 syndrome:0x0 -  area:DRAM err_code:0001:0090 socket:0 channel_mask:1 rank:0)
 
+A CXL error injection example with $dport_dev=0000:e0:01.1::
+
+    # cd /sys/kernel/debug/cxl/
+    # ls
+    0000:e0:01.1 0000:0c:00.0
+    # cat einj_types                # See which errors can be injected
+	0x00008000  CXL.mem Protocol Correctable
+	0x00010000  CXL.mem Protocol Uncorrectable non-fatal
+	0x00020000  CXL.mem Protocol Uncorrectable fatal
+    # cd 0000:e0:01.1               # Navigate to dport to inject into
+    # echo 0x8000 > einj_inject     # Inject error
+
 Special notes for injection into SGX enclaves:
 
 There may be a separate BIOS setup option to enable SGX injection.
-- 
2.34.1

Re: [PATCH v14 4/4] EINJ, Documentation: Update EINJ kernel doc

[Jonathan Cameron]

On Mon, 26 Feb 2024 16:27:04 -0600
Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:

> Update EINJ kernel document to include how to inject CXL protocol error
> types, build the kernel to include CXL error types, and give an example
> injection.
> 
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>