On 05/29/2019 08:23 PM, Mark Rutland wrote: > On Wed, May 29, 2019 at 06:04:44PM +0530, Anshuman Khandual wrote: >> This consolidates page fault information capture and move them bit earlier. >> While here it also adds an wrapper is_write_abort(). It also saves some >> cycles by replacing multiple user_mode() calls into a single one earlier >> during the fault. > > To be honest, I doubt this has any measureable impact, but I agree that > using variables _may_ make the flow control easier to understand. > >> >> Signed-off-by: Anshuman Khandual <anshuman.khand...@arm.com> >> Cc: Catalin Marinas <catalin.mari...@arm.com> >> Cc: Will Deacon <will.dea...@arm.com> >> Cc: Mark Rutland <mark.rutl...@arm.com> >> Cc: James Morse <james.mo...@arm.com> >> Cc: Andrey Konovalov <andreyk...@google.com> >> --- >> arch/arm64/mm/fault.c | 22 +++++++++++++++------- >> 1 file changed, 15 insertions(+), 7 deletions(-) >> >> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c >> index da02678..170c71f 100644 >> --- a/arch/arm64/mm/fault.c >> +++ b/arch/arm64/mm/fault.c >> @@ -435,6 +435,11 @@ static bool is_el0_instruction_abort(unsigned int esr) >> return ESR_ELx_EC(esr) == ESR_ELx_EC_IABT_LOW; >> } >> >> +static bool is_write_abort(unsigned int esr) >> +{ >> + return (esr & ESR_ELx_WNR) && !(esr & ESR_ELx_CM); >> +} > > In off-list review, I mentioned that this isn't true for EL1, and I > think that we should name this 'is_el0_write_abort()' or add a comment > explaining the caveats if factored into a helper. > > Thanks, > Mark. Okay will change the wrapper name to is_el0_write_abort() and add a comment explaining how this is only applicable to aborts originating from EL0.