From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from alexa-out-sd-01.qualcomm.com (alexa-out-sd-01.qualcomm.com [199.106.114.38]) by mx.groups.io with SMTP id smtpd.web12.6832.1667907279841693658 for ; Tue, 08 Nov 2022 03:34:39 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@quicinc.com header.s=qcdkim header.b=kytf+CEA; spf=permerror, err=parse error for token &{10 18 %{ir}.%{v}.%{d}.spf.has.pphosted.com}: invalid domain name (domain: quicinc.com, ip: 199.106.114.38, mailfrom: quic_rcran@quicinc.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; i=@quicinc.com; q=dns/txt; s=qcdkim; t=1667907279; x=1699443279; h=from:message-id:date:mime-version:subject:to:references: in-reply-to:content-transfer-encoding; bh=xMYokqjOA/U5NQpc2NDOlo2H9TaAXO3NfSxVEonW0U4=; b=kytf+CEAdGDJDpMl/pVwpAAqCu99II3v91QpG5nvfGnmayIJisbNNK9S +RoTrxG7opo0MRaRmKfpNr6hJDuHRZquFqpGbQgVv6WJeT0yARUYo7aKo jWIarYOAQ/7Dv2PpHH1JOPKdnNoIxsLzR1zkHti064E90yl2B5Y0Eumxp w=; Received: from unknown (HELO ironmsg04-sd.qualcomm.com) ([10.53.140.144]) by alexa-out-sd-01.qualcomm.com with ESMTP; 08 Nov 2022 03:34:39 -0800 From: "Rebecca Cran" X-QCInternal: smtphost Received: from nasanex01b.na.qualcomm.com ([10.46.141.250]) by ironmsg04-sd.qualcomm.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Nov 2022 03:34:39 -0800 Received: from [10.110.16.76] (10.80.80.8) by nasanex01b.na.qualcomm.com (10.46.141.250) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.29; Tue, 8 Nov 2022 03:34:38 -0800 Message-ID: <80e7980d-73a3-0d50-0952-4a0f3fc1deae@quicinc.com> Date: Tue, 8 Nov 2022 04:34:37 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [edk2-devel] [PATCH v4 1/1] MdePkg: Use ANSI colors to indicate debug message severity To: , , Michael D Kinney , Gao Liming , "Liu Zhiguang" , Andrew Fish References: <172201BE20D562E0.7329@groups.io> In-Reply-To: <172201BE20D562E0.7329@groups.io> Return-Path: rebecca@quicinc.com X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nasanex01b.na.qualcomm.com (10.46.141.250) Content-Language: en-US Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit 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 > --- > 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.
> # Copyright (c) 2007 - 2022, Intel Corporation. All rights reserved.
> # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
> # (C) Copyright 2016 - 2021 Hewlett Packard Enterprise Development LP
> @@ -1977,6 +1978,11 @@ [PcdsFeatureFlag] > # @Prompt Validate ORDERED_COLLECTION structure > gEfiMdePkgTokenSpaceGuid.PcdValidateOrderedCollection|FALSE|BOOLEAN|0x0000002a > > + ## Indicates if DEBUG output should use ANSI sequences.

> + # TRUE - Will use ANSI sequences in DEBUG output.
> + # FALSE - Will not use ANSI sequences in DEBUG output.
> + 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 > //