On 22 March 2017 at 13:18, Leif Lindholm <leif.lindh...@linaro.org> wrote:
> 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?
>
Yeah, good point. This looks like a suitable spot to dedicate some
space to explain what's going on
>> +
>> // 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
