Skip to contentSkip to conversation

acpi-apei-einj: add extensions to EINJ from rev 5.0 of acpi spec

PATCH In review Luck, Tony 3 participants 7 messages Nov 29, 2011 – Jan 17, 2012

Subsystems (4)
Documentation ACPI ACPI component architecture (acpica) ACPI APEI
lore
Share & tools
lore

Luck, Tony

author reviewer
#0 lore raw

PATCH 1/1 acpi-apei-einj: add extensions to EINJ from rev 5.0 of acpi spec

ACPI 5.0 provides extensions to the EINJ mechanism to specify the target for the error injection - by APICID for cpu related errors, by address for memory related errors, and by segment/bus/device/function for PCIe related errors. Also extensions for vendor specific error injections.

Signed-off-by Tony Luck <[email protected]>
---
 drivers/acpi/apei/einj.c |  217 ++++++++++++++++++++++++++++++++++++++--------
 include/acpi/actbl1.h    |    3 +-
 2 files changed, 183 insertions(+), 37 deletions(-)
drivers/acpi/apei/einj.c +181 -36
index 589b96c..b65d80d 100644
@@ -43,6 +43,42 @@
 #define FIRMWARE_TIMEOUT	(1 * NSEC_PER_MSEC)
 
 /*
+ * ACPI version 5 provides a SET_ERROR_TYPE_WITH_ADDRESS action.
+ */
+static int acpi5;
+
+struct set_error_type_with_address {
+	u32	type;
+	u32	vendor_extension;
+	u32	flags;
+	u32	apicid;
+	u64	memory_address;
+	u64	memory_address_range;
+	u32	pcie_sbdf;
+};
+enum {
+	SETWA_FLAGS_APICID = 1,
+	SETWA_FLAGS_MEM = 2,
+	SETWA_FLAGS_PCIE_SBDF = 4,
+};
+
+/*
+ * Vendor extensions for platform specific operations
+ */
+struct vendor_error_type_extension {
+	u32	length;
+	u32	pcie_sbdf;
+	u16	vendor_id;
+	u16	device_id;
+	u8	rev_id;
+	u8	reserved[3];
+};
+
+static u32 vendor_flags;
+static struct debugfs_blob_wrapper vendor_blob;
+static char vendor_dev[64];
+
+/*
  * Some BIOSes allow parameters to the SET_ERROR_TYPE entries in the
  * EINJ table through an unpublished extension. Use with caution as
  * most will ignore the parameter and make their own choice of address

@@ -103,7 +139,7 @@ static struct apei_exec_ins_type einj_ins_type[] = {
  */
 static DEFINE_MUTEX(einj_mutex);
 
-static struct einj_parameter *einj_param;
+static void *einj_param;
 
 #ifndef writeq
 static inline void writeq(__u64 val, volatile void __iomem *addr)

@@ -158,10 +194,31 @@ static int einj_timedout(u64 *t)
 	return 0;
 }
 
-static u64 einj_get_parameter_address(void)
+static void check_vendor_extension(u64 paddr,
+				   struct set_error_type_with_address *v5param)
+{
+	int	offset = readl(&v5param->vendor_extension);
+	struct	vendor_error_type_extension *v;
+	u32	sbdf;
+
+	if (!offset)
+		return;
+	v = ioremap(paddr + offset, sizeof(*v));
+	if (!v)
+		return;
+	sbdf = readl(&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,
+		 readw(&v->vendor_id), readw(&v->device_id),
+		readb(&v->rev_id));
+	iounmap(v);
+}
+
+static void *einj_get_parameter_address(void)
 {
 	int i;
-	u64 paddr = 0;
+	u64 paddrv4 = 0, paddrv5 = 0;
+	void *param;
 	struct acpi_whea_header *entry;
 
 	entry = EINJ_TAB_ENTRY(einj_tab);

@@ -170,12 +227,40 @@ static u64 einj_get_parameter_address(void)
 		    entry->instruction == ACPI_EINJ_WRITE_REGISTER &&
 		    entry->register_region.space_id ==
 		    ACPI_ADR_SPACE_SYSTEM_MEMORY)
-			memcpy(&paddr, &entry->register_region.address,
-			       sizeof(paddr));
+			memcpy(&paddrv4, &entry->register_region.address,
+			       sizeof(paddrv4));
+		if (entry->action == ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS &&
+		    entry->instruction == ACPI_EINJ_WRITE_REGISTER &&
+		    entry->register_region.space_id ==
+		    ACPI_ADR_SPACE_SYSTEM_MEMORY)
+			memcpy(&paddrv5, &entry->register_region.address,
+			       sizeof(paddrv5));
 		entry++;
 	}
+	if (paddrv5) {
+		struct set_error_type_with_address *v5param;
+
+		v5param = ioremap(paddrv5, sizeof(*v5param));
+		if (v5param) {
+			acpi5 = 1;
+			check_vendor_extension(paddrv5, v5param);
+			return v5param;
+		}
+	}
+	if (paddrv4) {
+		struct einj_parameter *v4param;
+
+		v4param = ioremap(paddrv4, sizeof(*v4param));
+		if (!v4param)
+			return 0;
+		if (readq(&v4param->reserved1) || readq(&v4param->reserved2)) {
+			iounmap(param);
+			return 0;
+		}
+		return v4param;
+	}
 
-	return paddr;
+	return 0;
 }
 
 /* do sanity check to trigger table */

@@ -293,12 +378,56 @@ static int __einj_error_inject(u32 type, u64 param1, u64 param2)
 	if (rc)
 		return rc;
 	apei_exec_ctx_set_input(&ctx, type);
-	rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
-	if (rc)
-		return rc;
-	if (einj_param) {
-		writeq(param1, &einj_param->param1);
-		writeq(param2, &einj_param->param2);
+	if (acpi5) {
+		struct set_error_type_with_address *v5param = einj_param;
+
+		writel(type, &v5param->type);
+		if (type & 0x80000000) {
+			switch (vendor_flags) {
+			case SETWA_FLAGS_APICID:
+				writel(param1, &v5param->apicid);
+				break;
+			case SETWA_FLAGS_MEM:
+				writeq(param1, &v5param->memory_address);
+				writeq(param2, &v5param->memory_address_range);
+				break;
+			case SETWA_FLAGS_PCIE_SBDF:
+				writel(param1, &v5param->pcie_sbdf);
+				break;
+			}
+			writel(vendor_flags, &v5param->flags);
+		} else {
+			switch (type) {
+			case ACPI_EINJ_PROCESSOR_CORRECTABLE:
+			case ACPI_EINJ_PROCESSOR_UNCORRECTABLE:
+			case ACPI_EINJ_PROCESSOR_FATAL:
+				writel(param1, &v5param->apicid);
+				writel(SETWA_FLAGS_APICID, &v5param->flags);
+				break;
+			case ACPI_EINJ_MEMORY_CORRECTABLE:
+			case ACPI_EINJ_MEMORY_UNCORRECTABLE:
+			case ACPI_EINJ_MEMORY_FATAL:
+				writeq(param1, &v5param->memory_address);
+				writeq(param2, &v5param->memory_address_range);
+				writel(SETWA_FLAGS_MEM, &v5param->flags);
+				break;
+			case ACPI_EINJ_PCIX_CORRECTABLE:
+			case ACPI_EINJ_PCIX_UNCORRECTABLE:
+			case ACPI_EINJ_PCIX_FATAL:
+				writel(param1, &v5param->pcie_sbdf);
+				writel(SETWA_FLAGS_PCIE_SBDF, &v5param->flags);
+				break;
+			}
+		}
+	} else {
+		rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
+		if (rc)
+			return rc;
+		if (einj_param) {
+			struct einj_parameter *v4param = einj_param;
+			writeq(param1, &v4param->param1);
+			writeq(param2, &v4param->param2);
+		}
 	}
 	rc = apei_exec_run(&ctx, ACPI_EINJ_EXECUTE_OPERATION);
 	if (rc)

@@ -408,15 +537,25 @@ static int error_type_set(void *data, u64 val)
 {
 	int rc;
 	u32 available_error_type = 0;
+	u32 tval, vendor;
+
+	/*
+	 * Vendor defined types have 0x80000000 bit set, and
+	 * are not enumerated by ACPI_EINJ_GET_ERROR_TYPE
+	 */
+	vendor = val & 0x80000000;
+	tval = val & 0x7fffffff;
 
 	/* Only one error type can be specified */
-	if (val & (val - 1))
-		return -EINVAL;
-	rc = einj_get_available_error_type(&available_error_type);
-	if (rc)
-		return rc;
-	if (!(val & available_error_type))
+	if (tval & (tval - 1))
 		return -EINVAL;
+	if (!vendor) {
+		rc = einj_get_available_error_type(&available_error_type);
+		if (rc)
+			return rc;
+		if (!(val & available_error_type))
+			return -EINVAL;
+	}
 	error_type = val;
 
 	return 0;

@@ -455,7 +594,6 @@ static int einj_check_table(struct acpi_table_einj *einj_tab)
 static int __init einj_init(void)
 {
 	int rc;
-	u64 param_paddr;
 	acpi_status status;
 	struct dentry *fentry;
 	struct apei_exec_context ctx;

@@ -509,23 +647,30 @@ static int __init einj_init(void)
 	rc = apei_exec_pre_map_gars(&ctx);
 	if (rc)
 		goto err_release;
-	if (param_extension) {
-		param_paddr = einj_get_parameter_address();
-		if (param_paddr) {
-			einj_param = ioremap(param_paddr, sizeof(*einj_param));
-			rc = -ENOMEM;
-			if (!einj_param)
-				goto err_unmap;
-			fentry = debugfs_create_x64("param1", S_IRUSR | S_IWUSR,
-						    einj_debug_dir, &error_param1);
-			if (!fentry)
-				goto err_unmap;
-			fentry = debugfs_create_x64("param2", S_IRUSR | S_IWUSR,
-						    einj_debug_dir, &error_param2);
-			if (!fentry)
-				goto err_unmap;
-		} else
-			pr_warn(EINJ_PFX "Parameter extension is not supported.\n");
+
+	einj_param = einj_get_parameter_address();
+	if ((param_extension || acpi5) && einj_param) {
+		fentry = debugfs_create_x64("param1", S_IRUSR | S_IWUSR,
+					    einj_debug_dir, &error_param1);
+		if (!fentry)
+			goto err_unmap;
+		fentry = debugfs_create_x64("param2", S_IRUSR | S_IWUSR,
+					    einj_debug_dir, &error_param2);
+		if (!fentry)
+			goto err_unmap;
+	}
+
+	if (vendor_dev[0]) {
+		vendor_blob.data = vendor_dev;
+		vendor_blob.size = strlen(vendor_dev);
+		fentry = debugfs_create_blob("vendor", S_IRUSR,
+					     einj_debug_dir, &vendor_blob);
+		if (!fentry)
+			goto err_unmap;
+		fentry = debugfs_create_x32("vendor_flags", S_IRUSR | S_IWUSR,
+					    einj_debug_dir, &vendor_flags);
+		if (!fentry)
+			goto err_unmap;
 	}
 
 	pr_info(EINJ_PFX "Error INJection is initialized.\n");
include/acpi/actbl1.h +2 -1
index 7504bc9..f25d7ef 100644
@@ -228,7 +228,8 @@ enum acpi_einj_actions {
 	ACPI_EINJ_EXECUTE_OPERATION = 5,
 	ACPI_EINJ_CHECK_BUSY_STATUS = 6,
 	ACPI_EINJ_GET_COMMAND_STATUS = 7,
-	ACPI_EINJ_ACTION_RESERVED = 8,	/* 8 and greater are reserved */
+	ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS = 8,
+	ACPI_EINJ_ACTION_RESERVED = 9,	/* 9 and greater are reserved */
 	ACPI_EINJ_TRIGGER_ERROR = 0xFF	/* Except for this value */
 };
Signature · 1 line
1.7.3.1
#1 lore raw
Quoted 164 lines · Tony

ACPI 5.0 provides extensions to the EINJ mechanism to specify the target for the error injection - by APICID for cpu related errors, by address for memory related errors, and by segment/bus/device/function for PCIe related errors. Also extensions for vendor specific error injections.

Signed-off-by Tony Luck<[email protected]>
---
  drivers/acpi/apei/einj.c |  217 ++++++++++++++++++++++++++++++++++++++--------
  include/acpi/actbl1.h    |    3 +-
  2 files changed, 183 insertions(+), 37 deletions(-)
drivers/acpi/apei/einj.c +99 -12
index 589b96c..b65d80d 100644
@@ -43,6 +43,42 @@
  #define FIRMWARE_TIMEOUT	(1 * NSEC_PER_MSEC)
 
  /*
+ * ACPI version 5 provides a SET_ERROR_TYPE_WITH_ADDRESS action.
+ */
+static int acpi5;
+
+struct set_error_type_with_address {
+	u32	type;
+	u32	vendor_extension;
+	u32	flags;
+	u32	apicid;
+	u64	memory_address;
+	u64	memory_address_range;
+	u32	pcie_sbdf;
+};
+enum {
+	SETWA_FLAGS_APICID = 1,
+	SETWA_FLAGS_MEM = 2,
+	SETWA_FLAGS_PCIE_SBDF = 4,
+};
+
+/*
+ * Vendor extensions for platform specific operations
+ */
+struct vendor_error_type_extension {
+	u32	length;
+	u32	pcie_sbdf;
+	u16	vendor_id;
+	u16	device_id;
+	u8	rev_id;
+	u8	reserved[3];
+};
+
+static u32 vendor_flags;
+static struct debugfs_blob_wrapper vendor_blob;
+static char vendor_dev[64];
+
+/*
   * Some BIOSes allow parameters to the SET_ERROR_TYPE entries in the
   * EINJ table through an unpublished extension. Use with caution as
   * most will ignore the parameter and make their own choice of address

@@ -103,7 +139,7 @@ static struct apei_exec_ins_type einj_ins_type[] = {
   */
  static DEFINE_MUTEX(einj_mutex);
 
-static struct einj_parameter *einj_param;
+static void *einj_param;
 
  #ifndef writeq
  static inline void writeq(__u64 val, volatile void __iomem *addr)

@@ -158,10 +194,31 @@ static int einj_timedout(u64 *t)
  	return 0;
  }
 
-static u64 einj_get_parameter_address(void)
+static void check_vendor_extension(u64 paddr,
+				   struct set_error_type_with_address *v5param)
+{
+	int	offset = readl(&v5param->vendor_extension);
+	struct	vendor_error_type_extension *v;
+	u32	sbdf;
+
+	if (!offset)
+		return;
+	v = ioremap(paddr + offset, sizeof(*v));
+	if (!v)
+		return;
+	sbdf = readl(&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,
+		 readw(&v->vendor_id), readw(&v->device_id),
+		readb(&v->rev_id));
+	iounmap(v);
+}
+
+static void *einj_get_parameter_address(void)
  {
  	int i;
-	u64 paddr = 0;
+	u64 paddrv4 = 0, paddrv5 = 0;
+	void *param;
  	struct acpi_whea_header *entry;
 
  	entry = EINJ_TAB_ENTRY(einj_tab);

@@ -170,12 +227,40 @@ static u64 einj_get_parameter_address(void)
  		    entry->instruction == ACPI_EINJ_WRITE_REGISTER&&
  		entry->register_region.space_id ==
  		    ACPI_ADR_SPACE_SYSTEM_MEMORY)
-			memcpy(&paddr,&entry->register_region.address,
-			       sizeof(paddr));
+			memcpy(&paddrv4,&entry->register_region.address,
+			       sizeof(paddrv4));
+		if (entry->action == ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS&&
+		    entry->instruction == ACPI_EINJ_WRITE_REGISTER&&
+		    entry->register_region.space_id ==
+		    ACPI_ADR_SPACE_SYSTEM_MEMORY)
+			memcpy(&paddrv5,&entry->register_region.address,
+			       sizeof(paddrv5));
  		entry++;
  	}
+	if (paddrv5) {
+		struct set_error_type_with_address *v5param;
+
+		v5param = ioremap(paddrv5, sizeof(*v5param));
+		if (v5param) {
+			acpi5 = 1;
+			check_vendor_extension(paddrv5, v5param);
+			return v5param;
+		}
+	}
+	if (paddrv4) {
+		struct einj_parameter *v4param;
+
+		v4param = ioremap(paddrv4, sizeof(*v4param));
+		if (!v4param)
+			return 0;
+		if (readq(&v4param->reserved1) || readq(&v4param->reserved2)) {
+			iounmap(param);
+			return 0;
+		}
+		return v4param;
+	}
 
-	return paddr;
+	return 0;
  }
 
  /* do sanity check to trigger table */

@@ -293,12 +378,56 @@ static int __einj_error_inject(u32 type, u64 param1, u64 param2)
  	if (rc)
  		return rc;
  	apei_exec_ctx_set_input(&ctx, type);
-	rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
-	if (rc)
-		return rc;
-	if (einj_param) {
-		writeq(param1,&einj_param->param1);
-		writeq(param2,&einj_param->param2);
+	if (acpi5) {
+		struct set_error_type_with_address *v5param = einj_param;
+
+		writel(type,&v5param->type);
+		if (type&  0x80000000) {
+			switch (vendor_flags) {
+			case SETWA_FLAGS_APICID:
+				writel(param1,&v5param->apicid);

drivers/acpi/apei/einj.c static int __einj_error_inject(u32 type, u64 param1, u64 param2) · line 398

Quoted 10 lines
drivers/acpi/apei/einj.c +10 -0
+				break;
+			case SETWA_FLAGS_MEM:
+				writeq(param1,&v5param->memory_address);
+				writeq(param2,&v5param->memory_address_range);
+				break;
+			case SETWA_FLAGS_PCIE_SBDF:
+				writel(param1,&v5param->pcie_sbdf);
+				break;
+			}
+			writel(vendor_flags,&v5param->flags);

I don't think one knows how to set *vendor_flags* correctly if no any instructions.

Quoted 36 lines
drivers/acpi/apei/einj.c +32 -0
+		} else {
+			switch (type) {
+			case ACPI_EINJ_PROCESSOR_CORRECTABLE:
+			case ACPI_EINJ_PROCESSOR_UNCORRECTABLE:
+			case ACPI_EINJ_PROCESSOR_FATAL:
+				writel(param1,&v5param->apicid);
+				writel(SETWA_FLAGS_APICID,&v5param->flags);
+				break;
+			case ACPI_EINJ_MEMORY_CORRECTABLE:
+			case ACPI_EINJ_MEMORY_UNCORRECTABLE:
+			case ACPI_EINJ_MEMORY_FATAL:
+				writeq(param1,&v5param->memory_address);
+				writeq(param2,&v5param->memory_address_range);
+				writel(SETWA_FLAGS_MEM,&v5param->flags);
+				break;
+			case ACPI_EINJ_PCIX_CORRECTABLE:
+			case ACPI_EINJ_PCIX_UNCORRECTABLE:
+			case ACPI_EINJ_PCIX_FATAL:
+				writel(param1,&v5param->pcie_sbdf);
+				writel(SETWA_FLAGS_PCIE_SBDF,&v5param->flags);
+				break;
+			}
+		}
+	} else {
+		rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
+		if (rc)
+			return rc;
+		if (einj_param) {
+			struct einj_parameter *v4param = einj_param;
+			writeq(param1,&v4param->param1);
+			writeq(param2,&v4param->param2);
+		}
  	}
  	rc = apei_exec_run(&ctx, ACPI_EINJ_EXECUTE_OPERATION);
  	if (rc)

drivers/acpi/apei/einj.c static int error_type_set(void *data, u64 val) · line 546

Quoted 10 lines
drivers/acpi/apei/einj.c +7 -0
@@ -408,15 +537,25 @@ static int error_type_set(void *data, u64 val)
  {
  	int rc;
  	u32 available_error_type = 0;
+	u32 tval, vendor;
+
+	/*
+	 * Vendor defined types have 0x80000000 bit set, and
+	 * are not enumerated by ACPI_EINJ_GET_ERROR_TYPE
+	 */
+	vendor = val&  0x80000000;

need explanation in the document how to set error type in ACPI 5.0.

In the spec it doesn't say this bit field is filled by the user, from the codes I have following conclusion: one can set error type as 0x2, 0x4, 0x8 etc. or 0x80000002, 0x80000004, 0x80000008 etc, right?

drivers/acpi/apei/einj.c static int error_type_set(void *data, u64 val) · line 552

Quoted 12 lines
drivers/acpi/apei/einj.c +3 -6
+	tval = val&  0x7fffffff;
 
  	/* Only one error type can be specified */
-	if (val&  (val - 1))
-		return -EINVAL;
-	rc = einj_get_available_error_type(&available_error_type);
-	if (rc)
-		return rc;
-	if (!(val&  available_error_type))
+	if (tval&  (tval - 1))
  		return -EINVAL;
+	if (!vendor) {

Why checking input parameter *val* only when *vendor* is assigned? I think any time input parameter should be checked.

Quoted 70 lines
drivers/acpi/apei/einj.c +30 -18
+		rc = einj_get_available_error_type(&available_error_type);
+		if (rc)
+			return rc;
+		if (!(val&  available_error_type))
+			return -EINVAL;
+	}
  	error_type = val;
 
  	return 0;

@@ -455,7 +594,6 @@ static int einj_check_table(struct acpi_table_einj *einj_tab)
  static int __init einj_init(void)
  {
  	int rc;
-	u64 param_paddr;
  	acpi_status status;
  	struct dentry *fentry;
  	struct apei_exec_context ctx;

@@ -509,23 +647,30 @@ static int __init einj_init(void)
  	rc = apei_exec_pre_map_gars(&ctx);
  	if (rc)
  		goto err_release;
-	if (param_extension) {
-		param_paddr = einj_get_parameter_address();
-		if (param_paddr) {
-			einj_param = ioremap(param_paddr, sizeof(*einj_param));
-			rc = -ENOMEM;
-			if (!einj_param)
-				goto err_unmap;
-			fentry = debugfs_create_x64("param1", S_IRUSR | S_IWUSR,
-						    einj_debug_dir,&error_param1);
-			if (!fentry)
-				goto err_unmap;
-			fentry = debugfs_create_x64("param2", S_IRUSR | S_IWUSR,
-						    einj_debug_dir,&error_param2);
-			if (!fentry)
-				goto err_unmap;
-		} else
-			pr_warn(EINJ_PFX "Parameter extension is not supported.\n");
+
+	einj_param = einj_get_parameter_address();
+	if ((param_extension || acpi5)&&  einj_param) {
+		fentry = debugfs_create_x64("param1", S_IRUSR | S_IWUSR,
+					    einj_debug_dir,&error_param1);
+		if (!fentry)
+			goto err_unmap;
+		fentry = debugfs_create_x64("param2", S_IRUSR | S_IWUSR,
+					    einj_debug_dir,&error_param2);
+		if (!fentry)
+			goto err_unmap;
+	}
+
+	if (vendor_dev[0]) {
+		vendor_blob.data = vendor_dev;
+		vendor_blob.size = strlen(vendor_dev);
+		fentry = debugfs_create_blob("vendor", S_IRUSR,
+					     einj_debug_dir,&vendor_blob);
+		if (!fentry)
+			goto err_unmap;
+		fentry = debugfs_create_x32("vendor_flags", S_IRUSR | S_IWUSR,
+					    einj_debug_dir,&vendor_flags);
+		if (!fentry)
+			goto err_unmap;
  	}
 
  	pr_info(EINJ_PFX "Error INJection is initialized.\n");
include/acpi/actbl1.h +0 -0
index 7504bc9..f25d7ef 100644

include/acpi/actbl1.h enum acpi_einj_actions { · line 235

Quoted 9 lines
include/acpi/actbl1.h +2 -1
@@ -228,7 +228,8 @@ enum acpi_einj_actions {
  	ACPI_EINJ_EXECUTE_OPERATION = 5,
  	ACPI_EINJ_CHECK_BUSY_STATUS = 6,
  	ACPI_EINJ_GET_COMMAND_STATUS = 7,
-	ACPI_EINJ_ACTION_RESERVED = 8,	/* 8 and greater are reserved */
+	ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS = 8,
+	ACPI_EINJ_ACTION_RESERVED = 9,	/* 9 and greater are reserved */
  	ACPI_EINJ_TRIGGER_ERROR = 0xFF	/* Except for this value */
  };

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author reviewer
#2 lore raw
Quoted 2 lines · Chen Gong

I don't think one knows how to set *vendor_flags* correctly if no any instructions.

Agreed - more documentation would help here - but I suspect that it needs to come
from a vendor implementing an extension for specifics ... but for general how to use this - see more comments below.

Quoted 5 lines

need explanation in the document how to set error type in ACPI 5.0.

In the spec it doesn't say this bit field is filled by the user, from the codes I have following conclusion: one can set error type as 0x2, 0x4, 0x8 etc. or 0x80000002, 0x80000004, 0x80000008 etc, right?

The spec is pretty vague here - I'm assuming that as soon as the 0x80000000 bit is seen, the rest of the bits are fair game for the vendor to do anything they
like. So I provided a way to set the error_type because I don't think there is any association between bits and error type. E.g. one vendor could use 0x80000001 for some special memory operation - so expect a memory address and mask, while another vendor could use the same 0x80000001 for a PCIe operation, so want to see a segment/bus/device/function. So I tried to be flexible. I'd expect that an application trying to use this would look at the contents of the /sys/kernel/debug/apei/einj/vendor to know that they were on the right platform, with the device/rev_id to use whatever vendor extension they want. But this needs more (some :-) ) Documentation

Of course there is still some vendor defined memory at the very end of the structure that I have absolutely no idea what to do with. I've ignored it for now,
but if some vendor does do something with it, we will need another vendor_blob interface to allow arbitrary bytes to be written to (and read from?) that area (just
checking the size).

The whole vendor extension part is very speculative on my part as I don't have any solid examples to test. I do have a platform that provides the structure. But
I don't know whether it actually implements any extended operations.

-Tony

author reviewer
#3 lore raw

PATCH 1/1 acpi/apei/einj: Add extensions to EINJ from rev 5.0 of acpi spec

In reply to Luck, Tony

ACPI 5.0 provides extensions to the EINJ mechanism to specify the target for the error injection - by APICID for cpu related errors, by address for memory related errors, and by segment/bus/device/function for PCIe related errors. Also extensions for vendor specific error injections.

Tested-by Chen Gong <[email protected]>
Signed-off-by Tony Luck <[email protected]>

---

[Code is identical to version posted on November 29th, 2011 - but added changes
to einj.txt in response to all the questions from Chen Gong]

 Documentation/acpi/apei/einj.txt |   43 ++++++--
 drivers/acpi/apei/einj.c         |  217 +++++++++++++++++++++++++++++++------
 include/acpi/actbl1.h            |    3 +-
 3 files changed, 215 insertions(+), 48 deletions(-)
Documentation/acpi/apei/einj.txt +32 -11
index 5cc699b..95a2b22 100644
@@ -47,20 +47,41 @@ directory apei/einj. The following files are provided.
 
 - param1
   This file is used to set the first error parameter value. Effect of
-  parameter depends on error_type specified. For memory error, this is
-  physical memory address.  Only available if param_extension module
-  parameter is specified.
+  parameter depends on error_type specified.
 
 - param2
   This file is used to set the second error parameter value. Effect of
-  parameter depends on error_type specified. For memory error, this is
-  physical memory address mask.  Only available if param_extension
-  module parameter is specified.
+  parameter depends on error_type specified.
 
-Injecting parameter support is a BIOS version specific extension, that
-is, it only works on some BIOS version.  If you want to use it, please
-make sure your BIOS version has the proper support and specify
-"param_extension=y" in module parameter.
+BIOS versions based in the ACPI 4.0 specification have limited options
+to control where the errors are injected.  Your BIOS may support an
+extension (enabled with the param_extension=1 module parameter, or
+boot command line einj.param_extension=1). This allows the address
+and mask for memory injections to be specified by the param1 and
+param2 files in apei/einj.
+
+BIOS versions using the ACPI 5.0 specification have more control over
+the target of the injection. For processor related errors (type 0x1,
+0x2 and 0x4) the APICID of the target should be provided using the
+param1 file in apei/einj. For memory errors (type 0x8, 0x10 and 0x20)
+the address is set using param1 with a mask in param2 (0x0 is equivalent
+to all ones). For PCI express errors (type 0x80, 0x80 and 0x100) the
+segment, bus, device and function are specified using param1:
+
+         31     24 23    16 15    11 10      8  7        0
+	+-------------------------------------------------+
+	| segment |   bus  | device | function | reserved |
+	+-------------------------------------------------+
+
+An ACPI 5.0 BIOS may also allow vendor specific errors to be injected.
+In this case a file named vendor will contain identifying information
+from the BIOS that hopefully will allow an application wishing to use
+the vendor specific extension to tell that they are running on a BIOS
+that supports it. All vendor extensions have the 0x80000000 bit set in
+error_type. A file vendor_flags controls the interpretation of param1
+and param2 (1 = PROCESSOR, 2 = MEMORY, 4 = PCI). See your BIOS vendor
+documentation for details (and expect changes to this API if vendors
+creativity in using this feature expands beyond our expectations).
 
 For more information about EINJ, please refer to ACPI specification
-version 4.0, section 17.5.
+version 4.0, section 17.5 and ACPI 5.0, section 18.6.
drivers/acpi/apei/einj.c +181 -36

273 lines not on this page · raw message

include/acpi/actbl1.h +2 -1
index 7504bc9..f25d7ef 100644
@@ -228,7 +228,8 @@ enum acpi_einj_actions {
 	ACPI_EINJ_EXECUTE_OPERATION = 5,
 	ACPI_EINJ_CHECK_BUSY_STATUS = 6,
 	ACPI_EINJ_GET_COMMAND_STATUS = 7,
-	ACPI_EINJ_ACTION_RESERVED = 8,	/* 8 and greater are reserved */
+	ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS = 8,
+	ACPI_EINJ_ACTION_RESERVED = 9,	/* 9 and greater are reserved */
 	ACPI_EINJ_TRIGGER_ERROR = 0xFF	/* Except for this value */
 };
Signature · 1 line
1.7.3.1
#4 lore raw

Re: [PATCH] acpi/apei/einj: Add extensions to EINJ from rev 5.0 of acpi spec

Quoted 45 lines · Tony Luck

ACPI 5.0 provides extensions to the EINJ mechanism to specify the target for the error injection - by APICID for cpu related errors, by address for memory related errors, and by segment/bus/device/function for PCIe related errors. Also extensions for vendor specific error injections.

Tested-by Chen Gong<[email protected]>
Signed-off-by Tony Luck<[email protected]>

---

[Code is identical to version posted on November 29th, 2011 - but added changes
to einj.txt in response to all the questions from Chen Gong]

  Documentation/acpi/apei/einj.txt |   43 ++++++--
  drivers/acpi/apei/einj.c         |  217 +++++++++++++++++++++++++++++++------
  include/acpi/actbl1.h            |    3 +-
  3 files changed, 215 insertions(+), 48 deletions(-)
Documentation/acpi/apei/einj.txt +5 -10
index 5cc699b..95a2b22 100644
@@ -47,20 +47,41 @@ directory apei/einj. The following files are provided.
 
  - param1
    This file is used to set the first error parameter value. Effect of
-  parameter depends on error_type specified. For memory error, this is
-  physical memory address.  Only available if param_extension module
-  parameter is specified.
+  parameter depends on error_type specified.
 
  - param2
    This file is used to set the second error parameter value. Effect of
-  parameter depends on error_type specified. For memory error, this is
-  physical memory address mask.  Only available if param_extension
-  module parameter is specified.
+  parameter depends on error_type specified.
 
-Injecting parameter support is a BIOS version specific extension, that
-is, it only works on some BIOS version.  If you want to use it, please
-make sure your BIOS version has the proper support and specify
-"param_extension=y" in module parameter.
+BIOS versions based in the ACPI 4.0 specification have limited options
+to control where the errors are injected.  Your BIOS may support an
+extension (enabled with the param_extension=1 module parameter, or

Documentation/acpi/apei/einj.txt directory apei/einj. The following files are provided. · line 68

Quoted 10 lines
Documentation/acpi/apei/einj.txt +10 -0
+boot command line einj.param_extension=1). This allows the address
+and mask for memory injections to be specified by the param1 and
+param2 files in apei/einj.
+
+BIOS versions using the ACPI 5.0 specification have more control over
+the target of the injection. For processor related errors (type 0x1,
+0x2 and 0x4) the APICID of the target should be provided using the
+param1 file in apei/einj. For memory errors (type 0x8, 0x10 and 0x20)
+the address is set using param1 with a mask in param2 (0x0 is equivalent
+to all ones). For PCI express errors (type 0x80, 0x80 and 0x100) the

                                         ^^^^^^^^^^^^^^^^^^^^^^^^^
                                    should be 0x40, 0x80 and 0x100
Quoted 10 lines
Documentation/acpi/apei/einj.txt +10 -0
+segment, bus, device and function are specified using param1:
+
+         31     24 23    16 15    11 10      8  7        0
+	+-------------------------------------------------+
+	| segment |   bus  | device | function | reserved |
+	+-------------------------------------------------+
+
+An ACPI 5.0 BIOS may also allow vendor specific errors to be injected.
+In this case a file named vendor will contain identifying information
+from the BIOS that hopefully will allow an application wishing to use

Documentation/acpi/apei/einj.txt directory apei/einj. The following files are provided. · line 87

Quoted 10 lines
Documentation/acpi/apei/einj.txt +7 -1
+the vendor specific extension to tell that they are running on a BIOS
+that supports it. All vendor extensions have the 0x80000000 bit set in
+error_type. A file vendor_flags controls the interpretation of param1
+and param2 (1 = PROCESSOR, 2 = MEMORY, 4 = PCI). See your BIOS vendor
+documentation for details (and expect changes to this API if vendors
+creativity in using this feature expands beyond our expectations).
 
  For more information about EINJ, please refer to ACPI specification
-version 4.0, section 17.5.
+version 4.0, section 17.5 and ACPI 5.0, section 18.6.

In my own opinion, it is too hard for the one to use all kinds of combinations to control test conditions. If still keeping current parameters, I suggest adding some examples to clarify them.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

reviewer
#5 lore raw

Re: [PATCH] acpi/apei/einj: Add extensions to EINJ from rev 5.0 of acpi spec

In reply to Tony Luck

this patch doesn't build in 32-bit mode:

drivers/acpi/apei/einj.c:256:3: error: implicit declaration of function ‘readq’ [-Werror=implicit-function-declaration]

author reviewer
#6 lore raw

RE: [PATCH] acpi/apei/einj: Add extensions to EINJ from rev 5.0 of acpi spec

Quoted 1 line

drivers/acpi/apei/einj.c:256:3: error: implicit declaration of function 'readq' [-Werror=implicit-function-declaration]

Ah - einj.c has a special version of writeq ... I guess it needs a "readq" too.

I'll respin the patch (needs to fix a typo and add some Examples to the Documentation, plus fix a line too long for checkpatch).

-Tony