public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/1] MdePkg: Use ANSI colors to indicate debug message severity
@ 2022-09-02 11:23 Rebecca Cran
  2022-09-02 11:23 ` [PATCH v3 1/1] " Rebecca Cran
  0 siblings, 1 reply; 4+ messages in thread
From: Rebecca Cran @ 2022-09-02 11:23 UTC (permalink / raw)
  To: devel, Michael D Kinney, Liming Gao, Zhiguang Liu, Andrew Fish
  Cc: Rebecca Cran

Changes in v3:

Renamed the FeatureFlag to PcdDebugAnsiSeqSupport.

Rebecca Cran (1):
  MdePkg: Use ANSI colors to indicate debug message severity

 MdePkg/MdePkg.dec                                                |   6 +
 MdePkg/MdePkg.dsc                                                |   2 +
 MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf |   2 +-
 MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf         |   2 +-
 MdePkg/Include/Library/PrintLib.h                                |  39 ++++++
 MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c                 |   4 +
 MdePkg/Library/BasePrintLib/PrintLib.c                           | 128 ++++++++++++++++++++
 MdePkg/Library/UefiDebugLibConOut/DebugLib.c                     |   4 +
 8 files changed, 185 insertions(+), 2 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 1/1] MdePkg: Use ANSI colors to indicate debug message severity
  2022-09-02 11:23 [PATCH v3 0/1] MdePkg: Use ANSI colors to indicate debug message severity Rebecca Cran
@ 2022-09-02 11:23 ` Rebecca Cran
  2022-09-06 18:40   ` Michael D Kinney
  0 siblings, 1 reply; 4+ messages in thread
From: Rebecca Cran @ 2022-09-02 11:23 UTC (permalink / raw)
  To: devel, Michael D Kinney, Liming Gao, Zhiguang Liu, Andrew Fish
  Cc: Rebecca Cran

There currently isn't a way to differentiate the different
levels of DEBUG output: DEBUG_ERROR, DEBUG_WARN, DEBUG_INFO
etc.

To improve this, wrap DEBUG_ERROR and DEBUG_WARN level
messages in ANSI color code escape sequences. DEBUG_ERROR
messages will be displayed in red text, and DEBUG_WARN
in yellow.

Only enable this new functionality if the FeatureFlag
gEfiMdePkgTokenSpaceGuid.PcdDebugAnsiSeqSupport
is set to TRUE. By default it's FALSE.

Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
---
 MdePkg/MdePkg.dec                                                |   6 +
 MdePkg/MdePkg.dsc                                                |   2 +
 MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf |   2 +-
 MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf         |   2 +-
 MdePkg/Include/Library/PrintLib.h                                |  39 ++++++
 MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c                 |   4 +
 MdePkg/Library/BasePrintLib/PrintLib.c                           | 128 ++++++++++++++++++++
 MdePkg/Library/UefiDebugLibConOut/DebugLib.c                     |   4 +
 8 files changed, 185 insertions(+), 2 deletions(-)

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index f1ebf9e251c1..00b50cedd510 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -4,6 +4,7 @@
 # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of
 # EFI1.10/UEFI2.7/PI1.7 and some Industry Standards.
 #
+# Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.<BR>
 # Copyright (c) 2007 - 2022, Intel Corporation. All rights reserved.<BR>
 # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
 # (C) Copyright 2016 - 2021 Hewlett Packard Enterprise Development LP<BR>
@@ -1973,6 +1974,11 @@ [PcdsFeatureFlag]
   # @Prompt Validate ORDERED_COLLECTION structure
   gEfiMdePkgTokenSpaceGuid.PcdValidateOrderedCollection|FALSE|BOOLEAN|0x0000002a
 
+  ## Indicates if DEBUG output should use ANSI sequences.<BR><BR>
+  #   TRUE  - Will use ANSI sequences in DEBUG output.<BR>
+  #   FALSE - Will not use ANSI sequences in DEBUG output.<BR>
+  gEfiMdePkgTokenSpaceGuid.PcdDebugAnsiSeqSupport|FALSE|BOOLEAN|0x0000002f
+
 [PcdsFixedAtBuild]
   ## Status code value for indicating a watchdog timer has expired.
   # EFI_COMPUTING_UNIT_HOST_PROCESSOR | EFI_CU_HP_EC_TIMER_EXPIRED
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index cc1ac196a931..1ecbe6526ac9 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -1,6 +1,7 @@
 ## @file
 # EFI/PI MdePkg Package
 #
+# Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.<BR>
 # Copyright (c) 2007 - 2022, Intel Corporation. All rights reserved.<BR>
 # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
 # (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
@@ -25,6 +26,7 @@ [Defines]
 
 [PcdsFeatureFlag]
   gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport|TRUE
+  gEfiMdePkgTokenSpaceGuid.PcdDebugAnsiSeqSupport|FALSE
 
 [PcdsFixedAtBuild]
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x0f
diff --git a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
index 7504faee67f0..8d6ed759e974 100644
--- a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
+++ b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
@@ -41,4 +41,4 @@ [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue  ## SOMETIMES_CONSUMES
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask      ## CONSUMES
   gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel ## CONSUMES
-
+  gEfiMdePkgTokenSpaceGuid.PcdDebugAnsiSeqSupport       ## CONSUMES
diff --git a/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf b/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
index 53bbc8ce3f65..694494ffc7a3 100644
--- a/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
+++ b/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
@@ -50,4 +50,4 @@ [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue        ## SOMETIMES_CONSUMES
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask            ## CONSUMES
   gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel    ## CONSUMES
-
+  gEfiMdePkgTokenSpaceGuid.PcdDebugAnsiSeqSupport          ## CONSUMES
diff --git a/MdePkg/Include/Library/PrintLib.h b/MdePkg/Include/Library/PrintLib.h
index 8d523cac528d..06d9761df897 100644
--- a/MdePkg/Include/Library/PrintLib.h
+++ b/MdePkg/Include/Library/PrintLib.h
@@ -2,6 +2,7 @@
   Provides services to print a formatted string to a buffer. All combinations of
   Unicode and ASCII strings are supported.
 
+Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.<BR>
 Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -931,4 +932,42 @@ SPrintLengthAsciiFormat (
   IN  VA_LIST      Marker
   );
 
+/**
+  Wraps a message with ANSI color escape codes.
+
+  @param String     The string to wrap.
+  @param StringLen  The size of the String buffer in characters.
+  @param ErrorLevel The error level.
+
+  @retval RETURN_SUCCESS          The string was successfully updated.
+  @retval RETURN_BUFFER_TOO_SMALL The buffer is too small.
+
+**/
+RETURN_STATUS
+EFIAPI
+AsciiDebugGetColorString (
+  IN OUT CHAR8  *String,
+  IN UINTN      StringLen,
+  IN UINTN      ErrorLevel
+  );
+
+/**
+  Wraps a message with ANSI color escape codes.
+
+  @param String     The string to wrap.
+  @param StringLen  The size of the String buffer in Unicode characters.
+  @param ErrorLevel The error level.
+
+  @retval RETURN_SUCCESS          The string was successfully updated.
+  @retval RETURN_BUFFER_TOO_SMALL The buffer is too small.
+
+**/
+RETURN_STATUS
+EFIAPI
+UnicodeDebugGetColorString (
+  IN OUT CHAR16  *String,
+  IN UINTN       StringLen,
+  IN UINTN       ErrorLevel
+  );
+
 #endif
diff --git a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
index bd5686947712..0971c9a57c88 100644
--- a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
+++ b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
@@ -125,6 +125,10 @@ DebugPrintMarker (
     AsciiBSPrint (Buffer, sizeof (Buffer), Format, BaseListMarker);
   }
 
+  if (FeaturePcdGet (PcdDebugAnsiSeqSupport)) {
+    AsciiDebugGetColorString (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ErrorLevel);
+  }
+
   //
   // Send the print string to a Serial Port
   //
diff --git a/MdePkg/Library/BasePrintLib/PrintLib.c b/MdePkg/Library/BasePrintLib/PrintLib.c
index e6f4042bb90b..95135d45aff9 100644
--- a/MdePkg/Library/BasePrintLib/PrintLib.c
+++ b/MdePkg/Library/BasePrintLib/PrintLib.c
@@ -1,12 +1,15 @@
 /** @file
   Base Print Library instance implementation.
 
+  Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.<BR>
   Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
   Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
+#include <Library/BaseMemoryLib.h>
+
 #include "PrintLibInternal.h"
 
 //
@@ -19,6 +22,19 @@ VA_LIST  gNullVaList;
 
 #define ASSERT_UNICODE_BUFFER(Buffer)  ASSERT ((((UINTN) (Buffer)) & 0x01) == 0)
 
+//
+// Define the maximum debug and assert message length that this library supports
+//
+#define MAX_DEBUG_MESSAGE_LENGTH  0x100
+
+#define ASCII_RED_ESC_SEQ     "\033[31m"
+#define ASCII_YELLOW_ESC_SEQ  "\033[33m"
+#define ASCII_END_ESC_SEQ     "\033[0m"
+
+#define RED_ESC_SEQ     L"\033[31m"
+#define YELLOW_ESC_SEQ  L"\033[33m"
+#define END_ESC_SEQ     L"\033[0m"
+
 /**
   Produces a Null-terminated Unicode string in an output buffer based on
   a Null-terminated Unicode format string and a VA_LIST argument list.
@@ -834,3 +850,115 @@ SPrintLengthAsciiFormat (
 {
   return BasePrintLibSPrintMarker (NULL, 0, OUTPUT_UNICODE | COUNT_ONLY_NO_PRINT, (CHAR8 *)FormatString, Marker, NULL);
 }
+
+/**
+  Wraps a message with ANSI color escape codes.
+
+  @param String     The string to wrap.
+  @param StringLen  The size of the String buffer in characters.
+  @param ErrorLevel The error level.
+
+  @retval RETURN_SUCCESS          The string was successfully updated.
+  @retval RETURN_BUFFER_TOO_SMALL The buffer is too small.
+
+**/
+RETURN_STATUS
+EFIAPI
+AsciiDebugGetColorString (
+  IN OUT CHAR8  *String,
+  IN UINTN      StringLen,
+  IN UINTN      ErrorLevel
+  )
+{
+  CHAR8  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
+  UINTN  ReqBufferLen;
+
+  ReqBufferLen = AsciiStrLen (String) +
+                 AsciiStrLen (ASCII_RED_ESC_SEQ) +
+                 AsciiStrLen (ASCII_END_ESC_SEQ) +
+                 1;
+
+  if (StringLen < ReqBufferLen) {
+    return RETURN_BUFFER_TOO_SMALL;
+  }
+
+  ZeroMem (Buffer, sizeof (Buffer));
+
+  switch (ErrorLevel) {
+    case DEBUG_WARN:
+      AsciiStrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ASCII_YELLOW_ESC_SEQ);
+      break;
+    case DEBUG_ERROR:
+      AsciiStrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ASCII_RED_ESC_SEQ);
+      break;
+  }
+
+  AsciiStrCatS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, String);
+
+  switch (ErrorLevel) {
+    case DEBUG_WARN:
+    case DEBUG_ERROR:
+      AsciiStrCatS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ASCII_END_ESC_SEQ);
+      break;
+  }
+
+  AsciiStrCpyS (String, StringLen, Buffer);
+
+  return RETURN_SUCCESS;
+}
+
+/**
+  Wraps a message with ANSI color escape codes.
+
+  @param String     The string to wrap.
+  @param StringLen  The size of the String buffer in Unicode characters.
+  @param ErrorLevel The error level.
+
+  @retval RETURN_SUCCESS          The string was successfully updated.
+  @retval RETURN_BUFFER_TOO_SMALL The buffer is too small.
+
+**/
+RETURN_STATUS
+EFIAPI
+UnicodeDebugGetColorString (
+  IN OUT CHAR16  *String,
+  IN UINTN       StringLen,
+  IN UINTN       ErrorLevel
+  )
+{
+  CHAR16  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
+  UINTN   ReqBufferLen;
+
+  ReqBufferLen = StrLen (String) +
+                 StrLen (RED_ESC_SEQ) +
+                 StrLen (END_ESC_SEQ) +
+                 1;
+
+  if (StringLen < ReqBufferLen) {
+    return RETURN_BUFFER_TOO_SMALL;
+  }
+
+  ZeroMem (Buffer, sizeof (Buffer));
+
+  switch (ErrorLevel) {
+    case DEBUG_WARN:
+      StrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, YELLOW_ESC_SEQ);
+      break;
+    case DEBUG_ERROR:
+      StrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, RED_ESC_SEQ);
+      break;
+  }
+
+  StrCatS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, String);
+
+  switch (ErrorLevel) {
+    case DEBUG_WARN:
+    case DEBUG_ERROR:
+      StrCatS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, END_ESC_SEQ);
+      break;
+  }
+
+  StrCpyS (String, StringLen, Buffer);
+
+  return RETURN_SUCCESS;
+}
diff --git a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
index 65c8dc2b4654..aac5b6d6e022 100644
--- a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
+++ b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
@@ -108,6 +108,10 @@ DebugPrintMarker (
       UnicodeBSPrintAsciiFormat (Buffer, sizeof (Buffer), Format, BaseListMarker);
     }
 
+    if (FeaturePcdGet (PcdDebugAnsiSeqSupport)) {
+      UnicodeDebugGetColorString (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ErrorLevel);
+    }
+
     //
     // Send the print string to the Console Output device
     //
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 1/1] MdePkg: Use ANSI colors to indicate debug message severity
  2022-09-02 11:23 ` [PATCH v3 1/1] " Rebecca Cran
@ 2022-09-06 18:40   ` Michael D Kinney
  2022-09-06 21:51     ` Rebecca Cran
  0 siblings, 1 reply; 4+ messages in thread
From: Michael D Kinney @ 2022-09-06 18:40 UTC (permalink / raw)
  To: Rebecca Cran, devel@edk2.groups.io, Gao, Liming, Liu, Zhiguang,
	Andrew Fish, Kinney, Michael D

Rebecca,

BasePrintLib is used for much more than just DEBUG() macros.  This features should
not modify the behavior of BasePrintLib.

For example, a single module that links to BasePrintLib can use those services
for non DebugLib services with no expectation that ANSI sequences are added
and that same module can link against DebugLib for DEBUG() macro support that
expects ANSI sequences to be added.  This is a conflict.

The scope of this change should be only the DebugLib instance.

Mike

> -----Original Message-----
> From: Rebecca Cran <rebecca@quicinc.com>
> Sent: Friday, September 2, 2022 4:23 AM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; Andrew Fish <afish@apple.com>
> Cc: Rebecca Cran <rebecca@quicinc.com>
> Subject: [PATCH v3 1/1] MdePkg: Use ANSI colors to indicate debug message severity
> 
> There currently isn't a way to differentiate the different
> levels of DEBUG output: DEBUG_ERROR, DEBUG_WARN, DEBUG_INFO
> etc.
> 
> To improve this, wrap DEBUG_ERROR and DEBUG_WARN level
> messages in ANSI color code escape sequences. DEBUG_ERROR
> messages will be displayed in red text, and DEBUG_WARN
> in yellow.
> 
> Only enable this new functionality if the FeatureFlag
> gEfiMdePkgTokenSpaceGuid.PcdDebugAnsiSeqSupport
> is set to TRUE. By default it's FALSE.
> 
> Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
> ---
>  MdePkg/MdePkg.dec                                                |   6 +
>  MdePkg/MdePkg.dsc                                                |   2 +
>  MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf |   2 +-
>  MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf         |   2 +-
>  MdePkg/Include/Library/PrintLib.h                                |  39 ++++++
>  MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c                 |   4 +
>  MdePkg/Library/BasePrintLib/PrintLib.c                           | 128 ++++++++++++++++++++
>  MdePkg/Library/UefiDebugLibConOut/DebugLib.c                     |   4 +
>  8 files changed, 185 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index f1ebf9e251c1..00b50cedd510 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -4,6 +4,7 @@
>  # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of
>  # EFI1.10/UEFI2.7/PI1.7 and some Industry Standards.
>  #
> +# Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.<BR>
>  # Copyright (c) 2007 - 2022, Intel Corporation. All rights reserved.<BR>
>  # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
>  # (C) Copyright 2016 - 2021 Hewlett Packard Enterprise Development LP<BR>
> @@ -1973,6 +1974,11 @@ [PcdsFeatureFlag]
>    # @Prompt Validate ORDERED_COLLECTION structure
>    gEfiMdePkgTokenSpaceGuid.PcdValidateOrderedCollection|FALSE|BOOLEAN|0x0000002a
> 
> +  ## Indicates if DEBUG output should use ANSI sequences.<BR><BR>
> +  #   TRUE  - Will use ANSI sequences in DEBUG output.<BR>
> +  #   FALSE - Will not use ANSI sequences in DEBUG output.<BR>
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugAnsiSeqSupport|FALSE|BOOLEAN|0x0000002f
> +
>  [PcdsFixedAtBuild]
>    ## Status code value for indicating a watchdog timer has expired.
>    # EFI_COMPUTING_UNIT_HOST_PROCESSOR | EFI_CU_HP_EC_TIMER_EXPIRED
> diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
> index cc1ac196a931..1ecbe6526ac9 100644
> --- a/MdePkg/MdePkg.dsc
> +++ b/MdePkg/MdePkg.dsc
> @@ -1,6 +1,7 @@
>  ## @file
>  # EFI/PI MdePkg Package
>  #
> +# Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.<BR>
>  # Copyright (c) 2007 - 2022, Intel Corporation. All rights reserved.<BR>
>  # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
>  # (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> @@ -25,6 +26,7 @@ [Defines]
> 
>  [PcdsFeatureFlag]
>    gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport|TRUE
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugAnsiSeqSupport|FALSE

The DEC default value is FALSE, so this line is not required.

> 
>  [PcdsFixedAtBuild]
>    gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x0f
> diff --git a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> index 7504faee67f0..8d6ed759e974 100644
> --- a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> +++ b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> @@ -41,4 +41,4 @@ [Pcd]
>    gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue  ## SOMETIMES_CONSUMES
>    gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask      ## CONSUMES
>    gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel ## CONSUMES
> -
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugAnsiSeqSupport       ## CONSUMES
> diff --git a/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
> b/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
> index 53bbc8ce3f65..694494ffc7a3 100644
> --- a/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
> +++ b/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
> @@ -50,4 +50,4 @@ [Pcd]
>    gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue        ## SOMETIMES_CONSUMES
>    gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask            ## CONSUMES
>    gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel    ## CONSUMES
> -
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugAnsiSeqSupport          ## CONSUMES
> diff --git a/MdePkg/Include/Library/PrintLib.h b/MdePkg/Include/Library/PrintLib.h
> index 8d523cac528d..06d9761df897 100644
> --- a/MdePkg/Include/Library/PrintLib.h
> +++ b/MdePkg/Include/Library/PrintLib.h
> @@ -2,6 +2,7 @@
>    Provides services to print a formatted string to a buffer. All combinations of
>    Unicode and ASCII strings are supported.
> 
> +Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.<BR>
>  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -931,4 +932,42 @@ SPrintLengthAsciiFormat (
>    IN  VA_LIST      Marker
>    );
> 
> +/**
> +  Wraps a message with ANSI color escape codes.
> +
> +  @param String     The string to wrap.
> +  @param StringLen  The size of the String buffer in characters.
> +  @param ErrorLevel The error level.
> +
> +  @retval RETURN_SUCCESS          The string was successfully updated.
> +  @retval RETURN_BUFFER_TOO_SMALL The buffer is too small.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +AsciiDebugGetColorString (
> +  IN OUT CHAR8  *String,
> +  IN UINTN      StringLen,
> +  IN UINTN      ErrorLevel
> +  );
> +
> +/**
> +  Wraps a message with ANSI color escape codes.
> +
> +  @param String     The string to wrap.
> +  @param StringLen  The size of the String buffer in Unicode characters.
> +  @param ErrorLevel The error level.
> +
> +  @retval RETURN_SUCCESS          The string was successfully updated.
> +  @retval RETURN_BUFFER_TOO_SMALL The buffer is too small.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +UnicodeDebugGetColorString (
> +  IN OUT CHAR16  *String,
> +  IN UINTN       StringLen,
> +  IN UINTN       ErrorLevel
> +  );
> +
>  #endif
> diff --git a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
> index bd5686947712..0971c9a57c88 100644
> --- a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
> +++ b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
> @@ -125,6 +125,10 @@ DebugPrintMarker (
>      AsciiBSPrint (Buffer, sizeof (Buffer), Format, BaseListMarker);
>    }
> 
> +  if (FeaturePcdGet (PcdDebugAnsiSeqSupport)) {
> +    AsciiDebugGetColorString (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ErrorLevel);
> +  }
> +
>    //
>    // Send the print string to a Serial Port
>    //
> diff --git a/MdePkg/Library/BasePrintLib/PrintLib.c b/MdePkg/Library/BasePrintLib/PrintLib.c
> index e6f4042bb90b..95135d45aff9 100644
> --- a/MdePkg/Library/BasePrintLib/PrintLib.c
> +++ b/MdePkg/Library/BasePrintLib/PrintLib.c
> @@ -1,12 +1,15 @@
>  /** @file
>    Base Print Library instance implementation.
> 
> +  Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.<BR>
>    Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>    Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
> +#include <Library/BaseMemoryLib.h>
> +
>  #include "PrintLibInternal.h"
> 
>  //
> @@ -19,6 +22,19 @@ VA_LIST  gNullVaList;
> 
>  #define ASSERT_UNICODE_BUFFER(Buffer)  ASSERT ((((UINTN) (Buffer)) & 0x01) == 0)
> 
> +//
> +// Define the maximum debug and assert message length that this library supports
> +//
> +#define MAX_DEBUG_MESSAGE_LENGTH  0x100
> +
> +#define ASCII_RED_ESC_SEQ     "\033[31m"
> +#define ASCII_YELLOW_ESC_SEQ  "\033[33m"
> +#define ASCII_END_ESC_SEQ     "\033[0m"
> +
> +#define RED_ESC_SEQ     L"\033[31m"
> +#define YELLOW_ESC_SEQ  L"\033[33m"
> +#define END_ESC_SEQ     L"\033[0m"
> +
>  /**
>    Produces a Null-terminated Unicode string in an output buffer based on
>    a Null-terminated Unicode format string and a VA_LIST argument list.
> @@ -834,3 +850,115 @@ SPrintLengthAsciiFormat (
>  {
>    return BasePrintLibSPrintMarker (NULL, 0, OUTPUT_UNICODE | COUNT_ONLY_NO_PRINT, (CHAR8 *)FormatString, Marker, NULL);
>  }
> +
> +/**
> +  Wraps a message with ANSI color escape codes.
> +
> +  @param String     The string to wrap.
> +  @param StringLen  The size of the String buffer in characters.
> +  @param ErrorLevel The error level.
> +
> +  @retval RETURN_SUCCESS          The string was successfully updated.
> +  @retval RETURN_BUFFER_TOO_SMALL The buffer is too small.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +AsciiDebugGetColorString (
> +  IN OUT CHAR8  *String,
> +  IN UINTN      StringLen,
> +  IN UINTN      ErrorLevel
> +  )
> +{
> +  CHAR8  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> +  UINTN  ReqBufferLen;
> +
> +  ReqBufferLen = AsciiStrLen (String) +
> +                 AsciiStrLen (ASCII_RED_ESC_SEQ) +
> +                 AsciiStrLen (ASCII_END_ESC_SEQ) +
> +                 1;
> +
> +  if (StringLen < ReqBufferLen) {
> +    return RETURN_BUFFER_TOO_SMALL;
> +  }
> +
> +  ZeroMem (Buffer, sizeof (Buffer));
> +
> +  switch (ErrorLevel) {
> +    case DEBUG_WARN:
> +      AsciiStrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ASCII_YELLOW_ESC_SEQ);
> +      break;
> +    case DEBUG_ERROR:
> +      AsciiStrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ASCII_RED_ESC_SEQ);
> +      break;
> +  }
> +
> +  AsciiStrCatS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, String);
> +
> +  switch (ErrorLevel) {
> +    case DEBUG_WARN:
> +    case DEBUG_ERROR:
> +      AsciiStrCatS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ASCII_END_ESC_SEQ);
> +      break;
> +  }
> +
> +  AsciiStrCpyS (String, StringLen, Buffer);
> +
> +  return RETURN_SUCCESS;
> +}
> +
> +/**
> +  Wraps a message with ANSI color escape codes.
> +
> +  @param String     The string to wrap.
> +  @param StringLen  The size of the String buffer in Unicode characters.
> +  @param ErrorLevel The error level.
> +
> +  @retval RETURN_SUCCESS          The string was successfully updated.
> +  @retval RETURN_BUFFER_TOO_SMALL The buffer is too small.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +UnicodeDebugGetColorString (
> +  IN OUT CHAR16  *String,
> +  IN UINTN       StringLen,
> +  IN UINTN       ErrorLevel
> +  )
> +{
> +  CHAR16  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> +  UINTN   ReqBufferLen;
> +
> +  ReqBufferLen = StrLen (String) +
> +                 StrLen (RED_ESC_SEQ) +
> +                 StrLen (END_ESC_SEQ) +
> +                 1;
> +
> +  if (StringLen < ReqBufferLen) {
> +    return RETURN_BUFFER_TOO_SMALL;
> +  }
> +
> +  ZeroMem (Buffer, sizeof (Buffer));
> +
> +  switch (ErrorLevel) {
> +    case DEBUG_WARN:
> +      StrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, YELLOW_ESC_SEQ);
> +      break;
> +    case DEBUG_ERROR:
> +      StrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, RED_ESC_SEQ);
> +      break;
> +  }
> +
> +  StrCatS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, String);
> +
> +  switch (ErrorLevel) {
> +    case DEBUG_WARN:
> +    case DEBUG_ERROR:
> +      StrCatS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, END_ESC_SEQ);
> +      break;
> +  }
> +
> +  StrCpyS (String, StringLen, Buffer);
> +
> +  return RETURN_SUCCESS;
> +}
> diff --git a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
> index 65c8dc2b4654..aac5b6d6e022 100644
> --- a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
> +++ b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
> @@ -108,6 +108,10 @@ DebugPrintMarker (
>        UnicodeBSPrintAsciiFormat (Buffer, sizeof (Buffer), Format, BaseListMarker);
>      }
> 
> +    if (FeaturePcdGet (PcdDebugAnsiSeqSupport)) {
> +      UnicodeDebugGetColorString (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ErrorLevel);
> +    }
> +
>      //
>      // Send the print string to the Console Output device
>      //
> --
> 2.30.2


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 1/1] MdePkg: Use ANSI colors to indicate debug message severity
  2022-09-06 18:40   ` Michael D Kinney
@ 2022-09-06 21:51     ` Rebecca Cran
  0 siblings, 0 replies; 4+ messages in thread
From: Rebecca Cran @ 2022-09-06 21:51 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, Gao, Liming,
	Liu, Zhiguang, Andrew Fish

On 9/6/22 12:40, Kinney, Michael D wrote:
> Rebecca,
>
> BasePrintLib is used for much more than just DEBUG() macros.  This features should
> not modify the behavior of BasePrintLib.
>
> For example, a single module that links to BasePrintLib can use those services
> for non DebugLib services with no expectation that ANSI sequences are added
> and that same module can link against DebugLib for DEBUG() macro support that
> expects ANSI sequences to be added.  This is a conflict.
>
> The scope of this change should be only the DebugLib instance.

The reason I put the functions in BasePrintLib is because I couldn't see 
any other common location to put them.

But since one deals with Unicode and the other ASCII, I think defining 
them in their respective libraries might be fine, since we won't end up 
duplicating code.


>>   [PcdsFeatureFlag]
>>     gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport|TRUE
>> +  gEfiMdePkgTokenSpaceGuid.PcdDebugAnsiSeqSupport|FALSE
> The DEC default value is FALSE, so this line is not required.

Thanks, I'll fix it in the next patch version.

-- 
Rebecca Cran

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-09-06 22:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-02 11:23 [PATCH v3 0/1] MdePkg: Use ANSI colors to indicate debug message severity Rebecca Cran
2022-09-02 11:23 ` [PATCH v3 1/1] " Rebecca Cran
2022-09-06 18:40   ` Michael D Kinney
2022-09-06 21:51     ` Rebecca Cran

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox