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

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

[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 <gong.chen@linux.intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---

Fix typo in Documentation and add examples (per Chen Gong) comment.
Break long line in two (checkpatch appeasement)
Provide stub "readq" definition so build on 32-bit will work [UNTESTED!!!]

 Documentation/acpi/apei/einj.txt |   55 ++++++++--
 drivers/acpi/apei/einj.c         |  225 ++++++++++++++++++++++++++++++++------
 include/acpi/actbl1.h            |    3 +-
 3 files changed, 235 insertions(+), 48 deletions(-)

diff --git a/Documentation/acpi/apei/einj.txt b/Documentation/acpi/apei/einj.txt
index 5cc699b..7558b3b 100644
--- a/Documentation/acpi/apei/einj.txt
+++ b/Documentation/acpi/apei/einj.txt
@@ -47,20 +47,53 @@ 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.
+
+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 0x40, 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).
+
+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
+# echo 0x12345000 > param1		# Set memory address for injection
+# echo 0xfffffffffffff000 > param2	# Mask - anywhere in this page
+# echo 0x8 > error_type			# Choose correctable memory error 
+# echo 1 > error_inject			# Inject now
 
-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.
 
 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.
diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
index 589b96c..aacbf5e 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -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,14 @@ 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 readq
+static inline __u64 readq(volatile void __iomem *addr)
+{
+	return ((__u64)readl(addr+4) << 32) + readl(addr);
+}
+#endif
 
 #ifndef writeq
 static inline void writeq(__u64 val, volatile void __iomem *addr)
@@ -158,10 +201,32 @@ 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 +235,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 +386,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 +545,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 +602,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 +655,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");
diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 7504bc9..f25d7ef 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -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 */
 };
 
-- 
1.7.3.1

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

[Jiang Liu]

Hi Tony and Gong,
	Should we remove the target memory address from the resource list of 
ACPI5 trigger too? Otherwise we may also encounter resource conflict with ACPI5
trigger table.

@@ -365,7 +365,7 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
         * This will cause resource conflict with regular memory.  So
         * remove it from trigger table resources.
         */
-       if (param_extension && (type & 0x0038) && param2) {
+       if ((param_extension || acpi5) && (type & 0x0038) && param2) {
                struct apei_resources addr_resources;
                apei_resources_init(&addr_resources);
                trigger_param_region = einj_get_trigger_parameter_region(
	
On 01/18/2012 04:10 AM, Tony Luck wrote:
> @@ -293,12 +386,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;
> +			}
> +		}
I'm a little confused about the ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS command in ACPI5.
The above code seems like a hard-coded version of 
apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS).
Hi Gong, could you please share the ACPI5 version of EINJ table with me? So I could
get better understanding of the ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS command.

> +	} 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 +545,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))
The ACPI5 spec doesn't specify the format of vendor specific error type, so would it
be better to check above condition only if it's a standard error type?

>  		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;

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

[Chen Gong]

于 2012/2/12 17:33, Jiang Liu 写道:
> Hi Tony and Gong,
> 	Should we remove the target memory address from the resource list of
> ACPI5 trigger too? Otherwise we may also encounter resource conflict with ACPI5
> trigger table.
>
> @@ -365,7 +365,7 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
>           * This will cause resource conflict with regular memory.  So
>           * remove it from trigger table resources.
>           */
> -       if (param_extension&&  (type&  0x0038)&&  param2) {
> +       if ((param_extension || acpi5)&&  (type&  0x0038)&&  param2) {
>                  struct apei_resources addr_resources;
>                  apei_resources_init(&addr_resources);
>                  trigger_param_region = einj_get_trigger_parameter_region(
> 	
> On 01/18/2012 04:10 AM, Tony Luck wrote:
>> @@ -293,12 +386,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;
>> +			}
>> +		}
> I'm a little confused about the ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS command in ACPI5.
> The above code seems like a hard-coded version of
> apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS).
> Hi Gong, could you please share the ACPI5 version of EINJ table with me? So I could
> get better understanding of the ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS command.
>

You can check ACPI5.0 spec 18.6 for detail. We have internal BIOS to support
ACPI5 and I'm not sure if there is external product to support ACPI5 formally.

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

[Luck, Tony]

>	Should we remove the target memory address from the resource list of 
> ACPI5 trigger too? Otherwise we may also encounter resource conflict with ACPI5
> trigger table.

I haven't seen any problems while testing this - but perhaps the trigger sequence
generated by this BIOS in ACPI 5 mode doesn't access the target address (though
that would be strange).

-Tony

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

[Luck, Tony]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 666 bytes --]

> I'm a little confused about the ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS command in ACPI5.
> The above code seems like a hard-coded version of
> apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS).

If I mashed this code into the wrong place ... then help me understand how it
should fit in stylistically with the existing code.

Basic summary is that ACPI 5 added the _WITH_ADDRESS functionality. If it is present,
then there seems no point in using the old SET_ERROR_TYPE action from ACPI4.

-Tony
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

[Luck, Tony]

> 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

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

[Len Brown]

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]

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

[Chen Gong]

于 2012/1/5 6:29, 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<gong.chen@linux.intel.com>
> Signed-off-by: Tony Luck<tony.luck@intel.com>
> ---
>
> [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(-)
>
> diff --git a/Documentation/acpi/apei/einj.txt b/Documentation/acpi/apei/einj.txt
> index 5cc699b..95a2b22 100644
> --- a/Documentation/acpi/apei/einj.txt
> +++ b/Documentation/acpi/apei/einj.txt
> @@ -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
                                          ^^^^^^^^^^^^^^^^^^^^^^^^^
                                     should be 0x40, 0x80 and 0x100

> +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.

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.

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

[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 <gong.chen@linux.intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---

[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(-)

diff --git a/Documentation/acpi/apei/einj.txt b/Documentation/acpi/apei/einj.txt
index 5cc699b..95a2b22 100644
--- a/Documentation/acpi/apei/einj.txt
+++ b/Documentation/acpi/apei/einj.txt
@@ -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.
diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
index 589b96c..1bc1f309 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -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");
diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 7504bc9..f25d7ef 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -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 */
 };
 
-- 
1.7.3.1