[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20230509141850.GA2894321@e124191.cambridge.arm.com>
Date: Tue, 9 May 2023 15:18:50 +0100
From: Joey Gouly <joey.gouly@....com>
To: Mark Brown <broonie@...nel.org>
CC: Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <nd@....com>
Subject: Re: [PATCH 2/2] arm64/esr: Add decode of ISS2 to data abort reporting
Hi Mark,
On Tue, Apr 18, 2023 at 02:57:32PM +0100, Mark Brown wrote:
> The architecture has added more information about faults to ISS2 within
> ESR. Add decode of this to our data abort fault decode to aid diagnostics.
> Features that are not currently enabled are included here for completeness.
>
> Since the architecture specifies the values of bits within ISS2 in terms
> of ISS2 rather than in terms of the register as a whole we do so for our
> definitions as well, this makes it easier to review bitfield definitions.
>
> Signed-off-by: Mark Brown <broonie@...nel.org>
> ---
> arch/arm64/include/asm/esr.h | 17 +++++++++++++++++
> arch/arm64/mm/fault.c | 14 ++++++++++++--
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
> index 0bd879007168..17bc6536ffea 100644
> --- a/arch/arm64/include/asm/esr.h
> +++ b/arch/arm64/include/asm/esr.h
> @@ -77,6 +77,9 @@
> #define ESR_ELx_IL (UL(1) << ESR_ELx_IL_SHIFT)
> #define ESR_ELx_ISS_MASK (GENMASK(24, 0))
> #define ESR_ELx_ISS(esr) ((esr) & ESR_ELx_ISS_MASK)
> +#define ESR_ELx_ISS2_SHIFT (32)
> +#define ESR_ELx_ISS2_MASK (GENMASK_ULL(55, 32))
> +#define ESR_ELx_ISS2(esr) ((esr) & ESR_ELx_ISS_MASK)
Typo here, should be ESR_ELx_ISS2_MASK.
>
> /* ISS field definitions shared by different classes */
> #define ESR_ELx_WNR_SHIFT (6)
> @@ -140,6 +143,20 @@
> #define ESR_ELx_CM_SHIFT (8)
> #define ESR_ELx_CM (UL(1) << ESR_ELx_CM_SHIFT)
>
> +/* ISS2 field definitions for Data Aborts */
> +#define ESR_ELx_TnD_SHIFT (10)
> +#define ESR_ELx_TnD (UL(1) << ESR_ELx_TnD_SHIFT)
> +#define ESR_ELx_TagAccess_SHIFT (9)
> +#define ESR_ELx_TagAccess (UL(1) << ESR_ELx_TagAccess_SHIFT)
> +#define ESR_ELx_GCS_SHIFT (8)
> +#define ESR_ELx_GCS (UL(1) << ESR_ELx_GCS_SHIFT)
> +#define ESR_ELx_Overlay_SHIFT (6)
> +#define ESR_ELx_Overlay (UL(1) << ESR_ELx_Overlay_SHIFT)
> +#define ESR_ELx_DirtyBit_SHIFT (5)
> +#define ESR_ELx_DirtyBit (UL(1) << ESR_ELx_DirtyBit_SHIFT)
> +#define ESR_ELx_Xs_SHIFT (0)
> +#define ESR_ELx_Xs_MASK (GENMASK_ULL(4, 0))
> +
> /* ISS field definitions for exceptions taken in to Hyp */
> #define ESR_ELx_CV (UL(1) << 24)
> #define ESR_ELx_COND_SHIFT (20)
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index f4cb0f85ccf4..2e76dc613c86 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -66,6 +66,8 @@ static inline const struct fault_info *esr_to_debug_fault_info(unsigned long esr
>
> static void data_abort_decode(unsigned long esr)
> {
> + u64 iss2 = ESR_ELx_ISS2(esr);
> +
> pr_alert("Data abort info:\n");
>
> if (esr & ESR_ELx_ISV) {
> @@ -81,9 +83,17 @@ static void data_abort_decode(unsigned long esr)
> pr_alert(" ISV = 0, ISS = 0x%08lx\n", esr & ESR_ELx_ISS_MASK);
> }
>
> - pr_alert(" CM = %lu, WnR = %lu\n",
> + pr_alert(" CM = %lu, WnR = %lu, TnD = %llu, TagAccess = %lld\n",
> (esr & ESR_ELx_CM) >> ESR_ELx_CM_SHIFT,
> - (esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT);
> + (esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT,
> + (iss2 & ESR_ELx_TnD) >> ESR_ELx_TnD_SHIFT,
> + (iss2 & ESR_ELx_TagAccess) >> ESR_ELx_TagAccess_SHIFT);
> +
> + pr_alert(" GCS = %lld, Overlay = %llu, DirtyBit = %llu, Xs = %llu\n",
> + (iss2 & ESR_ELx_GCS) >> ESR_ELx_GCS_SHIFT,
> + (iss2 & ESR_ELx_Overlay) >> ESR_ELx_Overlay_SHIFT,
> + (iss2 & ESR_ELx_DirtyBit) >> ESR_ELx_DirtyBit_SHIFT,
> + (iss2 & ESR_ELx_Xs_MASK) >> ESR_ELx_Xs_SHIFT);
> }
>
> static void mem_abort_decode(unsigned long esr)
Thanks,
Joey
Powered by blists - more mailing lists