* [PATCH v4 1/1] MdePkg: Use ANSI colors to indicate debug message severity
@ 2022-10-27 18:51 Rebecca Cran
2022-11-10 19:11 ` Michael D Kinney
0 siblings, 1 reply; 6+ messages in thread
From: Rebecca Cran @ 2022-10-27 18:51 UTC (permalink / raw)
To: devel, Michael D Kinney, Gao Liming, Liu Zhiguang, 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/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf | 2 +-
MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf | 2 +-
MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c | 64 ++++++++++++++++++++
MdePkg/Library/UefiDebugLibConOut/DebugLib.c | 64 ++++++++++++++++++++
5 files changed, 136 insertions(+), 2 deletions(-)
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 4c81cbd75ab2..8ddc46b62e7d 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>
@@ -1977,6 +1978,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/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/Library/BaseDebugLibSerialPort/DebugLib.c b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
index bd5686947712..df31bd1ffb9f 100644
--- a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
+++ b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
@@ -26,6 +26,10 @@
//
#define MAX_DEBUG_MESSAGE_LENGTH 0x100
+#define RED_ESC_SEQ "\033[31m"
+#define YELLOW_ESC_SEQ "\033[33m"
+#define END_ESC_SEQ "\033[0m"
+
//
// VA_LIST can not initialize to NULL for all compiler, so we use this to
// indicate a null VA_LIST
@@ -77,6 +81,62 @@ DebugPrint (
VA_END (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.
+
+**/
+STATIC
+RETURN_STATUS
+AsciiDebugGetColorString (
+ IN OUT CHAR8 *String,
+ IN UINTN StringLen,
+ IN UINTN ErrorLevel
+ )
+{
+ CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH];
+ UINTN ReqBufferLen;
+
+ ReqBufferLen = AsciiStrLen (String) +
+ AsciiStrLen (RED_ESC_SEQ) +
+ AsciiStrLen (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, YELLOW_ESC_SEQ);
+ break;
+ case DEBUG_ERROR:
+ AsciiStrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, 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, END_ESC_SEQ);
+ break;
+ }
+
+ AsciiStrCpyS (String, StringLen, Buffer);
+
+ return RETURN_SUCCESS;
+}
+
/**
Prints a debug message to the debug output device if the specified
error level is enabled base on Null-terminated format string and a
@@ -125,6 +185,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/UefiDebugLibConOut/DebugLib.c b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
index 65c8dc2b4654..521298a7c8a7 100644
--- a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
+++ b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
@@ -20,6 +20,10 @@
//
#define MAX_DEBUG_MESSAGE_LENGTH 0x100
+#define RED_ESC_SEQ L"\033[31m"
+#define YELLOW_ESC_SEQ L"\033[33m"
+#define END_ESC_SEQ L"\033[0m"
+
//
// VA_LIST can not initialize to NULL for all compiler, so we use this to
// indicate a null VA_LIST
@@ -59,6 +63,62 @@ DebugPrint (
VA_END (Marker);
}
+/**
+ 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.
+
+**/
+STATIC
+RETURN_STATUS
+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;
+}
+
/**
Prints a debug message to the debug output device if the specified
error level is enabled base on Null-terminated format string and a
@@ -108,6 +168,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] 6+ messages in thread
* Re: [edk2-devel] [PATCH v4 1/1] MdePkg: Use ANSI colors to indicate debug message severity
[not found] <172201BE20D562E0.7329@groups.io>
@ 2022-11-08 11:34 ` Rebecca Cran
0 siblings, 0 replies; 6+ messages in thread
From: Rebecca Cran @ 2022-11-08 11:34 UTC (permalink / raw)
To: devel, quic_rcran, Michael D Kinney, Gao Liming, Liu Zhiguang,
Andrew Fish
Could I get some reviews on this please?
Thanks.
Rebecca Cran
On 10/27/22 12:51, Rebecca Cran wrote:
> 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/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf | 2 +-
> MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf | 2 +-
> MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c | 64 ++++++++++++++++++++
> MdePkg/Library/UefiDebugLibConOut/DebugLib.c | 64 ++++++++++++++++++++
> 5 files changed, 136 insertions(+), 2 deletions(-)
>
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index 4c81cbd75ab2..8ddc46b62e7d 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>
> @@ -1977,6 +1978,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/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/Library/BaseDebugLibSerialPort/DebugLib.c b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
> index bd5686947712..df31bd1ffb9f 100644
> --- a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
> +++ b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
> @@ -26,6 +26,10 @@
> //
> #define MAX_DEBUG_MESSAGE_LENGTH 0x100
>
> +#define RED_ESC_SEQ "\033[31m"
> +#define YELLOW_ESC_SEQ "\033[33m"
> +#define END_ESC_SEQ "\033[0m"
> +
> //
> // VA_LIST can not initialize to NULL for all compiler, so we use this to
> // indicate a null VA_LIST
> @@ -77,6 +81,62 @@ DebugPrint (
> VA_END (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.
> +
> +**/
> +STATIC
> +RETURN_STATUS
> +AsciiDebugGetColorString (
> + IN OUT CHAR8 *String,
> + IN UINTN StringLen,
> + IN UINTN ErrorLevel
> + )
> +{
> + CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> + UINTN ReqBufferLen;
> +
> + ReqBufferLen = AsciiStrLen (String) +
> + AsciiStrLen (RED_ESC_SEQ) +
> + AsciiStrLen (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, YELLOW_ESC_SEQ);
> + break;
> + case DEBUG_ERROR:
> + AsciiStrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, 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, END_ESC_SEQ);
> + break;
> + }
> +
> + AsciiStrCpyS (String, StringLen, Buffer);
> +
> + return RETURN_SUCCESS;
> +}
> +
> /**
> Prints a debug message to the debug output device if the specified
> error level is enabled base on Null-terminated format string and a
> @@ -125,6 +185,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/UefiDebugLibConOut/DebugLib.c b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
> index 65c8dc2b4654..521298a7c8a7 100644
> --- a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
> +++ b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
> @@ -20,6 +20,10 @@
> //
> #define MAX_DEBUG_MESSAGE_LENGTH 0x100
>
> +#define RED_ESC_SEQ L"\033[31m"
> +#define YELLOW_ESC_SEQ L"\033[33m"
> +#define END_ESC_SEQ L"\033[0m"
> +
> //
> // VA_LIST can not initialize to NULL for all compiler, so we use this to
> // indicate a null VA_LIST
> @@ -59,6 +63,62 @@ DebugPrint (
> VA_END (Marker);
> }
>
> +/**
> + 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.
> +
> +**/
> +STATIC
> +RETURN_STATUS
> +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;
> +}
> +
> /**
> Prints a debug message to the debug output device if the specified
> error level is enabled base on Null-terminated format string and a
> @@ -108,6 +168,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
> //
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/1] MdePkg: Use ANSI colors to indicate debug message severity
2022-10-27 18:51 [PATCH v4 1/1] MdePkg: Use ANSI colors to indicate debug message severity Rebecca Cran
@ 2022-11-10 19:11 ` Michael D Kinney
2022-11-10 23:14 ` Michael D Kinney
2023-02-20 19:14 ` Rebecca Cran
0 siblings, 2 replies; 6+ messages in thread
From: Michael D Kinney @ 2022-11-10 19:11 UTC (permalink / raw)
To: Rebecca Cran, devel@edk2.groups.io, Gao, Liming, Liu, Zhiguang,
Andrew Fish, Kinney, Michael D
Hi Rebecca,
In the edk2 repo, I see the following instances of the DebugPrintMarker() API
where you are currently adding he ANSI color sequences. Should all of these
be updated? I did not review the edk2-platforms repo. And there may be
downstream custom DebugLib instances. It would better if this feature could
be enabled for all existing DebugLib instances, but the only common location
is the DEBUG() macro definition in MdePkg/Include/Library/DebugLib.h and
adding code to that macro adds statements to the module calling the DebugLib
services, which can increase code size.
ArmPkg\Library\SemiHostingDebugLib\DebugLib.c:
75 VOID
76: DebugPrintMarker (
77 IN UINTN ErrorLevel,
IntelFsp2Pkg\Library\BaseFspDebugLibSerialPort\DebugLib.c:
89 VOID
90: DebugPrintMarker (
91 IN UINTN ErrorLevel,
MdeModulePkg\Library\PeiDxeDebugLibReportStatusCode\DebugLib.c:
85 VOID
86: DebugPrintMarker (
87 IN UINTN ErrorLevel,
MdePkg\Library\BaseDebugLibSerialPort\DebugLib.c:
97 VOID
98: DebugPrintMarker (
99 IN UINTN ErrorLevel,
MdePkg\Library\DxeRuntimeDebugLibSerialPort\DebugLib.c:
156 VOID
157: DebugPrintMarker (
158 IN UINTN ErrorLevel,
MdePkg\Library\UefiDebugLibConOut\DebugLib.c:
79 VOID
80: DebugPrintMarker (
81 IN UINTN ErrorLevel,
MdePkg\Library\UefiDebugLibDebugPortProtocol\DebugLib.c:
137 VOID
138: DebugPrintMarker (
139 IN UINTN ErrorLevel,
MdePkg\Library\UefiDebugLibStdErr\DebugLib.c:
79 VOID
80: DebugPrintMarker (
81 IN UINTN ErrorLevel,
OvmfPkg\Library\PlatformDebugLibIoPort\DebugLib.c:
79 VOID
80: DebugPrintMarker (
81 IN UINTN ErrorLevel,
Also, the ErrorLevel parameter is a bitmask. It cannot be used in
a switch/case statement for only 1 bit being set. To test for a
debug message of type ERROR or WARN, a check must be done for that
one bit being set in ErrorLevel.
The DebugLib also provides support for ASSERT() macros. Should
ASSERT() messages have the same color as an ERROR message? Or
its own color?
The logic below changes the color for the message based on message
type. But is does not save/restore the current ANSI color setting.
It see it using the END_ESC_SEQ. Does that put the color settings
back to the previous setting?
Thanks,
Mike
> -----Original Message-----
> From: Rebecca Cran <rebecca@quicinc.com>
> Sent: Thursday, October 27, 2022 11:51 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 v4 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/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf | 2 +-
> MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf | 2 +-
> MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c | 64 ++++++++++++++++++++
> MdePkg/Library/UefiDebugLibConOut/DebugLib.c | 64 ++++++++++++++++++++
> 5 files changed, 136 insertions(+), 2 deletions(-)
>
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index 4c81cbd75ab2..8ddc46b62e7d 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>
> @@ -1977,6 +1978,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/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/Library/BaseDebugLibSerialPort/DebugLib.c b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
> index bd5686947712..df31bd1ffb9f 100644
> --- a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
> +++ b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
> @@ -26,6 +26,10 @@
> //
> #define MAX_DEBUG_MESSAGE_LENGTH 0x100
>
> +#define RED_ESC_SEQ "\033[31m"
> +#define YELLOW_ESC_SEQ "\033[33m"
> +#define END_ESC_SEQ "\033[0m"
> +
> //
> // VA_LIST can not initialize to NULL for all compiler, so we use this to
> // indicate a null VA_LIST
> @@ -77,6 +81,62 @@ DebugPrint (
> VA_END (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.
> +
> +**/
> +STATIC
> +RETURN_STATUS
> +AsciiDebugGetColorString (
> + IN OUT CHAR8 *String,
> + IN UINTN StringLen,
> + IN UINTN ErrorLevel
> + )
> +{
> + CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> + UINTN ReqBufferLen;
> +
> + ReqBufferLen = AsciiStrLen (String) +
> + AsciiStrLen (RED_ESC_SEQ) +
> + AsciiStrLen (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, YELLOW_ESC_SEQ);
> + break;
> + case DEBUG_ERROR:
> + AsciiStrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, 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, END_ESC_SEQ);
> + break;
> + }
> +
> + AsciiStrCpyS (String, StringLen, Buffer);
> +
> + return RETURN_SUCCESS;
> +}
> +
> /**
> Prints a debug message to the debug output device if the specified
> error level is enabled base on Null-terminated format string and a
> @@ -125,6 +185,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/UefiDebugLibConOut/DebugLib.c b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
> index 65c8dc2b4654..521298a7c8a7 100644
> --- a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
> +++ b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
> @@ -20,6 +20,10 @@
> //
> #define MAX_DEBUG_MESSAGE_LENGTH 0x100
>
> +#define RED_ESC_SEQ L"\033[31m"
> +#define YELLOW_ESC_SEQ L"\033[33m"
> +#define END_ESC_SEQ L"\033[0m"
> +
> //
> // VA_LIST can not initialize to NULL for all compiler, so we use this to
> // indicate a null VA_LIST
> @@ -59,6 +63,62 @@ DebugPrint (
> VA_END (Marker);
> }
>
> +/**
> + 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.
> +
> +**/
> +STATIC
> +RETURN_STATUS
> +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;
> +}
> +
> /**
> Prints a debug message to the debug output device if the specified
> error level is enabled base on Null-terminated format string and a
> @@ -108,6 +168,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] 6+ messages in thread
* Re: [PATCH v4 1/1] MdePkg: Use ANSI colors to indicate debug message severity
2022-11-10 19:11 ` Michael D Kinney
@ 2022-11-10 23:14 ` Michael D Kinney
2023-02-20 19:44 ` [edk2-devel] " Rebecca Cran
2023-02-20 19:14 ` Rebecca Cran
1 sibling, 1 reply; 6+ messages in thread
From: Michael D Kinney @ 2022-11-10 23:14 UTC (permalink / raw)
To: Rebecca Cran, devel@edk2.groups.io, Gao, Liming, Liu, Zhiguang,
Andrew Fish, Kinney, Michael D
Hi Rebecca,
Here is an alternate way to implement this feature. Just an
experiment because it does not follow the recommended methods
to add a feature that can be enabled/disabled. I have tested
this on EmulatorPkg and OvmfPkg and it works as expected.
Does not require any updates to any DebugLib instances.
https://github.com/mdkinney/edk2/tree/POC_DebugLibAnsi
https://github.com/mdkinney/edk2/commit/8961ee82e95495651eca53145ae76c4f0097c637
In general we do not like using #if to enable/disable features.
However, this feature does increase the size of modules to enable
ANSI color sequences. This approach has zero size impact
if MDEPKG_DEBUGANSI is not defined.
There are also challenges with this feature for platform FW
builds that may integrate binaries from other sources if there
is a mix of modules that enable or disable the feature.
Only the platform developer knows if the device that all
DEBUG()/ASSERT() messages are routed supports ANSI color
sequences or not. This means adding the sequences needs
to be in a platform component. Or we need a mode where all
generated messages are more like trace messages that include
the message type and error level, and let the receiver of
the trace messages reformat as needed.
Best regards,
Mike
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Thursday, November 10, 2022 11:12 AM
> To: Rebecca Cran <rebecca@quicinc.com>; devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Andrew Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [PATCH v4 1/1] MdePkg: Use ANSI colors to indicate debug message severity
>
> Hi Rebecca,
>
> In the edk2 repo, I see the following instances of the DebugPrintMarker() API
> where you are currently adding he ANSI color sequences. Should all of these
> be updated? I did not review the edk2-platforms repo. And there may be
> downstream custom DebugLib instances. It would better if this feature could
> be enabled for all existing DebugLib instances, but the only common location
> is the DEBUG() macro definition in MdePkg/Include/Library/DebugLib.h and
> adding code to that macro adds statements to the module calling the DebugLib
> services, which can increase code size.
>
> ArmPkg\Library\SemiHostingDebugLib\DebugLib.c:
> 75 VOID
> 76: DebugPrintMarker (
> 77 IN UINTN ErrorLevel,
>
> IntelFsp2Pkg\Library\BaseFspDebugLibSerialPort\DebugLib.c:
> 89 VOID
> 90: DebugPrintMarker (
> 91 IN UINTN ErrorLevel,
>
> MdeModulePkg\Library\PeiDxeDebugLibReportStatusCode\DebugLib.c:
> 85 VOID
> 86: DebugPrintMarker (
> 87 IN UINTN ErrorLevel,
>
> MdePkg\Library\BaseDebugLibSerialPort\DebugLib.c:
> 97 VOID
> 98: DebugPrintMarker (
> 99 IN UINTN ErrorLevel,
>
> MdePkg\Library\DxeRuntimeDebugLibSerialPort\DebugLib.c:
> 156 VOID
> 157: DebugPrintMarker (
> 158 IN UINTN ErrorLevel,
>
> MdePkg\Library\UefiDebugLibConOut\DebugLib.c:
> 79 VOID
> 80: DebugPrintMarker (
> 81 IN UINTN ErrorLevel,
>
> MdePkg\Library\UefiDebugLibDebugPortProtocol\DebugLib.c:
> 137 VOID
> 138: DebugPrintMarker (
> 139 IN UINTN ErrorLevel,
>
> MdePkg\Library\UefiDebugLibStdErr\DebugLib.c:
> 79 VOID
> 80: DebugPrintMarker (
> 81 IN UINTN ErrorLevel,
>
> OvmfPkg\Library\PlatformDebugLibIoPort\DebugLib.c:
> 79 VOID
> 80: DebugPrintMarker (
> 81 IN UINTN ErrorLevel,
>
> Also, the ErrorLevel parameter is a bitmask. It cannot be used in
> a switch/case statement for only 1 bit being set. To test for a
> debug message of type ERROR or WARN, a check must be done for that
> one bit being set in ErrorLevel.
>
> The DebugLib also provides support for ASSERT() macros. Should
> ASSERT() messages have the same color as an ERROR message? Or
> its own color?
>
> The logic below changes the color for the message based on message
> type. But is does not save/restore the current ANSI color setting.
> It see it using the END_ESC_SEQ. Does that put the color settings
> back to the previous setting?
>
> Thanks,
>
> Mike
>
> > -----Original Message-----
> > From: Rebecca Cran <rebecca@quicinc.com>
> > Sent: Thursday, October 27, 2022 11:51 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 v4 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/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf | 2 +-
> > MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf | 2 +-
> > MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c | 64 ++++++++++++++++++++
> > MdePkg/Library/UefiDebugLibConOut/DebugLib.c | 64 ++++++++++++++++++++
> > 5 files changed, 136 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> > index 4c81cbd75ab2..8ddc46b62e7d 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>
> > @@ -1977,6 +1978,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/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/Library/BaseDebugLibSerialPort/DebugLib.c b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
> > index bd5686947712..df31bd1ffb9f 100644
> > --- a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
> > +++ b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
> > @@ -26,6 +26,10 @@
> > //
> > #define MAX_DEBUG_MESSAGE_LENGTH 0x100
> >
> > +#define RED_ESC_SEQ "\033[31m"
> > +#define YELLOW_ESC_SEQ "\033[33m"
> > +#define END_ESC_SEQ "\033[0m"
> > +
> > //
> > // VA_LIST can not initialize to NULL for all compiler, so we use this to
> > // indicate a null VA_LIST
> > @@ -77,6 +81,62 @@ DebugPrint (
> > VA_END (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.
> > +
> > +**/
> > +STATIC
> > +RETURN_STATUS
> > +AsciiDebugGetColorString (
> > + IN OUT CHAR8 *String,
> > + IN UINTN StringLen,
> > + IN UINTN ErrorLevel
> > + )
> > +{
> > + CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> > + UINTN ReqBufferLen;
> > +
> > + ReqBufferLen = AsciiStrLen (String) +
> > + AsciiStrLen (RED_ESC_SEQ) +
> > + AsciiStrLen (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, YELLOW_ESC_SEQ);
> > + break;
> > + case DEBUG_ERROR:
> > + AsciiStrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, 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, END_ESC_SEQ);
> > + break;
> > + }
> > +
> > + AsciiStrCpyS (String, StringLen, Buffer);
> > +
> > + return RETURN_SUCCESS;
> > +}
> > +
> > /**
> > Prints a debug message to the debug output device if the specified
> > error level is enabled base on Null-terminated format string and a
> > @@ -125,6 +185,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/UefiDebugLibConOut/DebugLib.c b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
> > index 65c8dc2b4654..521298a7c8a7 100644
> > --- a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
> > +++ b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
> > @@ -20,6 +20,10 @@
> > //
> > #define MAX_DEBUG_MESSAGE_LENGTH 0x100
> >
> > +#define RED_ESC_SEQ L"\033[31m"
> > +#define YELLOW_ESC_SEQ L"\033[33m"
> > +#define END_ESC_SEQ L"\033[0m"
> > +
> > //
> > // VA_LIST can not initialize to NULL for all compiler, so we use this to
> > // indicate a null VA_LIST
> > @@ -59,6 +63,62 @@ DebugPrint (
> > VA_END (Marker);
> > }
> >
> > +/**
> > + 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.
> > +
> > +**/
> > +STATIC
> > +RETURN_STATUS
> > +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;
> > +}
> > +
> > /**
> > Prints a debug message to the debug output device if the specified
> > error level is enabled base on Null-terminated format string and a
> > @@ -108,6 +168,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] 6+ messages in thread
* Re: [PATCH v4 1/1] MdePkg: Use ANSI colors to indicate debug message severity
2022-11-10 19:11 ` Michael D Kinney
2022-11-10 23:14 ` Michael D Kinney
@ 2023-02-20 19:14 ` Rebecca Cran
1 sibling, 0 replies; 6+ messages in thread
From: Rebecca Cran @ 2023-02-20 19:14 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io, Gao, Liming,
Liu, Zhiguang, Andrew Fish
On 11/10/22 12:11, Kinney, Michael D wrote:
> In the edk2 repo, I see the following instances of the DebugPrintMarker() API
> where you are currently adding he ANSI color sequences. Should all of these
> be updated? I did not review the edk2-platforms repo. And there may be
> downstream custom DebugLib instances. It would better if this feature could
> be enabled for all existing DebugLib instances, but the only common location
> is the DEBUG() macro definition in MdePkg/Include/Library/DebugLib.h and
> adding code to that macro adds statements to the module calling the DebugLib
> services, which can increase code size.
I'll update the other instances. Given it's purely an aesthetic change,
I'm not sure we need to worry about custom instances?
> Also, the ErrorLevel parameter is a bitmask. It cannot be used in
> a switch/case statement for only 1 bit being set. To test for a
> debug message of type ERROR or WARN, a check must be done for that
> one bit being set in ErrorLevel.
Thanks, I'll fix that.
>
> The DebugLib also provides support for ASSERT() macros. Should
> ASSERT() messages have the same color as an ERROR message? Or
> its own color?
I wasn't thinking about ASSERT messages because those are super clear
being one of the last messages printed.
> The logic below changes the color for the message based on message
> type. But is does not save/restore the current ANSI color setting.
> It see it using the END_ESC_SEQ. Does that put the color settings
> back to the previous setting?
No, it resets it to the terminal's default color scheme. I'm not aware
of a way to save/restore the color.
--
Rebecca Cran
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH v4 1/1] MdePkg: Use ANSI colors to indicate debug message severity
2022-11-10 23:14 ` Michael D Kinney
@ 2023-02-20 19:44 ` Rebecca Cran
0 siblings, 0 replies; 6+ messages in thread
From: Rebecca Cran @ 2023-02-20 19:44 UTC (permalink / raw)
To: devel, michael.d.kinney, Rebecca Cran, Gao, Liming, Liu, Zhiguang,
Andrew Fish
Hi Mike,
Sorry, I only just saw this message.
Your implementation looks better since as you say it doesn't require
updates to the DebugLib instances.
Since you're more familiar with the gotchas around adding this, I'll
leave it to you to decide if this can be committed or not.
--
Rebecca Cran
On 11/10/22 16:14, Michael D Kinney wrote:
> Hi Rebecca,
>
> Here is an alternate way to implement this feature. Just an
> experiment because it does not follow the recommended methods
> to add a feature that can be enabled/disabled. I have tested
> this on EmulatorPkg and OvmfPkg and it works as expected.
> Does not require any updates to any DebugLib instances.
>
> https://github.com/mdkinney/edk2/tree/POC_DebugLibAnsi
> https://github.com/mdkinney/edk2/commit/8961ee82e95495651eca53145ae76c4f0097c637
>
> In general we do not like using #if to enable/disable features.
> However, this feature does increase the size of modules to enable
> ANSI color sequences. This approach has zero size impact
> if MDEPKG_DEBUGANSI is not defined.
>
> There are also challenges with this feature for platform FW
> builds that may integrate binaries from other sources if there
> is a mix of modules that enable or disable the feature.
>
> Only the platform developer knows if the device that all
> DEBUG()/ASSERT() messages are routed supports ANSI color
> sequences or not. This means adding the sequences needs
> to be in a platform component. Or we need a mode where all
> generated messages are more like trace messages that include
> the message type and error level, and let the receiver of
> the trace messages reformat as needed.
>
> Best regards,
>
> Mike
>
>> -----Original Message-----
>> From: Kinney, Michael D <michael.d.kinney@intel.com>
>> Sent: Thursday, November 10, 2022 11:12 AM
>> To: Rebecca Cran <rebecca@quicinc.com>; devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
>> <zhiguang.liu@intel.com>; Andrew Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
>> Subject: RE: [PATCH v4 1/1] MdePkg: Use ANSI colors to indicate debug message severity
>>
>> Hi Rebecca,
>>
>> In the edk2 repo, I see the following instances of the DebugPrintMarker() API
>> where you are currently adding he ANSI color sequences. Should all of these
>> be updated? I did not review the edk2-platforms repo. And there may be
>> downstream custom DebugLib instances. It would better if this feature could
>> be enabled for all existing DebugLib instances, but the only common location
>> is the DEBUG() macro definition in MdePkg/Include/Library/DebugLib.h and
>> adding code to that macro adds statements to the module calling the DebugLib
>> services, which can increase code size.
>>
>> ArmPkg\Library\SemiHostingDebugLib\DebugLib.c:
>> 75 VOID
>> 76: DebugPrintMarker (
>> 77 IN UINTN ErrorLevel,
>>
>> IntelFsp2Pkg\Library\BaseFspDebugLibSerialPort\DebugLib.c:
>> 89 VOID
>> 90: DebugPrintMarker (
>> 91 IN UINTN ErrorLevel,
>>
>> MdeModulePkg\Library\PeiDxeDebugLibReportStatusCode\DebugLib.c:
>> 85 VOID
>> 86: DebugPrintMarker (
>> 87 IN UINTN ErrorLevel,
>>
>> MdePkg\Library\BaseDebugLibSerialPort\DebugLib.c:
>> 97 VOID
>> 98: DebugPrintMarker (
>> 99 IN UINTN ErrorLevel,
>>
>> MdePkg\Library\DxeRuntimeDebugLibSerialPort\DebugLib.c:
>> 156 VOID
>> 157: DebugPrintMarker (
>> 158 IN UINTN ErrorLevel,
>>
>> MdePkg\Library\UefiDebugLibConOut\DebugLib.c:
>> 79 VOID
>> 80: DebugPrintMarker (
>> 81 IN UINTN ErrorLevel,
>>
>> MdePkg\Library\UefiDebugLibDebugPortProtocol\DebugLib.c:
>> 137 VOID
>> 138: DebugPrintMarker (
>> 139 IN UINTN ErrorLevel,
>>
>> MdePkg\Library\UefiDebugLibStdErr\DebugLib.c:
>> 79 VOID
>> 80: DebugPrintMarker (
>> 81 IN UINTN ErrorLevel,
>>
>> OvmfPkg\Library\PlatformDebugLibIoPort\DebugLib.c:
>> 79 VOID
>> 80: DebugPrintMarker (
>> 81 IN UINTN ErrorLevel,
>>
>> Also, the ErrorLevel parameter is a bitmask. It cannot be used in
>> a switch/case statement for only 1 bit being set. To test for a
>> debug message of type ERROR or WARN, a check must be done for that
>> one bit being set in ErrorLevel.
>>
>> The DebugLib also provides support for ASSERT() macros. Should
>> ASSERT() messages have the same color as an ERROR message? Or
>> its own color?
>>
>> The logic below changes the color for the message based on message
>> type. But is does not save/restore the current ANSI color setting.
>> It see it using the END_ESC_SEQ. Does that put the color settings
>> back to the previous setting?
>>
>> Thanks,
>>
>> Mike
>>
>>> -----Original Message-----
>>> From: Rebecca Cran <rebecca@quicinc.com>
>>> Sent: Thursday, October 27, 2022 11:51 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 v4 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/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf | 2 +-
>>> MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf | 2 +-
>>> MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c | 64 ++++++++++++++++++++
>>> MdePkg/Library/UefiDebugLibConOut/DebugLib.c | 64 ++++++++++++++++++++
>>> 5 files changed, 136 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
>>> index 4c81cbd75ab2..8ddc46b62e7d 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>
>>> @@ -1977,6 +1978,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/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/Library/BaseDebugLibSerialPort/DebugLib.c b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
>>> index bd5686947712..df31bd1ffb9f 100644
>>> --- a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
>>> +++ b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
>>> @@ -26,6 +26,10 @@
>>> //
>>> #define MAX_DEBUG_MESSAGE_LENGTH 0x100
>>>
>>> +#define RED_ESC_SEQ "\033[31m"
>>> +#define YELLOW_ESC_SEQ "\033[33m"
>>> +#define END_ESC_SEQ "\033[0m"
>>> +
>>> //
>>> // VA_LIST can not initialize to NULL for all compiler, so we use this to
>>> // indicate a null VA_LIST
>>> @@ -77,6 +81,62 @@ DebugPrint (
>>> VA_END (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.
>>> +
>>> +**/
>>> +STATIC
>>> +RETURN_STATUS
>>> +AsciiDebugGetColorString (
>>> + IN OUT CHAR8 *String,
>>> + IN UINTN StringLen,
>>> + IN UINTN ErrorLevel
>>> + )
>>> +{
>>> + CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH];
>>> + UINTN ReqBufferLen;
>>> +
>>> + ReqBufferLen = AsciiStrLen (String) +
>>> + AsciiStrLen (RED_ESC_SEQ) +
>>> + AsciiStrLen (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, YELLOW_ESC_SEQ);
>>> + break;
>>> + case DEBUG_ERROR:
>>> + AsciiStrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, 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, END_ESC_SEQ);
>>> + break;
>>> + }
>>> +
>>> + AsciiStrCpyS (String, StringLen, Buffer);
>>> +
>>> + return RETURN_SUCCESS;
>>> +}
>>> +
>>> /**
>>> Prints a debug message to the debug output device if the specified
>>> error level is enabled base on Null-terminated format string and a
>>> @@ -125,6 +185,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/UefiDebugLibConOut/DebugLib.c b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>>> index 65c8dc2b4654..521298a7c8a7 100644
>>> --- a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>>> +++ b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>>> @@ -20,6 +20,10 @@
>>> //
>>> #define MAX_DEBUG_MESSAGE_LENGTH 0x100
>>>
>>> +#define RED_ESC_SEQ L"\033[31m"
>>> +#define YELLOW_ESC_SEQ L"\033[33m"
>>> +#define END_ESC_SEQ L"\033[0m"
>>> +
>>> //
>>> // VA_LIST can not initialize to NULL for all compiler, so we use this to
>>> // indicate a null VA_LIST
>>> @@ -59,6 +63,62 @@ DebugPrint (
>>> VA_END (Marker);
>>> }
>>>
>>> +/**
>>> + 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.
>>> +
>>> +**/
>>> +STATIC
>>> +RETURN_STATUS
>>> +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;
>>> +}
>>> +
>>> /**
>>> Prints a debug message to the debug output device if the specified
>>> error level is enabled base on Null-terminated format string and a
>>> @@ -108,6 +168,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] 6+ messages in thread
end of thread, other threads:[~2023-02-20 19:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-27 18:51 [PATCH v4 1/1] MdePkg: Use ANSI colors to indicate debug message severity Rebecca Cran
2022-11-10 19:11 ` Michael D Kinney
2022-11-10 23:14 ` Michael D Kinney
2023-02-20 19:44 ` [edk2-devel] " Rebecca Cran
2023-02-20 19:14 ` Rebecca Cran
[not found] <172201BE20D562E0.7329@groups.io>
2022-11-08 11:34 ` [edk2-devel] " Rebecca Cran
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox