On 13 August 2015 at 05:10, Heyi Guo <[email protected]> wrote:
> Interrupt must be disabled before we storing ELR and other system
> registers, or else ELR will be overridden by interrupt reentrance.
>
> This bug is critical as we may get occasional exception or dead loop
> when interrupt reentrance occurs:
>
>   After increasing SP ... Before popping out registers
> Or
>   After restoring ELR
>
> The 1st circumstance could also be resolved by optimizing SP operation
> (Pop out registers before adding SP back), but the 2nd could not be
> resolved by disabling interrupt.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Heyi Guo <[email protected]>
> Cc: Leif Lindholm <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> ---
>  ArmPkg/Drivers/CpuDxe/AArch64/ExceptionSupport.S | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/ExceptionSupport.S 
> b/ArmPkg/Drivers/CpuDxe/AArch64/ExceptionSupport.S
> index 2682f4f..ca6c9a1 100644
> --- a/ArmPkg/Drivers/CpuDxe/AArch64/ExceptionSupport.S
> +++ b/ArmPkg/Drivers/CpuDxe/AArch64/ExceptionSupport.S
> @@ -358,6 +358,14 @@ ASM_PFX(AsmCommonExceptionEntry):
>  #define REG_PAIR(REG1, REG2, OFFSET, CONTEXT_SIZE)    ldp  REG1, REG2, [sp, 
> #(OFFSET-CONTEXT_SIZE)]
>  #define REG_ONE(REG1, OFFSET, CONTEXT_SIZE)           ldur REG1, [sp, 
> #(OFFSET-CONTEXT_SIZE)]
>
> +  //
> +  // Disable interrupt(IRQ and FIQ) before restoring context,
> +  // or else the context will be corrupted by interrupt reentrance.
> +  // Interrupt mask will be restored from spsr by hardware when we call eret
> +  //
> +  msr   daifset, #3
> +  isb
> +

Are you sure this is necessary? According to the ARM ARM

"""
On taking any exception to an Exception level using AArch64, all of
PSTATE.{A, I, F} are set to 1, masking all
interrupts that target that Exception level.
"""

Since you are disabling interrupts after your exception handler
returns, could it be the case that that function is reenabling
interrupts too early?


>    // Adjust SP to pop system registers
>    add     sp, sp, #(GP_CONTEXT_SIZE + FP_CONTEXT_SIZE + SYS_CONTEXT_SIZE)
>    ALL_SYS_REGS
> --
> 2.1.4
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to