* [PATCH 0/4] ArmPkg: use console for minimal 'exception occurred' message
@ 2018-12-20 17:31 Ard Biesheuvel
2018-12-20 17:31 ` [PATCH 1/4] ArmPkg/DebugAgentSymbolsBaseLib: remove exception handling Ard Biesheuvel
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-12-20 17:31 UTC (permalink / raw)
To: edk2-devel; +Cc: leif.lindholm, philmd, lersek, Ard Biesheuvel
When running with a graphical console, no message whatsoever is printed
when the systems hits an unexpected exception and hangs, because even
the minimal 'exception occurred' message is only sent to the serial port.
So let's fix that, by updating DefaultExceptionHandlerLib to take the
availability of a console into account. (#4)
This requires some preparatory decruftication so that we can safely refer
to the system table and console (#1 .. #3).
Ard Biesheuvel (4):
ArmPkg/DebugAgentSymbolsBaseLib: remove exception handling
ArmPkg/DefaultExceptionHandlerLib: declare the permitted usage context
ArmPkg/DefaultExceptionHandlerLib: drop BASE variant
ArmPkg/DefaultExceptionHandlerLib: use console if available
ArmPkg/ArmPkg.dsc | 1 -
.../AArch64/DebugAgentException.S | 96 ------
.../Arm/DebugAgentException.S | 277 ------------------
.../Arm/DebugAgentException.asm | 273 -----------------
.../DebugAgentSymbolsBaseLib.c | 7 -
.../DebugAgentSymbolsBaseLib.inf | 9 -
.../AArch64/DefaultExceptionHandler.c | 16 +-
.../Arm/DefaultExceptionHandler.c | 7 +-
.../DefaultExceptionHandlerBase.c | 35 ---
.../DefaultExceptionHandlerLib.inf | 5 +-
.../DefaultExceptionHandlerLibBase.inf | 45 ---
ArmVirtPkg/ArmVirt.dsc.inc | 1 -
12 files changed, 22 insertions(+), 750 deletions(-)
delete mode 100644 ArmPkg/Library/DebugAgentSymbolsBaseLib/AArch64/DebugAgentException.S
delete mode 100644 ArmPkg/Library/DebugAgentSymbolsBaseLib/Arm/DebugAgentException.S
delete mode 100644 ArmPkg/Library/DebugAgentSymbolsBaseLib/Arm/DebugAgentException.asm
delete mode 100644 ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c
delete mode 100644 ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
--
2.19.2
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/4] ArmPkg/DebugAgentSymbolsBaseLib: remove exception handling 2018-12-20 17:31 [PATCH 0/4] ArmPkg: use console for minimal 'exception occurred' message Ard Biesheuvel @ 2018-12-20 17:31 ` Ard Biesheuvel 2018-12-20 17:31 ` [PATCH 2/4] ArmPkg/DefaultExceptionHandlerLib: declare the permitted usage context Ard Biesheuvel ` (3 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Ard Biesheuvel @ 2018-12-20 17:31 UTC (permalink / raw) To: edk2-devel; +Cc: leif.lindholm, philmd, lersek, Ard Biesheuvel DebugAgentSymbolsBaseLib is an optional library that is in charge of extracting debug headers from SEC and PEI_CORE images in memory so the filename and the offset in memory can be reported via the UART, allowing a developer to load debugging symbols into his debugger. Interestingly enough, DebugAgentSymbolsBaseLib is also in charge of exception handling before this duty is taken over by either the PEI core, or the CPU DXE driver when running under PrePi. Since exceptions are not actually handled at all on AArch64, and simply routed to the DefaultExceptionHandlerLib (for which a special version has been created to be usable this early), let's get rid of this dubious functionality altogether. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPkg/Library/DebugAgentSymbolsBaseLib/AArch64/DebugAgentException.S | 96 ------- ArmPkg/Library/DebugAgentSymbolsBaseLib/Arm/DebugAgentException.S | 277 -------------------- ArmPkg/Library/DebugAgentSymbolsBaseLib/Arm/DebugAgentException.asm | 273 ------------------- ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.c | 7 - ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf | 9 - 5 files changed, 662 deletions(-) diff --git a/ArmPkg/Library/DebugAgentSymbolsBaseLib/AArch64/DebugAgentException.S b/ArmPkg/Library/DebugAgentSymbolsBaseLib/AArch64/DebugAgentException.S deleted file mode 100644 index f33a07a19bad..000000000000 --- a/ArmPkg/Library/DebugAgentSymbolsBaseLib/AArch64/DebugAgentException.S +++ /dev/null @@ -1,96 +0,0 @@ -#------------------------------------------------------------------------------ -# -# Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.<BR> -# -# This program and the accompanying materials -# are licensed and made available under the terms and conditions of the BSD License -# which accompanies this distribution. The full text of the license may be found at -# http://opensource.org/licenses/bsd-license.php -# -# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, -# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. -# -#------------------------------------------------------------------------------ - -#include <Chipset/AArch64.h> - -GCC_ASM_IMPORT(DefaultExceptionHandler) - -.text -VECTOR_BASE(DebugAgentVectorTable) - -// -// Current EL with SP0 : 0x0 - 0x180 -// -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_CUR_SP0_SYNC) -ASM_PFX(SynchronousExceptionSP0): - b ASM_PFX(SynchronousExceptionSP0) - -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_CUR_SP0_IRQ) -ASM_PFX(IrqSP0): - b ASM_PFX(IrqSP0) - -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_CUR_SP0_FIQ) -ASM_PFX(FiqSP0): - b ASM_PFX(FiqSP0) - -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_CUR_SP0_SERR) -ASM_PFX(SErrorSP0): - b ASM_PFX(SErrorSP0) - -// -// Current EL with SPx: 0x200 - 0x380 -// -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_CUR_SPx_SYNC) -ASM_PFX(SynchronousExceptionSPx): - b ASM_PFX(SynchronousExceptionSPx) - -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_CUR_SPx_IRQ) -ASM_PFX(IrqSPx): - b ASM_PFX(IrqSPx) - -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_CUR_SPx_FIQ) -ASM_PFX(FiqSPx): - b ASM_PFX(FiqSPx) - -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_CUR_SPx_SERR) -ASM_PFX(SErrorSPx): - b ASM_PFX(SErrorSPx) - -/* Lower EL using AArch64 : 0x400 - 0x580 */ -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_LOW_A64_SYNC) -ASM_PFX(SynchronousExceptionA64): - b ASM_PFX(SynchronousExceptionA64) - -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_LOW_A64_IRQ) -ASM_PFX(IrqA64): - b ASM_PFX(IrqA64) - -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_LOW_A64_FIQ) -ASM_PFX(FiqA64): - b ASM_PFX(FiqA64) - -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_LOW_A64_SERR) -ASM_PFX(SErrorA64): - b ASM_PFX(SErrorA64) - -// -// Lower EL using AArch32 : 0x600 - 0x780 -// -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_LOW_A32_SYNC) -ASM_PFX(SynchronousExceptionA32): - b ASM_PFX(SynchronousExceptionA32) - -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_LOW_A32_IRQ) -ASM_PFX(IrqA32): - b ASM_PFX(IrqA32) - -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_LOW_A32_FIQ) -ASM_PFX(FiqA32): - b ASM_PFX(FiqA32) - -VECTOR_ENTRY(DebugAgentVectorTable, ARM_VECTOR_LOW_A32_SERR) -ASM_PFX(SErrorA32): - b ASM_PFX(SErrorA32) - -VECTOR_END(DebugAgentVectorTable) diff --git a/ArmPkg/Library/DebugAgentSymbolsBaseLib/Arm/DebugAgentException.S b/ArmPkg/Library/DebugAgentSymbolsBaseLib/Arm/DebugAgentException.S deleted file mode 100644 index 215181460803..000000000000 --- a/ArmPkg/Library/DebugAgentSymbolsBaseLib/Arm/DebugAgentException.S +++ /dev/null @@ -1,277 +0,0 @@ -#------------------------------------------------------------------------------ -# -# Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR> -# Copyright (c) 2011 - 2012, ARM Ltd. All rights reserved.<BR> -# -# This program and the accompanying materials -# are licensed and made available under the terms and conditions of the BSD License -# which accompanies this distribution. The full text of the license may be found at -# http://opensource.org/licenses/bsd-license.php -# -# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, -# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. -# -#------------------------------------------------------------------------------ - -#include <Library/PcdLib.h> - -/* - -This is the stack constructed by the exception handler (low address to high address) - # R0 - IFAR is EFI_SYSTEM_CONTEXT for ARM - Reg Offset - === ====== - R0 0x00 # stmfd SP!,{R0-R12} - R1 0x04 - R2 0x08 - R3 0x0c - R4 0x10 - R5 0x14 - R6 0x18 - R7 0x1c - R8 0x20 - R9 0x24 - R10 0x28 - R11 0x2c - R12 0x30 - SP 0x34 # reserved via adding 0x20 (32) to the SP - LR 0x38 - PC 0x3c - CPSR 0x40 - DFSR 0x44 - DFAR 0x48 - IFSR 0x4c - IFAR 0x50 - - LR 0x54 # SVC Link register (we need to restore it) - - LR 0x58 # pushed by srsfd - CPSR 0x5c - - */ - -GCC_ASM_EXPORT(DebugAgentVectorTable) -GCC_ASM_IMPORT(DefaultExceptionHandler) - -.text -.syntax unified -#if !defined(__APPLE__) -.fpu neon @ makes vpush/vpop assemble -#endif -.align 5 - - -// -// This code gets copied to the ARM vector table -// ExceptionHandlersStart - ExceptionHandlersEnd gets copied -// -ASM_PFX(DebugAgentVectorTable): - b ASM_PFX(ResetEntry) - b ASM_PFX(UndefinedInstructionEntry) - b ASM_PFX(SoftwareInterruptEntry) - b ASM_PFX(PrefetchAbortEntry) - b ASM_PFX(DataAbortEntry) - b ASM_PFX(ReservedExceptionEntry) - b ASM_PFX(IrqEntry) - b ASM_PFX(FiqEntry) - -ASM_PFX(ResetEntry): - srsdb #0x13! @ Store return state on SVC stack - @ We are already in SVC mode - - stmfd SP!,{LR} @ Store the link register for the current mode - sub SP,SP,#0x20 @ Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} @ Store the register state - - mov R0,#0 @ ExceptionType - ldr R1,ASM_PFX(CommonExceptionEntry) - bx R1 - -ASM_PFX(UndefinedInstructionEntry): - sub LR, LR, #4 @ Only -2 for Thumb, adjust in CommonExceptionEntry - srsdb #0x13! @ Store return state on SVC stack - cps #0x13 @ Switch to SVC for common stack - stmfd SP!,{LR} @ Store the link register for the current mode - sub SP,SP,#0x20 @ Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} @ Store the register state - - mov R0,#1 @ ExceptionType - ldr R1,ASM_PFX(CommonExceptionEntry) - bx R1 - -ASM_PFX(SoftwareInterruptEntry): - sub LR, LR, #4 @ Only -2 for Thumb, adjust in CommonExceptionEntry - srsdb #0x13! @ Store return state on SVC stack - @ We are already in SVC mode - stmfd SP!,{LR} @ Store the link register for the current mode - sub SP,SP,#0x20 @ Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} @ Store the register state - - mov R0,#2 @ ExceptionType - ldr R1,ASM_PFX(CommonExceptionEntry) - bx R1 - -ASM_PFX(PrefetchAbortEntry): - sub LR,LR,#4 - srsdb #0x13! @ Store return state on SVC stack - cps #0x13 @ Switch to SVC for common stack - stmfd SP!,{LR} @ Store the link register for the current mode - sub SP,SP,#0x20 @ Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} @ Store the register state - - mov R0,#3 @ ExceptionType - ldr R1,ASM_PFX(CommonExceptionEntry) - bx R1 - -ASM_PFX(DataAbortEntry): - sub LR,LR,#8 - srsdb #0x13! @ Store return state on SVC stack - cps #0x13 @ Switch to SVC for common stack - stmfd SP!,{LR} @ Store the link register for the current mode - sub SP,SP,#0x20 @ Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} @ Store the register state - - mov R0,#4 - ldr R1,ASM_PFX(CommonExceptionEntry) - bx R1 - -ASM_PFX(ReservedExceptionEntry): - srsdb #0x13! @ Store return state on SVC stack - cps #0x13 @ Switch to SVC for common stack - stmfd SP!,{LR} @ Store the link register for the current mode - sub SP,SP,#0x20 @ Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} @ Store the register state - - mov R0,#5 - ldr R1,ASM_PFX(CommonExceptionEntry) - bx R1 - -ASM_PFX(IrqEntry): - sub LR,LR,#4 - srsdb #0x13! @ Store return state on SVC stack - cps #0x13 @ Switch to SVC for common stack - stmfd SP!,{LR} @ Store the link register for the current mode - sub SP,SP,#0x20 @ Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} @ Store the register state - - mov R0,#6 @ ExceptionType - ldr R1,ASM_PFX(CommonExceptionEntry) - bx R1 - -ASM_PFX(FiqEntry): - sub LR,LR,#4 - srsdb #0x13! @ Store return state on SVC stack - cps #0x13 @ Switch to SVC for common stack - stmfd SP!,{LR} @ Store the link register for the current mode - sub SP,SP,#0x20 @ Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} @ Store the register state - @ Since we have already switch to SVC R8_fiq - R12_fiq - @ never get used or saved - mov R0,#7 @ ExceptionType - ldr R1,ASM_PFX(CommonExceptionEntry) - bx R1 - -// -// This gets patched by the C code that patches in the vector table -// -ASM_PFX(CommonExceptionEntry): - .word ASM_PFX(AsmCommonExceptionEntry) - -ASM_PFX(ExceptionHandlersEnd): - -// -// This code runs from CpuDxe driver loaded address. It is patched into -// CommonExceptionEntry. -// -ASM_PFX(AsmCommonExceptionEntry): - mrc p15, 0, R1, c6, c0, 2 @ Read IFAR - str R1, [SP, #0x50] @ Store it in EFI_SYSTEM_CONTEXT_ARM.IFAR - - mrc p15, 0, R1, c5, c0, 1 @ Read IFSR - str R1, [SP, #0x4c] @ Store it in EFI_SYSTEM_CONTEXT_ARM.IFSR - - mrc p15, 0, R1, c6, c0, 0 @ Read DFAR - str R1, [SP, #0x48] @ Store it in EFI_SYSTEM_CONTEXT_ARM.DFAR - - mrc p15, 0, R1, c5, c0, 0 @ Read DFSR - str R1, [SP, #0x44] @ Store it in EFI_SYSTEM_CONTEXT_ARM.DFSR - - ldr R1, [SP, #0x5c] @ srsdb saved pre-exception CPSR on the stack - str R1, [SP, #0x40] @ Store it in EFI_SYSTEM_CONTEXT_ARM.CPSR - - add R2, SP, #0x38 @ Make R2 point to EFI_SYSTEM_CONTEXT_ARM.LR - and R3, R1, #0x1f @ Check CPSR to see if User or System Mode - cmp R3, #0x1f @ if ((CPSR == 0x10) || (CPSR == 0x1df)) - cmpne R3, #0x10 @ - stmdaeq R2, {lr}^ @ save unbanked lr - @ else - stmdane R2, {lr} @ save SVC lr - - - ldr R5, [SP, #0x58] @ PC is the LR pushed by srsfd - @ Check to see if we have to adjust for Thumb entry - sub r4, r0, #1 @ if (ExceptionType == 1 || ExceptionType ==2)) { - cmp r4, #1 @ // UND & SVC have different LR adjust for Thumb - bhi NoAdjustNeeded - - tst r1, #0x20 @ if ((CPSR & T)) == T) { // Thumb Mode on entry - addne R5, R5, #2 @ PC += 2@ - str R5,[SP,#0x58] @ Update LR value pused by srsfd - -NoAdjustNeeded: - - str R5, [SP, #0x3c] @ Store it in EFI_SYSTEM_CONTEXT_ARM.PC - - sub R1, SP, #0x60 @ We pused 0x60 bytes on the stack - str R1, [SP, #0x34] @ Store it in EFI_SYSTEM_CONTEXT_ARM.SP - - @ R0 is ExceptionType - mov R1,SP @ R1 is SystemContext - -#if (FixedPcdGet32(PcdVFPEnabled)) - vpush {d0-d15} @ save vstm registers in case they are used in optimizations -#endif - -/* -VOID -EFIAPI -DefaultExceptionHandler ( - IN EFI_EXCEPTION_TYPE ExceptionType, R0 - IN OUT EFI_SYSTEM_CONTEXT SystemContext R1 - ) - -*/ - blx ASM_PFX(DefaultExceptionHandler) @ Call exception handler - -#if (FixedPcdGet32(PcdVFPEnabled)) - vpop {d0-d15} -#endif - - ldr R1, [SP, #0x4c] @ Restore EFI_SYSTEM_CONTEXT_ARM.IFSR - mcr p15, 0, R1, c5, c0, 1 @ Write IFSR - - ldr R1, [SP, #0x44] @ sRestore EFI_SYSTEM_CONTEXT_ARM.DFSR - mcr p15, 0, R1, c5, c0, 0 @ Write DFSR - - ldr R1,[SP,#0x3c] @ EFI_SYSTEM_CONTEXT_ARM.PC - str R1,[SP,#0x58] @ Store it back to srsfd stack slot so it can be restored - - ldr R1,[SP,#0x40] @ EFI_SYSTEM_CONTEXT_ARM.CPSR - str R1,[SP,#0x5c] @ Store it back to srsfd stack slot so it can be restored - - add R3, SP, #0x54 @ Make R3 point to SVC LR saved on entry - add R2, SP, #0x38 @ Make R2 point to EFI_SYSTEM_CONTEXT_ARM.LR - and R1, R1, #0x1f @ Check to see if User or System Mode - cmp R1, #0x1f @ if ((CPSR == 0x10) || (CPSR == 0x1f)) - cmpne R1, #0x10 @ - ldmibeq R2, {lr}^ @ restore unbanked lr - @ else - ldmibne R3, {lr} @ restore SVC lr, via ldmfd SP!, {LR} - - ldmfd SP!,{R0-R12} @ Restore general purpose registers - @ Exception handler can not change SP - - add SP,SP,#0x20 @ Clear out the remaining stack space - ldmfd SP!,{LR} @ restore the link register for this context - rfefd SP! @ return from exception via srsfd stack slot - diff --git a/ArmPkg/Library/DebugAgentSymbolsBaseLib/Arm/DebugAgentException.asm b/ArmPkg/Library/DebugAgentSymbolsBaseLib/Arm/DebugAgentException.asm deleted file mode 100644 index cf59447bed10..000000000000 --- a/ArmPkg/Library/DebugAgentSymbolsBaseLib/Arm/DebugAgentException.asm +++ /dev/null @@ -1,273 +0,0 @@ -//------------------------------------------------------------------------------ -// -// Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR> -// Copyright (c) 2011 - 2012, ARM Ltd. All rights reserved.<BR> -// -// This program and the accompanying materials -// are licensed and made available under the terms and conditions of the BSD License -// which accompanies this distribution. The full text of the license may be found at -// http://opensource.org/licenses/bsd-license.php -// -// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, -// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. -// -//------------------------------------------------------------------------------ - -#include <Library/PcdLib.h> - -/* - -This is the stack constructed by the exception handler (low address to high address) - # R0 - IFAR is EFI_SYSTEM_CONTEXT for ARM - Reg Offset - === ====== - R0 0x00 # stmfd SP!,{R0-R12} - R1 0x04 - R2 0x08 - R3 0x0c - R4 0x10 - R5 0x14 - R6 0x18 - R7 0x1c - R8 0x20 - R9 0x24 - R10 0x28 - R11 0x2c - R12 0x30 - SP 0x34 # reserved via adding 0x20 (32) to the SP - LR 0x38 - PC 0x3c - CPSR 0x40 - DFSR 0x44 - DFAR 0x48 - IFSR 0x4c - IFAR 0x50 - - LR 0x54 # SVC Link register (we need to restore it) - - LR 0x58 # pushed by srsfd - CPSR 0x5c - - */ - - EXPORT DebugAgentVectorTable - IMPORT DefaultExceptionHandler - - PRESERVE8 - AREA DebugAgentException, CODE, READONLY, CODEALIGN, ALIGN=5 - -// -// This code gets copied to the ARM vector table -// ExceptionHandlersStart - ExceptionHandlersEnd gets copied -// -DebugAgentVectorTable FUNCTION - b ResetEntry - b UndefinedInstructionEntry - b SoftwareInterruptEntry - b PrefetchAbortEntry - b DataAbortEntry - b ReservedExceptionEntry - b IrqEntry - b FiqEntry - ENDFUNC - -ResetEntry - srsfd #0x13! ; Store return state on SVC stack - ; We are already in SVC mode - stmfd SP!,{LR} ; Store the link register for the current mode - sub SP,SP,#0x20 ; Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} ; Store the register state - - mov R0,#0 ; ExceptionType - ldr R1,CommonExceptionEntry - bx R1 - -UndefinedInstructionEntry - sub LR, LR, #4 ; Only -2 for Thumb, adjust in CommonExceptionEntry - srsfd #0x13! ; Store return state on SVC stack - cps #0x13 ; Switch to SVC for common stack - stmfd SP!,{LR} ; Store the link register for the current mode - sub SP,SP,#0x20 ; Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} ; Store the register state - - mov R0,#1 ; ExceptionType - ldr R1,CommonExceptionEntry; - bx R1 - -SoftwareInterruptEntry - sub LR, LR, #4 ; Only -2 for Thumb, adjust in CommonExceptionEntry - srsfd #0x13! ; Store return state on SVC stack - ; We are already in SVC mode - stmfd SP!,{LR} ; Store the link register for the current mode - sub SP,SP,#0x20 ; Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} ; Store the register state - - mov R0,#2 ; ExceptionType - ldr R1,CommonExceptionEntry - bx R1 - -PrefetchAbortEntry - sub LR,LR,#4 - srsfd #0x13! ; Store return state on SVC stack - cps #0x13 ; Switch to SVC for common stack - stmfd SP!,{LR} ; Store the link register for the current mode - sub SP,SP,#0x20 ; Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} ; Store the register state - - mov R0,#3 ; ExceptionType - ldr R1,CommonExceptionEntry - bx R1 - -DataAbortEntry - sub LR,LR,#8 - srsfd #0x13! ; Store return state on SVC stack - cps #0x13 ; Switch to SVC for common stack - stmfd SP!,{LR} ; Store the link register for the current mode - sub SP,SP,#0x20 ; Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} ; Store the register state - - mov R0,#4 ; ExceptionType - ldr R1,CommonExceptionEntry - bx R1 - -ReservedExceptionEntry - srsfd #0x13! ; Store return state on SVC stack - cps #0x13 ; Switch to SVC for common stack - stmfd SP!,{LR} ; Store the link register for the current mode - sub SP,SP,#0x20 ; Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} ; Store the register state - - mov R0,#5 ; ExceptionType - ldr R1,CommonExceptionEntry - bx R1 - -IrqEntry - sub LR,LR,#4 - srsfd #0x13! ; Store return state on SVC stack - cps #0x13 ; Switch to SVC for common stack - stmfd SP!,{LR} ; Store the link register for the current mode - sub SP,SP,#0x20 ; Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} ; Store the register state - - mov R0,#6 ; ExceptionType - ldr R1,CommonExceptionEntry - bx R1 - -FiqEntry - sub LR,LR,#4 - srsfd #0x13! ; Store return state on SVC stack - cps #0x13 ; Switch to SVC for common stack - stmfd SP!,{LR} ; Store the link register for the current mode - sub SP,SP,#0x20 ; Save space for SP, LR, PC, IFAR - CPSR - stmfd SP!,{R0-R12} ; Store the register state - ; Since we have already switch to SVC R8_fiq - R12_fiq - ; never get used or saved - mov R0,#7 ; ExceptionType - ldr R1,CommonExceptionEntry - bx R1 - -// -// This gets patched by the C code that patches in the vector table -// -CommonExceptionEntry - dcd AsmCommonExceptionEntry - -ExceptionHandlersEnd - -// -// This code runs from CpuDxe driver loaded address. It is patched into -// CommonExceptionEntry. -// -AsmCommonExceptionEntry - mrc p15, 0, R1, c6, c0, 2 ; Read IFAR - str R1, [SP, #0x50] ; Store it in EFI_SYSTEM_CONTEXT_ARM.IFAR - - mrc p15, 0, R1, c5, c0, 1 ; Read IFSR - str R1, [SP, #0x4c] ; Store it in EFI_SYSTEM_CONTEXT_ARM.IFSR - - mrc p15, 0, R1, c6, c0, 0 ; Read DFAR - str R1, [SP, #0x48] ; Store it in EFI_SYSTEM_CONTEXT_ARM.DFAR - - mrc p15, 0, R1, c5, c0, 0 ; Read DFSR - str R1, [SP, #0x44] ; Store it in EFI_SYSTEM_CONTEXT_ARM.DFSR - - ldr R1, [SP, #0x5c] ; srsfd saved pre-exception CPSR on the stack - str R1, [SP, #0x40] ; Store it in EFI_SYSTEM_CONTEXT_ARM.CPSR - - add R2, SP, #0x38 ; Make R2 point to EFI_SYSTEM_CONTEXT_ARM.LR - and R3, R1, #0x1f ; Check CPSR to see if User or System Mode - cmp R3, #0x1f ; if ((CPSR == 0x10) || (CPSR == 0x1df)) - cmpne R3, #0x10 ; - stmeqed R2, {lr}^ ; save unbanked lr - ; else - stmneed R2, {lr} ; save SVC lr - - - ldr R5, [SP, #0x58] ; PC is the LR pushed by srsfd - ; Check to see if we have to adjust for Thumb entry - sub r4, r0, #1 ; if (ExceptionType == 1 || ExceptionType ==2)) { - cmp r4, #1 ; // UND & SVC have different LR adjust for Thumb - bhi NoAdjustNeeded - - tst r1, #0x20 ; if ((CPSR & T)) == T) { // Thumb Mode on entry - addne R5, R5, #2 ; PC += 2; - str R5,[SP,#0x58] ; Update LR value pused by srsfd - -NoAdjustNeeded - - str R5, [SP, #0x3c] ; Store it in EFI_SYSTEM_CONTEXT_ARM.PC - - sub R1, SP, #0x60 ; We pused 0x60 bytes on the stack - str R1, [SP, #0x34] ; Store it in EFI_SYSTEM_CONTEXT_ARM.SP - - ; R0 is ExceptionType - mov R1,SP ; R1 is SystemContext - -#if (FixedPcdGet32(PcdVFPEnabled)) - vpush {d0-d15} ; save vstm registers in case they are used in optimizations -#endif - -/* -VOID -EFIAPI -DefaultExceptionHandler ( - IN EFI_EXCEPTION_TYPE ExceptionType, R0 - IN OUT EFI_SYSTEM_CONTEXT SystemContext R1 - ) - -*/ - blx DefaultExceptionHandler ; Call exception handler - -#if (FixedPcdGet32(PcdVFPEnabled)) - vpop {d0-d15} -#endif - - ldr R1, [SP, #0x4c] ; Restore EFI_SYSTEM_CONTEXT_ARM.IFSR - mcr p15, 0, R1, c5, c0, 1 ; Write IFSR - - ldr R1, [SP, #0x44] ; sRestore EFI_SYSTEM_CONTEXT_ARM.DFSR - mcr p15, 0, R1, c5, c0, 0 ; Write DFSR - - ldr R1,[SP,#0x3c] ; EFI_SYSTEM_CONTEXT_ARM.PC - str R1,[SP,#0x58] ; Store it back to srsfd stack slot so it can be restored - - ldr R1,[SP,#0x40] ; EFI_SYSTEM_CONTEXT_ARM.CPSR - str R1,[SP,#0x5c] ; Store it back to srsfd stack slot so it can be restored - - add R3, SP, #0x54 ; Make R3 point to SVC LR saved on entry - add R2, SP, #0x38 ; Make R2 point to EFI_SYSTEM_CONTEXT_ARM.LR - and R1, R1, #0x1f ; Check to see if User or System Mode - cmp R1, #0x1f ; if ((CPSR == 0x10) || (CPSR == 0x1f)) - cmpne R1, #0x10 ; - ldmeqed R2, {lr}^ ; restore unbanked lr - ; else - ldmneed R3, {lr} ; restore SVC lr, via ldmfd SP!, {LR} - - ldmfd SP!,{R0-R12} ; Restore general purpose registers - ; Exception handler can not change SP - - add SP,SP,#0x20 ; Clear out the remaining stack space - ldmfd SP!,{LR} ; restore the link register for this context - rfefd SP! ; return from exception via srsfd stack slot - - END diff --git a/ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.c b/ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.c index 9c0cf0de38e1..f47f4250d1e8 100644 --- a/ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.c +++ b/ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.c @@ -14,7 +14,6 @@ **/ #include <Uefi.h> -#include <Library/ArmLib.h> #include <Library/BaseLib.h> #include <Library/BaseMemoryLib.h> #include <Library/DebugLib.h> @@ -283,12 +282,6 @@ InitializeDebugAgent ( EFI_FFS_FILE_HEADER *FfsHeader; PE_COFF_LOADER_IMAGE_CONTEXT ImageContext; - // Now we've got UART, check the Debug Agent Vector Table - // Note: The AArch64 Vector table must be 2k-byte aligned - if this assertion fails ensure - // 'Align=4K' is defined into your FDF for this module. - ASSERT (((UINTN)DebugAgentVectorTable & ARM_VECTOR_TABLE_ALIGNMENT) == 0); - ArmWriteVBar ((UINTN)DebugAgentVectorTable); - // We use InitFlag to know if DebugAgent has been initialized from // Sec (DEBUG_AGENT_INIT_PREMEM_SEC) or PrePi (DEBUG_AGENT_INIT_POSTMEM_SEC) // modules diff --git a/ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf b/ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf index 6b784f749b5e..671c1b6474b2 100644 --- a/ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf +++ b/ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf @@ -22,22 +22,13 @@ [Sources.common] DebugAgentSymbolsBaseLib.c -[Sources.ARM] - Arm/DebugAgentException.asm | RVCT - Arm/DebugAgentException.S | GCC - -[Sources.AARCH64] - AArch64/DebugAgentException.S - [Packages] MdePkg/MdePkg.dec MdeModulePkg/MdeModulePkg.dec ArmPkg/ArmPkg.dec [LibraryClasses] - ArmLib DebugLib - DefaultExceptionHandlerLib PcdLib PeCoffExtraActionLib PeCoffLib -- 2.19.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] ArmPkg/DefaultExceptionHandlerLib: declare the permitted usage context 2018-12-20 17:31 [PATCH 0/4] ArmPkg: use console for minimal 'exception occurred' message Ard Biesheuvel 2018-12-20 17:31 ` [PATCH 1/4] ArmPkg/DebugAgentSymbolsBaseLib: remove exception handling Ard Biesheuvel @ 2018-12-20 17:31 ` Ard Biesheuvel 2018-12-20 17:31 ` [PATCH 3/4] ArmPkg/DefaultExceptionHandlerLib: drop BASE variant Ard Biesheuvel ` (2 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Ard Biesheuvel @ 2018-12-20 17:31 UTC (permalink / raw) To: edk2-devel; +Cc: leif.lindholm, philmd, lersek, Ard Biesheuvel Declare that this library is only usable in the context of DXE core or a DXE driver. Set the MODULE_TYPE to BASE: this only affects the prototype of the constructor (if present) but doesn't actually restrict the usage context otherwise. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf index f5421b1240a1..7609f82d89a1 100644 --- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf @@ -17,9 +17,9 @@ INF_VERSION = 0x00010005 BASE_NAME = DefaultExceptionHandlerLib FILE_GUID = EACDB354-DF1A-4AF9-A171-499737ED818F - MODULE_TYPE = UEFI_DRIVER + MODULE_TYPE = BASE VERSION_STRING = 1.0 - LIBRARY_CLASS = DefaultExceptionHandlerLib + LIBRARY_CLASS = DefaultExceptionHandlerLib|DXE_CORE DXE_DRIVER [Sources.common] DefaultExceptionHandlerUefi.c -- 2.19.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] ArmPkg/DefaultExceptionHandlerLib: drop BASE variant 2018-12-20 17:31 [PATCH 0/4] ArmPkg: use console for minimal 'exception occurred' message Ard Biesheuvel 2018-12-20 17:31 ` [PATCH 1/4] ArmPkg/DebugAgentSymbolsBaseLib: remove exception handling Ard Biesheuvel 2018-12-20 17:31 ` [PATCH 2/4] ArmPkg/DefaultExceptionHandlerLib: declare the permitted usage context Ard Biesheuvel @ 2018-12-20 17:31 ` Ard Biesheuvel 2018-12-26 21:27 ` Laszlo Ersek 2018-12-20 17:31 ` [PATCH 4/4] ArmPkg/DefaultExceptionHandlerLib: use console if available Ard Biesheuvel 2019-01-11 17:49 ` [PATCH 0/4] ArmPkg: use console for minimal 'exception occurred' message Leif Lindholm 4 siblings, 1 reply; 10+ messages in thread From: Ard Biesheuvel @ 2018-12-20 17:31 UTC (permalink / raw) To: edk2-devel; +Cc: leif.lindholm, philmd, lersek, Ard Biesheuvel Drop the redundant BASE variant, which is no longer used anywhere now that DebugAgentSymbolsBaseLib no longer incorporates a vector table and exception handling. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPkg/ArmPkg.dsc | 1 - ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c | 35 --------------- ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf | 45 -------------------- ArmVirtPkg/ArmVirt.dsc.inc | 1 - 4 files changed, 82 deletions(-) diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc index 5d83c18b143e..d9f9935d70b6 100644 --- a/ArmPkg/ArmPkg.dsc +++ b/ArmPkg/ArmPkg.dsc @@ -141,7 +141,6 @@ ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf ArmPkg/Library/ArmLib/ArmBaseLib.inf ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf - ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c deleted file mode 100644 index 4a54298b1189..000000000000 --- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c +++ /dev/null @@ -1,35 +0,0 @@ -/** @file - - Copyright (c) 2012, ARM Ltd. All rights reserved.<BR> - - This program and the accompanying materials - are licensed and made available under the terms and conditions of the BSD License - which accompanies this distribution. The full text of the license may be found at - http://opensource.org/licenses/bsd-license.php - - THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, - WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. - -**/ - -#include <Base.h> - -/** - - @param FaultAddress Address to find PE/COFF image for. - @param ImageBase Return load address of found image - @param PeCoffSizeOfHeaders Return the size of the PE/COFF header for the image that was found - - @retval NULL FaultAddress not in a loaded PE/COFF image. - @retval Path and file name of PE/COFF image. - -**/ -CHAR8 * -GetImageName ( - IN UINTN FaultAddress, - OUT UINTN *ImageBase, - OUT UINTN *PeCoffSizeOfHeaders - ) -{ - return NULL; -} diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf deleted file mode 100644 index b53a5e89f507..000000000000 --- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf +++ /dev/null @@ -1,45 +0,0 @@ -#/** @file -# -# Copyright (c) 2012, ARM Ltd. All rights reserved.<BR> -# -# This program and the accompanying materials -# are licensed and made available under the terms and conditions of the BSD License -# which accompanies this distribution. The full text of the license may be found at -# http://opensource.org/licenses/bsd-license.php -# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, -# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. -# -# -#**/ - -[Defines] - INF_VERSION = 0x00010005 - BASE_NAME = DefaultExceptionHandlerBaseLib - FILE_GUID = 3d5261d5-5eb7-4559-98e7-475aa9d0dc42 - MODULE_TYPE = BASE - VERSION_STRING = 1.0 - LIBRARY_CLASS = DefaultExceptionHandlerLib - -[Sources.common] - DefaultExceptionHandlerBase.c - -[Sources.ARM] - Arm/DefaultExceptionHandler.c - -[Sources.AARCH64] - AArch64/DefaultExceptionHandler.c - -[Packages] - MdePkg/MdePkg.dec - ArmPkg/ArmPkg.dec - -[LibraryClasses] - BaseLib - PrintLib - DebugLib - PeCoffGetEntryPointLib - ArmDisassemblerLib - SerialPortLib - -[Guids] - gEfiDebugImageInfoTableGuid diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc index 89c2db074711..c47955be940c 100644 --- a/ArmVirtPkg/ArmVirt.dsc.inc +++ b/ArmVirtPkg/ArmVirt.dsc.inc @@ -174,7 +174,6 @@ BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf DebugAgentLib|ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf - DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf SerialPortLib|ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf -- 2.19.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] ArmPkg/DefaultExceptionHandlerLib: drop BASE variant 2018-12-20 17:31 ` [PATCH 3/4] ArmPkg/DefaultExceptionHandlerLib: drop BASE variant Ard Biesheuvel @ 2018-12-26 21:27 ` Laszlo Ersek 2018-12-27 9:43 ` Ard Biesheuvel 0 siblings, 1 reply; 10+ messages in thread From: Laszlo Ersek @ 2018-12-26 21:27 UTC (permalink / raw) To: Ard Biesheuvel, edk2-devel On 12/20/18 18:31, Ard Biesheuvel wrote: > Drop the redundant BASE variant, which is no longer used anywhere > now that DebugAgentSymbolsBaseLib no longer incorporates a vector > table and exception handling. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmPkg/ArmPkg.dsc | 1 - > ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c | 35 --------------- > ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf | 45 -------------------- > ArmVirtPkg/ArmVirt.dsc.inc | 1 - > 4 files changed, 82 deletions(-) > > diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc > index 5d83c18b143e..d9f9935d70b6 100644 > --- a/ArmPkg/ArmPkg.dsc > +++ b/ArmPkg/ArmPkg.dsc > @@ -141,7 +141,6 @@ > ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf > ArmPkg/Library/ArmLib/ArmBaseLib.inf > ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf > - ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf > ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf > ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > > diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c > deleted file mode 100644 > index 4a54298b1189..000000000000 > --- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c > +++ /dev/null > @@ -1,35 +0,0 @@ > -/** @file > - > - Copyright (c) 2012, ARM Ltd. All rights reserved.<BR> > - > - This program and the accompanying materials > - are licensed and made available under the terms and conditions of the BSD License > - which accompanies this distribution. The full text of the license may be found at > - http://opensource.org/licenses/bsd-license.php > - > - THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > - WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > - > -**/ > - > -#include <Base.h> > - > -/** > - > - @param FaultAddress Address to find PE/COFF image for. > - @param ImageBase Return load address of found image > - @param PeCoffSizeOfHeaders Return the size of the PE/COFF header for the image that was found > - > - @retval NULL FaultAddress not in a loaded PE/COFF image. > - @retval Path and file name of PE/COFF image. > - > -**/ > -CHAR8 * > -GetImageName ( > - IN UINTN FaultAddress, > - OUT UINTN *ImageBase, > - OUT UINTN *PeCoffSizeOfHeaders > - ) > -{ > - return NULL; > -} > diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf > deleted file mode 100644 > index b53a5e89f507..000000000000 > --- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf > +++ /dev/null > @@ -1,45 +0,0 @@ > -#/** @file > -# > -# Copyright (c) 2012, ARM Ltd. All rights reserved.<BR> > -# > -# This program and the accompanying materials > -# are licensed and made available under the terms and conditions of the BSD License > -# which accompanies this distribution. The full text of the license may be found at > -# http://opensource.org/licenses/bsd-license.php > -# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > -# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > -# > -# > -#**/ > - > -[Defines] > - INF_VERSION = 0x00010005 > - BASE_NAME = DefaultExceptionHandlerBaseLib > - FILE_GUID = 3d5261d5-5eb7-4559-98e7-475aa9d0dc42 > - MODULE_TYPE = BASE > - VERSION_STRING = 1.0 > - LIBRARY_CLASS = DefaultExceptionHandlerLib > - > -[Sources.common] > - DefaultExceptionHandlerBase.c > - > -[Sources.ARM] > - Arm/DefaultExceptionHandler.c > - > -[Sources.AARCH64] > - AArch64/DefaultExceptionHandler.c > - > -[Packages] > - MdePkg/MdePkg.dec > - ArmPkg/ArmPkg.dec > - > -[LibraryClasses] > - BaseLib > - PrintLib > - DebugLib > - PeCoffGetEntryPointLib > - ArmDisassemblerLib > - SerialPortLib > - > -[Guids] > - gEfiDebugImageInfoTableGuid > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc > index 89c2db074711..c47955be940c 100644 > --- a/ArmVirtPkg/ArmVirt.dsc.inc > +++ b/ArmVirtPkg/ArmVirt.dsc.inc > @@ -174,7 +174,6 @@ > BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf > > DebugAgentLib|ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf > - DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf > SerialPortLib|ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf > HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf > PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf > Can you split the ArmVirtPkg hunk to a separate patch? I think that, after patch #1, it should be possible to remove the [LibraryClasses.common.SEC] resolution from "ArmVirt.dsc.inc". Then the present patch can continue saying "... no longer used anywhere". Such a split would be more idiomatic to edk2, and also more faithful to the current subject line (which says "ArmPkg/DefaultExceptionHandlerLib: ..."). Thanks Laszlo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] ArmPkg/DefaultExceptionHandlerLib: drop BASE variant 2018-12-26 21:27 ` Laszlo Ersek @ 2018-12-27 9:43 ` Ard Biesheuvel 0 siblings, 0 replies; 10+ messages in thread From: Ard Biesheuvel @ 2018-12-27 9:43 UTC (permalink / raw) To: Laszlo Ersek Cc: edk2-devel@lists.01.org, Leif Lindholm, Philippe Mathieu-Daudé On Wed, 26 Dec 2018 at 22:27, Laszlo Ersek <lersek@redhat.com> wrote: > > On 12/20/18 18:31, Ard Biesheuvel wrote: > > Drop the redundant BASE variant, which is no longer used anywhere > > now that DebugAgentSymbolsBaseLib no longer incorporates a vector > > table and exception handling. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > ArmPkg/ArmPkg.dsc | 1 - > > ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c | 35 --------------- > > ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf | 45 -------------------- > > ArmVirtPkg/ArmVirt.dsc.inc | 1 - > > 4 files changed, 82 deletions(-) > > > > diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc > > index 5d83c18b143e..d9f9935d70b6 100644 > > --- a/ArmPkg/ArmPkg.dsc > > +++ b/ArmPkg/ArmPkg.dsc > > @@ -141,7 +141,6 @@ > > ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf > > ArmPkg/Library/ArmLib/ArmBaseLib.inf > > ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf > > - ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf > > ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf > > ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > > > > diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c > > deleted file mode 100644 > > index 4a54298b1189..000000000000 > > --- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c > > +++ /dev/null > > @@ -1,35 +0,0 @@ > > -/** @file > > - > > - Copyright (c) 2012, ARM Ltd. All rights reserved.<BR> > > - > > - This program and the accompanying materials > > - are licensed and made available under the terms and conditions of the BSD License > > - which accompanies this distribution. The full text of the license may be found at > > - http://opensource.org/licenses/bsd-license.php > > - > > - THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > > - WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > - > > -**/ > > - > > -#include <Base.h> > > - > > -/** > > - > > - @param FaultAddress Address to find PE/COFF image for. > > - @param ImageBase Return load address of found image > > - @param PeCoffSizeOfHeaders Return the size of the PE/COFF header for the image that was found > > - > > - @retval NULL FaultAddress not in a loaded PE/COFF image. > > - @retval Path and file name of PE/COFF image. > > - > > -**/ > > -CHAR8 * > > -GetImageName ( > > - IN UINTN FaultAddress, > > - OUT UINTN *ImageBase, > > - OUT UINTN *PeCoffSizeOfHeaders > > - ) > > -{ > > - return NULL; > > -} > > diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf > > deleted file mode 100644 > > index b53a5e89f507..000000000000 > > --- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf > > +++ /dev/null > > @@ -1,45 +0,0 @@ > > -#/** @file > > -# > > -# Copyright (c) 2012, ARM Ltd. All rights reserved.<BR> > > -# > > -# This program and the accompanying materials > > -# are licensed and made available under the terms and conditions of the BSD License > > -# which accompanies this distribution. The full text of the license may be found at > > -# http://opensource.org/licenses/bsd-license.php > > -# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > > -# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > -# > > -# > > -#**/ > > - > > -[Defines] > > - INF_VERSION = 0x00010005 > > - BASE_NAME = DefaultExceptionHandlerBaseLib > > - FILE_GUID = 3d5261d5-5eb7-4559-98e7-475aa9d0dc42 > > - MODULE_TYPE = BASE > > - VERSION_STRING = 1.0 > > - LIBRARY_CLASS = DefaultExceptionHandlerLib > > - > > -[Sources.common] > > - DefaultExceptionHandlerBase.c > > - > > -[Sources.ARM] > > - Arm/DefaultExceptionHandler.c > > - > > -[Sources.AARCH64] > > - AArch64/DefaultExceptionHandler.c > > - > > -[Packages] > > - MdePkg/MdePkg.dec > > - ArmPkg/ArmPkg.dec > > - > > -[LibraryClasses] > > - BaseLib > > - PrintLib > > - DebugLib > > - PeCoffGetEntryPointLib > > - ArmDisassemblerLib > > - SerialPortLib > > - > > -[Guids] > > - gEfiDebugImageInfoTableGuid > > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc > > index 89c2db074711..c47955be940c 100644 > > --- a/ArmVirtPkg/ArmVirt.dsc.inc > > +++ b/ArmVirtPkg/ArmVirt.dsc.inc > > @@ -174,7 +174,6 @@ > > BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf > > > > DebugAgentLib|ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf > > - DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf > > SerialPortLib|ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf > > HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf > > PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf > > > > Can you split the ArmVirtPkg hunk to a separate patch? I think that, > after patch #1, it should be possible to remove the > [LibraryClasses.common.SEC] resolution from "ArmVirt.dsc.inc". Then the > present patch can continue saying "... no longer used anywhere". > > Such a split would be more idiomatic to edk2, and also more faithful to > the current subject line (which says "ArmPkg/DefaultExceptionHandlerLib: > ..."). > Sure ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] ArmPkg/DefaultExceptionHandlerLib: use console if available 2018-12-20 17:31 [PATCH 0/4] ArmPkg: use console for minimal 'exception occurred' message Ard Biesheuvel ` (2 preceding siblings ...) 2018-12-20 17:31 ` [PATCH 3/4] ArmPkg/DefaultExceptionHandlerLib: drop BASE variant Ard Biesheuvel @ 2018-12-20 17:31 ` Ard Biesheuvel 2018-12-26 21:33 ` Laszlo Ersek 2019-01-11 17:49 ` [PATCH 0/4] ArmPkg: use console for minimal 'exception occurred' message Leif Lindholm 4 siblings, 1 reply; 10+ messages in thread From: Ard Biesheuvel @ 2018-12-20 17:31 UTC (permalink / raw) To: edk2-devel; +Cc: leif.lindholm, philmd, lersek, Ard Biesheuvel Print the minimal 'exception occurred' message to the console instead of straight to the serial port if the console is available. This makes such messages visible on systems where the console is graphical and the serial is not connected. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c | 16 +++++++++++++--- ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c | 7 ++++++- ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf | 1 + 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c index 1024bf48c63d..1aaf3c88f21e 100644 --- a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c @@ -22,6 +22,7 @@ #include <Library/PrintLib.h> #include <Library/ArmDisassemblerLib.h> #include <Library/SerialPortLib.h> +#include <Library/UefiBootServicesTableLib.h> #include <Guid/DebugImageInfoTable.h> #include <Protocol/DebugSupport.h> @@ -159,14 +160,23 @@ DefaultExceptionHandler ( INT32 Offset; if (mRecursiveException) { - CharCount = AsciiSPrint (Buffer, sizeof (Buffer),"\nRecursive exception occurred while dumping the CPU state\n"); - SerialPortWrite ((UINT8 *) Buffer, CharCount); + STATIC CHAR8 CONST Message[] = "\nRecursive exception occurred while dumping the CPU state\n"; + + if (gST->ConOut != NULL) { + AsciiPrint (Message); + } else { + SerialPortWrite ((UINT8 *)Message, AsciiStrLen (Message)); + } CpuDeadLoop (); } mRecursiveException = TRUE; CharCount = AsciiSPrint (Buffer,sizeof (Buffer),"\n\n%a Exception at 0x%016lx\n", gExceptionTypeString[ExceptionType], SystemContext.SystemContextAArch64->ELR); - SerialPortWrite ((UINT8 *) Buffer, CharCount); + if (gST->ConOut != NULL) { + AsciiPrint (Buffer); + } else { + SerialPortWrite ((UINT8 *)Buffer, CharCount); + } DEBUG_CODE_BEGIN (); CHAR8 *Pdb, *PrevPdb; diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c index 0b9da031b47d..9159b579da6f 100644 --- a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c @@ -21,6 +21,7 @@ #include <Library/PrintLib.h> #include <Library/ArmDisassemblerLib.h> #include <Library/SerialPortLib.h> +#include <Library/UefiBootServicesTableLib.h> #include <Guid/DebugImageInfoTable.h> @@ -194,7 +195,11 @@ DefaultExceptionHandler ( CharCount = AsciiSPrint (Buffer,sizeof (Buffer),"\n%a Exception PC at 0x%08x CPSR 0x%08x ", gExceptionTypeString[ExceptionType], SystemContext.SystemContextArm->PC, SystemContext.SystemContextArm->CPSR); - SerialPortWrite ((UINT8 *) Buffer, CharCount); + if (gST->ConOut != NULL) { + AsciiPrint (Buffer); + } else { + SerialPortWrite ((UINT8 *)Buffer, CharCount); + } DEBUG_CODE_BEGIN (); CHAR8 *Pdb; diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf index 7609f82d89a1..6bc48714c9dc 100644 --- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf @@ -42,6 +42,7 @@ PeCoffGetEntryPointLib ArmDisassemblerLib SerialPortLib + UefiBootServicesTableLib [Guids] gEfiDebugImageInfoTableGuid -- 2.19.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] ArmPkg/DefaultExceptionHandlerLib: use console if available 2018-12-20 17:31 ` [PATCH 4/4] ArmPkg/DefaultExceptionHandlerLib: use console if available Ard Biesheuvel @ 2018-12-26 21:33 ` Laszlo Ersek 2018-12-27 9:43 ` Ard Biesheuvel 0 siblings, 1 reply; 10+ messages in thread From: Laszlo Ersek @ 2018-12-26 21:33 UTC (permalink / raw) To: Ard Biesheuvel, edk2-devel On 12/20/18 18:31, Ard Biesheuvel wrote: > Print the minimal 'exception occurred' message to the console instead > of straight to the serial port if the console is available. This makes > such messages visible on systems where the console is graphical and > the serial is not connected. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c | 16 +++++++++++++--- > ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c | 7 ++++++- > ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf | 1 + > 3 files changed, 20 insertions(+), 4 deletions(-) > > diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c > index 1024bf48c63d..1aaf3c88f21e 100644 > --- a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c > +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c > @@ -22,6 +22,7 @@ > #include <Library/PrintLib.h> > #include <Library/ArmDisassemblerLib.h> > #include <Library/SerialPortLib.h> > +#include <Library/UefiBootServicesTableLib.h> > > #include <Guid/DebugImageInfoTable.h> > #include <Protocol/DebugSupport.h> > @@ -159,14 +160,23 @@ DefaultExceptionHandler ( > INT32 Offset; > > if (mRecursiveException) { > - CharCount = AsciiSPrint (Buffer, sizeof (Buffer),"\nRecursive exception occurred while dumping the CPU state\n"); > - SerialPortWrite ((UINT8 *) Buffer, CharCount); > + STATIC CHAR8 CONST Message[] = "\nRecursive exception occurred while dumping the CPU state\n"; > + > + if (gST->ConOut != NULL) { > + AsciiPrint (Message); > + } else { > + SerialPortWrite ((UINT8 *)Message, AsciiStrLen (Message)); > + } > CpuDeadLoop (); > } > mRecursiveException = TRUE; > > CharCount = AsciiSPrint (Buffer,sizeof (Buffer),"\n\n%a Exception at 0x%016lx\n", gExceptionTypeString[ExceptionType], SystemContext.SystemContextAArch64->ELR); > - SerialPortWrite ((UINT8 *) Buffer, CharCount); > + if (gST->ConOut != NULL) { > + AsciiPrint (Buffer); > + } else { > + SerialPortWrite ((UINT8 *)Buffer, CharCount); > + } > > DEBUG_CODE_BEGIN (); > CHAR8 *Pdb, *PrevPdb; > diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c > index 0b9da031b47d..9159b579da6f 100644 > --- a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c > +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c > @@ -21,6 +21,7 @@ > #include <Library/PrintLib.h> > #include <Library/ArmDisassemblerLib.h> > #include <Library/SerialPortLib.h> > +#include <Library/UefiBootServicesTableLib.h> > > #include <Guid/DebugImageInfoTable.h> > > @@ -194,7 +195,11 @@ DefaultExceptionHandler ( > > CharCount = AsciiSPrint (Buffer,sizeof (Buffer),"\n%a Exception PC at 0x%08x CPSR 0x%08x ", > gExceptionTypeString[ExceptionType], SystemContext.SystemContextArm->PC, SystemContext.SystemContextArm->CPSR); > - SerialPortWrite ((UINT8 *) Buffer, CharCount); > + if (gST->ConOut != NULL) { > + AsciiPrint (Buffer); > + } else { > + SerialPortWrite ((UINT8 *)Buffer, CharCount); > + } > > DEBUG_CODE_BEGIN (); > CHAR8 *Pdb; > diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf > index 7609f82d89a1..6bc48714c9dc 100644 > --- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf > +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf > @@ -42,6 +42,7 @@ > PeCoffGetEntryPointLib > ArmDisassemblerLib > SerialPortLib > + UefiBootServicesTableLib > > [Guids] > gEfiDebugImageInfoTableGuid > I feel that invoking such high level functionality from an exception handler is risky, but I do understand this is a last resort / best effort action, and if it *happens* to work on systems with no serial, then it's already a win. However, I'd suggest improving the robustness by preserving the serial write first, and then performing the (optional) console write in addition, second. I guess it can lead to the message appearing twice on serial (if the console is available, and it happens to be multiplexed to the serial port as well), but I think that's a better compromise than possibly losing the message altogether. I don't feel strongly about it either way, I just thought this worth raising. Thanks, Laszlo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] ArmPkg/DefaultExceptionHandlerLib: use console if available 2018-12-26 21:33 ` Laszlo Ersek @ 2018-12-27 9:43 ` Ard Biesheuvel 0 siblings, 0 replies; 10+ messages in thread From: Ard Biesheuvel @ 2018-12-27 9:43 UTC (permalink / raw) To: Laszlo Ersek; +Cc: edk2-devel@lists.01.org On Wed, 26 Dec 2018 at 22:34, Laszlo Ersek <lersek@redhat.com> wrote: > > On 12/20/18 18:31, Ard Biesheuvel wrote: > > Print the minimal 'exception occurred' message to the console instead > > of straight to the serial port if the console is available. This makes > > such messages visible on systems where the console is graphical and > > the serial is not connected. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c | 16 +++++++++++++--- > > ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c | 7 ++++++- > > ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf | 1 + > > 3 files changed, 20 insertions(+), 4 deletions(-) > > > > diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c > > index 1024bf48c63d..1aaf3c88f21e 100644 > > --- a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c > > +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c > > @@ -22,6 +22,7 @@ > > #include <Library/PrintLib.h> > > #include <Library/ArmDisassemblerLib.h> > > #include <Library/SerialPortLib.h> > > +#include <Library/UefiBootServicesTableLib.h> > > > > #include <Guid/DebugImageInfoTable.h> > > #include <Protocol/DebugSupport.h> > > @@ -159,14 +160,23 @@ DefaultExceptionHandler ( > > INT32 Offset; > > > > if (mRecursiveException) { > > - CharCount = AsciiSPrint (Buffer, sizeof (Buffer),"\nRecursive exception occurred while dumping the CPU state\n"); > > - SerialPortWrite ((UINT8 *) Buffer, CharCount); > > + STATIC CHAR8 CONST Message[] = "\nRecursive exception occurred while dumping the CPU state\n"; > > + > > + if (gST->ConOut != NULL) { > > + AsciiPrint (Message); > > + } else { > > + SerialPortWrite ((UINT8 *)Message, AsciiStrLen (Message)); > > + } > > CpuDeadLoop (); > > } > > mRecursiveException = TRUE; > > > > CharCount = AsciiSPrint (Buffer,sizeof (Buffer),"\n\n%a Exception at 0x%016lx\n", gExceptionTypeString[ExceptionType], SystemContext.SystemContextAArch64->ELR); > > - SerialPortWrite ((UINT8 *) Buffer, CharCount); > > + if (gST->ConOut != NULL) { > > + AsciiPrint (Buffer); > > + } else { > > + SerialPortWrite ((UINT8 *)Buffer, CharCount); > > + } > > > > DEBUG_CODE_BEGIN (); > > CHAR8 *Pdb, *PrevPdb; > > diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c > > index 0b9da031b47d..9159b579da6f 100644 > > --- a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c > > +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c > > @@ -21,6 +21,7 @@ > > #include <Library/PrintLib.h> > > #include <Library/ArmDisassemblerLib.h> > > #include <Library/SerialPortLib.h> > > +#include <Library/UefiBootServicesTableLib.h> > > > > #include <Guid/DebugImageInfoTable.h> > > > > @@ -194,7 +195,11 @@ DefaultExceptionHandler ( > > > > CharCount = AsciiSPrint (Buffer,sizeof (Buffer),"\n%a Exception PC at 0x%08x CPSR 0x%08x ", > > gExceptionTypeString[ExceptionType], SystemContext.SystemContextArm->PC, SystemContext.SystemContextArm->CPSR); > > - SerialPortWrite ((UINT8 *) Buffer, CharCount); > > + if (gST->ConOut != NULL) { > > + AsciiPrint (Buffer); > > + } else { > > + SerialPortWrite ((UINT8 *)Buffer, CharCount); > > + } > > > > DEBUG_CODE_BEGIN (); > > CHAR8 *Pdb; > > diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf > > index 7609f82d89a1..6bc48714c9dc 100644 > > --- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf > > +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf > > @@ -42,6 +42,7 @@ > > PeCoffGetEntryPointLib > > ArmDisassemblerLib > > SerialPortLib > > + UefiBootServicesTableLib > > > > [Guids] > > gEfiDebugImageInfoTableGuid > > > > I feel that invoking such high level functionality from an exception > handler is risky, but I do understand this is a last resort / best > effort action, and if it *happens* to work on systems with no serial, > then it's already a win. > > However, I'd suggest improving the robustness by preserving the serial > write first, and then performing the (optional) console write in > addition, second. I guess it can lead to the message appearing twice on > serial (if the console is available, and it happens to be multiplexed to > the serial port as well), but I think that's a better compromise than > possibly losing the message altogether. > > I don't feel strongly about it either way, I just thought this worth > raising. > Excellent point. I'll change this in v2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/4] ArmPkg: use console for minimal 'exception occurred' message 2018-12-20 17:31 [PATCH 0/4] ArmPkg: use console for minimal 'exception occurred' message Ard Biesheuvel ` (3 preceding siblings ...) 2018-12-20 17:31 ` [PATCH 4/4] ArmPkg/DefaultExceptionHandlerLib: use console if available Ard Biesheuvel @ 2019-01-11 17:49 ` Leif Lindholm 4 siblings, 0 replies; 10+ messages in thread From: Leif Lindholm @ 2019-01-11 17:49 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: edk2-devel, philmd, lersek On Thu, Dec 20, 2018 at 06:31:00PM +0100, Ard Biesheuvel wrote: > When running with a graphical console, no message whatsoever is printed > when the systems hits an unexpected exception and hangs, because even > the minimal 'exception occurred' message is only sent to the serial port. > > So let's fix that, by updating DefaultExceptionHandlerLib to take the > availability of a console into account. (#4) > > This requires some preparatory decruftication so that we can safely refer > to the system table and console (#1 .. #3). > > Ard Biesheuvel (4): > ArmPkg/DebugAgentSymbolsBaseLib: remove exception handling > ArmPkg/DefaultExceptionHandlerLib: declare the permitted usage context > ArmPkg/DefaultExceptionHandlerLib: drop BASE variant > ArmPkg/DefaultExceptionHandlerLib: use console if available For the series: Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > ArmPkg/ArmPkg.dsc | 1 - > .../AArch64/DebugAgentException.S | 96 ------ > .../Arm/DebugAgentException.S | 277 ------------------ > .../Arm/DebugAgentException.asm | 273 ----------------- > .../DebugAgentSymbolsBaseLib.c | 7 - > .../DebugAgentSymbolsBaseLib.inf | 9 - > .../AArch64/DefaultExceptionHandler.c | 16 +- > .../Arm/DefaultExceptionHandler.c | 7 +- > .../DefaultExceptionHandlerBase.c | 35 --- > .../DefaultExceptionHandlerLib.inf | 5 +- > .../DefaultExceptionHandlerLibBase.inf | 45 --- > ArmVirtPkg/ArmVirt.dsc.inc | 1 - > 12 files changed, 22 insertions(+), 750 deletions(-) > delete mode 100644 ArmPkg/Library/DebugAgentSymbolsBaseLib/AArch64/DebugAgentException.S > delete mode 100644 ArmPkg/Library/DebugAgentSymbolsBaseLib/Arm/DebugAgentException.S > delete mode 100644 ArmPkg/Library/DebugAgentSymbolsBaseLib/Arm/DebugAgentException.asm > delete mode 100644 ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c > delete mode 100644 ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf > > -- > 2.19.2 > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-01-11 17:49 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-20 17:31 [PATCH 0/4] ArmPkg: use console for minimal 'exception occurred' message Ard Biesheuvel 2018-12-20 17:31 ` [PATCH 1/4] ArmPkg/DebugAgentSymbolsBaseLib: remove exception handling Ard Biesheuvel 2018-12-20 17:31 ` [PATCH 2/4] ArmPkg/DefaultExceptionHandlerLib: declare the permitted usage context Ard Biesheuvel 2018-12-20 17:31 ` [PATCH 3/4] ArmPkg/DefaultExceptionHandlerLib: drop BASE variant Ard Biesheuvel 2018-12-26 21:27 ` Laszlo Ersek 2018-12-27 9:43 ` Ard Biesheuvel 2018-12-20 17:31 ` [PATCH 4/4] ArmPkg/DefaultExceptionHandlerLib: use console if available Ard Biesheuvel 2018-12-26 21:33 ` Laszlo Ersek 2018-12-27 9:43 ` Ard Biesheuvel 2019-01-11 17:49 ` [PATCH 0/4] ArmPkg: use console for minimal 'exception occurred' message Leif Lindholm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox