public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [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

* [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 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 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 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

* 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