diff mbox series

[edk2,v2,5/5] ArmPkg/DefaultExceptionHandlerLib: use console if available

Message ID 20190115082345.3711-6-ard.biesheuvel@linaro.org
State Accepted
Commit 31f5388006fcb6e8582e5bce3f5438ae707de986
Headers show
Series ArmPkg: use console for minimal 'exception occurred' message | expand

Commit Message

Ard Biesheuvel Jan. 15, 2019, 8:23 a.m. UTC 
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>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

---
 ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c | 12 ++++++++++--
 ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c     |  7 ++++++-
 ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf    |  1 +
 3 files changed, 17 insertions(+), 3 deletions(-)

-- 
2.17.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek Jan. 15, 2019, 10:09 a.m. UTC | #1 
On 01/15/19 09:23, 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>

> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---

>  ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c | 12 ++++++++++--

>  ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c     |  7 ++++++-

>  ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf    |  1 +

>  3 files changed, 17 insertions(+), 3 deletions(-)


Please consider updating the following, before pushing the patch:

> diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c

> index 1024bf48c63d..362acd5ba6d2 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,21 @@ 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";

> +

> +    SerialPortWrite ((UINT8 *)Message, AsciiStrLen (Message));


(1) A micro-optimization could be (sizeof Message - 1) rather than
AsciiStrLen (Message), but it's mostly irrelevant.

> +    if (gST->ConOut != NULL) {

> +      AsciiPrint (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);

> +  }

>  

>    DEBUG_CODE_BEGIN ();

>      CHAR8  *Pdb, *PrevPdb;

> diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c

> index cc79cb2fa301..a79f73725aed 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);

> +  }


(2) I think the "serial PLUS console" reporting applies to the 32-bit
lib instance as well, so I assume not updating this from v1 was an
oversight.

>  

>    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 @@ [LibraryClasses]

>    PeCoffGetEntryPointLib

>    ArmDisassemblerLib

>    SerialPortLib

> +  UefiBootServicesTableLib

>  

>  [Guids]

>    gEfiDebugImageInfoTableGuid

> 


With (2) updated, and regardless of (1):

Acked-by: Laszlo Ersek <lersek@redhat.com>


Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Jan. 15, 2019, 11:14 a.m. UTC | #2 
On Tue, 15 Jan 2019 at 11:09, Laszlo Ersek <lersek@redhat.com> wrote:
>

> On 01/15/19 09:23, 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>

> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> > ---

> >  ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c | 12 ++++++++++--

> >  ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c     |  7 ++++++-

> >  ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf    |  1 +

> >  3 files changed, 17 insertions(+), 3 deletions(-)

>

> Please consider updating the following, before pushing the patch:

>

> > diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c

> > index 1024bf48c63d..362acd5ba6d2 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,21 @@ 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";

> > +

> > +    SerialPortWrite ((UINT8 *)Message, AsciiStrLen (Message));

>

> (1) A micro-optimization could be (sizeof Message - 1) rather than

> AsciiStrLen (Message), but it's mostly irrelevant.

>


OK

> > +    if (gST->ConOut != NULL) {

> > +      AsciiPrint (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);

> > +  }

> >

> >    DEBUG_CODE_BEGIN ();

> >      CHAR8  *Pdb, *PrevPdb;

> > diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c

> > index cc79cb2fa301..a79f73725aed 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);

> > +  }

>

> (2) I think the "serial PLUS console" reporting applies to the 32-bit

> lib instance as well, so I assume not updating this from v1 was an

> oversight.

>


Yes it was. Thanks for spotting that.

> >

> >    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 @@ [LibraryClasses]

> >    PeCoffGetEntryPointLib

> >    ArmDisassemblerLib

> >    SerialPortLib

> > +  UefiBootServicesTableLib

> >

> >  [Guids]

> >    gEfiDebugImageInfoTableGuid

> >

>

> With (2) updated, and regardless of (1):

>

> Acked-by: Laszlo Ersek <lersek@redhat.com>

>

> Thanks!

> Laszlo

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c
index 1024bf48c63d..362acd5ba6d2 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,21 @@  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";
+
+    SerialPortWrite ((UINT8 *)Message, AsciiStrLen (Message));
+    if (gST->ConOut != NULL) {
+      AsciiPrint (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);
+  }
 
   DEBUG_CODE_BEGIN ();
     CHAR8  *Pdb, *PrevPdb;
diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c
index cc79cb2fa301..a79f73725aed 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 @@  [LibraryClasses]
   PeCoffGetEntryPointLib
   ArmDisassemblerLib
   SerialPortLib
+  UefiBootServicesTableLib
 
 [Guids]
   gEfiDebugImageInfoTableGuid