[PATCH] ArmPkg/ArmExceptionLib: use static buffer for sp_el0

[PATCH] ArmPkg/ArmExceptionLib: use static buffer for sp_el0

[guoheyi]

The exception library is also used in DxeMain before memory services
are available, and AllocatePages() will fail in this case and cause
sp_el0 remains 0. Then if any exception occurs before CpuDxe driver is
loaded, a recursive exception will be trigged by page translation
fault for sp = 0 - 0x130.

Use static buffer instead to fix this issue.

Signed-off-by: Heyi Guo <guoheyi@linux.alibaba.com>

---

Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c b/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c
index be1cdcf..1e2aeaa 100644
--- a/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c
+++ b/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c
@@ -19,7 +19,8 @@ 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
+#define EL0_STACK_SIZE  EFI_PAGES_TO_SIZE(2)
+STATIC UINTN mNewStackBase[EL0_STACK_SIZE / sizeof (UINTN)];
 
 VOID
 RegisterEl0Stack (
@@ -31,14 +32,11 @@ RETURN_STATUS ArchVectorConfig(
   )
 {
   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));
+  // Round down sp by 16 bytes alignment
+  RegisterEl0Stack (
+    (VOID *) (((UINTN) mNewStackBase + EL0_STACK_SIZE) & ~0xFUL)
+    );
 
   if (ArmReadCurrentEL() == AARCH64_EL2) {
     HcrReg = ArmReadHcr();
-- 
2.7.4

Re: [PATCH] ArmPkg/ArmExceptionLib: use static buffer for sp_el0

[Ard Biesheuvel]

On 6/9/20 3:26 AM, Heyi Guo wrote:
> The exception library is also used in DxeMain before memory services
> are available, and AllocatePages() will fail in this case and cause
> sp_el0 remains 0. Then if any exception occurs before CpuDxe driver is
> loaded, a recursive exception will be trigged by page translation
> fault for sp = 0 - 0x130.
> 
> Use static buffer instead to fix this issue.
> 
> Signed-off-by: Heyi Guo <guoheyi@linux.alibaba.com>
> 

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Merged as #679

> ---
> 
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>   ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c | 14 ++++++--------
>   1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c b/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c
> index be1cdcf..1e2aeaa 100644
> --- a/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c
> +++ b/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c
> @@ -19,7 +19,8 @@ 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
> +#define EL0_STACK_SIZE  EFI_PAGES_TO_SIZE(2)
> +STATIC UINTN mNewStackBase[EL0_STACK_SIZE / sizeof (UINTN)];
>   
>   VOID
>   RegisterEl0Stack (
> @@ -31,14 +32,11 @@ RETURN_STATUS ArchVectorConfig(
>     )
>   {
>     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));
> +  // Round down sp by 16 bytes alignment
> +  RegisterEl0Stack (
> +    (VOID *) (((UINTN) mNewStackBase + EL0_STACK_SIZE) & ~0xFUL)
> +    );
>   
>     if (ArmReadCurrentEL() == AARCH64_EL2) {
>       HcrReg = ArmReadHcr();
>