[edk2] [PATCH 3/3] ArmPkg/ArmExceptionLib: use EL0 stack for synchronous exceptions

Ard Biesheuvel posted 3 patches 7 years, 7 months ago
[edk2] [PATCH 3/3] ArmPkg/ArmExceptionLib: use EL0 stack for synchronous exceptions
Posted by Ard Biesheuvel 7 years, 7 months ago
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.biesheuvel@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
+
   // 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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 3/3] ArmPkg/ArmExceptionLib: use EL0 stack for synchronous exceptions
Posted by Leif Lindholm 7 years, 7 months ago
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.biesheuvel@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.lindholm@linaro.org>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 3/3] ArmPkg/ArmExceptionLib: use EL0 stack for synchronous exceptions
Posted by Ard Biesheuvel 7 years, 7 months ago
On 22 March 2017 at 13:18, Leif Lindholm <leif.lindholm@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.biesheuvel@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.lindholm@linaro.org>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 3/3] ArmPkg/ArmExceptionLib: use EL0 stack for synchronous exceptions
Posted by Ard Biesheuvel 7 years, 7 months ago
On 22 March 2017 at 13:18, Leif Lindholm <leif.lindholm@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.biesheuvel@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.lindholm@linaro.org>

No word back from Eugene, so I am going to assume that he is on board with this.

I have added the following comment

//
// Our backtrace and register dump code is written in C and so it requires
// a stack. This makes it difficult to produce meaningful diagnostics when
// the stack pointer has been corrupted. So in such cases (i.e., when taking
// synchronous exceptions), this macro is expanded with \sp set to SP0, in
// which case we switch to the SP_EL0 stack pointer, which has been
// initialized to point to a buffer that has been set aside for this purpose.
//
// Since 'sp' may no longer refer to the stack frame that was active when
// the exception was taken, we may have to switch back and forth between
// SP_EL0 and SP_ELx to record the correct value for SP in the context struct.
//

Pushed as 21dbcff5be3c
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 3/3] ArmPkg/ArmExceptionLib: use EL0 stack for synchronous exceptions
Posted by Ard Biesheuvel 7 years, 7 months ago
On 27 March 2017 at 13:54, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 22 March 2017 at 13:18, Leif Lindholm <leif.lindholm@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.biesheuvel@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.lindholm@linaro.org>
>
> No word back from Eugene, so I am going to assume that he is on board with this.
>
> I have added the following comment
>
> //
> // Our backtrace and register dump code is written in C and so it requires
> // a stack. This makes it difficult to produce meaningful diagnostics when
> // the stack pointer has been corrupted. So in such cases (i.e., when taking
> // synchronous exceptions), this macro is expanded with \sp set to SP0, in
> // which case we switch to the SP_EL0 stack pointer, which has been
> // initialized to point to a buffer that has been set aside for this purpose.
> //
> // Since 'sp' may no longer refer to the stack frame that was active when
> // the exception was taken, we may have to switch back and forth between
> // SP_EL0 and SP_ELx to record the correct value for SP in the context struct.
> //
>
> Pushed as 21dbcff5be3c

.. or rather, 2d120489583a
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel