[PATCH v7 0/9] Enable EINJv2 Support

[PATCH v7 0/9] Enable EINJv2 Support

[Zaid Alali]

The goal of this update is to allow the driver to simultaneously
support EINJ and EINJv2. The implementation follows the approved ACPI
specs[1][2] that enables the driver to discover system capabilities
through GET_ERROR_TYPE.

Note: The first two ACPICA patches are to be dropped once merged in
ACPICA project, see pull request[3].

Link: https://github.com/tianocore/edk2/issues/9449 [1]
Link: https://github.com/tianocore/edk2/issues/9017 [2]
Link: https://github.com/acpica/acpica/pull/977 [3]

V5:
        *Users no longer input component array size, instead it
         is counted by parsing the component array itself.
V6:
        *Fix memory leak.
        *If EINJv2 initialization failed, EINJv1 will still work, and
         probe function will continue with disabled EINJv2.
V7:
        *Update component array to take 128-bit values to match ACPI specs.
        *Enable Vendor EINJv2 injections
        *Moved component array parsing and validating to a separate
         function to improve readability.

Zaid Alali (9):
  ACPICA: Update values to hex to follow ACPI specs
  ACPICA: Add EINJv2 get error type action
  ACPI: APEI: EINJ: Fix kernel test sparse warnings
  ACPI: APEI: EINJ: Remove redundant calls to
    einj_get_available_error_type
  ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities
  ACPI: APEI: EINJ: Add einjv2 extension struct
  ACPI: APEI: EINJ: Add debugfs files for EINJv2 support
  ACPI: APEI: EINJ: Enable EINJv2 error injections
  ACPI: APEI: EINJ: Update the documentation for EINJv2 support

 .../firmware-guide/acpi/apei/einj.rst         |  32 ++
 drivers/acpi/apei/apei-internal.h             |   2 +-
 drivers/acpi/apei/einj-core.c                 | 410 ++++++++++++++----
 drivers/acpi/apei/einj-cxl.c                  |   2 +-
 include/acpi/actbl1.h                         |  25 +-
 5 files changed, 379 insertions(+), 92 deletions(-)

-- 
2.43.0

[PATCH v7 1/9] ACPICA: Update values to hex to follow ACPI specs

[Zaid Alali]

ACPI specs[1] define Error Injection Actions in hex values.
This commit intends to update values from decimal to hex to be
consistent with ACPI specs. This commit and the following one are
not to be merged and will come form ACPICA project see pull request[2].

Link: https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html [1]
Link: https://github.com/acpica/acpica/pull/977 [2]

Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 include/acpi/actbl1.h | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 387fc821703a..c701c434976c 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -1024,18 +1024,18 @@ struct acpi_einj_entry {
 /* Values for Action field above */
 
 enum acpi_einj_actions {
-	ACPI_EINJ_BEGIN_OPERATION = 0,
-	ACPI_EINJ_GET_TRIGGER_TABLE = 1,
-	ACPI_EINJ_SET_ERROR_TYPE = 2,
-	ACPI_EINJ_GET_ERROR_TYPE = 3,
-	ACPI_EINJ_END_OPERATION = 4,
-	ACPI_EINJ_EXECUTE_OPERATION = 5,
-	ACPI_EINJ_CHECK_BUSY_STATUS = 6,
-	ACPI_EINJ_GET_COMMAND_STATUS = 7,
-	ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS = 8,
-	ACPI_EINJ_GET_EXECUTE_TIMINGS = 9,
-	ACPI_EINJ_ACTION_RESERVED = 10,	/* 10 and greater are reserved */
-	ACPI_EINJ_TRIGGER_ERROR = 0xFF	/* Except for this value */
+	ACPI_EINJ_BEGIN_OPERATION =		0x0,
+	ACPI_EINJ_GET_TRIGGER_TABLE =		0x1,
+	ACPI_EINJ_SET_ERROR_TYPE =		0x2,
+	ACPI_EINJ_GET_ERROR_TYPE =		0x3,
+	ACPI_EINJ_END_OPERATION =		0x4,
+	ACPI_EINJ_EXECUTE_OPERATION =		0x5,
+	ACPI_EINJ_CHECK_BUSY_STATUS =		0x6,
+	ACPI_EINJ_GET_COMMAND_STATUS =		0x7,
+	ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS =	0x8,
+	ACPI_EINJ_GET_EXECUTE_TIMINGS =		0x9,
+	ACPI_EINJ_ACTION_RESERVED =		0xA,	/* 0xA and greater are reserved */
+	ACPI_EINJ_TRIGGER_ERROR =		0xFF	/* Except for this value */
 };
 
 /* Values for Instruction field above */
-- 
2.43.0

[PATCH v7 2/9] ACPICA: Add EINJv2 get error type action

[Zaid Alali]

Add EINJV2_GET_ERROR_TYPE as defined in the approved new ACPI
specs[1][2].

Proposed ACPI spces for EINJv2:
Link: https://github.com/tianocore/edk2/issues/9449 [1]
Link: https://github.com/tianocore/edk2/issues/9017 [2]

This commit is not a direct merge, it will come from ACPICA
project, see pull request[3].

Link: https://github.com/acpica/acpica/pull/977 [3]

Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 include/acpi/actbl1.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index c701c434976c..f52d5cafaf76 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -1034,7 +1034,8 @@ enum acpi_einj_actions {
 	ACPI_EINJ_GET_COMMAND_STATUS =		0x7,
 	ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS =	0x8,
 	ACPI_EINJ_GET_EXECUTE_TIMINGS =		0x9,
-	ACPI_EINJ_ACTION_RESERVED =		0xA,	/* 0xA and greater are reserved */
+	ACPI_EINJV2_GET_ERROR_TYPE =		0x11,
+	ACPI_EINJ_ACTION_RESERVED =		0x12,	/* 0x12 and greater are reserved */
 	ACPI_EINJ_TRIGGER_ERROR =		0xFF	/* Except for this value */
 };
 
-- 
2.43.0

[PATCH v7 3/9] ACPI: APEI: EINJ: Fix kernel test sparse warnings

[Zaid Alali]

This patch fixes the kernel test robot warning reported here:
Link: https://lore.kernel.org/all/202410241620.oApALow5-lkp@intel.com/

Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 drivers/acpi/apei/einj-core.c | 106 +++++++++++++++++++---------------
 1 file changed, 60 insertions(+), 46 deletions(-)

diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index 04731a5b01fa..47abd9317fef 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -149,7 +149,7 @@ static DEFINE_MUTEX(einj_mutex);
  */
 bool einj_initialized __ro_after_init;
 
-static void *einj_param;
+static void __iomem *einj_param;
 
 static void einj_exec_ctx_init(struct apei_exec_context *ctx)
 {
@@ -214,24 +214,26 @@ static void check_vendor_extension(u64 paddr,
 				   struct set_error_type_with_address *v5param)
 {
 	int	offset = v5param->vendor_extension;
-	struct	vendor_error_type_extension *v;
+	struct	vendor_error_type_extension v;
+	struct vendor_error_type_extension __iomem *p;
 	u32	sbdf;
 
 	if (!offset)
 		return;
-	v = acpi_os_map_iomem(paddr + offset, sizeof(*v));
-	if (!v)
+	p = acpi_os_map_iomem(paddr + offset, sizeof(*p));
+	if (!p)
 		return;
-	get_oem_vendor_struct(paddr, offset, v);
-	sbdf = v->pcie_sbdf;
+	memcpy_fromio(&v, p, sizeof(v));
+	get_oem_vendor_struct(paddr, offset, &v);
+	sbdf = v.pcie_sbdf;
 	sprintf(vendor_dev, "%x:%x:%x.%x vendor_id=%x device_id=%x rev_id=%x\n",
 		sbdf >> 24, (sbdf >> 16) & 0xff,
 		(sbdf >> 11) & 0x1f, (sbdf >> 8) & 0x7,
-		 v->vendor_id, v->device_id, v->rev_id);
-	acpi_os_unmap_iomem(v, sizeof(*v));
+		 v.vendor_id, v.device_id, v.rev_id);
+	acpi_os_unmap_iomem(p, sizeof(v));
 }
 
-static void *einj_get_parameter_address(void)
+static void __iomem *einj_get_parameter_address(void)
 {
 	int i;
 	u64 pa_v4 = 0, pa_v5 = 0;
@@ -252,26 +254,30 @@ static void *einj_get_parameter_address(void)
 		entry++;
 	}
 	if (pa_v5) {
-		struct set_error_type_with_address *v5param;
+		struct set_error_type_with_address v5param;
+		struct set_error_type_with_address __iomem *p;
 
-		v5param = acpi_os_map_iomem(pa_v5, sizeof(*v5param));
-		if (v5param) {
+		p = acpi_os_map_iomem(pa_v5, sizeof(*p));
+		if (p) {
+			memcpy_fromio(&v5param, p, sizeof(v5param));
 			acpi5 = 1;
-			check_vendor_extension(pa_v5, v5param);
-			return v5param;
+			check_vendor_extension(pa_v5, &v5param);
+			return p;
 		}
 	}
 	if (param_extension && pa_v4) {
-		struct einj_parameter *v4param;
+		struct einj_parameter v4param;
+		struct einj_parameter __iomem *p;
 
-		v4param = acpi_os_map_iomem(pa_v4, sizeof(*v4param));
-		if (!v4param)
+		p = acpi_os_map_iomem(pa_v4, sizeof(*p));
+		if (!p)
 			return NULL;
-		if (v4param->reserved1 || v4param->reserved2) {
-			acpi_os_unmap_iomem(v4param, sizeof(*v4param));
+		memcpy_fromio(&v4param, p, sizeof(v4param));
+		if (v4param.reserved1 || v4param.reserved2) {
+			acpi_os_unmap_iomem(p, sizeof(v4param));
 			return NULL;
 		}
-		return v4param;
+		return p;
 	}
 
 	return NULL;
@@ -317,7 +323,7 @@ static struct acpi_generic_address *einj_get_trigger_parameter_region(
 static int __einj_error_trigger(u64 trigger_paddr, u32 type,
 				u64 param1, u64 param2)
 {
-	struct acpi_einj_trigger *trigger_tab = NULL;
+	struct acpi_einj_trigger trigger_tab;
 	struct apei_exec_context trigger_ctx;
 	struct apei_resources trigger_resources;
 	struct acpi_whea_header *trigger_entry;
@@ -325,54 +331,57 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
 	u32 table_size;
 	int rc = -EIO;
 	struct acpi_generic_address *trigger_param_region = NULL;
+	struct acpi_einj_trigger __iomem *p;
 
-	r = request_mem_region(trigger_paddr, sizeof(*trigger_tab),
+	r = request_mem_region(trigger_paddr, sizeof(trigger_tab),
 			       "APEI EINJ Trigger Table");
 	if (!r) {
 		pr_err("Can not request [mem %#010llx-%#010llx] for Trigger table\n",
 		       (unsigned long long)trigger_paddr,
 		       (unsigned long long)trigger_paddr +
-			    sizeof(*trigger_tab) - 1);
+			    sizeof(trigger_tab) - 1);
 		goto out;
 	}
-	trigger_tab = ioremap_cache(trigger_paddr, sizeof(*trigger_tab));
-	if (!trigger_tab) {
+	p = ioremap_cache(trigger_paddr, sizeof(*p));
+	if (!p) {
 		pr_err("Failed to map trigger table!\n");
 		goto out_rel_header;
 	}
-	rc = einj_check_trigger_header(trigger_tab);
+	memcpy_fromio(&trigger_tab, p, sizeof(trigger_tab));
+	rc = einj_check_trigger_header(&trigger_tab);
 	if (rc) {
 		pr_warn(FW_BUG "Invalid trigger error action table.\n");
 		goto out_rel_header;
 	}
 
 	/* No action structures in the TRIGGER_ERROR table, nothing to do */
-	if (!trigger_tab->entry_count)
+	if (!trigger_tab.entry_count)
 		goto out_rel_header;
 
 	rc = -EIO;
-	table_size = trigger_tab->table_size;
-	r = request_mem_region(trigger_paddr + sizeof(*trigger_tab),
-			       table_size - sizeof(*trigger_tab),
+	table_size = trigger_tab.table_size;
+	r = request_mem_region(trigger_paddr + sizeof(trigger_tab),
+			       table_size - sizeof(trigger_tab),
 			       "APEI EINJ Trigger Table");
 	if (!r) {
 		pr_err("Can not request [mem %#010llx-%#010llx] for Trigger Table Entry\n",
-		       (unsigned long long)trigger_paddr + sizeof(*trigger_tab),
+		       (unsigned long long)trigger_paddr + sizeof(trigger_tab),
 		       (unsigned long long)trigger_paddr + table_size - 1);
 		goto out_rel_header;
 	}
-	iounmap(trigger_tab);
-	trigger_tab = ioremap_cache(trigger_paddr, table_size);
-	if (!trigger_tab) {
+	iounmap(p);
+	p = ioremap_cache(trigger_paddr, table_size);
+	if (!p) {
 		pr_err("Failed to map trigger table!\n");
 		goto out_rel_entry;
 	}
+	memcpy_fromio(&trigger_tab, p, sizeof(trigger_tab));
 	trigger_entry = (struct acpi_whea_header *)
-		((char *)trigger_tab + sizeof(struct acpi_einj_trigger));
+		((char *)&trigger_tab + sizeof(struct acpi_einj_trigger));
 	apei_resources_init(&trigger_resources);
 	apei_exec_ctx_init(&trigger_ctx, einj_ins_type,
 			   ARRAY_SIZE(einj_ins_type),
-			   trigger_entry, trigger_tab->entry_count);
+			   trigger_entry, trigger_tab.entry_count);
 	rc = apei_exec_collect_resources(&trigger_ctx, &trigger_resources);
 	if (rc)
 		goto out_fini;
@@ -390,7 +399,7 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
 
 		apei_resources_init(&addr_resources);
 		trigger_param_region = einj_get_trigger_parameter_region(
-			trigger_tab, param1, param2);
+			&trigger_tab, param1, param2);
 		if (trigger_param_region) {
 			rc = apei_resources_add(&addr_resources,
 				trigger_param_region->address,
@@ -419,13 +428,13 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
 out_fini:
 	apei_resources_fini(&trigger_resources);
 out_rel_entry:
-	release_mem_region(trigger_paddr + sizeof(*trigger_tab),
-			   table_size - sizeof(*trigger_tab));
+	release_mem_region(trigger_paddr + sizeof(trigger_tab),
+			   table_size - sizeof(trigger_tab));
 out_rel_header:
-	release_mem_region(trigger_paddr, sizeof(*trigger_tab));
+	release_mem_region(trigger_paddr, sizeof(trigger_tab));
 out:
-	if (trigger_tab)
-		iounmap(trigger_tab);
+	if (p)
+		iounmap(p);
 
 	return rc;
 }
@@ -444,8 +453,10 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 		return rc;
 	apei_exec_ctx_set_input(&ctx, type);
 	if (acpi5) {
-		struct set_error_type_with_address *v5param = einj_param;
+		struct set_error_type_with_address *v5param, v5_struct;
 
+		v5param = &v5_struct;
+		memcpy_fromio(v5param, einj_param, sizeof(*v5param));
 		v5param->type = type;
 		if (type & ACPI5_VENDOR_BIT) {
 			switch (vendor_flags) {
@@ -490,15 +501,18 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 				break;
 			}
 		}
+		memcpy_toio(einj_param, v5param, sizeof(*v5param));
 	} else {
 		rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
 		if (rc)
 			return rc;
 		if (einj_param) {
-			struct einj_parameter *v4param = einj_param;
+			struct einj_parameter v4param;
 
-			v4param->param1 = param1;
-			v4param->param2 = param2;
+			memcpy_fromio(&v4param, einj_param, sizeof(v4param));
+			v4param.param1 = param1;
+			v4param.param2 = param2;
+			memcpy_toio(einj_param, &v4param, sizeof(v4param));
 		}
 	}
 	rc = apei_exec_run(&ctx, ACPI_EINJ_EXECUTE_OPERATION);
-- 
2.43.0

[PATCH v7 4/9] ACPI: APEI: EINJ: Remove redundant calls to einj_get_available_error_type

[Zaid Alali]

A single call to einj_get_available_error_type in init function is
sufficient to save the return value in a global variable to be used
later in various places in the code. This commit does not introduce
any functional changes, but only removing unnecessary redundant
function calls.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 drivers/acpi/apei/einj-core.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index 47abd9317fef..ada1d7026af5 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -83,6 +83,8 @@ static struct debugfs_blob_wrapper vendor_blob;
 static struct debugfs_blob_wrapper vendor_errors;
 static char vendor_dev[64];
 
+static u32 available_error_type;
+
 /*
  * Some BIOSes allow parameters to the SET_ERROR_TYPE entries in the
  * EINJ table through an unpublished extension. Use with caution as
@@ -662,14 +664,9 @@ static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
 
 static int available_error_type_show(struct seq_file *m, void *v)
 {
-	int rc;
-	u32 error_type = 0;
 
-	rc = einj_get_available_error_type(&error_type);
-	if (rc)
-		return rc;
 	for (int pos = 0; pos < ARRAY_SIZE(einj_error_type_string); pos++)
-		if (error_type & einj_error_type_string[pos].mask)
+		if (available_error_type & einj_error_type_string[pos].mask)
 			seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
 				   einj_error_type_string[pos].str);
 
@@ -692,8 +689,7 @@ bool einj_is_cxl_error_type(u64 type)
 
 int einj_validate_error_type(u64 type)
 {
-	u32 tval, vendor, available_error_type = 0;
-	int rc;
+	u32 tval, vendor;
 
 	/* Only low 32 bits for error type are valid */
 	if (type & GENMASK_ULL(63, 32))
@@ -709,13 +705,9 @@ int einj_validate_error_type(u64 type)
 	/* Only one error type can be specified */
 	if (tval & (tval - 1))
 		return -EINVAL;
-	if (!vendor) {
-		rc = einj_get_available_error_type(&available_error_type);
-		if (rc)
-			return rc;
+	if (!vendor)
 		if (!(type & available_error_type))
 			return -EINVAL;
-	}
 
 	return 0;
 }
@@ -791,6 +783,10 @@ static int __init einj_probe(struct platform_device *pdev)
 		goto err_put_table;
 	}
 
+	rc = einj_get_available_error_type(&available_error_type);
+	if (rc)
+		return rc;
+
 	rc = -ENOMEM;
 	einj_debug_dir = debugfs_create_dir("einj", apei_get_debugfs_dir());
 
-- 
2.43.0

[PATCH v7 5/9] ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities

[Zaid Alali]

Enable the driver to show all supported error injections for EINJ
and EINJv2 at the same time. EINJv2 capabilities can be discovered
by checking the return value of get_error_type, where bit 30 set
indicates EINJv2 support.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 drivers/acpi/apei/apei-internal.h |  2 +-
 drivers/acpi/apei/einj-core.c     | 75 +++++++++++++++++++++++++------
 drivers/acpi/apei/einj-cxl.c      |  2 +-
 3 files changed, 63 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
index cd2766c69d78..77c10a7a7a9f 100644
--- a/drivers/acpi/apei/apei-internal.h
+++ b/drivers/acpi/apei/apei-internal.h
@@ -131,7 +131,7 @@ 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_get_available_error_type(u32 *type, int einj_action);
 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,
diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index ada1d7026af5..ee26df0398fc 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -33,6 +33,7 @@
 #define SLEEP_UNIT_MAX		5000			/* 5ms */
 /* Firmware should respond within 1 seconds */
 #define FIRMWARE_TIMEOUT	(1 * USEC_PER_SEC)
+#define ACPI65_EINJV2_SUPP	BIT(30)
 #define ACPI5_VENDOR_BIT	BIT(31)
 #define MEM_ERROR_MASK		(ACPI_EINJ_MEMORY_CORRECTABLE | \
 				ACPI_EINJ_MEMORY_UNCORRECTABLE | \
@@ -84,6 +85,7 @@ static struct debugfs_blob_wrapper vendor_errors;
 static char vendor_dev[64];
 
 static u32 available_error_type;
+static u32 available_error_type_v2;
 
 /*
  * Some BIOSes allow parameters to the SET_ERROR_TYPE entries in the
@@ -159,13 +161,13 @@ static void einj_exec_ctx_init(struct apei_exec_context *ctx)
 			   EINJ_TAB_ENTRY(einj_tab), einj_tab->entries);
 }
 
-static int __einj_get_available_error_type(u32 *type)
+static int __einj_get_available_error_type(u32 *type, int einj_action)
 {
 	struct apei_exec_context ctx;
 	int rc;
 
 	einj_exec_ctx_init(&ctx);
-	rc = apei_exec_run(&ctx, ACPI_EINJ_GET_ERROR_TYPE);
+	rc = apei_exec_run(&ctx, einj_action);
 	if (rc)
 		return rc;
 	*type = apei_exec_ctx_get_output(&ctx);
@@ -174,17 +176,34 @@ static int __einj_get_available_error_type(u32 *type)
 }
 
 /* Get error injection capabilities of the platform */
-int einj_get_available_error_type(u32 *type)
+int einj_get_available_error_type(u32 *type, int einj_action)
 {
 	int rc;
 
 	mutex_lock(&einj_mutex);
-	rc = __einj_get_available_error_type(type);
+	rc = __einj_get_available_error_type(type, einj_action);
 	mutex_unlock(&einj_mutex);
 
 	return rc;
 }
 
+static int einj_get_available_error_types(u32 *type1, u32 *type2)
+{
+	int rc;
+
+	rc = einj_get_available_error_type(type1, ACPI_EINJ_GET_ERROR_TYPE);
+	if (rc)
+		return rc;
+	if (*type1 & ACPI65_EINJV2_SUPP) {
+		rc = einj_get_available_error_type(type2,
+						   ACPI_EINJV2_GET_ERROR_TYPE);
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
+
 static int einj_timedout(u64 *t)
 {
 	if ((s64)*t < SLEEP_UNIT_MIN) {
@@ -646,6 +665,7 @@ static u64 error_param2;
 static u64 error_param3;
 static u64 error_param4;
 static struct dentry *einj_debug_dir;
+static char einj_buf[32];
 static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
 	{ BIT(0), "Processor Correctable" },
 	{ BIT(1), "Processor Uncorrectable non-fatal" },
@@ -662,6 +682,12 @@ static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
 	{ BIT(31), "Vendor Defined Error Types" },
 };
 
+static struct { u32 mask; const char *str; } const einjv2_error_type_string[] = {
+	{ BIT(0), "EINJV2 Processor Error" },
+	{ BIT(1), "EINJV2 Memory Error" },
+	{ BIT(2), "EINJV2 PCI Express Error" },
+};
+
 static int available_error_type_show(struct seq_file *m, void *v)
 {
 
@@ -669,17 +695,22 @@ static int available_error_type_show(struct seq_file *m, void *v)
 		if (available_error_type & einj_error_type_string[pos].mask)
 			seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
 				   einj_error_type_string[pos].str);
-
+	if (available_error_type & ACPI65_EINJV2_SUPP) {
+		for (int pos = 0; pos < ARRAY_SIZE(einjv2_error_type_string); pos++) {
+			if (available_error_type_v2 & einjv2_error_type_string[pos].mask)
+				seq_printf(m, "V2_0x%08x\t%s\n", einjv2_error_type_string[pos].mask,
+					   einjv2_error_type_string[pos].str);
+		}
+	}
 	return 0;
 }
 
 DEFINE_SHOW_ATTRIBUTE(available_error_type);
 
-static int error_type_get(void *data, u64 *val)
+static ssize_t error_type_get(struct file *file, char __user *buf,
+				size_t count, loff_t *ppos)
 {
-	*val = error_type;
-
-	return 0;
+	return simple_read_from_buffer(buf, count, ppos, einj_buf, strlen(einj_buf));
 }
 
 bool einj_is_cxl_error_type(u64 type)
@@ -712,9 +743,23 @@ int einj_validate_error_type(u64 type)
 	return 0;
 }
 
-static int error_type_set(void *data, u64 val)
+static ssize_t error_type_set(struct file *file, const char __user *buf,
+				size_t count, loff_t *ppos)
 {
 	int rc;
+	u64 val;
+
+	memset(einj_buf, 0, sizeof(einj_buf));
+	if (copy_from_user(einj_buf, buf, count))
+		return -EFAULT;
+
+	if (strncmp(einj_buf, "V2_", 3) == 0) {
+		if (!sscanf(einj_buf, "V2_%llx", &val))
+			return -EINVAL;
+	} else {
+		if (!sscanf(einj_buf, "%llx", &val))
+			return -EINVAL;
+	}
 
 	rc = einj_validate_error_type(val);
 	if (rc)
@@ -722,11 +767,13 @@ static int error_type_set(void *data, u64 val)
 
 	error_type = val;
 
-	return 0;
+	return count;
 }
 
-DEFINE_DEBUGFS_ATTRIBUTE(error_type_fops, error_type_get, error_type_set,
-			 "0x%llx\n");
+static const struct file_operations error_type_fops = {
+	.read		= error_type_get,
+	.write		= error_type_set,
+};
 
 static int error_inject_set(void *data, u64 val)
 {
@@ -783,7 +830,7 @@ static int __init einj_probe(struct platform_device *pdev)
 		goto err_put_table;
 	}
 
-	rc = einj_get_available_error_type(&available_error_type);
+	rc = einj_get_available_error_types(&available_error_type, &available_error_type_v2);
 	if (rc)
 		return rc;
 
diff --git a/drivers/acpi/apei/einj-cxl.c b/drivers/acpi/apei/einj-cxl.c
index 78da9ae543a2..e70a416ec925 100644
--- a/drivers/acpi/apei/einj-cxl.c
+++ b/drivers/acpi/apei/einj-cxl.c
@@ -30,7 +30,7 @@ int einj_cxl_available_error_type_show(struct seq_file *m, void *v)
 	int cxl_err, rc;
 	u32 available_error_type = 0;
 
-	rc = einj_get_available_error_type(&available_error_type);
+	rc = einj_get_available_error_type(&available_error_type, ACPI_EINJ_GET_ERROR_TYPE);
 	if (rc)
 		return rc;
 
-- 
2.43.0

[PATCH v7 6/9] ACPI: APEI: EINJ: Add einjv2 extension struct

[Zaid Alali]

Add einjv2 extension struct and EINJv2 error types to prepare
the driver for EINJv2 support. ACPI specifications[1] enables
EINJv2 by extending set_error_type_with_address struct.

Link: https://github.com/tianocore/edk2/issues/9449 [1]

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 drivers/acpi/apei/einj-core.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index ee26df0398fc..60e4f3dc7055 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -50,6 +50,28 @@
  */
 static int acpi5;
 
+struct syndrome_array {
+	union {
+		u8	acpi_id[16];
+		u8	device_id[16];
+		u8	pcie_sbdf[16];
+		u8	vendor_id[16];
+	} comp_id;
+	union {
+		u8	proc_synd[16];
+		u8	mem_synd[16];
+		u8	pcie_synd[16];
+		u8	vendor_synd[16];
+	} comp_synd;
+};
+
+struct einjv2_extension_struct {
+	u32 length;
+	u16 revision;
+	u16 component_arr_count;
+	struct syndrome_array component_arr[] __counted_by(component_arr_count);
+};
+
 struct set_error_type_with_address {
 	u32	type;
 	u32	vendor_extension;
@@ -58,6 +80,7 @@ struct set_error_type_with_address {
 	u64	memory_address;
 	u64	memory_address_range;
 	u32	pcie_sbdf;
+	struct	einjv2_extension_struct einjv2_struct;
 };
 enum {
 	SETWA_FLAGS_APICID = 1,
-- 
2.43.0

[PATCH v7 7/9] ACPI: APEI: EINJ: Add debugfs files for EINJv2 support

[Zaid Alali]

Create a debugfs blob file to be used for reading the user
input for the component array. EINJv2 enables users to inject
errors to multiple components/devices at the same time using
component array.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 drivers/acpi/apei/einj-core.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index 60e4f3dc7055..44f9166c3881 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -33,6 +33,7 @@
 #define SLEEP_UNIT_MAX		5000			/* 5ms */
 /* Firmware should respond within 1 seconds */
 #define FIRMWARE_TIMEOUT	(1 * USEC_PER_SEC)
+#define COMP_ARR_SIZE		1024
 #define ACPI65_EINJV2_SUPP	BIT(30)
 #define ACPI5_VENDOR_BIT	BIT(31)
 #define MEM_ERROR_MASK		(ACPI_EINJ_MEMORY_CORRECTABLE | \
@@ -107,6 +108,8 @@ static struct debugfs_blob_wrapper vendor_blob;
 static struct debugfs_blob_wrapper vendor_errors;
 static char vendor_dev[64];
 
+static struct debugfs_blob_wrapper einjv2_component_arr;
+static void *user_input;
 static u32 available_error_type;
 static u32 available_error_type_v2;
 
@@ -689,6 +692,7 @@ static u64 error_param3;
 static u64 error_param4;
 static struct dentry *einj_debug_dir;
 static char einj_buf[32];
+static bool EINJv2_enabled;
 static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
 	{ BIT(0), "Processor Correctable" },
 	{ BIT(1), "Processor Uncorrectable non-fatal" },
@@ -718,7 +722,7 @@ static int available_error_type_show(struct seq_file *m, void *v)
 		if (available_error_type & einj_error_type_string[pos].mask)
 			seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
 				   einj_error_type_string[pos].str);
-	if (available_error_type & ACPI65_EINJV2_SUPP) {
+	if ((available_error_type & ACPI65_EINJV2_SUPP) && EINJv2_enabled) {
 		for (int pos = 0; pos < ARRAY_SIZE(einjv2_error_type_string); pos++) {
 			if (available_error_type_v2 & einjv2_error_type_string[pos].mask)
 				seq_printf(m, "V2_0x%08x\t%s\n", einjv2_error_type_string[pos].mask,
@@ -901,8 +905,22 @@ static int __init einj_probe(struct platform_device *pdev)
 				   &error_param4);
 		debugfs_create_x32("notrigger", S_IRUSR | S_IWUSR,
 				   einj_debug_dir, &notrigger);
+		if (available_error_type & ACPI65_EINJV2_SUPP) {
+			user_input = kzalloc(COMP_ARR_SIZE, GFP_KERNEL);
+			if (!user_input) {
+				EINJv2_enabled = false;
+				pr_info("EINJv2 is disabled: not enough memory\n");
+				goto skip_EINJv2;
+			}
+			EINJv2_enabled = true;
+			einjv2_component_arr.data = user_input;
+			einjv2_component_arr.size = COMP_ARR_SIZE;
+			debugfs_create_blob("einjv2_component_array", S_IRUSR | S_IWUSR,
+					einj_debug_dir, &einjv2_component_arr);
+		}
 	}
 
+skip_EINJv2:
 	if (vendor_dev[0]) {
 		vendor_blob.data = vendor_dev;
 		vendor_blob.size = strlen(vendor_dev);
@@ -944,6 +962,7 @@ static void __exit einj_remove(struct platform_device *pdev)
 		if (vendor_errors.size)
 			acpi_os_unmap_memory(vendor_errors.data, vendor_errors.size);
 	}
+	kfree(user_input);
 	einj_exec_ctx_init(&ctx);
 	apei_exec_post_unmap_gars(&ctx);
 	apei_resources_release(&einj_resources);
-- 
2.43.0

[PATCH v7 8/9] ACPI: APEI: EINJ: Enable EINJv2 error injections

[Zaid Alali]

Enable the driver to inject EINJv2 type errors. The component
array values are parsed from user_input and expected to contain
hex values for component id and syndrome separated by space,
and multiple components are separated by new line as follows:

component_id1 component_syndrome1
component_id2 component_syndrome2
 :
component_id(n) component_syndrome(n)

for example:

$comp_arr="0x1 0x2
>0x1 0x4
>0x2 0x4"
$cd /sys/kernel/debug/apei/einj/
$echo "$comp_arr" > einjv2_component_array

Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 drivers/acpi/apei/einj-core.c | 177 +++++++++++++++++++++++++++++++---
 1 file changed, 166 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index 44f9166c3881..491d33529a3d 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -34,6 +34,7 @@
 /* Firmware should respond within 1 seconds */
 #define FIRMWARE_TIMEOUT	(1 * USEC_PER_SEC)
 #define COMP_ARR_SIZE		1024
+#define COMPONENT_LEN		16
 #define ACPI65_EINJV2_SUPP	BIT(30)
 #define ACPI5_VENDOR_BIT	BIT(31)
 #define MEM_ERROR_MASK		(ACPI_EINJ_MEMORY_CORRECTABLE | \
@@ -87,6 +88,14 @@ enum {
 	SETWA_FLAGS_APICID = 1,
 	SETWA_FLAGS_MEM = 2,
 	SETWA_FLAGS_PCIE_SBDF = 4,
+	SETWA_FLAGS_EINJV2 = 8,
+};
+
+enum {
+	EINJV2_PROCESSOR_ERROR = 0x1,
+	EINJV2_MEMORY_ERROR = 0x2,
+	EINJV2_PCIE_ERROR = 0x4,
+	EINJV2_VENDOR_ERROR = 0x80000000,
 };
 
 /*
@@ -110,6 +119,7 @@ static char vendor_dev[64];
 
 static struct debugfs_blob_wrapper einjv2_component_arr;
 static void *user_input;
+static int nr_components;
 static u32 available_error_type;
 static u32 available_error_type_v2;
 
@@ -180,6 +190,8 @@ static DEFINE_MUTEX(einj_mutex);
 bool einj_initialized __ro_after_init;
 
 static void __iomem *einj_param;
+static u32 v5param_size;
+static bool is_V2;
 
 static void einj_exec_ctx_init(struct apei_exec_context *ctx)
 {
@@ -304,11 +316,31 @@ static void __iomem *einj_get_parameter_address(void)
 		struct set_error_type_with_address v5param;
 		struct set_error_type_with_address __iomem *p;
 
+		v5param_size = sizeof(v5param);
 		p = acpi_os_map_iomem(pa_v5, sizeof(*p));
 		if (p) {
-			memcpy_fromio(&v5param, p, sizeof(v5param));
+			int offset, len;
+
+			memcpy_fromio(&v5param, p, v5param_size);
 			acpi5 = 1;
 			check_vendor_extension(pa_v5, &v5param);
+			if (available_error_type & ACPI65_EINJV2_SUPP) {
+				len = v5param.einjv2_struct.length;
+				offset = offsetof(struct einjv2_extension_struct, component_arr);
+				nr_components = (len - offset) /
+						sizeof(v5param.einjv2_struct.component_arr[0]);
+				/*
+				 * The first call to acpi_os_map_iomem above does not include the
+				 * component array, instead it is used to read and calculate maximum
+				 * number of components supported by the system. Below, the mapping
+				 * is expanded to include the component array.
+				 */
+				acpi_os_unmap_iomem(p, v5param_size);
+				offset = offsetof(struct set_error_type_with_address, einjv2_struct);
+				v5param_size = offset + struct_size(&v5param.einjv2_struct,
+					component_arr, nr_components);
+				p = acpi_os_map_iomem(pa_v5, v5param_size);
+			}
 			return p;
 		}
 	}
@@ -486,6 +518,104 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
 	return rc;
 }
 
+static int parse_hex_to_u8(char *str, u8 *arr)
+{
+	char *ptr, val[32];
+	int pad, str_len;
+
+
+	if (str[0] == '0' && (str[1] == 'x' || str[1] == 'X'))
+		str += 2;
+
+	str_len = strlen(str);
+	if (str_len > 32)
+		return -EINVAL;
+
+	memcpy(val, str, str_len);
+
+	ptr = val;
+	while (*ptr != '\0') {
+		if (!isxdigit(*ptr))
+			return -EINVAL;
+		ptr++;
+	}
+
+	pad = 32 - str_len;
+
+	memmove(val + pad, val, str_len);
+	memset(val, '0', pad);
+
+	for (int i = 0; i < COMPONENT_LEN; ++i) {
+		char byte_str[3] = {val[i * 2], val[i * 2 + 1], '\0'};
+		/* write bytes in little endian format to follow ACPI specs */
+		arr[COMPONENT_LEN - i - 1] = (u8)strtoul(byte_str, NULL, 16);
+	}
+
+	return 0;
+}
+
+static int read_component_array(struct set_error_type_with_address *v5param)
+{
+	int count = 0, str_len;
+	u8 comp_arr[COMPONENT_LEN], synd_arr[COMPONENT_LEN];
+	struct syndrome_array *component_arr;
+	char *tok, *comp_str, *synd_str, *user;
+
+	component_arr = v5param->einjv2_struct.component_arr;
+	str_len = strlen(user_input);
+	user = user_input;
+	user[str_len - 1] = '\0';
+	while ((tok = strsep(&user, "\n")) != NULL) {
+		if (count >= nr_components)
+			return -EINVAL;
+
+		comp_str = strsep(&tok, " \t");
+		synd_str = strsep(&tok, " \t");
+
+		if (!comp_str || !synd_str)
+			return -EINVAL;
+
+		if (parse_hex_to_u8(comp_str, comp_arr))
+			return -EINVAL;
+		if (parse_hex_to_u8(synd_str, synd_arr))
+			return -EINVAL;
+
+		switch (v5param->type) {
+		case EINJV2_PROCESSOR_ERROR:
+			for (int i = 0; i < COMPONENT_LEN; ++i) {
+				component_arr[count].comp_id.acpi_id[i] = comp_arr[i];
+				component_arr[count].comp_synd.proc_synd[i] = synd_arr[i];
+			}
+			break;
+		case EINJV2_MEMORY_ERROR:
+			for (int i = 0; i < COMPONENT_LEN; ++i) {
+				component_arr[count].comp_id.device_id[i] = comp_arr[i];
+				component_arr[count].comp_synd.mem_synd[i] = synd_arr[i];
+			}
+			break;
+		case EINJV2_PCIE_ERROR:
+			for (int i = 0; i < COMPONENT_LEN; ++i) {
+				component_arr[count].comp_id.pcie_sbdf[i] = comp_arr[i];
+				component_arr[count].comp_synd.pcie_synd[i] = synd_arr[i];
+			}
+			break;
+		case EINJV2_VENDOR_ERROR:
+			for (int i = 0; i < COMPONENT_LEN; ++i) {
+				component_arr[count].comp_id.vendor_id[i] = comp_arr[i];
+				component_arr[count].comp_synd.vendor_synd[i] = synd_arr[i];
+			}
+			break;
+		}
+		count++;
+
+	}
+	v5param->einjv2_struct.component_arr_count = count;
+
+	/* clear buffer after user input for next injection */
+	memset(user_input, 0, COMP_ARR_SIZE);
+	return 0;
+}
+
 static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 			       u64 param3, u64 param4)
 {
@@ -500,10 +630,10 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 		return rc;
 	apei_exec_ctx_set_input(&ctx, type);
 	if (acpi5) {
-		struct set_error_type_with_address *v5param, v5_struct;
+		struct set_error_type_with_address *v5param;
 
-		v5param = &v5_struct;
-		memcpy_fromio(v5param, einj_param, sizeof(*v5param));
+		v5param = kmalloc(v5param_size, GFP_KERNEL);
+		memcpy_fromio(v5param, einj_param, v5param_size);
 		v5param->type = type;
 		if (type & ACPI5_VENDOR_BIT) {
 			switch (vendor_flags) {
@@ -523,8 +653,19 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 			v5param->flags = flags;
 			v5param->memory_address = param1;
 			v5param->memory_address_range = param2;
-			v5param->apicid = param3;
-			v5param->pcie_sbdf = param4;
+
+			if (is_V2) {
+				rc = read_component_array(v5param);
+				if (rc) {
+					kfree(v5param);
+					goto err_out;
+				}
+				/* clear buffer after user input for next injection */
+				memset(user_input, 0, COMP_ARR_SIZE);
+			} else {
+				v5param->apicid = param3;
+				v5param->pcie_sbdf = param4;
+			}
 		} else {
 			switch (type) {
 			case ACPI_EINJ_PROCESSOR_CORRECTABLE:
@@ -548,7 +689,8 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 				break;
 			}
 		}
-		memcpy_toio(einj_param, v5param, sizeof(*v5param));
+		memcpy_toio(einj_param, v5param, v5param_size);
+		kfree(v5param);
 	} else {
 		rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
 		if (rc)
@@ -600,6 +742,9 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 	rc = apei_exec_run_optional(&ctx, ACPI_EINJ_END_OPERATION);
 
 	return rc;
+err_out:
+	memset(user_input, 0, COMP_ARR_SIZE);
+	return -EINVAL;
 }
 
 /* Inject the specified hardware error */
@@ -610,10 +755,15 @@ int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
 	u64 base_addr, size;
 
 	/* If user manually set "flags", make sure it is legal */
-	if (flags && (flags &
-		~(SETWA_FLAGS_APICID|SETWA_FLAGS_MEM|SETWA_FLAGS_PCIE_SBDF)))
+	if (flags && (flags & ~(SETWA_FLAGS_APICID | SETWA_FLAGS_MEM |
+		      SETWA_FLAGS_PCIE_SBDF | SETWA_FLAGS_EINJV2)))
 		return -EINVAL;
 
+	/* check if type is a valid EINJv2 error type */
+	if (is_V2) {
+		if (!(type & available_error_type_v2))
+			return -EINVAL;
+	}
 	/*
 	 * We need extra sanity checks for memory errors.
 	 * Other types leap directly to injection.
@@ -764,7 +914,7 @@ int einj_validate_error_type(u64 type)
 	if (tval & (tval - 1))
 		return -EINVAL;
 	if (!vendor)
-		if (!(type & available_error_type))
+		if (!(type & (available_error_type | available_error_type_v2)))
 			return -EINVAL;
 
 	return 0;
@@ -783,9 +933,11 @@ static ssize_t error_type_set(struct file *file, const char __user *buf,
 	if (strncmp(einj_buf, "V2_", 3) == 0) {
 		if (!sscanf(einj_buf, "V2_%llx", &val))
 			return -EINVAL;
+		is_V2 = true;
 	} else {
 		if (!sscanf(einj_buf, "%llx", &val))
 			return -EINVAL;
+		is_V2 = false;
 	}
 
 	rc = einj_validate_error_type(val);
@@ -807,6 +959,9 @@ static int error_inject_set(void *data, u64 val)
 	if (!error_type)
 		return -EINVAL;
 
+	if (is_V2)
+		error_flags |= SETWA_FLAGS_EINJV2;
+
 	return einj_error_inject(error_type, error_flags, error_param1, error_param2,
 		error_param3, error_param4);
 }
@@ -955,7 +1110,7 @@ static void __exit einj_remove(struct platform_device *pdev)
 
 	if (einj_param) {
 		acpi_size size = (acpi5) ?
-			sizeof(struct set_error_type_with_address) :
+			v5param_size :
 			sizeof(struct einj_parameter);
 
 		acpi_os_unmap_iomem(einj_param, size);
-- 
2.43.0

[PATCH v7 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Zaid Alali]

Add documentation for the updated ACPI specs for EINJv2[1][2]

Link: https://github.com/tianocore/edk2/issues/9449 [1]
Link: https://github.com/tianocore/edk2/issues/9017 [2]

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 .../firmware-guide/acpi/apei/einj.rst         | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/Documentation/firmware-guide/acpi/apei/einj.rst b/Documentation/firmware-guide/acpi/apei/einj.rst
index c52b9da08fa9..edf3a2165e75 100644
--- a/Documentation/firmware-guide/acpi/apei/einj.rst
+++ b/Documentation/firmware-guide/acpi/apei/einj.rst
@@ -59,6 +59,9 @@ The following files belong to it:
   0x00000200        Platform Correctable
   0x00000400        Platform Uncorrectable non-fatal
   0x00000800        Platform Uncorrectable fatal
+  V2_0x00000001     EINJV2 Processor Error
+  V2_0x00000002     EINJV2 Memory Error
+  V2_0x00000004     EINJV2 PCI Express Error
   ================  ===================================
 
   The format of the file contents are as above, except present are only
@@ -88,6 +91,8 @@ The following files belong to it:
       Memory address and mask valid (param1 and param2).
     Bit 2
       PCIe (seg,bus,dev,fn) valid (see param4 below).
+    Bit 3
+      EINJv2 extension structure is valid
 
   If set to zero, legacy behavior is mimicked where the type of
   injection specifies just one bit set, and param1 is multiplexed.
@@ -122,6 +127,13 @@ The following files belong to it:
   this actually works depends on what operations the BIOS actually
   includes in the trigger phase.
 
+- einjv2_component_array
+
+  The contents of this file are used to set the "Component Array" field
+  of the EINJv2 Extension Structure. The expected format is hex values
+  for component id and syndrome separated by space, and multiple
+  components are separated by new line.
+
 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:
@@ -194,6 +206,26 @@ An error injection example::
   # echo 0x8 > error_type			# Choose correctable memory error
   # echo 1 > error_inject			# Inject now
 
+An EINJv2 error injection example::
+
+  # cd /sys/kernel/debug/apei/einj
+  # cat available_error_type			# See which errors can be injected
+  0x00000002	Processor Uncorrectable non-fatal
+  0x00000008	Memory Correctable
+  0x00000010	Memory Uncorrectable non-fatal
+  V2_0x00000001	EINJV2 Processor Error
+  V2_0x00000002	EINJV2 Memory Error
+
+  # echo 0x12345000 > param1			# Set memory address for injection
+  # echo 0xfffffffffffff000 > param2		# Range - anywhere in this page
+  # comp_arr="0x1 0x2				# Fill in the component array
+    >0x1 0x4
+    >0x2 0x4"
+  # echo "$comp_arr" > einjv2_component_array
+  # echo V2_0x2 > error_type			# Choose EINJv2 memory error
+  # echo 0xa > flags				# set flags to indicate EINJv2
+  # echo 1 > error_inject			# Inject now
+
 You should see something like this in dmesg::
 
   [22715.830801] EDAC sbridge MC3: HANDLING MCE MEMORY ERROR
-- 
2.43.0

Re: [PATCH v7 1/9] ACPICA: Update values to hex to follow ACPI specs

[Rafael J. Wysocki]

On Tue, May 6, 2025 at 11:38 PM Zaid Alali
<zaidal@os.amperecomputing.com> wrote:
>
> ACPI specs[1] define Error Injection Actions in hex values.
> This commit intends to update values from decimal to hex to be
> consistent with ACPI specs. This commit and the following one are
> not to be merged and will come form ACPICA project see pull request[2].
>
> Link: https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html [1]
> Link: https://github.com/acpica/acpica/pull/977 [2]
>
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>

An equivalent patch is already there in linux-next:

https://web.git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=5d2f7e76b70121fe06fd12315a6ea439e3bf0414

> ---
>  include/acpi/actbl1.h | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
> index 387fc821703a..c701c434976c 100644
> --- a/include/acpi/actbl1.h
> +++ b/include/acpi/actbl1.h
> @@ -1024,18 +1024,18 @@ struct acpi_einj_entry {
>  /* Values for Action field above */
>
>  enum acpi_einj_actions {
> -       ACPI_EINJ_BEGIN_OPERATION = 0,
> -       ACPI_EINJ_GET_TRIGGER_TABLE = 1,
> -       ACPI_EINJ_SET_ERROR_TYPE = 2,
> -       ACPI_EINJ_GET_ERROR_TYPE = 3,
> -       ACPI_EINJ_END_OPERATION = 4,
> -       ACPI_EINJ_EXECUTE_OPERATION = 5,
> -       ACPI_EINJ_CHECK_BUSY_STATUS = 6,
> -       ACPI_EINJ_GET_COMMAND_STATUS = 7,
> -       ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS = 8,
> -       ACPI_EINJ_GET_EXECUTE_TIMINGS = 9,
> -       ACPI_EINJ_ACTION_RESERVED = 10, /* 10 and greater are reserved */
> -       ACPI_EINJ_TRIGGER_ERROR = 0xFF  /* Except for this value */
> +       ACPI_EINJ_BEGIN_OPERATION =             0x0,
> +       ACPI_EINJ_GET_TRIGGER_TABLE =           0x1,
> +       ACPI_EINJ_SET_ERROR_TYPE =              0x2,
> +       ACPI_EINJ_GET_ERROR_TYPE =              0x3,
> +       ACPI_EINJ_END_OPERATION =               0x4,
> +       ACPI_EINJ_EXECUTE_OPERATION =           0x5,
> +       ACPI_EINJ_CHECK_BUSY_STATUS =           0x6,
> +       ACPI_EINJ_GET_COMMAND_STATUS =          0x7,
> +       ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS = 0x8,
> +       ACPI_EINJ_GET_EXECUTE_TIMINGS =         0x9,
> +       ACPI_EINJ_ACTION_RESERVED =             0xA,    /* 0xA and greater are reserved */
> +       ACPI_EINJ_TRIGGER_ERROR =               0xFF    /* Except for this value */
>  };
>
>  /* Values for Instruction field above */
> --
> 2.43.0
>

Re: [PATCH v7 2/9] ACPICA: Add EINJv2 get error type action

[Rafael J. Wysocki]

On Tue, May 6, 2025 at 11:38 PM Zaid Alali
<zaidal@os.amperecomputing.com> wrote:
>
> Add EINJV2_GET_ERROR_TYPE as defined in the approved new ACPI
> specs[1][2].
>
> Proposed ACPI spces for EINJv2:
> Link: https://github.com/tianocore/edk2/issues/9449 [1]
> Link: https://github.com/tianocore/edk2/issues/9017 [2]
>
> This commit is not a direct merge, it will come from ACPICA
> project, see pull request[3].
>
> Link: https://github.com/acpica/acpica/pull/977 [3]
>
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>

An equivalent patch is already there in linux-next:

https://web.git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=56b594fdb6dd68d085067a69f5f78171aa6f1657

> ---
>  include/acpi/actbl1.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
> index c701c434976c..f52d5cafaf76 100644
> --- a/include/acpi/actbl1.h
> +++ b/include/acpi/actbl1.h
> @@ -1034,7 +1034,8 @@ enum acpi_einj_actions {
>         ACPI_EINJ_GET_COMMAND_STATUS =          0x7,
>         ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS = 0x8,
>         ACPI_EINJ_GET_EXECUTE_TIMINGS =         0x9,
> -       ACPI_EINJ_ACTION_RESERVED =             0xA,    /* 0xA and greater are reserved */
> +       ACPI_EINJV2_GET_ERROR_TYPE =            0x11,
> +       ACPI_EINJ_ACTION_RESERVED =             0x12,   /* 0x12 and greater are reserved */
>         ACPI_EINJ_TRIGGER_ERROR =               0xFF    /* Except for this value */
>  };
>
> --
> 2.43.0
>

Re: [PATCH v7 3/9] ACPI: APEI: EINJ: Fix kernel test sparse warnings

[Rafael J. Wysocki]

On Tue, May 6, 2025 at 11:38 PM Zaid Alali
<zaidal@os.amperecomputing.com> wrote:
>
> This patch fixes the kernel test robot warning reported here:
> Link: https://lore.kernel.org/all/202410241620.oApALow5-lkp@intel.com/

The Link: tag should be adjacent to the other tags.

Also, it is not sufficient to point to a problem report from a patch
changelog.  Please describe both the problem and the fix.

> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> ---
>  drivers/acpi/apei/einj-core.c | 106 +++++++++++++++++++---------------
>  1 file changed, 60 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index 04731a5b01fa..47abd9317fef 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -149,7 +149,7 @@ static DEFINE_MUTEX(einj_mutex);
>   */
>  bool einj_initialized __ro_after_init;
>
> -static void *einj_param;
> +static void __iomem *einj_param;
>
>  static void einj_exec_ctx_init(struct apei_exec_context *ctx)
>  {
> @@ -214,24 +214,26 @@ static void check_vendor_extension(u64 paddr,
>                                    struct set_error_type_with_address *v5param)
>  {
>         int     offset = v5param->vendor_extension;
> -       struct  vendor_error_type_extension *v;
> +       struct  vendor_error_type_extension v;
> +       struct vendor_error_type_extension __iomem *p;
>         u32     sbdf;
>
>         if (!offset)
>                 return;
> -       v = acpi_os_map_iomem(paddr + offset, sizeof(*v));
> -       if (!v)
> +       p = acpi_os_map_iomem(paddr + offset, sizeof(*p));
> +       if (!p)
>                 return;
> -       get_oem_vendor_struct(paddr, offset, v);
> -       sbdf = v->pcie_sbdf;
> +       memcpy_fromio(&v, p, sizeof(v));
> +       get_oem_vendor_struct(paddr, offset, &v);
> +       sbdf = v.pcie_sbdf;
>         sprintf(vendor_dev, "%x:%x:%x.%x vendor_id=%x device_id=%x rev_id=%x\n",
>                 sbdf >> 24, (sbdf >> 16) & 0xff,
>                 (sbdf >> 11) & 0x1f, (sbdf >> 8) & 0x7,
> -                v->vendor_id, v->device_id, v->rev_id);
> -       acpi_os_unmap_iomem(v, sizeof(*v));
> +                v.vendor_id, v.device_id, v.rev_id);
> +       acpi_os_unmap_iomem(p, sizeof(v));
>  }
>
> -static void *einj_get_parameter_address(void)
> +static void __iomem *einj_get_parameter_address(void)
>  {
>         int i;
>         u64 pa_v4 = 0, pa_v5 = 0;
> @@ -252,26 +254,30 @@ static void *einj_get_parameter_address(void)
>                 entry++;
>         }
>         if (pa_v5) {
> -               struct set_error_type_with_address *v5param;
> +               struct set_error_type_with_address v5param;
> +               struct set_error_type_with_address __iomem *p;
>
> -               v5param = acpi_os_map_iomem(pa_v5, sizeof(*v5param));
> -               if (v5param) {
> +               p = acpi_os_map_iomem(pa_v5, sizeof(*p));
> +               if (p) {
> +                       memcpy_fromio(&v5param, p, sizeof(v5param));
>                         acpi5 = 1;
> -                       check_vendor_extension(pa_v5, v5param);
> -                       return v5param;
> +                       check_vendor_extension(pa_v5, &v5param);
> +                       return p;
>                 }
>         }
>         if (param_extension && pa_v4) {
> -               struct einj_parameter *v4param;
> +               struct einj_parameter v4param;
> +               struct einj_parameter __iomem *p;
>
> -               v4param = acpi_os_map_iomem(pa_v4, sizeof(*v4param));
> -               if (!v4param)
> +               p = acpi_os_map_iomem(pa_v4, sizeof(*p));
> +               if (!p)
>                         return NULL;
> -               if (v4param->reserved1 || v4param->reserved2) {
> -                       acpi_os_unmap_iomem(v4param, sizeof(*v4param));
> +               memcpy_fromio(&v4param, p, sizeof(v4param));
> +               if (v4param.reserved1 || v4param.reserved2) {
> +                       acpi_os_unmap_iomem(p, sizeof(v4param));
>                         return NULL;
>                 }
> -               return v4param;
> +               return p;
>         }
>
>         return NULL;
> @@ -317,7 +323,7 @@ static struct acpi_generic_address *einj_get_trigger_parameter_region(
>  static int __einj_error_trigger(u64 trigger_paddr, u32 type,
>                                 u64 param1, u64 param2)
>  {
> -       struct acpi_einj_trigger *trigger_tab = NULL;
> +       struct acpi_einj_trigger trigger_tab;
>         struct apei_exec_context trigger_ctx;
>         struct apei_resources trigger_resources;
>         struct acpi_whea_header *trigger_entry;
> @@ -325,54 +331,57 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
>         u32 table_size;
>         int rc = -EIO;
>         struct acpi_generic_address *trigger_param_region = NULL;
> +       struct acpi_einj_trigger __iomem *p;
>
> -       r = request_mem_region(trigger_paddr, sizeof(*trigger_tab),
> +       r = request_mem_region(trigger_paddr, sizeof(trigger_tab),
>                                "APEI EINJ Trigger Table");
>         if (!r) {
>                 pr_err("Can not request [mem %#010llx-%#010llx] for Trigger table\n",
>                        (unsigned long long)trigger_paddr,
>                        (unsigned long long)trigger_paddr +
> -                           sizeof(*trigger_tab) - 1);
> +                           sizeof(trigger_tab) - 1);
>                 goto out;
>         }
> -       trigger_tab = ioremap_cache(trigger_paddr, sizeof(*trigger_tab));
> -       if (!trigger_tab) {
> +       p = ioremap_cache(trigger_paddr, sizeof(*p));
> +       if (!p) {
>                 pr_err("Failed to map trigger table!\n");
>                 goto out_rel_header;
>         }
> -       rc = einj_check_trigger_header(trigger_tab);
> +       memcpy_fromio(&trigger_tab, p, sizeof(trigger_tab));
> +       rc = einj_check_trigger_header(&trigger_tab);
>         if (rc) {
>                 pr_warn(FW_BUG "Invalid trigger error action table.\n");
>                 goto out_rel_header;
>         }
>
>         /* No action structures in the TRIGGER_ERROR table, nothing to do */
> -       if (!trigger_tab->entry_count)
> +       if (!trigger_tab.entry_count)
>                 goto out_rel_header;
>
>         rc = -EIO;
> -       table_size = trigger_tab->table_size;
> -       r = request_mem_region(trigger_paddr + sizeof(*trigger_tab),
> -                              table_size - sizeof(*trigger_tab),
> +       table_size = trigger_tab.table_size;
> +       r = request_mem_region(trigger_paddr + sizeof(trigger_tab),
> +                              table_size - sizeof(trigger_tab),
>                                "APEI EINJ Trigger Table");
>         if (!r) {
>                 pr_err("Can not request [mem %#010llx-%#010llx] for Trigger Table Entry\n",
> -                      (unsigned long long)trigger_paddr + sizeof(*trigger_tab),
> +                      (unsigned long long)trigger_paddr + sizeof(trigger_tab),
>                        (unsigned long long)trigger_paddr + table_size - 1);
>                 goto out_rel_header;
>         }
> -       iounmap(trigger_tab);
> -       trigger_tab = ioremap_cache(trigger_paddr, table_size);
> -       if (!trigger_tab) {
> +       iounmap(p);
> +       p = ioremap_cache(trigger_paddr, table_size);
> +       if (!p) {
>                 pr_err("Failed to map trigger table!\n");
>                 goto out_rel_entry;
>         }
> +       memcpy_fromio(&trigger_tab, p, sizeof(trigger_tab));
>         trigger_entry = (struct acpi_whea_header *)
> -               ((char *)trigger_tab + sizeof(struct acpi_einj_trigger));
> +               ((char *)&trigger_tab + sizeof(struct acpi_einj_trigger));
>         apei_resources_init(&trigger_resources);
>         apei_exec_ctx_init(&trigger_ctx, einj_ins_type,
>                            ARRAY_SIZE(einj_ins_type),
> -                          trigger_entry, trigger_tab->entry_count);
> +                          trigger_entry, trigger_tab.entry_count);
>         rc = apei_exec_collect_resources(&trigger_ctx, &trigger_resources);
>         if (rc)
>                 goto out_fini;
> @@ -390,7 +399,7 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
>
>                 apei_resources_init(&addr_resources);
>                 trigger_param_region = einj_get_trigger_parameter_region(
> -                       trigger_tab, param1, param2);
> +                       &trigger_tab, param1, param2);
>                 if (trigger_param_region) {
>                         rc = apei_resources_add(&addr_resources,
>                                 trigger_param_region->address,
> @@ -419,13 +428,13 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
>  out_fini:
>         apei_resources_fini(&trigger_resources);
>  out_rel_entry:
> -       release_mem_region(trigger_paddr + sizeof(*trigger_tab),
> -                          table_size - sizeof(*trigger_tab));
> +       release_mem_region(trigger_paddr + sizeof(trigger_tab),
> +                          table_size - sizeof(trigger_tab));
>  out_rel_header:
> -       release_mem_region(trigger_paddr, sizeof(*trigger_tab));
> +       release_mem_region(trigger_paddr, sizeof(trigger_tab));
>  out:
> -       if (trigger_tab)
> -               iounmap(trigger_tab);
> +       if (p)
> +               iounmap(p);
>
>         return rc;
>  }
> @@ -444,8 +453,10 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>                 return rc;
>         apei_exec_ctx_set_input(&ctx, type);
>         if (acpi5) {
> -               struct set_error_type_with_address *v5param = einj_param;
> +               struct set_error_type_with_address *v5param, v5_struct;
>
> +               v5param = &v5_struct;
> +               memcpy_fromio(v5param, einj_param, sizeof(*v5param));
>                 v5param->type = type;
>                 if (type & ACPI5_VENDOR_BIT) {
>                         switch (vendor_flags) {
> @@ -490,15 +501,18 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>                                 break;
>                         }
>                 }
> +               memcpy_toio(einj_param, v5param, sizeof(*v5param));
>         } else {
>                 rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
>                 if (rc)
>                         return rc;
>                 if (einj_param) {
> -                       struct einj_parameter *v4param = einj_param;
> +                       struct einj_parameter v4param;
>
> -                       v4param->param1 = param1;
> -                       v4param->param2 = param2;
> +                       memcpy_fromio(&v4param, einj_param, sizeof(v4param));
> +                       v4param.param1 = param1;
> +                       v4param.param2 = param2;
> +                       memcpy_toio(einj_param, &v4param, sizeof(v4param));
>                 }
>         }
>         rc = apei_exec_run(&ctx, ACPI_EINJ_EXECUTE_OPERATION);
> --
> 2.43.0
>

Re: [PATCH v7 4/9] ACPI: APEI: EINJ: Remove redundant calls to einj_get_available_error_type

[Rafael J. Wysocki]

On Tue, May 6, 2025 at 11:38 PM Zaid Alali
<zaidal@os.amperecomputing.com> wrote:
>
> A single call to einj_get_available_error_type in init function is
> sufficient to save the return value in a global variable to be used
> later in various places in the code. This commit does not introduce
> any functional changes, but only removing unnecessary redundant
> function calls.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>

Does this patch depend on patches [1-3/7]?  If not, I can pick it up
as an optimization.

> ---
>  drivers/acpi/apei/einj-core.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index 47abd9317fef..ada1d7026af5 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -83,6 +83,8 @@ static struct debugfs_blob_wrapper vendor_blob;
>  static struct debugfs_blob_wrapper vendor_errors;
>  static char vendor_dev[64];
>
> +static u32 available_error_type;
> +
>  /*
>   * Some BIOSes allow parameters to the SET_ERROR_TYPE entries in the
>   * EINJ table through an unpublished extension. Use with caution as
> @@ -662,14 +664,9 @@ static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
>
>  static int available_error_type_show(struct seq_file *m, void *v)
>  {
> -       int rc;
> -       u32 error_type = 0;
>
> -       rc = einj_get_available_error_type(&error_type);
> -       if (rc)
> -               return rc;
>         for (int pos = 0; pos < ARRAY_SIZE(einj_error_type_string); pos++)
> -               if (error_type & einj_error_type_string[pos].mask)
> +               if (available_error_type & einj_error_type_string[pos].mask)
>                         seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
>                                    einj_error_type_string[pos].str);
>
> @@ -692,8 +689,7 @@ bool einj_is_cxl_error_type(u64 type)
>
>  int einj_validate_error_type(u64 type)
>  {
> -       u32 tval, vendor, available_error_type = 0;
> -       int rc;
> +       u32 tval, vendor;
>
>         /* Only low 32 bits for error type are valid */
>         if (type & GENMASK_ULL(63, 32))
> @@ -709,13 +705,9 @@ int einj_validate_error_type(u64 type)
>         /* Only one error type can be specified */
>         if (tval & (tval - 1))
>                 return -EINVAL;
> -       if (!vendor) {
> -               rc = einj_get_available_error_type(&available_error_type);
> -               if (rc)
> -                       return rc;
> +       if (!vendor)
>                 if (!(type & available_error_type))
>                         return -EINVAL;
> -       }
>
>         return 0;
>  }
> @@ -791,6 +783,10 @@ static int __init einj_probe(struct platform_device *pdev)
>                 goto err_put_table;
>         }
>
> +       rc = einj_get_available_error_type(&available_error_type);
> +       if (rc)
> +               return rc;
> +
>         rc = -ENOMEM;
>         einj_debug_dir = debugfs_create_dir("einj", apei_get_debugfs_dir());
>
> --
> 2.43.0
>

Re: [PATCH v7 5/9] ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities

[Rafael J. Wysocki]

On Tue, May 6, 2025 at 11:38 PM Zaid Alali
<zaidal@os.amperecomputing.com> wrote:
>
> Enable the driver to show all supported error injections for EINJ
> and EINJv2 at the same time. EINJv2 capabilities can be discovered
> by checking the return value of get_error_type, where bit 30 set
> indicates EINJv2 support.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>

While the tag provided by Jonathan is appreciated, patches [5-9/9]
need to be reviewed by one of the reviewers listed in the APEI entry
in MAINTAINERS.

> ---
>  drivers/acpi/apei/apei-internal.h |  2 +-
>  drivers/acpi/apei/einj-core.c     | 75 +++++++++++++++++++++++++------
>  drivers/acpi/apei/einj-cxl.c      |  2 +-
>  3 files changed, 63 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
> index cd2766c69d78..77c10a7a7a9f 100644
> --- a/drivers/acpi/apei/apei-internal.h
> +++ b/drivers/acpi/apei/apei-internal.h
> @@ -131,7 +131,7 @@ 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_get_available_error_type(u32 *type, int einj_action);
>  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,
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index ada1d7026af5..ee26df0398fc 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -33,6 +33,7 @@
>  #define SLEEP_UNIT_MAX         5000                    /* 5ms */
>  /* Firmware should respond within 1 seconds */
>  #define FIRMWARE_TIMEOUT       (1 * USEC_PER_SEC)
> +#define ACPI65_EINJV2_SUPP     BIT(30)
>  #define ACPI5_VENDOR_BIT       BIT(31)
>  #define MEM_ERROR_MASK         (ACPI_EINJ_MEMORY_CORRECTABLE | \
>                                 ACPI_EINJ_MEMORY_UNCORRECTABLE | \
> @@ -84,6 +85,7 @@ static struct debugfs_blob_wrapper vendor_errors;
>  static char vendor_dev[64];
>
>  static u32 available_error_type;
> +static u32 available_error_type_v2;
>
>  /*
>   * Some BIOSes allow parameters to the SET_ERROR_TYPE entries in the
> @@ -159,13 +161,13 @@ static void einj_exec_ctx_init(struct apei_exec_context *ctx)
>                            EINJ_TAB_ENTRY(einj_tab), einj_tab->entries);
>  }
>
> -static int __einj_get_available_error_type(u32 *type)
> +static int __einj_get_available_error_type(u32 *type, int einj_action)
>  {
>         struct apei_exec_context ctx;
>         int rc;
>
>         einj_exec_ctx_init(&ctx);
> -       rc = apei_exec_run(&ctx, ACPI_EINJ_GET_ERROR_TYPE);
> +       rc = apei_exec_run(&ctx, einj_action);
>         if (rc)
>                 return rc;
>         *type = apei_exec_ctx_get_output(&ctx);
> @@ -174,17 +176,34 @@ static int __einj_get_available_error_type(u32 *type)
>  }
>
>  /* Get error injection capabilities of the platform */
> -int einj_get_available_error_type(u32 *type)
> +int einj_get_available_error_type(u32 *type, int einj_action)
>  {
>         int rc;
>
>         mutex_lock(&einj_mutex);
> -       rc = __einj_get_available_error_type(type);
> +       rc = __einj_get_available_error_type(type, einj_action);
>         mutex_unlock(&einj_mutex);
>
>         return rc;
>  }
>
> +static int einj_get_available_error_types(u32 *type1, u32 *type2)
> +{
> +       int rc;
> +
> +       rc = einj_get_available_error_type(type1, ACPI_EINJ_GET_ERROR_TYPE);
> +       if (rc)
> +               return rc;
> +       if (*type1 & ACPI65_EINJV2_SUPP) {
> +               rc = einj_get_available_error_type(type2,
> +                                                  ACPI_EINJV2_GET_ERROR_TYPE);
> +               if (rc)
> +                       return rc;
> +       }
> +
> +       return 0;
> +}
> +
>  static int einj_timedout(u64 *t)
>  {
>         if ((s64)*t < SLEEP_UNIT_MIN) {
> @@ -646,6 +665,7 @@ static u64 error_param2;
>  static u64 error_param3;
>  static u64 error_param4;
>  static struct dentry *einj_debug_dir;
> +static char einj_buf[32];
>  static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
>         { BIT(0), "Processor Correctable" },
>         { BIT(1), "Processor Uncorrectable non-fatal" },
> @@ -662,6 +682,12 @@ static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
>         { BIT(31), "Vendor Defined Error Types" },
>  };
>
> +static struct { u32 mask; const char *str; } const einjv2_error_type_string[] = {
> +       { BIT(0), "EINJV2 Processor Error" },
> +       { BIT(1), "EINJV2 Memory Error" },
> +       { BIT(2), "EINJV2 PCI Express Error" },
> +};
> +
>  static int available_error_type_show(struct seq_file *m, void *v)
>  {
>
> @@ -669,17 +695,22 @@ static int available_error_type_show(struct seq_file *m, void *v)
>                 if (available_error_type & einj_error_type_string[pos].mask)
>                         seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
>                                    einj_error_type_string[pos].str);
> -
> +       if (available_error_type & ACPI65_EINJV2_SUPP) {
> +               for (int pos = 0; pos < ARRAY_SIZE(einjv2_error_type_string); pos++) {
> +                       if (available_error_type_v2 & einjv2_error_type_string[pos].mask)
> +                               seq_printf(m, "V2_0x%08x\t%s\n", einjv2_error_type_string[pos].mask,
> +                                          einjv2_error_type_string[pos].str);
> +               }
> +       }
>         return 0;
>  }
>
>  DEFINE_SHOW_ATTRIBUTE(available_error_type);
>
> -static int error_type_get(void *data, u64 *val)
> +static ssize_t error_type_get(struct file *file, char __user *buf,
> +                               size_t count, loff_t *ppos)
>  {
> -       *val = error_type;
> -
> -       return 0;
> +       return simple_read_from_buffer(buf, count, ppos, einj_buf, strlen(einj_buf));
>  }
>
>  bool einj_is_cxl_error_type(u64 type)
> @@ -712,9 +743,23 @@ int einj_validate_error_type(u64 type)
>         return 0;
>  }
>
> -static int error_type_set(void *data, u64 val)
> +static ssize_t error_type_set(struct file *file, const char __user *buf,
> +                               size_t count, loff_t *ppos)
>  {
>         int rc;
> +       u64 val;
> +
> +       memset(einj_buf, 0, sizeof(einj_buf));
> +       if (copy_from_user(einj_buf, buf, count))
> +               return -EFAULT;
> +
> +       if (strncmp(einj_buf, "V2_", 3) == 0) {
> +               if (!sscanf(einj_buf, "V2_%llx", &val))
> +                       return -EINVAL;
> +       } else {
> +               if (!sscanf(einj_buf, "%llx", &val))
> +                       return -EINVAL;
> +       }
>
>         rc = einj_validate_error_type(val);
>         if (rc)
> @@ -722,11 +767,13 @@ static int error_type_set(void *data, u64 val)
>
>         error_type = val;
>
> -       return 0;
> +       return count;
>  }
>
> -DEFINE_DEBUGFS_ATTRIBUTE(error_type_fops, error_type_get, error_type_set,
> -                        "0x%llx\n");
> +static const struct file_operations error_type_fops = {
> +       .read           = error_type_get,
> +       .write          = error_type_set,
> +};
>
>  static int error_inject_set(void *data, u64 val)
>  {
> @@ -783,7 +830,7 @@ static int __init einj_probe(struct platform_device *pdev)
>                 goto err_put_table;
>         }
>
> -       rc = einj_get_available_error_type(&available_error_type);
> +       rc = einj_get_available_error_types(&available_error_type, &available_error_type_v2);
>         if (rc)
>                 return rc;
>
> diff --git a/drivers/acpi/apei/einj-cxl.c b/drivers/acpi/apei/einj-cxl.c
> index 78da9ae543a2..e70a416ec925 100644
> --- a/drivers/acpi/apei/einj-cxl.c
> +++ b/drivers/acpi/apei/einj-cxl.c
> @@ -30,7 +30,7 @@ int einj_cxl_available_error_type_show(struct seq_file *m, void *v)
>         int cxl_err, rc;
>         u32 available_error_type = 0;
>
> -       rc = einj_get_available_error_type(&available_error_type);
> +       rc = einj_get_available_error_type(&available_error_type, ACPI_EINJ_GET_ERROR_TYPE);
>         if (rc)
>                 return rc;
>
> --
> 2.43.0
>

Re: [PATCH v7 4/9] ACPI: APEI: EINJ: Remove redundant calls to einj_get_available_error_type

[Zaid Alali]

On Wed, May 07, 2025 at 01:34:35PM +0200, Rafael J. Wysocki wrote:
> On Tue, May 6, 2025 at 11:38 PM Zaid Alali
> <zaidal@os.amperecomputing.com> wrote:
> >
> > A single call to einj_get_available_error_type in init function is
> > sufficient to save the return value in a global variable to be used
> > later in various places in the code. This commit does not introduce
> > any functional changes, but only removing unnecessary redundant
> > function calls.
> >
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> 
> Does this patch depend on patches [1-3/7]?  If not, I can pick it up
> as an optimization.
Hi Rafael,

No, this patch is does not depend on 1-3 patches. You can pick it up!

-Zaid 

> 
> > ---
> >  drivers/acpi/apei/einj-core.c | 22 +++++++++-------------
> >  1 file changed, 9 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> > index 47abd9317fef..ada1d7026af5 100644
> > --- a/drivers/acpi/apei/einj-core.c
> > +++ b/drivers/acpi/apei/einj-core.c
> > @@ -83,6 +83,8 @@ static struct debugfs_blob_wrapper vendor_blob;
> >  static struct debugfs_blob_wrapper vendor_errors;
> >  static char vendor_dev[64];
> >
> > +static u32 available_error_type;
> > +
> >  /*
> >   * Some BIOSes allow parameters to the SET_ERROR_TYPE entries in the
> >   * EINJ table through an unpublished extension. Use with caution as
> > @@ -662,14 +664,9 @@ static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
> >
> >  static int available_error_type_show(struct seq_file *m, void *v)
> >  {
> > -       int rc;
> > -       u32 error_type = 0;
> >
> > -       rc = einj_get_available_error_type(&error_type);
> > -       if (rc)
> > -               return rc;
> >         for (int pos = 0; pos < ARRAY_SIZE(einj_error_type_string); pos++)
> > -               if (error_type & einj_error_type_string[pos].mask)
> > +               if (available_error_type & einj_error_type_string[pos].mask)
> >                         seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
> >                                    einj_error_type_string[pos].str);
> >
> > @@ -692,8 +689,7 @@ bool einj_is_cxl_error_type(u64 type)
> >
> >  int einj_validate_error_type(u64 type)
> >  {
> > -       u32 tval, vendor, available_error_type = 0;
> > -       int rc;
> > +       u32 tval, vendor;
> >
> >         /* Only low 32 bits for error type are valid */
> >         if (type & GENMASK_ULL(63, 32))
> > @@ -709,13 +705,9 @@ int einj_validate_error_type(u64 type)
> >         /* Only one error type can be specified */
> >         if (tval & (tval - 1))
> >                 return -EINVAL;
> > -       if (!vendor) {
> > -               rc = einj_get_available_error_type(&available_error_type);
> > -               if (rc)
> > -                       return rc;
> > +       if (!vendor)
> >                 if (!(type & available_error_type))
> >                         return -EINVAL;
> > -       }
> >
> >         return 0;
> >  }
> > @@ -791,6 +783,10 @@ static int __init einj_probe(struct platform_device *pdev)
> >                 goto err_put_table;
> >         }
> >
> > +       rc = einj_get_available_error_type(&available_error_type);
> > +       if (rc)
> > +               return rc;
> > +
> >         rc = -ENOMEM;
> >         einj_debug_dir = debugfs_create_dir("einj", apei_get_debugfs_dir());
> >
> > --
> > 2.43.0
> >

Re: [PATCH v7 4/9] ACPI: APEI: EINJ: Remove redundant calls to einj_get_available_error_type

[Rafael J. Wysocki]

On Thu, May 8, 2025 at 7:58 PM Zaid Alali <zaidal@os.amperecomputing.com> wrote:
>
> On Wed, May 07, 2025 at 01:34:35PM +0200, Rafael J. Wysocki wrote:
> > On Tue, May 6, 2025 at 11:38 PM Zaid Alali
> > <zaidal@os.amperecomputing.com> wrote:
> > >
> > > A single call to einj_get_available_error_type in init function is
> > > sufficient to save the return value in a global variable to be used
> > > later in various places in the code. This commit does not introduce
> > > any functional changes, but only removing unnecessary redundant
> > > function calls.
> > >
> > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> > > Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> >
> > Does this patch depend on patches [1-3/7]?  If not, I can pick it up
> > as an optimization.
> Hi Rafael,
>
> No, this patch is does not depend on 1-3 patches. You can pick it up!

Now applied as 6.16 material, thanks!

Re: [PATCH v7 6/9] ACPI: APEI: EINJ: Add einjv2 extension struct

[Luck, Tony]

On Tue, May 06, 2025 at 02:38:10PM -0700, Zaid Alali wrote:
> Add einjv2 extension struct and EINJv2 error types to prepare
> the driver for EINJv2 support. ACPI specifications[1] enables
> EINJv2 by extending set_error_type_with_address struct.
> 
> Link: https://github.com/tianocore/edk2/issues/9449 [1]
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> ---
>  drivers/acpi/apei/einj-core.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index ee26df0398fc..60e4f3dc7055 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -50,6 +50,28 @@
>   */
>  static int acpi5;
>  
> +struct syndrome_array {
> +	union {
> +		u8	acpi_id[16];
> +		u8	device_id[16];
> +		u8	pcie_sbdf[16];
> +		u8	vendor_id[16];
> +	} comp_id;
> +	union {
> +		u8	proc_synd[16];
> +		u8	mem_synd[16];
> +		u8	pcie_synd[16];
> +		u8	vendor_synd[16];
> +	} comp_synd;
> +};
> +
> +struct einjv2_extension_struct {
> +	u32 length;
> +	u16 revision;
> +	u16 component_arr_count;
> +	struct syndrome_array component_arr[] __counted_by(component_arr_count);
> +};
> +
>  struct set_error_type_with_address {
>  	u32	type;
>  	u32	vendor_extension;
> @@ -58,6 +80,7 @@ struct set_error_type_with_address {
>  	u64	memory_address;
>  	u64	memory_address_range;
>  	u32	pcie_sbdf;
> +	struct	einjv2_extension_struct einjv2_struct;

I can't make this match up with the ACPI v6.5 spec.  The spec defines
a whole new EINJV2_SET_ERROR_TYPE data structure in table 18.34 that
is NOT just a simple addition of new fields at the end of the existing
SET_ERROR_TYPE_WITH_ADDRESS data structure. E.g. the "flags" are now
in a 3-byte field at offset 5 instead of a 4-byte field at offset 8.
There is a new "length" field that descibes the total size of the
structure including the new flex array of syndrome values at the
end.

Shouldn't this look like this?

struct set_error_type_with_address_v2 {
	u32	type;
	u8	type_code;
	u8	flags[3];
	u32	length;
	u32	severity;
	u64	memory_address;
	u64	memory_address_range;
	u32	syndrome_count;
	struct syndrome_array syndrome syndromes[];
};

>  };
>  enum {
>  	SETWA_FLAGS_APICID = 1,
> -- 
> 2.43.0

-Tony

Re: [PATCH v7 5/9] ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities

[Luck, Tony]

On Tue, May 06, 2025 at 02:38:09PM -0700, Zaid Alali wrote:
> -static int error_type_set(void *data, u64 val)
> +static ssize_t error_type_set(struct file *file, const char __user *buf,
> +				size_t count, loff_t *ppos)
>  {
>  	int rc;
> +	u64 val;
> +
Add
	if (count > sizeof(einj_buf) - 1)
		return -EINVAL;

to make sure copy_from_user() can't corrupt memory after
the einj_buf[].

> +	memset(einj_buf, 0, sizeof(einj_buf));
> +	if (copy_from_user(einj_buf, buf, count))
> +		return -EFAULT;
> +
> +	if (strncmp(einj_buf, "V2_", 3) == 0) {
> +		if (!sscanf(einj_buf, "V2_%llx", &val))
> +			return -EINVAL;
> +	} else {
> +		if (!sscanf(einj_buf, "%llx", &val))
> +			return -EINVAL;
> +	}

-Tony

Re: [PATCH v7 6/9] ACPI: APEI: EINJ: Add einjv2 extension struct

[Luck, Tony]

On Wed, May 28, 2025 at 11:38:54AM -0700, Luck, Tony wrote:
> On Tue, May 06, 2025 at 02:38:10PM -0700, Zaid Alali wrote:
> >  struct set_error_type_with_address {
> >  	u32	type;
> >  	u32	vendor_extension;
> > @@ -58,6 +80,7 @@ struct set_error_type_with_address {
> >  	u64	memory_address;
> >  	u64	memory_address_range;
> >  	u32	pcie_sbdf;
> > +	struct	einjv2_extension_struct einjv2_struct;
> 
> I can't make this match up with the ACPI v6.5 spec.  The spec defines
> a whole new EINJV2_SET_ERROR_TYPE data structure in table 18.34 that
> is NOT just a simple addition of new fields at the end of the existing
> SET_ERROR_TYPE_WITH_ADDRESS data structure. E.g. the "flags" are now
> in a 3-byte field at offset 5 instead of a 4-byte field at offset 8.
> There is a new "length" field that descibes the total size of the
> structure including the new flex array of syndrome values at the
> end.

Someone pointed me to the ACPI 6.5 Errata A spec: https://uefi.org/specs/ACPI/6.5_A/

This code does match with the description there.

I'll continue looking at your patches with this as the reference.

Please make sure to reference this spec directly (not buried in links
to tianocore bugzilla entries) when you post next version.

-Tony

Re: [PATCH v7 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Luck, Tony]

On Tue, May 06, 2025 at 02:38:13PM -0700, Zaid Alali wrote:
> +  # echo 0x12345000 > param1			# Set memory address for injection
> +  # echo 0xfffffffffffff000 > param2		# Range - anywhere in this page
> +  # comp_arr="0x1 0x2				# Fill in the component array
> +    >0x1 0x4
> +    >0x2 0x4"
> +  # echo "$comp_arr" > einjv2_component_array

Seems complex (and may confuse people as the ">" in the lines setting
up the comp_arr are secondary prompts from bash, not part if the input).

If they miss the "" around $comp_arr in the last line they will
get all the values on one line which will be rejected with -EINVAL
during injection.

This works better (and is shorter too!):

# echo -e '0x1 0x2\n0x1 0x4\n0x2 0x4\n\0' > einjv2_component_array

I think explicitly terminating the input with '\0' is needed (and that
the kernel should NOT zero out the einjv2_component_array blob
on each injection.  That's unlike the other einj paramaters which
are "sticky". The user can repeat the same injection without resetting
all the parameters each time, just "echo 1 > error_inject" to do the
same thing again.

-Tony

Re: [PATCH v7 7/9] ACPI: APEI: EINJ: Add debugfs files for EINJv2 support

[Luck, Tony]

On Tue, May 06, 2025 at 02:38:11PM -0700, Zaid Alali wrote:
> Create a debugfs blob file to be used for reading the user
> input for the component array. EINJv2 enables users to inject
> errors to multiple components/devices at the same time using
> component array.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> ---
>  drivers/acpi/apei/einj-core.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index 60e4f3dc7055..44f9166c3881 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -33,6 +33,7 @@
>  #define SLEEP_UNIT_MAX		5000			/* 5ms */
>  /* Firmware should respond within 1 seconds */
>  #define FIRMWARE_TIMEOUT	(1 * USEC_PER_SEC)
> +#define COMP_ARR_SIZE		1024
>  #define ACPI65_EINJV2_SUPP	BIT(30)
>  #define ACPI5_VENDOR_BIT	BIT(31)
>  #define MEM_ERROR_MASK		(ACPI_EINJ_MEMORY_CORRECTABLE | \
> @@ -107,6 +108,8 @@ static struct debugfs_blob_wrapper vendor_blob;
>  static struct debugfs_blob_wrapper vendor_errors;
>  static char vendor_dev[64];
>  
> +static struct debugfs_blob_wrapper einjv2_component_arr;
> +static void *user_input;

Better name for this? "user_input" is too generic.

>  static u32 available_error_type;
>  static u32 available_error_type_v2;
>  
> @@ -689,6 +692,7 @@ static u64 error_param3;
>  static u64 error_param4;
>  static struct dentry *einj_debug_dir;
>  static char einj_buf[32];
> +static bool EINJv2_enabled;
>  static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
>  	{ BIT(0), "Processor Correctable" },
>  	{ BIT(1), "Processor Uncorrectable non-fatal" },
> @@ -718,7 +722,7 @@ static int available_error_type_show(struct seq_file *m, void *v)
>  		if (available_error_type & einj_error_type_string[pos].mask)
>  			seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
>  				   einj_error_type_string[pos].str);
> -	if (available_error_type & ACPI65_EINJV2_SUPP) {
> +	if ((available_error_type & ACPI65_EINJV2_SUPP) && EINJv2_enabled) {
>  		for (int pos = 0; pos < ARRAY_SIZE(einjv2_error_type_string); pos++) {
>  			if (available_error_type_v2 & einjv2_error_type_string[pos].mask)
>  				seq_printf(m, "V2_0x%08x\t%s\n", einjv2_error_type_string[pos].mask,
> @@ -901,8 +905,22 @@ static int __init einj_probe(struct platform_device *pdev)
>  				   &error_param4);
>  		debugfs_create_x32("notrigger", S_IRUSR | S_IWUSR,
>  				   einj_debug_dir, &notrigger);
> +		if (available_error_type & ACPI65_EINJV2_SUPP) {
> +			user_input = kzalloc(COMP_ARR_SIZE, GFP_KERNEL);
> +			if (!user_input) {
> +				EINJv2_enabled = false;
> +				pr_info("EINJv2 is disabled: not enough memory\n");
> +				goto skip_EINJv2;
> +			}
> +			EINJv2_enabled = true;
> +			einjv2_component_arr.data = user_input;
> +			einjv2_component_arr.size = COMP_ARR_SIZE;
> +			debugfs_create_blob("einjv2_component_array", S_IRUSR | S_IWUSR,
> +					einj_debug_dir, &einjv2_component_arr);
> +		}
>  	}
>  
> +skip_EINJv2:
>  	if (vendor_dev[0]) {
>  		vendor_blob.data = vendor_dev;
>  		vendor_blob.size = strlen(vendor_dev);
> @@ -944,6 +962,7 @@ static void __exit einj_remove(struct platform_device *pdev)
>  		if (vendor_errors.size)
>  			acpi_os_unmap_memory(vendor_errors.data, vendor_errors.size);
>  	}
> +	kfree(user_input);
>  	einj_exec_ctx_init(&ctx);
>  	apei_exec_post_unmap_gars(&ctx);
>  	apei_resources_release(&einj_resources);
> -- 
> 2.43.0
> 

Re: [PATCH v7 8/9] ACPI: APEI: EINJ: Enable EINJv2 error injections

[Luck, Tony]

On Tue, May 06, 2025 at 02:38:12PM -0700, Zaid Alali wrote:
> Enable the driver to inject EINJv2 type errors. The component
> array values are parsed from user_input and expected to contain
> hex values for component id and syndrome separated by space,
> and multiple components are separated by new line as follows:
> 
> component_id1 component_syndrome1
> component_id2 component_syndrome2
>  :
> component_id(n) component_syndrome(n)
> 
> for example:
> 
> $comp_arr="0x1 0x2
> >0x1 0x4
> >0x2 0x4"
> $cd /sys/kernel/debug/apei/einj/
> $echo "$comp_arr" > einjv2_component_array
> 
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> ---
>  drivers/acpi/apei/einj-core.c | 177 +++++++++++++++++++++++++++++++---
>  1 file changed, 166 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index 44f9166c3881..491d33529a3d 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -34,6 +34,7 @@
>  /* Firmware should respond within 1 seconds */
>  #define FIRMWARE_TIMEOUT	(1 * USEC_PER_SEC)
>  #define COMP_ARR_SIZE		1024
> +#define COMPONENT_LEN		16
>  #define ACPI65_EINJV2_SUPP	BIT(30)
>  #define ACPI5_VENDOR_BIT	BIT(31)
>  #define MEM_ERROR_MASK		(ACPI_EINJ_MEMORY_CORRECTABLE | \
> @@ -87,6 +88,14 @@ enum {
>  	SETWA_FLAGS_APICID = 1,
>  	SETWA_FLAGS_MEM = 2,
>  	SETWA_FLAGS_PCIE_SBDF = 4,
> +	SETWA_FLAGS_EINJV2 = 8,
> +};
> +
> +enum {
> +	EINJV2_PROCESSOR_ERROR = 0x1,
> +	EINJV2_MEMORY_ERROR = 0x2,
> +	EINJV2_PCIE_ERROR = 0x4,
> +	EINJV2_VENDOR_ERROR = 0x80000000,
>  };
>  
>  /*
> @@ -110,6 +119,7 @@ static char vendor_dev[64];
>  
>  static struct debugfs_blob_wrapper einjv2_component_arr;
>  static void *user_input;
> +static int nr_components;

Maybe call this "max_component_count"?

This code figures out the value by digging into the BIOS structure
pointed at by the ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS entry in
the "EINJ" table. But doesn't make this visible to the user. Only
way for them to find it out is try try injections with progressively
more components until they get the ubiquitous -EINVAL error.

Please add a read-only file in the einj debugfs directory that will
show this value.

>  static u32 available_error_type;
>  static u32 available_error_type_v2;
>  
> @@ -180,6 +190,8 @@ static DEFINE_MUTEX(einj_mutex);
>  bool einj_initialized __ro_after_init;
>  
>  static void __iomem *einj_param;
> +static u32 v5param_size;
> +static bool is_V2;
>  
>  static void einj_exec_ctx_init(struct apei_exec_context *ctx)
>  {
> @@ -304,11 +316,31 @@ static void __iomem *einj_get_parameter_address(void)
>  		struct set_error_type_with_address v5param;
>  		struct set_error_type_with_address __iomem *p;
>  
> +		v5param_size = sizeof(v5param);
>  		p = acpi_os_map_iomem(pa_v5, sizeof(*p));
>  		if (p) {
> -			memcpy_fromio(&v5param, p, sizeof(v5param));
> +			int offset, len;
> +
> +			memcpy_fromio(&v5param, p, v5param_size);
>  			acpi5 = 1;
>  			check_vendor_extension(pa_v5, &v5param);
> +			if (available_error_type & ACPI65_EINJV2_SUPP) {
> +				len = v5param.einjv2_struct.length;
> +				offset = offsetof(struct einjv2_extension_struct, component_arr);
> +				nr_components = (len - offset) /
> +						sizeof(v5param.einjv2_struct.component_arr[0]);
> +				/*
> +				 * The first call to acpi_os_map_iomem above does not include the
> +				 * component array, instead it is used to read and calculate maximum
> +				 * number of components supported by the system. Below, the mapping
> +				 * is expanded to include the component array.
> +				 */
> +				acpi_os_unmap_iomem(p, v5param_size);
> +				offset = offsetof(struct set_error_type_with_address, einjv2_struct);
> +				v5param_size = offset + struct_size(&v5param.einjv2_struct,
> +					component_arr, nr_components);
> +				p = acpi_os_map_iomem(pa_v5, v5param_size);
> +			}
>  			return p;
>  		}
>  	}
> @@ -486,6 +518,104 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
>  	return rc;
>  }
>  
> +static int parse_hex_to_u8(char *str, u8 *arr)
> +{
> +	char *ptr, val[32];
s/32/COMPONENT_LEN * 2/
> +	int pad, str_len;
> +
> +
> +	if (str[0] == '0' && (str[1] == 'x' || str[1] == 'X'))
> +		str += 2;
> +
> +	str_len = strlen(str);
> +	if (str_len > 32)
s/32/sizeof(val)/
> +		return -EINVAL;
> +
> +	memcpy(val, str, str_len);
Does not copy NUL string terminator. But see my next comments.
> +
> +	ptr = val;
> +	while (*ptr != '\0') {
But you scan "val" looking for the NUL!
> +		if (!isxdigit(*ptr))
> +			return -EINVAL;
> +		ptr++;
> +	}
Perhaps do this whole sanity check against "str" instead of "val"
and drop the memcpy() above?
> +
> +	pad = 32 - str_len;
> +
Comment here? /* prefix input with '0' characters */
> +	memmove(val + pad, val, str_len);
> +	memset(val, '0', pad);
> +
> +	for (int i = 0; i < COMPONENT_LEN; ++i) {
> +		char byte_str[3] = {val[i * 2], val[i * 2 + 1], '\0'};
> +		/* write bytes in little endian format to follow ACPI specs */
> +		arr[COMPONENT_LEN - i - 1] = (u8)strtoul(byte_str, NULL, 16);
> +	}
> +
> +	return 0;
> +}
Just to check the steps here.
1) User input 0x1234
2) val[] is set to "00000000000000000000000000001234"
3) arr[] is set to { 0x34, 0x12, 0x0 ... }
> +
> +static int read_component_array(struct set_error_type_with_address *v5param)
> +{
> +	int count = 0, str_len;
> +	u8 comp_arr[COMPONENT_LEN], synd_arr[COMPONENT_LEN];
> +	struct syndrome_array *component_arr;
> +	char *tok, *comp_str, *synd_str, *user;
> +
> +	component_arr = v5param->einjv2_struct.component_arr;
> +	str_len = strlen(user_input);
> +	user = user_input;
> +	user[str_len - 1] = '\0';
> +	while ((tok = strsep(&user, "\n")) != NULL) {
> +		if (count >= nr_components)
> +			return -EINVAL;
> +
> +		comp_str = strsep(&tok, " \t");
> +		synd_str = strsep(&tok, " \t");
> +
> +		if (!comp_str || !synd_str)
> +			return -EINVAL;
> +
> +		if (parse_hex_to_u8(comp_str, comp_arr))
> +			return -EINVAL;
> +		if (parse_hex_to_u8(synd_str, synd_arr))
> +			return -EINVAL;
> +
> +		switch (v5param->type) {
> +		case EINJV2_PROCESSOR_ERROR:
> +			for (int i = 0; i < COMPONENT_LEN; ++i) {
> +				component_arr[count].comp_id.acpi_id[i] = comp_arr[i];
> +				component_arr[count].comp_synd.proc_synd[i] = synd_arr[i];
> +			}
> +			break;
> +		case EINJV2_MEMORY_ERROR:
> +			for (int i = 0; i < COMPONENT_LEN; ++i) {
> +				component_arr[count].comp_id.device_id[i] = comp_arr[i];
> +				component_arr[count].comp_synd.mem_synd[i] = synd_arr[i];
> +			}
> +			break;
> +		case EINJV2_PCIE_ERROR:
> +			for (int i = 0; i < COMPONENT_LEN; ++i) {
> +				component_arr[count].comp_id.pcie_sbdf[i] = comp_arr[i];
> +				component_arr[count].comp_synd.pcie_synd[i] = synd_arr[i];
> +			}
> +			break;
> +		case EINJV2_VENDOR_ERROR:
> +			for (int i = 0; i < COMPONENT_LEN; ++i) {
> +				component_arr[count].comp_id.vendor_id[i] = comp_arr[i];
> +				component_arr[count].comp_synd.vendor_synd[i] = synd_arr[i];
> +			}
> +			break;

These four cases are all doing exactly the same thing since the targets
of the copies are overlapping "union" fields in the syndrome_array
structure. Could you just have:

struct syndrome_array {
	u8	component_id[16];
	u8	component_syndrome[16];
};

and then remove the switch((v5param->type) and just have:

	for (int i = 0; i < COMPONENT_LEN; ++i) {
		component_arr[count].component_id[i] = comp_arr[i];
		component_arr[count].component_syndrome[i] = synd_arr[i];
	}
		
> +		}
> +		count++;
> +
> +	}
> +	v5param->einjv2_struct.component_arr_count = count;
> +
> +	/* clear buffer after user input for next injection */
> +	memset(user_input, 0, COMP_ARR_SIZE);
No. User input should be sticky across injections.

> +	return 0;
> +}
> +
>  static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>  			       u64 param3, u64 param4)
>  {
> @@ -500,10 +630,10 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>  		return rc;
>  	apei_exec_ctx_set_input(&ctx, type);
>  	if (acpi5) {
> -		struct set_error_type_with_address *v5param, v5_struct;
> +		struct set_error_type_with_address *v5param;
>  
> -		v5param = &v5_struct;
> -		memcpy_fromio(v5param, einj_param, sizeof(*v5param));
> +		v5param = kmalloc(v5param_size, GFP_KERNEL);
> +		memcpy_fromio(v5param, einj_param, v5param_size);
>  		v5param->type = type;
>  		if (type & ACPI5_VENDOR_BIT) {
>  			switch (vendor_flags) {
> @@ -523,8 +653,19 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>  			v5param->flags = flags;
>  			v5param->memory_address = param1;
>  			v5param->memory_address_range = param2;
> -			v5param->apicid = param3;
> -			v5param->pcie_sbdf = param4;
> +
> +			if (is_V2) {
> +				rc = read_component_array(v5param);
> +				if (rc) {
> +					kfree(v5param);
> +					goto err_out;
> +				}
> +				/* clear buffer after user input for next injection */
> +				memset(user_input, 0, COMP_ARR_SIZE);
No. User input should be sticky across injections.

> +			} else {
> +				v5param->apicid = param3;
> +				v5param->pcie_sbdf = param4;
> +			}
>  		} else {
>  			switch (type) {
>  			case ACPI_EINJ_PROCESSOR_CORRECTABLE:
> @@ -548,7 +689,8 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>  				break;
>  			}
>  		}
> -		memcpy_toio(einj_param, v5param, sizeof(*v5param));
> +		memcpy_toio(einj_param, v5param, v5param_size);
> +		kfree(v5param);
>  	} else {
>  		rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
>  		if (rc)
> @@ -600,6 +742,9 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>  	rc = apei_exec_run_optional(&ctx, ACPI_EINJ_END_OPERATION);
>  
>  	return rc;
> +err_out:
> +	memset(user_input, 0, COMP_ARR_SIZE);
No. User input should be sticky across injections.

> +	return -EINVAL;
>  }
>  
>  /* Inject the specified hardware error */
> @@ -610,10 +755,15 @@ int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
>  	u64 base_addr, size;
>  
>  	/* If user manually set "flags", make sure it is legal */
> -	if (flags && (flags &
> -		~(SETWA_FLAGS_APICID|SETWA_FLAGS_MEM|SETWA_FLAGS_PCIE_SBDF)))
> +	if (flags && (flags & ~(SETWA_FLAGS_APICID | SETWA_FLAGS_MEM |
> +		      SETWA_FLAGS_PCIE_SBDF | SETWA_FLAGS_EINJV2)))
>  		return -EINVAL;
>  
> +	/* check if type is a valid EINJv2 error type */
> +	if (is_V2) {
> +		if (!(type & available_error_type_v2))
> +			return -EINVAL;
> +	}
>  	/*
>  	 * We need extra sanity checks for memory errors.
>  	 * Other types leap directly to injection.
> @@ -764,7 +914,7 @@ int einj_validate_error_type(u64 type)
>  	if (tval & (tval - 1))
>  		return -EINVAL;
>  	if (!vendor)
> -		if (!(type & available_error_type))
> +		if (!(type & (available_error_type | available_error_type_v2)))
>  			return -EINVAL;
>  
>  	return 0;
> @@ -783,9 +933,11 @@ static ssize_t error_type_set(struct file *file, const char __user *buf,
>  	if (strncmp(einj_buf, "V2_", 3) == 0) {
>  		if (!sscanf(einj_buf, "V2_%llx", &val))
>  			return -EINVAL;
> +		is_V2 = true;
>  	} else {
>  		if (!sscanf(einj_buf, "%llx", &val))
>  			return -EINVAL;
> +		is_V2 = false;
>  	}
>  
>  	rc = einj_validate_error_type(val);
> @@ -807,6 +959,9 @@ static int error_inject_set(void *data, u64 val)
>  	if (!error_type)
>  		return -EINVAL;
>  
> +	if (is_V2)
> +		error_flags |= SETWA_FLAGS_EINJV2;
> +
>  	return einj_error_inject(error_type, error_flags, error_param1, error_param2,
>  		error_param3, error_param4);
>  }
> @@ -955,7 +1110,7 @@ static void __exit einj_remove(struct platform_device *pdev)
>  
>  	if (einj_param) {
>  		acpi_size size = (acpi5) ?
> -			sizeof(struct set_error_type_with_address) :
> +			v5param_size :
>  			sizeof(struct einj_parameter);
>  
>  		acpi_os_unmap_iomem(einj_param, size);
> -- 
> 2.43.0
> 

Re: [PATCH v7 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Borislav Petkov]

On Tue, May 06, 2025 at 02:38:13PM -0700, Zaid Alali wrote:
> +- einjv2_component_array
> +
> +  The contents of this file are used to set the "Component Array" field
> +  of the EINJv2 Extension Structure. The expected format is hex values
> +  for component id and syndrome separated by space, and multiple
> +  components are separated by new line.

How is this a good design?

Do you guys not see that the other injection files are one value per file?

> +  # comp_arr="0x1 0x2				# Fill in the component array
> +    >0x1 0x4
> +    >0x2 0x4"
> +  # echo "$comp_arr" > einjv2_component_array

Oh boy. Srsly?!

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v7 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Luck, Tony]

On Fri, May 30, 2025 at 12:27:11PM +0200, Borislav Petkov wrote:
> On Tue, May 06, 2025 at 02:38:13PM -0700, Zaid Alali wrote:
> > +- einjv2_component_array
> > +
> > +  The contents of this file are used to set the "Component Array" field
> > +  of the EINJv2 Extension Structure. The expected format is hex values
> > +  for component id and syndrome separated by space, and multiple
> > +  components are separated by new line.
> 
> How is this a good design?
> 
> Do you guys not see that the other injection files are one value per file?
> 
> > +  # comp_arr="0x1 0x2				# Fill in the component array
> > +    >0x1 0x4
> > +    >0x2 0x4"
> > +  # echo "$comp_arr" > einjv2_component_array
> 
> Oh boy. Srsly?!

I've been staring at the debugfs blob used for einjv2_component_array
to try and come up with some sane way to use it ... but I think it is
a lost cause and I agree we need "one file, one value" like the rest of
the EINJ user interface.

I poked at the code a bit and mangled it into the patch below. I've
tested that the new files read/write as expected. But I don't have
an EINJV2 enabled system to run a full test.

New files in the einj directory (assuming the system reports that
it supports up to four simultaneous injections):

-rw-------. 1 root root 0 May 30 13:26 component_id0
-rw-------. 1 root root 0 May 30 13:26 component_id1
-rw-------. 1 root root 0 May 30 13:26 component_id2
-rw-------. 1 root root 0 May 30 13:26 component_id3
-rw-------. 1 root root 0 May 30 13:26 component_syndrome0
-rw-------. 1 root root 0 May 30 13:26 component_syndrome1
-rw-------. 1 root root 0 May 30 13:26 component_syndrome2
-rw-------. 1 root root 0 May 30 13:26 component_syndrome3
-r--------. 1 root root 0 May 30 13:26 max_nr_components
-rw-------. 1 root root 0 May 30 13:26 nr_components

Use case to inject to one device would be:

# echo 1 > nr_components
# echo 4 > component_id0
# echo A5A5A5A5 > component_syndrome0
... set other files and finish with usual
# echo 1 > error_inject

There isn't a fancy "debugfs_create_x128_le()" helper to manage these
128-bit little endian numbers. So I've coded with the basic building
blocks (though using copy_from_user() and copy_to_user() feels like
back in the stone age). If there some helpers that I missed I'd be
happy to see that part simplified.

Patch is on top of the existing v7 set. Obviously it needs to be folded
back into the earlier patches to make a clean history that doesn't add
functions and then replace them with different code.

-Tony

---

diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index ab3d20b51ff1..9f29fc97f6a6 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -65,7 +65,7 @@ struct syndrome_array {
 		u8	pcie_synd[16];
 		u8	vendor_synd[16];
 	} comp_synd;
-};
+} *syndrome_data;
 
 struct einjv2_extension_struct {
 	u32 length;
@@ -117,9 +117,7 @@ static struct debugfs_blob_wrapper vendor_blob;
 static struct debugfs_blob_wrapper vendor_errors;
 static char vendor_dev[64];
 
-static struct debugfs_blob_wrapper einjv2_component_arr;
-static void *user_input;
-static int nr_components;
+static u32 max_nr_components, nr_components;
 static u32 available_error_type;
 static u32 available_error_type_v2;
 
@@ -327,7 +325,7 @@ static void __iomem *einj_get_parameter_address(void)
 			if (available_error_type & ACPI65_EINJV2_SUPP) {
 				len = v5param.einjv2_struct.length;
 				offset = offsetof(struct einjv2_extension_struct, component_arr);
-				nr_components = (len - offset) /
+				max_nr_components = (len - offset) /
 						sizeof(v5param.einjv2_struct.component_arr[0]);
 				/*
 				 * The first call to acpi_os_map_iomem above does not include the
@@ -338,7 +336,7 @@ static void __iomem *einj_get_parameter_address(void)
 				acpi_os_unmap_iomem(p, v5param_size);
 				offset = offsetof(struct set_error_type_with_address, einjv2_struct);
 				v5param_size = offset + struct_size(&v5param.einjv2_struct,
-					component_arr, nr_components);
+					component_arr, max_nr_components);
 				p = acpi_os_map_iomem(pa_v5, v5param_size);
 			}
 			return p;
@@ -518,104 +516,6 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
 	return rc;
 }
 
-static int parse_hex_to_u8(char *str, u8 *arr)
-{
-	char *ptr, val[32];
-	int pad, str_len;
-
-
-	if (str[0] == '0' && (str[1] == 'x' || str[1] == 'X'))
-		str += 2;
-
-	str_len = strlen(str);
-	if (str_len > 32)
-		return -EINVAL;
-
-	memcpy(val, str, str_len);
-
-	ptr = val;
-	while (*ptr != '\0') {
-		if (!isxdigit(*ptr))
-			return -EINVAL;
-		ptr++;
-	}
-
-	pad = 32 - str_len;
-
-	memmove(val + pad, val, str_len);
-	memset(val, '0', pad);
-
-	for (int i = 0; i < COMPONENT_LEN; ++i) {
-		char byte_str[3] = {val[i * 2], val[i * 2 + 1], '\0'};
-		/* write bytes in little endian format to follow ACPI specs */
-		arr[COMPONENT_LEN - i - 1] = (u8)strtoul(byte_str, NULL, 16);
-	}
-
-	return 0;
-}
-
-static int read_component_array(struct set_error_type_with_address *v5param)
-{
-	int count = 0, str_len;
-	u8 comp_arr[COMPONENT_LEN], synd_arr[COMPONENT_LEN];
-	struct syndrome_array *component_arr;
-	char *tok, *comp_str, *synd_str, *user;
-
-	component_arr = v5param->einjv2_struct.component_arr;
-	str_len = strlen(user_input);
-	user = user_input;
-	user[str_len - 1] = '\0';
-	while ((tok = strsep(&user, "\n")) != NULL) {
-		if (count >= nr_components)
-			return -EINVAL;
-
-		comp_str = strsep(&tok, " \t");
-		synd_str = strsep(&tok, " \t");
-
-		if (!comp_str || !synd_str)
-			return -EINVAL;
-
-		if (parse_hex_to_u8(comp_str, comp_arr))
-			return -EINVAL;
-		if (parse_hex_to_u8(synd_str, synd_arr))
-			return -EINVAL;
-
-		switch (v5param->type) {
-		case EINJV2_PROCESSOR_ERROR:
-			for (int i = 0; i < COMPONENT_LEN; ++i) {
-				component_arr[count].comp_id.acpi_id[i] = comp_arr[i];
-				component_arr[count].comp_synd.proc_synd[i] = synd_arr[i];
-			}
-			break;
-		case EINJV2_MEMORY_ERROR:
-			for (int i = 0; i < COMPONENT_LEN; ++i) {
-				component_arr[count].comp_id.device_id[i] = comp_arr[i];
-				component_arr[count].comp_synd.mem_synd[i] = synd_arr[i];
-			}
-			break;
-		case EINJV2_PCIE_ERROR:
-			for (int i = 0; i < COMPONENT_LEN; ++i) {
-				component_arr[count].comp_id.pcie_sbdf[i] = comp_arr[i];
-				component_arr[count].comp_synd.pcie_synd[i] = synd_arr[i];
-			}
-			break;
-		case EINJV2_VENDOR_ERROR:
-			for (int i = 0; i < COMPONENT_LEN; ++i) {
-				component_arr[count].comp_id.vendor_id[i] = comp_arr[i];
-				component_arr[count].comp_synd.vendor_synd[i] = synd_arr[i];
-			}
-			break;
-		}
-		count++;
-
-	}
-	v5param->einjv2_struct.component_arr_count = count;
-
-	/* clear buffer after user input for next injection */
-	memset(user_input, 0, COMP_ARR_SIZE);
-	return 0;
-}
-
 static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 			       u64 param3, u64 param4)
 {
@@ -655,13 +555,17 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 			v5param->memory_address_range = param2;
 
 			if (is_V2) {
-				rc = read_component_array(v5param);
-				if (rc) {
+				if (nr_components < 1 || nr_components > max_nr_components) {
 					kfree(v5param);
-					goto err_out;
+					return -EINVAL;
+				}
+				v5param->einjv2_struct.component_arr_count = nr_components;
+				for (int i = 0; i < nr_components; i++) {
+					v5param->einjv2_struct.component_arr[i].comp_id =
+						syndrome_data[i].comp_id;
+					v5param->einjv2_struct.component_arr[i].comp_synd =
+						syndrome_data[i].comp_synd;
 				}
-				/* clear buffer after user input for next injection */
-				memset(user_input, 0, COMP_ARR_SIZE);
 			} else {
 				v5param->apicid = param3;
 				v5param->pcie_sbdf = param4;
@@ -742,9 +646,6 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 	rc = apei_exec_run_optional(&ctx, ACPI_EINJ_END_OPERATION);
 
 	return rc;
-err_out:
-	memset(user_input, 0, COMP_ARR_SIZE);
-	return -EINVAL;
 }
 
 /* Inject the specified hardware error */
@@ -984,6 +885,97 @@ static int einj_check_table(struct acpi_table_einj *einj_tab)
 	return 0;
 }
 
+static ssize_t u128_read(struct file *f, char __user *buf, size_t count, loff_t *off)
+{
+	char output[2 * COMPONENT_LEN + 1];
+	u8 *data = f->f_inode->i_private;
+	ssize_t n;
+	int i;
+
+	if (*off >= sizeof(output))
+		return 0;
+
+	for (i = 0; i < COMPONENT_LEN; i++)
+		sprintf(output + 2 * i, "%.02x", data[i]);
+	output[2 * COMPONENT_LEN] = '\n';
+
+	n = min(count, sizeof(output) - *off);
+	if (copy_to_user(buf, output + *off, n))
+		return -EFAULT;
+	*off += n;
+
+	return n;
+}
+
+static ssize_t u128_write(struct file *f, const char __user *buf, size_t count, loff_t *off)
+{
+	char input[2 + 2 * COMPONENT_LEN + 2];
+	u8 *save = f->f_inode->i_private;
+	u8 tmp[COMPONENT_LEN];
+	char byte[3] = {};
+	char *s, *e;
+	long val;
+	int i;
+
+	if (count > sizeof(input))
+		return -EINVAL;
+	if (copy_from_user(input, buf, count))
+		return -EFAULT;
+
+	if (input[0] == '0' && (input[1] == 'x' || input[1] == 'X'))
+		s = input + 2;
+	else
+		s = input;
+	e = input + count;
+	if (e[-1] == '\n')
+		e--;
+
+	for (i = 0; i < COMPONENT_LEN; i++) {
+		byte[1] = *--e;
+		byte[0] = e > s ? *--e : '0';
+		if (kstrtol(byte, 16, &val))
+			return -EINVAL;
+		tmp[i] = val;
+		if (e <= s)
+			break;
+	}
+	while (++i < COMPONENT_LEN)
+		tmp[i] = 0;
+
+	memcpy(save, tmp, COMPONENT_LEN);
+
+	return count;
+}
+
+static const struct file_operations u128_fops = {
+	.read	= u128_read,
+	.write	= u128_write,
+};
+
+static bool setup_einjv2_component_files(void)
+{
+	char name[32];
+
+	max_nr_components = 4; //FAKE!!!
+	debugfs_create_u32("max_nr_components", 0400, einj_debug_dir, &max_nr_components);
+	debugfs_create_u32("nr_components", 0600, einj_debug_dir, &nr_components);
+
+	syndrome_data = kcalloc(max_nr_components, sizeof(syndrome_data[0]), GFP_KERNEL);
+	if (!syndrome_data)
+		return false;
+
+	for (int i = 0; i < max_nr_components; i++) {
+		sprintf(name, "component_id%d", i);
+		debugfs_create_file(name, 0600, einj_debug_dir,
+				    &syndrome_data[i].comp_id, &u128_fops);
+		sprintf(name, "component_syndrome%d", i);
+		debugfs_create_file(name, 0600, einj_debug_dir,
+				    &syndrome_data[i].comp_synd, &u128_fops);
+	}
+
+	return true;
+}
+
 static int __init einj_probe(struct faux_device *fdev)
 {
 	int rc;
@@ -1056,21 +1048,10 @@ static int __init einj_probe(struct faux_device *fdev)
 		debugfs_create_x32("notrigger", S_IRUSR | S_IWUSR,
 				   einj_debug_dir, &notrigger);
 		if (available_error_type & ACPI65_EINJV2_SUPP) {
-			user_input = kzalloc(COMP_ARR_SIZE, GFP_KERNEL);
-			if (!user_input) {
-				EINJv2_enabled = false;
-				pr_info("EINJv2 is disabled: not enough memory\n");
-				goto skip_EINJv2;
-			}
-			EINJv2_enabled = true;
-			einjv2_component_arr.data = user_input;
-			einjv2_component_arr.size = COMP_ARR_SIZE;
-			debugfs_create_blob("einjv2_component_array", S_IRUSR | S_IWUSR,
-					einj_debug_dir, &einjv2_component_arr);
+			EINJv2_enabled = setup_einjv2_component_files();
 		}
 	}
 
-skip_EINJv2:
 	if (vendor_dev[0]) {
 		vendor_blob.data = vendor_dev;
 		vendor_blob.size = strlen(vendor_dev);
@@ -1112,7 +1093,7 @@ static void __exit einj_remove(struct faux_device *fdev)
 		if (vendor_errors.size)
 			acpi_os_unmap_memory(vendor_errors.data, vendor_errors.size);
 	}
-	kfree(user_input);
+	kfree(syndrome_data);
 	einj_exec_ctx_init(&ctx);
 	apei_exec_post_unmap_gars(&ctx);
 	apei_resources_release(&einj_resources);
-- 
2.49.0

Re: [PATCH v7 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Luck, Tony]

On Fri, May 30, 2025 at 01:52:41PM -0700, Luck, Tony wrote:
> On Fri, May 30, 2025 at 12:27:11PM +0200, Borislav Petkov wrote:
> > On Tue, May 06, 2025 at 02:38:13PM -0700, Zaid Alali wrote:
> There isn't a fancy "debugfs_create_x128_le()" helper to manage these
> 128-bit little endian numbers. So I've coded with the basic building
> blocks (though using copy_from_user() and copy_to_user() feels like
> back in the stone age). If there some helpers that I missed I'd be
> happy to see that part simplified.

simple_read_from_buffer() and simple_write_to_buffer() may be the
helpers that I didn't spot earlier.

-Tony

Re: [PATCH v7 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Borislav Petkov]

On Fri, May 30, 2025 at 01:52:39PM -0700, Luck, Tony wrote:
> Use case to inject to one device would be:
> 
> # echo 1 > nr_components

What does that do?

The interface probably needs a little refinement, but...

> # echo 4 > component_id0
> # echo A5A5A5A5 > component_syndrome0
> ... set other files and finish with usual
> # echo 1 > error_inject
> 
> There isn't a fancy "debugfs_create_x128_le()" helper to manage these
> 128-bit little endian numbers. So I've coded with the basic building
> blocks (though using copy_from_user() and copy_to_user() feels like
> back in the stone age). If there some helpers that I missed I'd be
> happy to see that part simplified.
> 
> Patch is on top of the existing v7 set. Obviously it needs to be folded
> back into the earlier patches to make a clean history that doesn't add
> functions and then replace them with different code.

... yes, definitely much better.

Thanks!

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v7 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Luck, Tony]

On Sat, May 31, 2025 at 11:20:50AM +0200, Borislav Petkov wrote:
> On Fri, May 30, 2025 at 01:52:39PM -0700, Luck, Tony wrote:
> > Use case to inject to one device would be:
> > 
> > # echo 1 > nr_components
> 
> What does that do?

EINJ V2 allows the user to perform multiple injections together.

The component_idN/component_syndromeN pairs of files direct the
"where" and the "what" of each injection.

But the kernel needs to know how many of these pairs to use
for an injection (to fill in a field in the structure passed
to the BIOS).

With EINJ V2 the user might want to inject to 2 locations with
one injection, and then just to 1 location on the next.

Zaid Alali's version took the approach of zeroing the input
after each injection so the user had to start from scratch
for each injection.

I wasn't fond of that because the existing Linux EINJ interface
saves all the paramters allowing the user to repeat the same
injection by just runniing "echo 1 > error_inject: over and over
(e.g. to force a soft offline by injecting multiple corrected
errors to the same address).

User interface options:

1) User can zero out the component_idN/component_syndromeN pairs
that they don't need and have the kernel count how many injections
are requested by looping to find the zero terminator.

2) Kernel could zero all pairs after an injection to make the user
explicitly set the list of targets each time.

3) User provides the count vis the nr_components file (perhaps
needs a better name?)

4) Something else?

> 
> The interface probably needs a little refinement, but...
> 
> > # echo 4 > component_id0
> > # echo A5A5A5A5 > component_syndrome0
> > ... set other files and finish with usual
> > # echo 1 > error_inject
> > 
> > There isn't a fancy "debugfs_create_x128_le()" helper to manage these
> > 128-bit little endian numbers. So I've coded with the basic building
> > blocks (though using copy_from_user() and copy_to_user() feels like
> > back in the stone age). If there some helpers that I missed I'd be
> > happy to see that part simplified.
> > 
> > Patch is on top of the existing v7 set. Obviously it needs to be folded
> > back into the earlier patches to make a clean history that doesn't add
> > functions and then replace them with different code.
> 
> ... yes, definitely much better.
> 
> Thanks!

You are welcome.
> 
> -- 
> Regards/Gruss,
>     Boris.

-Tony

Re: [PATCH v7 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Borislav Petkov]

Some questions inline...

On Sat, May 31, 2025 at 03:24:14PM -0700, Luck, Tony wrote:
> EINJ V2 allows the user to perform multiple injections together.
> 
> The component_idN/component_syndromeN pairs of files direct the
> "where" and the "what" of each injection.
> 
> But the kernel needs to know how many of these pairs to use
> for an injection (to fill in a field in the structure passed
> to the BIOS).

The kernel could realloc on each write. Or we could allocate the struct to max
elems and trim it before passing it down to BIOS.

> With EINJ V2 the user might want to inject to 2 locations with
> one injection, and then just to 1 location on the next.

Right.

> Zaid Alali's version took the approach of zeroing the input
> after each injection so the user had to start from scratch
> for each injection.
> 
> I wasn't fond of that because the existing Linux EINJ interface
> saves all the paramters allowing the user to repeat the same
> injection by just runniing "echo 1 > error_inject: over and over
> (e.g. to force a soft offline by injecting multiple corrected
> errors to the same address).

I agree with you here. Linux sysfs, etc interfaces do keep their values
usually.

> User interface options:
> 
> 1) User can zero out the component_idN/component_syndromeN pairs
> that they don't need and have the kernel count how many injections
> are requested by looping to find the zero terminator.
> 
> 2) Kernel could zero all pairs after an injection to make the user
> explicitly set the list of targets each time.
> 
> 3) User provides the count vis the nr_components file (perhaps
> needs a better name?)

Yap, agree that the name is not optimal.

> 4) Something else?

See above.

User can inject into each component pairs file and the kernel can put that in
the tracking struct. So you have:

# echo 4 > component_id0
# echo A5A5A5A5 > component_syndrome0
... set other files and finish with usual
# echo 1 > error_inject

<--- here, it goes through each component pair and builds the structure to
pass down the BIOS.

And you track valid component pairs by setting the IDs to -1 or something else
invalid.

All those component IDs which have remained invalid after the error_inject
write happens, get ignored - you gather only those which are valid and inject.

And this way you can keep the old values too and gather them again and inject
again, over and over again.

Right?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v7 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Luck, Tony]

On Sun, Jun 01, 2025 at 12:25:54PM +0200, Borislav Petkov wrote:
> Some questions inline...
> 
> On Sat, May 31, 2025 at 03:24:14PM -0700, Luck, Tony wrote:
> > EINJ V2 allows the user to perform multiple injections together.
> > 
> > The component_idN/component_syndromeN pairs of files direct the
> > "where" and the "what" of each injection.
> > 
> > But the kernel needs to know how many of these pairs to use
> > for an injection (to fill in a field in the structure passed
> > to the BIOS).
> 
> The kernel could realloc on each write. Or we could allocate the struct to max
> elems and trim it before passing it down to BIOS.

The actual structure passed to BIOS is the same each time. Just the
set_error_type_with_address::einjv2_struct::component_arr_count
changed to indicate how many errors to inject.  In theory the
driver could allocate and copy a correctly sized structure, but
Zaid's code here is simpler, an this is hardly a critical path.

> > User interface options:
> > 
> > 1) User can zero out the component_idN/component_syndromeN pairs
> > that they don't need and have the kernel count how many injections
> > are requested by looping to find the zero terminator.
> > 
> > 2) Kernel could zero all pairs after an injection to make the user
> > explicitly set the list of targets each time.
> > 
> > 3) User provides the count vis the nr_components file (perhaps
> > needs a better name?)
> 
> Yap, agree that the name is not optimal.

It can be dropped if we make the user zap previously supplied
component_idN/component_syndromeN pairs that are no longer
wanted.
> 
> User can inject into each component pairs file and the kernel can put that in
> the tracking struct. So you have:
> 
> # echo 4 > component_id0
> # echo A5A5A5A5 > component_syndrome0
> ... set other files and finish with usual
> # echo 1 > error_inject
> 
> <--- here, it goes through each component pair and builds the structure to
> pass down the BIOS.
> 
> And you track valid component pairs by setting the IDs to -1 or something else
> invalid.

This is just an improvement on my "option 1" (improved because all-ones
for the component ID is going to be invalid for sure, while all zeroes
could be a valid component).
> 
> All those component IDs which have remained invalid after the error_inject
> write happens, get ignored - you gather only those which are valid and inject.

Or just stop collecting on the first invalid one.

> And this way you can keep the old values too and gather them again and inject
> again, over and over again.
> 
> Right?

Yup.

-Tony

Re: [PATCH v7 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Zaid Alali]

On Mon, Jun 02, 2025 at 10:02:15AM -0700, Luck, Tony wrote:
> On Sun, Jun 01, 2025 at 12:25:54PM +0200, Borislav Petkov wrote:
> > Some questions inline...
> > 
> > On Sat, May 31, 2025 at 03:24:14PM -0700, Luck, Tony wrote:
> > > EINJ V2 allows the user to perform multiple injections together.
> > > 
> > > The component_idN/component_syndromeN pairs of files direct the
> > > "where" and the "what" of each injection.
> > > 
> > > But the kernel needs to know how many of these pairs to use
> > > for an injection (to fill in a field in the structure passed
> > > to the BIOS).
> > 
> > The kernel could realloc on each write. Or we could allocate the struct to max
> > elems and trim it before passing it down to BIOS.
> 
> The actual structure passed to BIOS is the same each time. Just the
> set_error_type_with_address::einjv2_struct::component_arr_count
> changed to indicate how many errors to inject.  In theory the
> driver could allocate and copy a correctly sized structure, but
> Zaid's code here is simpler, an this is hardly a critical path.
> 
> > > User interface options:
> > > 
> > > 1) User can zero out the component_idN/component_syndromeN pairs
> > > that they don't need and have the kernel count how many injections
> > > are requested by looping to find the zero terminator.
> > > 
> > > 2) Kernel could zero all pairs after an injection to make the user
> > > explicitly set the list of targets each time.
> > > 
> > > 3) User provides the count vis the nr_components file (perhaps
> > > needs a better name?)
> > 
> > Yap, agree that the name is not optimal.
> 
> It can be dropped if we make the user zap previously supplied
> component_idN/component_syndromeN pairs that are no longer
> wanted.
> > 
> > User can inject into each component pairs file and the kernel can put that in
> > the tracking struct. So you have:
> > 
> > # echo 4 > component_id0
> > # echo A5A5A5A5 > component_syndrome0
> > ... set other files and finish with usual
> > # echo 1 > error_inject
> > 
> > <--- here, it goes through each component pair and builds the structure to
> > pass down the BIOS.
> > 
> > And you track valid component pairs by setting the IDs to -1 or something else
> > invalid.
> 
> This is just an improvement on my "option 1" (improved because all-ones
> for the component ID is going to be invalid for sure, while all zeroes
> could be a valid component).
> > 
> > All those component IDs which have remained invalid after the error_inject
> > write happens, get ignored - you gather only those which are valid and inject.
> 
> Or just stop collecting on the first invalid one.
> 
> > And this way you can keep the old values too and gather them again and inject
> > again, over and over again.
> > 
> > Right?
> 
> Yup.
> 
> -Tony

Thank you Tony and Borislav, this is great feedback. I will update the patches
and send out a new revision that uses Tony's UI for the ID and syndrome.

-Zaid

Re: [PATCH v7 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Borislav Petkov]

On Mon, Jun 02, 2025 at 10:02:15AM -0700, Luck, Tony wrote:
> The actual structure passed to BIOS is the same each time. Just the
> set_error_type_with_address::einjv2_struct::component_arr_count
> changed to indicate how many errors to inject.  In theory the
> driver could allocate and copy a correctly sized structure, but
> Zaid's code here is simpler, an this is hardly a critical path.

Right, allocate it once on driver init and keep massaging it on every
injection. Simple.

> This is just an improvement on my "option 1" (improved because all-ones
> for the component ID is going to be invalid for sure, while all zeroes
> could be a valid component).

Right, you need to know at injection time which of the components are valid
and which are not.

> Or just stop collecting on the first invalid one.

That would mean that you punish the user at the first typo. :-P

Considering how complex those interfaces become perhaps not such a good
idea...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v7 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Luck, Tony]

On Tue, Jun 03, 2025 at 10:31:57AM +0200, Borislav Petkov wrote:
> On Mon, Jun 02, 2025 at 10:02:15AM -0700, Luck, Tony wrote:
> > This is just an improvement on my "option 1" (improved because all-ones
> > for the component ID is going to be invalid for sure, while all zeroes
> > could be a valid component).
> 
> Right, you need to know at injection time which of the components are valid
> and which are not.
> 
> > Or just stop collecting on the first invalid one.
> 
> That would mean that you punish the user at the first typo. :-P
> 
> Considering how complex those interfaces become perhaps not such a good
> idea...

Here's what EINJ V2 injection looks like to a user (to be included in
Documentation/firmware-guide/acpi/apei/einj.rst):

An EINJv2 error injection example::

  # cd /sys/kernel/debug/apei/einj
  # cat available_error_type                    # See which errors can be injected
  0x00000002    Processor Uncorrectable non-fatal
  0x00000008    Memory Correctable
  0x00000010    Memory Uncorrectable non-fatal
  V2_0x00000001 EINJV2 Processor Error
  V2_0x00000002 EINJV2 Memory Error

  # echo 0x12345000 > param1                    # Set memory address for injection
  # echo 0xfffffffffffff000 > param2            # Range - anywhere in this page
  # echo 0x1 > component_id0                    # First device ID
  # echo 0x4 > component_syndrome0              # First error syndrome
  # echo 0x2 > component_id1                    # Second device ID
  # echo 0x4 > component_syndrome1              # Second error syndrome
  # echo '' > component_id2                     # Mark id2 invalid to terminate list
  # echo V2_0x2 > error_type                    # Choose EINJv2 memory error
  # echo 0xa > flags                            # set flags to indicate EINJv2
  # echo 1 > error_inject                       # Inject now

Note the shorthand to write all-ones using an empty line because
typing:

  # echo fffffffffffffffffffffffffffffff > component_id2

would:
1) give carpal tunnel syndrome
2) wear out the "f" key on my keyboard
3) run the risk of miscounting the number of 'f' characters
   (above is wrong because there are only 31, but hard to spot that).

-Tony