From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935273AbdJRFF7 (ORCPT ); Wed, 18 Oct 2017 01:05:59 -0400 Received: from szxga05-in.huawei.com ([45.249.212.191]:8529 "EHLO szxga05-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933086AbdJRFF6 (ORCPT ); Wed, 18 Oct 2017 01:05:58 -0400 Subject: Re: [PATCH v5 2/2] acpi: apei: Add SEI notification type support for ARMv8 To: Borislav Petkov CC: , , , , , , , , , , , , , , , , , , , Huangshaoyu , Wuquanming References: <1508227341-15651-1-git-send-email-gengdongjiu@huawei.com> <1508227341-15651-2-git-send-email-gengdongjiu@huawei.com> <20171017170635.qb4sqyrsnndgbuni@pd.tnic> From: gengdongjiu Message-ID: <0bcba052-700b-5ee8-88fc-aaf8cc6b1d37@huawei.com> Date: Wed, 18 Oct 2017 13:00:44 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20171017170635.qb4sqyrsnndgbuni@pd.tnic> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.142.68.147] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020205.59E6E11C.0057,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 9f2ffa114906d14c9f391901f8b9004e Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Borislav, On 2017/10/18 1:06, Borislav Petkov wrote: > On Tue, Oct 17, 2017 at 04:02:21PM +0800, Dongjiu Geng wrote: >> ARMv8.2 requires implementation of the RAS extension, in >> this extension it adds SEI(SError Interrupt) notification >> type, this patch adds new GHES error source SEI handling >> functions. Because this error source parsing and handling >> methods are similar with the SEA. So share some SEA handling >> functions with the SEI >> >> Expose one API ghes_notify_abort() to external users. External >> modules can call this exposed API to parse and handle the >> SEA or SEI. >> >> Note: For the SEI(SError Interrupt), it is asynchronous external >> abort, the error address recorded by firmware may be not accurate. >> If not accurate, EL3 firmware needs to identify the address to a >> invalid value. >> >> Cc: Borislav Petkov >> Cc: James Morse >> Signed-off-by: Dongjiu Geng >> Tested-by: Tyler Baicar >> Tested-by: Dongjiu Geng >> --- >> arch/arm64/mm/fault.c | 4 +-- >> drivers/acpi/apei/Kconfig | 15 ++++++++++ >> drivers/acpi/apei/ghes.c | 71 ++++++++++++++++++++++++++++++++++------------- >> include/acpi/ghes.h | 2 +- >> 4 files changed, 70 insertions(+), 22 deletions(-) >> >> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c >> index 2509e4f..c98c1b3 100644 >> --- a/arch/arm64/mm/fault.c >> +++ b/arch/arm64/mm/fault.c >> @@ -585,7 +585,7 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) >> if (interrupts_enabled(regs)) >> nmi_enter(); >> >> - ret = ghes_notify_sea(); >> + ret = ghes_notify_abort(ACPI_HEST_NOTIFY_SEA); >> >> if (interrupts_enabled(regs)) >> nmi_exit(); >> @@ -682,7 +682,7 @@ int handle_guest_sea(phys_addr_t addr, unsigned int esr) >> int ret = -ENOENT; >> >> if (IS_ENABLED(CONFIG_ACPI_APEI_SEA)) >> - ret = ghes_notify_sea(); >> + ret = ghes_notify_abort(ACPI_HEST_NOTIFY_SEA); >> >> return ret; >> } >> diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig >> index de14d49..47fcb0c 100644 >> --- a/drivers/acpi/apei/Kconfig >> +++ b/drivers/acpi/apei/Kconfig >> @@ -54,6 +54,21 @@ config ACPI_APEI_SEA >> option allows the OS to look for such hardware error record, and >> take appropriate action. >> >> +config ACPI_APEI_SEI >> + bool "APEI Asynchronous SError Interrupt logging/recovering support" > > What is "SError" ? SError is System Error, which is a asynchronous exception in the Internal CPU. In the ARM RAS Extension, there are mainly two type abort for CPU: SEA(Synchronous External Abort) SEI(SError Interrupt) > >> + depends on ARM64 && ACPI_APEI_GHES >> + default y >> + help >> + This option should be enabled if the system supports >> + firmware first handling of SEI (asynchronous SError interrupt). >> + >> + SEI happens with asynchronous external abort for errors on device >> + memory reads on ARMv8 systems. If a system supports firmware first >> + handling of SEI, the platform analyzes and handles hardware error >> + notifications from SEI, and it may then form a HW error record for >> + the OS to parse and handle. This option allows the OS to look for >> + such hardware error record, and take appropriate action. >> + >> config ACPI_APEI_MEMORY_FAILURE >> bool "APEI memory error recovering support" >> depends on ACPI_APEI && MEMORY_FAILURE >> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c >> index 3eee30a..24b4233 100644 >> --- a/drivers/acpi/apei/ghes.c >> +++ b/drivers/acpi/apei/ghes.c >> @@ -815,43 +815,67 @@ static struct notifier_block ghes_notifier_hed = { >> >> #ifdef CONFIG_ACPI_APEI_SEA >> static LIST_HEAD(ghes_sea); >> +#endif >> + >> +#ifdef CONFIG_ACPI_APEI_SEI >> +static LIST_HEAD(ghes_sei); >> +#endif >> >> +#if defined(CONFIG_ACPI_APEI_SEA) || defined(CONFIG_ACPI_APEI_SEI) >> /* >> - * Return 0 only if one of the SEA error sources successfully reported an error >> - * record sent from the firmware. >> + * Return 0 only if one of the SEA or SEI error sources successfully >> + * reported an error record sent from the firmware. >> */ >> -int ghes_notify_sea(void) >> +int ghes_notify_abort(u8 type) > > Adding "abort" everywhere makes it worse: what does this function now > do, notify or abort or both? This function is used to notify APEI driver to parse APEI table and do some recovery, such as calling memrory_failure() to identify the address to a poisoned memory and deliver SIGBUS to related application. > > Ditto for the remaining ones. Please think of a better name. Ok, thanks for your good suggestion, I will consider to use a better name. > > > ghes_notify_sei() sounds much better to me, for example. And then you > can add a whole set of *_sei() functions similar to the *_sea() ones and > then you don't have to do all that checking of the type but simply call > the proper function set. They're not huge so that the duplication of > code should be minimal. Thanks for your suggestion again. I will flowing that and add a whole set of *sei() functions. >