On Mon, Mar 20, 2017 at 08:53:01PM +0000, Ard Biesheuvel wrote:
> In order to be able to produce meaningful diagnostic output when taking
> synchronous exceptions that have been caused by corruption of the stack
> pointer, prepare the EL0 stack pointer and switch to it when handling the
> 'Sync exception using SPx' exception class. Other exception classes (of
> which we really only care about IrqSPx) are left functionally intact.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> ---
>
> Note that some code has been moved around so that the macro doesn't grow too
> big to fit in a 128 byte slot, while keeping the code logically consistent.
>
> ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c | 17 +++-
> ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S | 86
> ++++++++++++--------
> ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 5 +-
> 3 files changed, 70 insertions(+), 38 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c
> b/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c
> index 3d6eb4974d74..bd307628af87 100644
> --- a/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c
> +++ b/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c
> @@ -16,7 +16,7 @@
> #include <Uefi.h>
>
> #include <Chipset/AArch64.h>
> -
> +#include <Library/MemoryAllocationLib.h>
> #include <Protocol/DebugSupport.h> // for MAX_AARCH64_EXCEPTION
>
> UINTN gMaxExceptionNumber = MAX_AARCH64_EXCEPTION;
> @@ -25,11 +25,26 @@ EFI_EXCEPTION_CALLBACK
> gDebuggerExceptionHandlers[MAX_AARCH64_EXCEPTION + 1] =
> PHYSICAL_ADDRESS gExceptionVectorAlignmentMask =
> ARM_VECTOR_TABLE_ALIGNMENT;
> UINTN gDebuggerNoHandlerValue = 0; // todo: define for
> AArch64
>
> +#define EL0_STACK_PAGES 2
> +
> +VOID
> +RegisterEl0Stack (
> + IN VOID *Stack
> + );
> +
> RETURN_STATUS ArchVectorConfig(
> IN UINTN VectorBaseAddress
> )
> {
> UINTN HcrReg;
> + UINT8 *Stack;
> +
> + Stack = AllocatePages (EL0_STACK_PAGES);
> + if (Stack == NULL) {
> + return RETURN_OUT_OF_RESOURCES;
> + }
> +
> + RegisterEl0Stack ((UINT8 *)Stack + EFI_PAGES_TO_SIZE (EL0_STACK_PAGES));
>
> if (ArmReadCurrentEL() == AARCH64_EL2) {
> HcrReg = ArmReadHcr();
> diff --git a/ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S
> b/ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S
> index ff1f5fc81316..ac426d72a150 100644
> --- a/ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S
> +++ b/ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S
> @@ -100,6 +100,7 @@
>
> GCC_ASM_EXPORT(ExceptionHandlersEnd)
> GCC_ASM_EXPORT(CommonCExceptionHandler)
> +GCC_ASM_EXPORT(RegisterEl0Stack)
>
> .text
>
> @@ -122,35 +123,41 @@ ASM_PFX(ExceptionHandlersStart):
> VECTOR_BASE(ExceptionHandlersStart)
> #endif
>
> - .macro ExceptionEntry, val
> + .macro ExceptionEntry, val, sp=SPx
> + .ifnc \sp, SPx
> + msr SPsel, xzr
> + .endif
Is this esoteric enough to motivate a comment?
> +
> // Move the stackpointer so we can reach our structure with the str
> instruction.
> sub sp, sp, #(FP_CONTEXT_SIZE + SYS_CONTEXT_SIZE)
>
> - // Push some GP registers so we can record the exception context
> + // Push the GP registers so we can record the exception context
> stp x0, x1, [sp, #-GP_CONTEXT_SIZE]!
> stp x2, x3, [sp, #0x10]
> stp x4, x5, [sp, #0x20]
> stp x6, x7, [sp, #0x30]
> + stp x8, x9, [sp, #0x40]
> + stp x10, x11, [sp, #0x50]
> + stp x12, x13, [sp, #0x60]
> + stp x14, x15, [sp, #0x70]
> + stp x16, x17, [sp, #0x80]
> + stp x18, x19, [sp, #0x90]
> + stp x20, x21, [sp, #0xa0]
> + stp x22, x23, [sp, #0xb0]
> + stp x24, x25, [sp, #0xc0]
> + stp x26, x27, [sp, #0xd0]
> + stp x28, x29, [sp, #0xe0]
> + add x28, sp, #(GP_CONTEXT_SIZE + FP_CONTEXT_SIZE + SYS_CONTEXT_SIZE)
>
> - EL1_OR_EL2_OR_EL3(x1)
> -1:mrs x2, elr_el1 // Exception Link Register
> - mrs x3, spsr_el1 // Saved Processor Status Register 32bit
> - mrs x5, esr_el1 // EL1 Exception syndrome register 32bit
> - mrs x6, far_el1 // EL1 Fault Address Register
> - b 4f
> -
> -2:mrs x2, elr_el2 // Exception Link Register
> - mrs x3, spsr_el2 // Saved Processor Status Register 32bit
> - mrs x5, esr_el2 // EL2 Exception syndrome register 32bit
> - mrs x6, far_el2 // EL2 Fault Address Register
> - b 4f
> -
> -3:mrs x2, elr_el3 // Exception Link Register
> - mrs x3, spsr_el3 // Saved Processor Status Register 32bit
> - mrs x5, esr_el3 // EL3 Exception syndrome register 32bit
> - mrs x6, far_el3 // EL3 Fault Address Register
> + .ifnc \sp, SPx
> + msr SPsel, #1 // Switch back to read the SP value upon entry
> + mov x7, sp
> + msr SPsel, xzr
> + .else
> + mov x7, x28 // x28 contains the SP value upon entry
> + .endif
>
> -4:mrs x4, fpsr // Floating point Status Register 32bit
> + stp x30, x7, [sp, #0xf0]
>
> // Record the type of exception that occurred.
> mov x0, #\val
> @@ -189,7 +196,7 @@ ASM_PFX(SErrorSP0):
> //
> VECTOR_ENTRY(ExceptionHandlersStart, ARM_VECTOR_CUR_SPx_SYNC)
> ASM_PFX(SynchronousExceptionSPx):
> - ExceptionEntry EXCEPT_AARCH64_SYNCHRONOUS_EXCEPTIONS
> + ExceptionEntry EXCEPT_AARCH64_SYNCHRONOUS_EXCEPTIONS, SP0
>
> VECTOR_ENTRY(ExceptionHandlersStart, ARM_VECTOR_CUR_SPx_IRQ)
> ASM_PFX(IrqSPx):
> @@ -248,20 +255,25 @@ ASM_PFX(ExceptionHandlersEnd):
>
> ASM_PFX(CommonExceptionEntry):
>
> - // Stack the remaining GP registers
> - stp x8, x9, [sp, #0x40]
> - stp x10, x11, [sp, #0x50]
> - stp x12, x13, [sp, #0x60]
> - stp x14, x15, [sp, #0x70]
> - stp x16, x17, [sp, #0x80]
> - stp x18, x19, [sp, #0x90]
> - stp x20, x21, [sp, #0xa0]
> - stp x22, x23, [sp, #0xb0]
> - stp x24, x25, [sp, #0xc0]
> - stp x26, x27, [sp, #0xd0]
> - stp x28, x29, [sp, #0xe0]
> - add x28, sp, #GP_CONTEXT_SIZE + FP_CONTEXT_SIZE + SYS_CONTEXT_SIZE
> - stp x30, x28, [sp, #0xf0]
> + EL1_OR_EL2_OR_EL3(x1)
> +1:mrs x2, elr_el1 // Exception Link Register
> + mrs x3, spsr_el1 // Saved Processor Status Register 32bit
> + mrs x5, esr_el1 // EL1 Exception syndrome register 32bit
> + mrs x6, far_el1 // EL1 Fault Address Register
> + b 4f
> +
> +2:mrs x2, elr_el2 // Exception Link Register
> + mrs x3, spsr_el2 // Saved Processor Status Register 32bit
> + mrs x5, esr_el2 // EL2 Exception syndrome register 32bit
> + mrs x6, far_el2 // EL2 Fault Address Register
> + b 4f
> +
> +3:mrs x2, elr_el3 // Exception Link Register
> + mrs x3, spsr_el3 // Saved Processor Status Register 32bit
> + mrs x5, esr_el3 // EL3 Exception syndrome register 32bit
> + mrs x6, far_el3 // EL3 Fault Address Register
> +
> +4:mrs x4, fpsr // Floating point Status Register 32bit
>
> // Save the SYS regs
> stp x2, x3, [x28, #-SYS_CONTEXT_SIZE]!
> @@ -368,3 +380,7 @@ ASM_PFX(CommonExceptionEntry):
> add sp, sp, #FP_CONTEXT_SIZE + SYS_CONTEXT_SIZE
>
> eret
> +
> +ASM_PFX(RegisterEl0Stack):
> + msr sp_el0, x0
> + ret
> diff --git a/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
> b/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
> index 10d9ae0f4afc..cd9149cf76c6 100644
> --- a/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
> +++ b/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
> @@ -53,10 +53,11 @@ [Packages]
>
> [LibraryClasses]
> ArmLib
> - DebugLib
> - DefaultExceptionHandlerLib
> BaseMemoryLib
> CacheMaintenanceLib
> + DebugLib
> + DefaultExceptionHandlerLib
> + MemoryAllocationLib
>
> [Pcd]
> gArmTokenSpaceGuid.PcdDebuggerExceptionSupport
> --
> 2.7.4
>
Not tested, but it looks quite plausible, so if Eugene agrees:
Reviewed-by: Leif Lindholm <leif.lindh...@linaro.org>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
