Parameter check for EINJ error injection - V3
Subsystems (3)
[PATCH 1/2 V3] ACPI/APEI: Add parameter check before error injection [PATCH 2/2 V3] ACPI/APEI: Update einj documentation for param1/param2
These two patches are employed to enhance the check for param1/param2 when einj is used. Param1/param2 are very subtle for the guy who lacks of experience. These two patches add some extra checks to avoid happening further errors to confuse users.
V3: only update the 1st patch based on Tony and Boris' suggestion.
V2: only update the 1st patch based on Boris' suggestion.
PATCH v3 1/2 ACPI/APEI: Add parameter check before error injection
When param1 is enabled in EINJ but not assigned with a valid value, sometimes it will cause the error like below:
APEI: Can not request [mem 0x7aaa7000-0x7aaa7007] for APEI EINJ Trigger registers
It is because some firmware will access target address specified in param1 to trigger the error when injecting memory error. This will cause resource conflict with regular memory. So It must be removed from trigger table resources, but incorrect param1/param2 combination will stop this action. Add extra check to avoid this kind of error.
v3 -> v2: update comments suggested by Tony Luck
v2 -> v1: update redundant logic following the suggestion from Boris
--- drivers/acpi/apei/einj.c | 44 +++++++++++++++++++++++++++++++++++++++++--- kernel/resource.c | 1 + 2 files changed, 42 insertions(+), 3 deletions(-)
drivers/acpi/apei/einj.c +41 -3
@@ -32,6 +32,7 @@#include <linux/seq_file.h> #include <linux/nmi.h> #include <linux/delay.h> +#include <linux/mm.h> #include <acpi/acpi.h> #include "apei-internal.h"
@@ -41,6 +42,10 @@#define SPIN_UNIT 100/* 100ns */ /* Firmware should respond within 1 milliseconds */ #define FIRMWARE_TIMEOUT (1 * NSEC_PER_MSEC) +#define ACPI5_VENDOR_BIT BIT(31) +#define MEM_ERROR_MASK (ACPI_EINJ_MEMORY_CORRECTABLE | \ + ACPI_EINJ_MEMORY_UNCORRECTABLE | \ + ACPI_EINJ_MEMORY_FATAL)/* * ACPI version 5 provides a SET_ERROR_TYPE_WITH_ADDRESS action.
@@ -367,7 +372,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 || acpi5) && (type &0x0038 ) && param2) { +if ((param_extension || acpi5) && (type &MEM_ERROR_MASK ) && param2) {struct apei_resources addr_resources;apei_resources_init (&addr_resources); trigger_param_region = einj_get_trigger_parameter_region(
@@ -427,7 +432,7 @@ static int __einj_error_inject(u32 type, u64 param1, u64 param2)struct set_error_type_with_address *v5param = einj_param; v5param->type = type; -if (type &0x80000000 ) { +if (type &ACPI5_VENDOR_BIT ) {switch (vendor_flags) {case SETWA_FLAGS_APICID : v5param->apicid = param1;
@@ -509,10 +514,43 @@ static int __einj_error_inject(u32 type, u64 param1, u64 param2) }/* Inject the specified hardware error */ + +/* +* When injection type is memory related, the param1/param2 should +* be check carefully. Especially if param2 is non-page-aligned +* like 0xf0f0f0f0f0f0f0f0, that must be strictly forbidden. +*/ static int einj_error_inject (u32 type,u64 param1,u64 param2) {int rc; +unsigned long pfn; + +/* +* We need extra sanity checks for memory errors. +* Other types leap directly to injection. +*/ + +/* ensure param1/param2 existed */ +if (!(param_extension || acpi5)) + goto inject; + +/* ensure injection is memory related */ +if (type &ACPI5_VENDOR_BIT ) { +if (vendor_flags !=SETWA_FLAGS_MEM ) + goto inject; + }else if (!(type &MEM_ERROR_MASK )) + goto inject; + +/* +* Disallow crazy address masks that give BIOS leeway to pick +* injection address almost anywhere. Insist on page or +* better granularity. +*/ + pfn =PFN_DOWN (param1 & param2); +if (!page_is_ram (pfn) || ((param2 &PAGE_MASK ) !=PAGE_MASK )) +return -EINVAL ; +inject:mutex_lock (&einj_mutex); rc =__einj_error_inject (type, param1, param2);mutex_unlock (&einj_mutex);
@@ -590,7 +628,7 @@ static int error_type_set(void *data, u64 val)* Vendor defined types have 0x80000000 bit set, and * are not enumerated by ACPI_EINJ_GET_ERROR_TYPE */ - vendor = val &0x80000000 ; + vendor = val &ACPI5_VENDOR_BIT ; tval = val &0x7fffffff ;/* Only one error type can be specified */
kernel/resource.c +1 -0
@@ -409,6 +409,7 @@ int __weak page_is_ram(unsigned long pfn) {return walk_system_ram_range (pfn,1 ,NULL , __is_ram) ==1 ; } +EXPORT_SYMBOL_GPL (page_is_ram);void __weak arch_remove_reservations (struct resource *avail) {
Signature · 1 line
1.7.10.4
PATCH v3 2/2 ACPI/APEI: Update einj documentation for param1/param2
In reply to Chen GongTo ensure EINJ working well when injecting errors via EINJ table, add some attentions for param1/param2.
--- Documentation/acpi/apei/einj.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
Documentation/acpi/apei/einj.txt +6 -2
@@ -47,11 +47,15 @@ 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. + parameter depends on error_type specified. For example, if error + type is memory related type, the param1 should be a valid physical + memory address. - param2 This file is used to set the second error parameter value. Effect of - parameter depends on error_type specified. + parameter depends on error_type specified. For example, if error + type is memory related type, the param2 should be a valid physical + memory address mask, say, 0xfffffffffffff000. - notrigger The EINJ mechanism is a two step process. First inject the error, then
Signature · 1 line
1.7.10.4
Quoted 10 lines · Chen Gong
Date: Wed, 22 May 2013 22:32:01 -0400
From: Chen Gong <[email protected]>
To: [email protected], [email protected]
Cc: [email protected]
Subject: Parameter check for EINJ error injection - V3
X-Mailer: git-send-email 1.7.10.4[PATCH 1/2 V3] ACPI/APEI: Add parameter check before error injection [PATCH 2/2 V3] ACPI/APEI: Update einj documentation for param1/param2
Quoted 8 lines
These two patches are employed to enhance the check for param1/param2 when einj is used. Param1/param2 are very subtle for the guy who lacks of experience. These two patches add some extra checks to avoid happening further errors to confuse users.
V3: only update the 1st patch based on Tony and Boris' suggestion.
V2: only update the 1st patch based on Boris' suggestion. --
So Tony/Boris
Have you picked this patch series into some RAS branch? I just don't find them.
Look in "tip" tree (x86/ras branch).
-Tony
Sent from my iPhone
Quoted 25 lines · Chen Gong
Quoted 10 lines · Chen Gong
Date: Wed, 22 May 2013 22:32:01 -0400
From: Chen Gong <[email protected]>
To: [email protected], [email protected]
Cc: [email protected]
Subject: Parameter check for EINJ error injection - V3
X-Mailer: git-send-email 1.7.10.4[PATCH 1/2 V3] ACPI/APEI: Add parameter check before error injection [PATCH 2/2 V3] ACPI/APEI: Update einj documentation for param1/param2
Quoted 8 lines
These two patches are employed to enhance the check for param1/param2 when einj is used. Param1/param2 are very subtle for the guy who lacks of experience. These two patches add some extra checks to avoid happening further errors to confuse users.
V3: only update the 1st patch based on Tony and Boris' suggestion.
V2: only update the 1st patch based on Boris' suggestion. --So Tony/Boris
Have you picked this patch series into some RAS branch? I just don't find them.