Re: [PATCH RFC 2/4] arm64: mm: Add RAS extension system register check to SEA handling

From: James Morse <james.morse@arm.com>
To: Tyler Baicar OS <baicar@os.amperecomputing.com>
Cc: "mark.rutland@arm.com" <mark.rutland@arm.com>,
	"lorenzo.pieralisi@arm.com" <lorenzo.pieralisi@arm.com>,
	"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
	"sudeep.holla@arm.com" <sudeep.holla@arm.com>,
	"rjw@rjwysocki.net" <rjw@rjwysocki.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Matteo.Carlini@arm.com" <Matteo.Carlini@arm.com>,
	"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	"tony.luck@intel.com" <tony.luck@intel.com>,
	"bp@alien8.de" <bp@alien8.de>,
	"guohanjun@huawei.com" <guohanjun@huawei.com>,
	"Andrew.Murray@arm.com" <Andrew.Murray@arm.com>,
	Open Source Submission <patches@amperecomputing.com>,
	"lenb@kernel.org" <lenb@kernel.org>,
	"will@kernel.org" <will@kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>
Subject: Re: [PATCH RFC 2/4] arm64: mm: Add RAS extension system register check to SEA handling
Date: Mon, 8 Jul 2019 11:00:19 +0100	[thread overview]
Message-ID: <df262b97-eda2-0556-d6ef-532a0d697131@arm.com> (raw)
In-Reply-To: <1562086280-5351-3-git-send-email-baicar@os.amperecomputing.com>

Hey Tyler,

On 02/07/2019 17:51, Tyler Baicar OS wrote:
> On systems that support the ARM RAS extension, synchronous external
> abort syndrome information could be captured in the core's RAS extension
> system registers. So, when handling SEAs check the RAS system registers
> for error syndrome information.

> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 2d11501..76b42ca 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -37,6 +37,7 @@
>  #include <asm/pgtable.h>
>  #include <asm/tlbflush.h>
>  #include <asm/traps.h>
> +#include <asm/ras.h>
>  
>  struct fault_info {
>  	int	(*fn)(unsigned long addr, unsigned int esr,
> @@ -632,6 +633,8 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
>  
>  	inf = esr_to_fault_info(esr);
>  
> +	arch_arm_ras_report_error();
> +
>  	/*
>  	 * Return value ignored as we rely on signal merging.
>  	 * Future patches will make this more robust.
> 

If we interrupted a preemptible context, do_sea() is preemptible too... This means we
can't know if we're still running on the same CPU as the one that took the external-abort.
(until this series, it hasn't mattered).

Fixing this means cramming something into entry.S's el1_da, as this may unmask interrupts
before calling do_mem_abort(). But its going to be ugly because some of do_mem_abort()s
ESR values need to be preemptible because they sleep, e.g. page-faults calling
handle_mm_fault().
For do_sea(), do_exit() will 'fix' the preempt count if we kill the thread, but if we
don't, it still needs to be balanced. Doing all this in assembly is going to be unreadable!

Mark Rutland has a series to move the entry assembly into C [0]. Based on that that it
should be possible for the new el1_abort() to spot a Synchronous-External-Abort ESR, and
wrap the do_mem_abort() with preempt enable/disable, before inheriting the flags. (which
for synchronous exceptions, I think we should always do)

Thanks,

James

[0] https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/entry-deasm

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel