From patchwork Mon May 20 07:20:32 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chen Gong X-Patchwork-Id: 2591091 Return-Path: X-Original-To: patchwork-linux-acpi@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 24D83DF2A2 for ; Mon, 20 May 2013 07:27:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753476Ab3ETH1L (ORCPT ); Mon, 20 May 2013 03:27:11 -0400 Received: from mga03.intel.com ([143.182.124.21]:31666 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753374Ab3ETH1K (ORCPT ); Mon, 20 May 2013 03:27:10 -0400 Received: from azsmga001.ch.intel.com ([10.2.17.19]) by azsmga101.ch.intel.com with ESMTP; 20 May 2013 00:27:09 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.87,706,1363158000"; d="scan'208";a="305018776" Received: from gchen-sby.bj.intel.com (HELO localhost) ([10.238.158.225]) by azsmga001.ch.intel.com with ESMTP; 20 May 2013 00:27:08 -0700 From: Chen Gong To: tony.luck@intel.com, bp@alien8.de Cc: linux-acpi@vger.kernel.org, Chen Gong Subject: [PATCH 1/2 V2] ACPI/APEI: Add parameter check before error injection Date: Mon, 20 May 2013 03:20:32 -0400 Message-Id: <1369034433-13666-2-git-send-email-gong.chen@linux.intel.com> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1369034433-13666-1-git-send-email-gong.chen@linux.intel.com> References: <1369034433-13666-1-git-send-email-gong.chen@linux.intel.com> Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org 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. v2 -> v1: update redundant logic following the suggestion from Boris Signed-off-by: Chen Gong --- drivers/acpi/apei/einj.c | 38 +++++++++++++++++++++++++++++++++++--- kernel/resource.c | 1 + 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c index 8d457b5..841765f 100644 --- a/drivers/acpi/apei/einj.c +++ b/drivers/acpi/apei/einj.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #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; @@ -512,7 +517,34 @@ static int __einj_error_inject(u32 type, u64 param1, u64 param2) static int einj_error_inject(u32 type, u64 param1, u64 param2) { int rc; + unsigned long pfn; + /* 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; + + /* + * When error injection type is memory related, param2 is the address + * mask of param1. This mask is used to ensure that the final address + * (param1 & param2) is meaningful. If param2 has a *weird* style + * like 0xf0f0f0f0f0f0f0f0, it means the injection address can be + * anywhere around param1, and that must be forbidden. In that reason, + * PAGE_MASK is employed to avoid injection address discontinuous. + * If one finds a special case not to satisfy this requirement, please + * fix it. + */ + 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 +622,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 */ diff --git a/kernel/resource.c b/kernel/resource.c index d738698..77bf11a 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -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) {