[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <86iky8j9ow.wl-maz@kernel.org>
Date: Mon, 17 Jun 2024 08:43:43 +0100
From: Marc Zyngier <maz@...nel.org>
To: Anshuman Khandual <anshuman.khandual@....com>
Cc: linux-arm-kernel@...ts.infradead.org,
mark.rutland@....com,
ryan.roberts@....com,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64/mm: Drop ESR_ELx_FSC_TYPE
On Mon, 17 Jun 2024 04:15:40 +0100,
Anshuman Khandual <anshuman.khandual@....com> wrote:
>
> Does the following re-worked patch looks okay ? Earlier set_thread_esr() changes
> can be dropped from arch/arm64/mm/fault.c and also the original commit message
> still makes sense.
>
> diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
> index 7abf09df7033..6cd13ac61005 100644
> --- a/arch/arm64/include/asm/esr.h
> +++ b/arch/arm64/include/asm/esr.h
> @@ -121,6 +121,13 @@
> #define ESR_ELx_FSC_SECC (0x18)
> #define ESR_ELx_FSC_SECC_TTW(n) (0x1c + (n))
>
> +/* Status codes for individual page table levels */
> +#define ESR_ELx_FSC_ACCESS_L(n) (ESR_ELx_FSC_ACCESS + n)
> +#define ESR_ELx_FSC_FAULT_nL (0x2C)
> +#define ESR_ELx_FSC_FAULT_L(n) (((n) < 0 ? ESR_ELx_FSC_FAULT_nL : \
> + ESR_ELx_FSC_FAULT) + (n))
> +#define ESR_ELx_FSC_PERM_L(n) (ESR_ELx_FSC_PERM + n)
> +
> /* ISS field definitions for Data Aborts */
> #define ESR_ELx_ISV_SHIFT (24)
> #define ESR_ELx_ISV (UL(1) << ESR_ELx_ISV_SHIFT)
> @@ -388,20 +395,33 @@ static inline bool esr_is_data_abort(unsigned long esr)
>
> static inline bool esr_fsc_is_translation_fault(unsigned long esr)
> {
> - /* Translation fault, level -1 */
> - if ((esr & ESR_ELx_FSC) == 0b101011)
> - return true;
> - return (esr & ESR_ELx_FSC_TYPE) == ESR_ELx_FSC_FAULT;
> + esr = esr & ESR_ELx_FSC;
> +
> + return (esr == ESR_ELx_FSC_FAULT_L(3)) ||
> + (esr == ESR_ELx_FSC_FAULT_L(2)) ||
> + (esr == ESR_ELx_FSC_FAULT_L(1)) ||
> + (esr == ESR_ELx_FSC_FAULT_L(0)) ||
> + (esr == ESR_ELx_FSC_FAULT_L(-1));
> }
>
> static inline bool esr_fsc_is_permission_fault(unsigned long esr)
> {
> - return (esr & ESR_ELx_FSC_TYPE) == ESR_ELx_FSC_PERM;
> + esr = esr & ESR_ELx_FSC;
> +
> + return (esr == ESR_ELx_FSC_PERM_L(3)) ||
> + (esr == ESR_ELx_FSC_PERM_L(2)) ||
> + (esr == ESR_ELx_FSC_PERM_L(1)) ||
> + (esr == ESR_ELx_FSC_PERM_L(0));
> }
>
> static inline bool esr_fsc_is_access_flag_fault(unsigned long esr)
> {
> - return (esr & ESR_ELx_FSC_TYPE) == ESR_ELx_FSC_ACCESS;
> + esr = esr & ESR_ELx_FSC;
> +
> + return (esr == ESR_ELx_FSC_ACCESS_L(3)) ||
> + (esr == ESR_ELx_FSC_ACCESS_L(2)) ||
> + (esr == ESR_ELx_FSC_ACCESS_L(1)) ||
> + (esr == ESR_ELx_FSC_ACCESS_L(0));
> }
>
> /* Indicate whether ESR.EC==0x1A is for an ERETAx instruction */
This looks better indeed.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists