From c72200357aed49fd51dc21e45d4396f5402df811 Mon Sep 17 00:00:00 2001 From: Manish Pandey Date: Mon, 3 Feb 2025 12:00:56 +0000 Subject: [PATCH] fix(el3-runtime): replace CTX_ESR_EL3 with CTX_DOUBLE_FAULT_ESR ESR_EL3 value is updated when an exception is taken to EL3 and its value does not change until a new exception is taken to EL3. We need to save ESR in context memory only when we expect nested exception in EL3. The scenarios where we would expect nested EL3 execution are related with FFH_SUPPORT, namely 1.Handling pending async EAs at EL3 boundry - It uses CTX_SAVED_ESR_EL3 to preserve origins esr_el3 2.Double fault handling - Introduce an explicit storage (CTX_DOUBLE_FAULT_ESR) for esr_el3 to take care of DobuleFault. As the ESR context has been removed, read the register directly instead of its context value in RD platform. Signed-off-by: Manish Pandey Change-Id: I7720c5f03903f894a77413a235e3cc05c86f9c17 --- bl31/aarch64/ea_delegate.S | 38 +++++++++---------- include/lib/el3_runtime/aarch64/context.h | 24 ++++++------ .../neoverse_rd/common/ras/nrd_ras_cpu.c | 3 +- 3 files changed, 30 insertions(+), 35 deletions(-) diff --git a/bl31/aarch64/ea_delegate.S b/bl31/aarch64/ea_delegate.S index 91ea75d5f..fce17e1d3 100644 --- a/bl31/aarch64/ea_delegate.S +++ b/bl31/aarch64/ea_delegate.S @@ -245,26 +245,30 @@ endfunc delegate_async_ea */ func ea_proceed /* - * If the ESR loaded earlier is not zero, we were processing an EA - * already, and this is a double fault. + * If it is a double fault invoke platform handler. + * Double fault scenario would arise when platform is handling a fault in + * lower EL using plat_ea_handler() and another fault happens which would + * trap into EL3 as FFH_SUPPORT is enabled for the platform. */ - ldr x5, [sp, #CTX_EL3STATE_OFFSET + CTX_ESR_EL3] + ldr x5, [sp, #CTX_EL3STATE_OFFSET + CTX_DOUBLE_FAULT_ESR] cbz x5, 1f no_ret plat_handle_double_fault 1: - /* Save EL3 state */ + /* Save EL3 state as handling might involve lower ELs */ mrs x2, spsr_el3 mrs x3, elr_el3 stp x2, x3, [sp, #CTX_EL3STATE_OFFSET + CTX_SPSR_EL3] + mrs x4, scr_el3 + str x4, [sp, #CTX_EL3STATE_OFFSET + CTX_SCR_EL3] /* - * Save ESR as handling might involve lower ELs, and returning back to - * EL3 from there would trample the original ESR. + * Save CTX_DOUBLE_FAULT_ESR, so that if another fault happens in lower EL, we + * catch it as DoubleFault in next invocation of ea_proceed() along with + * preserving original ESR_EL3. */ - mrs x4, scr_el3 mrs x5, esr_el3 - stp x4, x5, [sp, #CTX_EL3STATE_OFFSET + CTX_SCR_EL3] + str x5, [sp, #CTX_EL3STATE_OFFSET + CTX_DOUBLE_FAULT_ESR] /* * Setup rest of arguments, and call platform External Abort handler. @@ -305,23 +309,15 @@ func ea_proceed /* Make SP point to context */ msr spsel, #MODE_SP_ELX - /* Restore EL3 state and ESR */ + /* Clear Double Fault storage */ + str xzr, [sp, #CTX_EL3STATE_OFFSET + CTX_DOUBLE_FAULT_ESR] + + /* Restore EL3 state */ ldp x1, x2, [sp, #CTX_EL3STATE_OFFSET + CTX_SPSR_EL3] msr spsr_el3, x1 msr elr_el3, x2 - - /* Restore ESR_EL3 and SCR_EL3 */ - ldp x3, x4, [sp, #CTX_EL3STATE_OFFSET + CTX_SCR_EL3] + ldr x3, [sp, #CTX_EL3STATE_OFFSET + CTX_SCR_EL3] msr scr_el3, x3 - msr esr_el3, x4 - -#if ENABLE_ASSERTIONS - cmp x4, xzr - ASM_ASSERT(ne) -#endif - - /* Clear ESR storage */ - str xzr, [sp, #CTX_EL3STATE_OFFSET + CTX_ESR_EL3] ret x29 endfunc ea_proceed diff --git a/include/lib/el3_runtime/aarch64/context.h b/include/lib/el3_runtime/aarch64/context.h index 15d5204b8..d9a918866 100644 --- a/include/lib/el3_runtime/aarch64/context.h +++ b/include/lib/el3_runtime/aarch64/context.h @@ -67,25 +67,25 @@ ******************************************************************************/ #define CTX_EL3STATE_OFFSET (CTX_GPREGS_OFFSET + CTX_GPREGS_END) #define CTX_SCR_EL3 U(0x0) -#define CTX_ESR_EL3 U(0x8) -#define CTX_RUNTIME_SP U(0x10) -#define CTX_SPSR_EL3 U(0x18) -#define CTX_ELR_EL3 U(0x20) -#define CTX_PMCR_EL0 U(0x28) -#define CTX_IS_IN_EL3 U(0x30) -#define CTX_MDCR_EL3 U(0x38) +#define CTX_RUNTIME_SP U(0x8) +#define CTX_SPSR_EL3 U(0x10) +#define CTX_ELR_EL3 U(0x18) +#define CTX_PMCR_EL0 U(0x20) +#define CTX_IS_IN_EL3 U(0x28) +#define CTX_MDCR_EL3 U(0x30) /* Constants required in supporting nested exception in EL3 */ -#define CTX_SAVED_ELR_EL3 U(0x40) +#define CTX_SAVED_ELR_EL3 U(0x38) /* * General purpose flag, to save various EL3 states * FFH mode : Used to identify if handling nested exception * KFH mode : Used as counter value */ -#define CTX_NESTED_EA_FLAG U(0x48) +#define CTX_NESTED_EA_FLAG U(0x40) #if FFH_SUPPORT - #define CTX_SAVED_ESR_EL3 U(0x50) - #define CTX_SAVED_SPSR_EL3 U(0x58) - #define CTX_SAVED_GPREG_LR U(0x60) + #define CTX_SAVED_ESR_EL3 U(0x48) + #define CTX_SAVED_SPSR_EL3 U(0x50) + #define CTX_SAVED_GPREG_LR U(0x58) + #define CTX_DOUBLE_FAULT_ESR U(0x60) #define CTX_EL3STATE_END U(0x70) /* Align to the next 16 byte boundary */ #else #define CTX_EL3STATE_END U(0x50) /* Align to the next 16 byte boundary */ diff --git a/plat/arm/board/neoverse_rd/common/ras/nrd_ras_cpu.c b/plat/arm/board/neoverse_rd/common/ras/nrd_ras_cpu.c index dcee92c8e..19fb79654 100644 --- a/plat/arm/board/neoverse_rd/common/ras/nrd_ras_cpu.c +++ b/plat/arm/board/neoverse_rd/common/ras/nrd_ras_cpu.c @@ -129,8 +129,7 @@ static void populate_cpu_err_data(cpu_err_info *cpu_info, cpu_info->ErrCtxEl3Reg[0] = read_ctx_reg(get_el3state_ctx(ctx), CTX_ELR_EL3); - cpu_info->ErrCtxEl3Reg[1] = read_ctx_reg(get_el3state_ctx(ctx), - CTX_ESR_EL3); + cpu_info->ErrCtxEl3Reg[1] = read_esr_el3(); cpu_info->ErrCtxEl3Reg[2] = read_far_el3(); cpu_info->ErrCtxEl3Reg[4] = read_mair_el3(); cpu_info->ErrCtxEl3Reg[5] = read_sctlr_el3();