Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752371AbdHNH7D (ORCPT ); Mon, 14 Aug 2017 03:59:03 -0400 Received: from szxga05-in.huawei.com ([45.249.212.191]:3072 "EHLO szxga05-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751501AbdHNH7B (ORCPT ); Mon, 14 Aug 2017 03:59:01 -0400 Subject: Re: [PATCH V15 06/11] acpi: apei: handle SEA notification type for ARMv8 To: Tyler Baicar , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , References: <1492556723-9189-1-git-send-email-tbaicar@codeaurora.org> <1492556723-9189-7-git-send-email-tbaicar@codeaurora.org> From: Xiongfeng Wang Message-ID: <6a5ef607-ac92-a616-8e60-06c4e51bba04@huawei.com> Date: Mon, 14 Aug 2017 15:55:12 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1492556723-9189-7-git-send-email-tbaicar@codeaurora.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.32.209] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020202.599157A3.0035,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: c7507bd3717527a1b2ec4542839c25d5 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7382 Lines: 213 Hi Tyler, On 2017/4/19 7:05, Tyler Baicar wrote: > ARM APEI extension proposal added SEA (Synchronous External Abort) > notification type for ARMv8. > Add a new GHES error source handling function for SEA. If an error > source's notification type is SEA, then this function can be registered > into the SEA exception handler. That way GHES will parse and report > SEA exceptions when they occur. > An SEA can interrupt code that had interrupts masked and is treated as > an NMI. To aid this the page of address space for mapping APEI buffers > while in_nmi() is always reserved, and ghes_ioremap_pfn_nmi() is > changed to use the helper methods to find the prot_t to map with in > the same way as ghes_ioremap_pfn_irq(). > > Signed-off-by: Tyler Baicar > CC: Jonathan (Zhixiong) Zhang > Reviewed-by: James Morse > Acked-by: Catalin Marinas > --- > @@ -518,6 +520,17 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) > pr_err("Synchronous External Abort: %s (0x%08x) at 0x%016lx\n", > inf->name, esr, addr); > > + /* > + * Synchronous aborts may interrupt code which had interrupts masked. > + * Before calling out into the wider kernel tell the interested > + * subsystems. > + */ > + if (IS_ENABLED(CONFIG_ACPI_APEI_SEA)) { > + nmi_enter(); > + ghes_notify_sea(); > + nmi_exit(); > + } > + > info.si_signo = SIGBUS; > info.si_errno = 0; > info.si_code = 0; For instruction abort, if there exists memory section in ghes, we will call memory_failure() in ghes_notify_sea() and reread the instruction from the disk. In this case, we don't have to send SIGBUS to the application. But memory_failure() is scheduled in a work queue, we don't what the result of memory_failure will be when ghes_notify_sea() returned. Do you have any idea about how to fix this, so we don't have to kill the application in the instruction abort case. Thanks, Wang Xiongfeng > diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig > index b0140c8..de14d49 100644 > --- a/drivers/acpi/apei/Kconfig > +++ b/drivers/acpi/apei/Kconfig > @@ -39,6 +39,21 @@ config ACPI_APEI_PCIEAER > PCIe AER errors may be reported via APEI firmware first mode. > Turn on this option to enable the corresponding support. > > +config ACPI_APEI_SEA > + bool "APEI Synchronous External Abort logging/recovering support" > + depends on ARM64 && ACPI_APEI_GHES > + default y > + help > + This option should be enabled if the system supports > + firmware first handling of SEA (Synchronous External Abort). > + SEA happens with certain faults of data abort or instruction > + abort synchronous exceptions on ARMv8 systems. If a system > + supports firmware first handling of SEA, the platform analyzes > + and handles hardware error notifications from SEA, 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 dfb7dd2..2d387f8 100644 > --- a/drivers/acpi/apei/ghes.c > +++ b/drivers/acpi/apei/ghes.c > @@ -115,11 +115,7 @@ > * Two virtual pages are used, one for IRQ/PROCESS context, the other for > * NMI context (optionally). > */ > -#ifdef CONFIG_HAVE_ACPI_APEI_NMI > #define GHES_IOREMAP_PAGES 2 > -#else > -#define GHES_IOREMAP_PAGES 1 > -#endif > #define GHES_IOREMAP_IRQ_PAGE(base) (base) > #define GHES_IOREMAP_NMI_PAGE(base) ((base) + PAGE_SIZE) > > @@ -158,10 +154,14 @@ static void ghes_ioremap_exit(void) > static void __iomem *ghes_ioremap_pfn_nmi(u64 pfn) > { > unsigned long vaddr; > + phys_addr_t paddr; > + pgprot_t prot; > > vaddr = (unsigned long)GHES_IOREMAP_NMI_PAGE(ghes_ioremap_area->addr); > - ioremap_page_range(vaddr, vaddr + PAGE_SIZE, > - pfn << PAGE_SHIFT, PAGE_KERNEL); > + > + paddr = pfn << PAGE_SHIFT; > + prot = arch_apei_get_mem_attribute(paddr); > + ioremap_page_range(vaddr, vaddr + PAGE_SIZE, paddr, prot); > > return (void __iomem *)vaddr; > } > @@ -771,6 +771,50 @@ static int ghes_notify_sci(struct notifier_block *this, > .notifier_call = ghes_notify_sci, > }; > > +#ifdef CONFIG_ACPI_APEI_SEA > +static LIST_HEAD(ghes_sea); > + > +void ghes_notify_sea(void) > +{ > + struct ghes *ghes; > + > + /* > + * synchronize_rcu() will wait for nmi_exit(), so no need to > + * rcu_read_lock(). > + */ > + list_for_each_entry_rcu(ghes, &ghes_sea, list) { > + ghes_proc(ghes); > + } > +} > + > +static void ghes_sea_add(struct ghes *ghes) > +{ > + mutex_lock(&ghes_list_mutex); > + list_add_rcu(&ghes->list, &ghes_sea); > + mutex_unlock(&ghes_list_mutex); > +} > + > +static void ghes_sea_remove(struct ghes *ghes) > +{ > + mutex_lock(&ghes_list_mutex); > + list_del_rcu(&ghes->list); > + mutex_unlock(&ghes_list_mutex); > + synchronize_rcu(); > +} > +#else /* CONFIG_ACPI_APEI_SEA */ > +static inline void ghes_sea_add(struct ghes *ghes) > +{ > + pr_err(GHES_PFX "ID: %d, trying to add SEA notification which is not supported\n", > + ghes->generic->header.source_id); > +} > + > +static inline void ghes_sea_remove(struct ghes *ghes) > +{ > + pr_err(GHES_PFX "ID: %d, trying to remove SEA notification which is not supported\n", > + ghes->generic->header.source_id); > +} > +#endif /* CONFIG_ACPI_APEI_SEA */ > + > #ifdef CONFIG_HAVE_ACPI_APEI_NMI > /* > * printk is not safe in NMI context. So in NMI handler, we allocate > @@ -1016,6 +1060,14 @@ static int ghes_probe(struct platform_device *ghes_dev) > case ACPI_HEST_NOTIFY_EXTERNAL: > case ACPI_HEST_NOTIFY_SCI: > break; > + case ACPI_HEST_NOTIFY_SEA: > + if (!IS_ENABLED(CONFIG_ACPI_APEI_SEA)) { > + pr_warn(GHES_PFX "Generic hardware error source: %d notified via SEA is not supported\n", > + generic->header.source_id); > + rc = -ENOTSUPP; > + goto err; > + } > + break; > case ACPI_HEST_NOTIFY_NMI: > if (!IS_ENABLED(CONFIG_HAVE_ACPI_APEI_NMI)) { > pr_warn(GHES_PFX "Generic hardware error source: %d notified via NMI interrupt is not supported!\n", > @@ -1081,6 +1133,9 @@ static int ghes_probe(struct platform_device *ghes_dev) > list_add_rcu(&ghes->list, &ghes_sci); > mutex_unlock(&ghes_list_mutex); > break; > + case ACPI_HEST_NOTIFY_SEA: > + ghes_sea_add(ghes); > + break; > case ACPI_HEST_NOTIFY_NMI: > ghes_nmi_add(ghes); > break; > @@ -1124,6 +1179,9 @@ static int ghes_remove(struct platform_device *ghes_dev) > mutex_unlock(&ghes_list_mutex); > synchronize_rcu(); > break; > + case ACPI_HEST_NOTIFY_SEA: > + ghes_sea_remove(ghes); > + break; > case ACPI_HEST_NOTIFY_NMI: > ghes_nmi_remove(ghes); > break; > diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h > index b89361a..ef0040893 100644 > --- a/include/acpi/ghes.h > +++ b/include/acpi/ghes.h > @@ -1,3 +1,6 @@ > +#ifndef GHES_H > +#define GHES_H > + > #include > #include > > @@ -95,3 +98,7 @@ static inline void *acpi_hest_get_payload(struct acpi_hest_generic_data *gdata) > > return gdata + 1; > } > + > +void ghes_notify_sea(void); > + > +#endif /* GHES_H */ >