From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.65; helo=mga03.intel.com; envelope-from=liming.gao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 718C2211D9155 for ; Wed, 27 Mar 2019 19:04:10 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Mar 2019 19:04:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,278,1549958400"; d="scan'208";a="137875461" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga003.jf.intel.com with ESMTP; 27 Mar 2019 19:04:09 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 27 Mar 2019 19:04:09 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.74]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.158]) with mapi id 14.03.0415.000; Thu, 28 Mar 2019 10:04:07 +0800 From: "Gao, Liming" To: "Gao, Zhichao" , "edk2-devel@lists.01.org" CC: "Wang, Jian J" , "Wu, Hao A" , "Ni, Ray" , "Zeng, Star" , Sean Brogan , Michael Turner , Bret Barkelew Thread-Topic: [PATCH V4 16/17] MdeModulePkg/PeiDebugLibDebugPpi: Add PEI debug lib Thread-Index: AQHU3+8/X1bHnBsJX02cvbQcj9WNUKYgU3gg Date: Thu, 28 Mar 2019 02:04:06 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E40CD8B@SHSMSX104.ccr.corp.intel.com> References: <20190321140459.18304-1-zhichao.gao@intel.com> <20190321140459.18304-17-zhichao.gao@intel.com> In-Reply-To: <20190321140459.18304-17-zhichao.gao@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH V4 16/17] MdeModulePkg/PeiDebugLibDebugPpi: Add PEI debug lib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Mar 2019 02:04:11 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Zhichao: For new library instance, I have some comments.=20 1) DebugPrint(). If DebugPpi is not found, directly return instead of CpuDe= adLoop(). 2) DebugAssert(). If DebugPpi is not found, bases on PcdDebugPropertyMask v= alue to trig CpuDeadLoop() or CpuBreakPoint().=20 3) Please rename mDebugPpi as the specific prefix name to avoid the name co= nfliction.=20 4) What test is done for this library instance?=20 Thanks Liming >-----Original Message----- >From: Gao, Zhichao >Sent: Thursday, March 21, 2019 10:05 PM >To: edk2-devel@lists.01.org >Cc: Gao, Liming ; Wang, Jian J >; Wu, Hao A ; Ni, Ray >; Zeng, Star ; Sean Brogan >; Michael Turner >; Bret Barkelew > >Subject: [PATCH V4 16/17] MdeModulePkg/PeiDebugLibDebugPpi: Add PEI >debug lib > >From: Liming Gao > >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1395 > >Add a PEI debug library instance PeiDebugLibDebugPpi base on >DebugPpi. Using the combination of the DebugServicePei and >this lib instance can reduce the image size of PEI drivers. > >Contributed-under: TianoCore Contribution Agreement 1.1 >Signed-off-by: Zhichao Gao >Cc: Jian J Wang >Cc: Hao Wu >Cc: Ray Ni >Cc: Star Zeng >Cc: Liming Gao >Cc: Sean Brogan >Cc: Michael Turner >Cc: Bret Barkelew >--- > .../Library/PeiDebugLibDebugPpi/DebugLib.c | 456 >+++++++++++++++++++++ > .../PeiDebugLibDebugPpi/PeiDebugLibDebugPpi.inf | 55 +++ > 2 files changed, 511 insertions(+) > create mode 100644 >MdeModulePkg/Library/PeiDebugLibDebugPpi/DebugLib.c > create mode 100644 >MdeModulePkg/Library/PeiDebugLibDebugPpi/PeiDebugLibDebugPpi.inf > >diff --git a/MdeModulePkg/Library/PeiDebugLibDebugPpi/DebugLib.c >b/MdeModulePkg/Library/PeiDebugLibDebugPpi/DebugLib.c >new file mode 100644 >index 0000000000..75859163ae >--- /dev/null >+++ b/MdeModulePkg/Library/PeiDebugLibDebugPpi/DebugLib.c >@@ -0,0 +1,456 @@ >+/** @file >+ PEI debug lib instance base on DebugPpi to save size >+ >+ Copyright (c) 2019, Intel Corporation. All rights reserved.
>+ >+ This program and the accompanying materials >+ are licensed and made available under the terms and conditions of the B= SD >License >+ which accompanies this distribution. The full text of the license may = be >found at >+ http://opensource.org/licenses/bsd-license.php >+ >+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >BASIS, >+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER >EXPRESS OR IMPLIED. >+ >+**/ >+ >+#include >+#include >+#include >+#include >+#include >+#include >+#include >+ >+EDKII_DEBUG_PPI *mDebugPpi =3D NULL; >+ >+/** >+ Prints a debug message to the debug output device if the specified >+ error level is enabled. >+ >+ If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib functio= n >+ GetDebugPrintErrorLevel (), then print the message specified by Format >and >+ the associated variable argument list to the debug output device. >+ >+ If Format is NULL, then ASSERT(). >+ >+ @param ErrorLevel The error level of the debug message. >+ @param Format Format string for the debug message to print. >+ @param ... Variable argument list whose contents are accesse= d >+ based on the format string specified by Format. >+ >+**/ >+VOID >+EFIAPI >+DebugPrint ( >+ IN UINTN ErrorLevel, >+ IN CONST CHAR8 *Format, >+ ... >+ ) >+{ >+ VA_LIST Marker; >+ >+ VA_START (Marker, Format); >+ DebugVPrint (ErrorLevel, Format, Marker); >+ VA_END (Marker); >+} >+ >+ >+/** >+ Prints a debug message to the debug output device if the specified >+ error level is enabled. >+ This function use BASE_LIST which would provide a more compatible >+ service than VA_LIST. >+ >+ If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib functio= n >+ GetDebugPrintErrorLevel (), then print the message specified by Format >and >+ the associated variable argument list to the debug output device. >+ >+ If Format is NULL, then ASSERT(). >+ >+ @param ErrorLevel The error level of the debug message. >+ @param Format Format string for the debug message to print. >+ @param BaseListMarker BASE_LIST marker for the variable argument list= . >+ >+**/ >+VOID >+EFIAPI >+DebugBPrint ( >+ IN UINTN ErrorLevel, >+ IN CONST CHAR8 *Format, >+ IN BASE_LIST BaseListMarker >+ ) >+{ >+ EFI_STATUS Status; >+ >+ // >+ // If Format is NULL, then ASSERT(). >+ // >+ ASSERT (Format !=3D NULL); >+ >+ // >+ // Check driver Debug Level value and global debug level >+ // >+ if ((ErrorLevel & GetDebugPrintErrorLevel ()) =3D=3D 0) { >+ return; >+ } >+ >+ if (mDebugPpi =3D=3D NULL) { >+ Status =3D PeiServicesLocatePpi ( >+ &gEdkiiDebugPpiGuid, >+ 0, >+ NULL, >+ (VOID **)&mDebugPpi >+ ); >+ if (EFI_ERROR (Status)) { >+ CpuDeadLoop(); >+ } >+ } >+ >+ mDebugPpi->DebugBPrint ( >+ ErrorLevel, >+ Format, >+ BaseListMarker >+ ); >+} >+ >+ >+/** >+ Worker function that convert a VA_LIST to a BASE_LIST based on a >+ Null-terminated format string. >+ >+ @param Format Null-terminated format string. >+ @param VaListMarker VA_LIST style variable argument list consumed >+ by processing Format. >+ @param BaseListMarker BASE_LIST style variable argument list consumed >+ by processing Format. >+ @param Size The size, in bytes, of the BaseListMarker buffe= r. >+ >+ @return TRUE The VA_LIST has been converted to BASE_LIST. >+ @return FALSE The VA_LIST has not been converted to BASE_LIST. >+ >+**/ >+BOOLEAN >+VaListToBaseList ( >+ IN CONST CHAR8 *Format, >+ IN VA_LIST VaListMarker, >+ OUT BASE_LIST BaseListMarker, >+ IN UINTN Size >+ ) >+{ >+ BASE_LIST BaseListStart; >+ BOOLEAN Long; >+ >+ ASSERT (Format !=3D NULL); >+ >+ ASSERT (BaseListMarker !=3D NULL); >+ >+ BaseListStart =3D BaseListMarker; >+ >+ for (; *Format !=3D '\0'; Format++) { >+ // >+ // Only format with prefix % is processed. >+ // >+ if (*Format !=3D '%') { >+ continue; >+ } >+ >+ Long =3D FALSE; >+ >+ // >+ // Parse Flags and Width >+ // >+ for (Format++; TRUE; Format++) { >+ if (*Format =3D=3D '.' || *Format =3D=3D '-' || *Format =3D=3D '+' = || *Format =3D=3D ' ') { >+ // >+ // These characters in format field are omitted. >+ // >+ continue; >+ } >+ if (*Format >=3D '0' && *Format <=3D '9') { >+ // >+ // These characters in format field are omitted. >+ // >+ continue; >+ } >+ if (*Format =3D=3D 'L' || *Format =3D=3D 'l') { >+ // >+ // 'L" or "l" in format field means the number being printed is a= UINT64 >+ // >+ Long =3D TRUE; >+ continue; >+ } >+ if (*Format =3D=3D '*') { >+ // >+ // '*' in format field means the precision of the field is specif= ied by >+ // a UINTN argument in the argument list. >+ // >+ BASE_ARG (BaseListMarker, UINTN) =3D VA_ARG (VaListMarker, UINTN)= ; >+ continue; >+ } >+ if (*Format =3D=3D '\0') { >+ // >+ // Make no output if Format string terminates unexpectedly when >+ // looking up for flag, width, precision and type. >+ // >+ Format--; >+ } >+ // >+ // When valid argument type detected or format string terminates >unexpectedly, >+ // the inner loop is done. >+ // >+ break; >+ } >+ >+ // >+ // Pack variable arguments into the storage area following >EFI_DEBUG_INFO. >+ // >+ if ((*Format =3D=3D 'p') && (sizeof (VOID *) > 4)) { >+ Long =3D TRUE; >+ } >+ if (*Format =3D=3D 'p' || *Format =3D=3D 'X' || *Format =3D=3D 'x' ||= *Format =3D=3D 'd' || >*Format =3D=3D 'u') { >+ if (Long) { >+ BASE_ARG (BaseListMarker, INT64) =3D VA_ARG (VaListMarker, INT64)= ; >+ } else { >+ BASE_ARG (BaseListMarker, int) =3D VA_ARG (VaListMarker, int); >+ } >+ } else if (*Format =3D=3D 's' || *Format =3D=3D 'S' || *Format =3D=3D= 'a' || *Format =3D=3D >'g' || *Format =3D=3D 't') { >+ BASE_ARG (BaseListMarker, VOID *) =3D VA_ARG (VaListMarker, VOID *)= ; >+ } else if (*Format =3D=3D 'c') { >+ BASE_ARG (BaseListMarker, UINTN) =3D VA_ARG (VaListMarker, UINTN); >+ } else if (*Format =3D=3D 'r') { >+ BASE_ARG (BaseListMarker, RETURN_STATUS) =3D VA_ARG (VaListMarker, >RETURN_STATUS); >+ } >+ >+ // >+ // If the converted BASE_LIST is larger than the size of BaseListMark= er, >then return FALSE >+ // >+ if (((UINTN)BaseListMarker - (UINTN)BaseListStart) > Size) { >+ return FALSE; >+ } >+ } >+ >+ return TRUE; >+} >+ >+ >+/** >+ Prints a debug message to the debug output device if the specified >+ error level is enabled. >+ >+ If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib functio= n >+ GetDebugPrintErrorLevel (), then print the message specified by Format >and >+ the associated variable argument list to the debug output device. >+ >+ If Format is NULL, then ASSERT(). >+ >+ @param ErrorLevel The error level of the debug message. >+ @param Format Format string for the debug message to print. >+ @param VaListMarker VA_LIST marker for the variable argument list. >+ >+**/ >+VOID >+EFIAPI >+DebugVPrint ( >+ IN UINTN ErrorLevel, >+ IN CONST CHAR8 *Format, >+ IN VA_LIST VaListMarker >+ ) >+{ >+ UINT64 BaseListMarker[256 / sizeof (UINT64)]; >+ BOOLEAN Converted; >+ >+ // >+ // Convert the VaList to BaseList >+ // >+ Converted =3D VaListToBaseList ( >+ Format, >+ VaListMarker, >+ (BASE_LIST)BaseListMarker, >+ sizeof (BaseListMarker) - 8 >+ ); >+ >+ if (!Converted) { >+ return; >+ } >+ >+ DebugBPrint (ErrorLevel, Format, (BASE_LIST)BaseListMarker); >+} >+ >+ >+/** >+ Prints an assert message containing a filename, line number, and >description. >+ This may be followed by a breakpoint or a dead loop. >+ >+ Print a message of the form "ASSERT (): >\n" >+ to the debug output device. If >DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED bit of >+ PcdDebugProperyMask is set then CpuBreakpoint() is called. Otherwise, i= f >+ DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED bit of >PcdDebugProperyMask is set then >+ CpuDeadLoop() is called. If neither of these bits are set, then this f= unction >+ returns immediately after the message is printed to the debug output >device. >+ DebugAssert() must actively prevent recursion. If DebugAssert() is cal= led >while >+ processing another DebugAssert(), then DebugAssert() must return >immediately. >+ >+ If FileName is NULL, then a string of "(NULL) Filename" is >printed. >+ If Description is NULL, then a string of "(NULL) Descript= ion" is >printed. >+ >+ @param FileName The pointer to the name of the source file that >generated the assert condition. >+ @param LineNumber The line number in the source file that generated >the assert condition >+ @param Description The pointer to the description of the assert condi= tion. >+ >+**/ >+VOID >+EFIAPI >+DebugAssert ( >+ IN CONST CHAR8 *FileName, >+ IN UINTN LineNumber, >+ IN CONST CHAR8 *Description >+ ) >+{ >+ EFI_STATUS Status; >+ >+ if (mDebugPpi =3D=3D NULL) { >+ Status =3D PeiServicesLocatePpi ( >+ &gEdkiiDebugPpiGuid, >+ 0, >+ NULL, >+ (VOID **)&mDebugPpi >+ ); >+ if (EFI_ERROR (Status)) { >+ CpuDeadLoop(); >+ } >+ } >+ >+ mDebugPpi->DebugAssert ( >+ FileName, >+ LineNumber, >+ Description >+ ); >+} >+ >+ >+/** >+ Fills a target buffer with PcdDebugClearMemoryValue, and returns the >target buffer. >+ >+ This function fills Length bytes of Buffer with the value specified by >+ PcdDebugClearMemoryValue, and returns Buffer. >+ >+ If Buffer is NULL, then ASSERT(). >+ If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT(). >+ >+ @param Buffer The pointer to the target buffer to be filled with >PcdDebugClearMemoryValue. >+ @param Length The number of bytes in Buffer to fill with zeros >PcdDebugClearMemoryValue. >+ >+ @return Buffer The pointer to the target buffer filled with >PcdDebugClearMemoryValue. >+ >+**/ >+VOID * >+EFIAPI >+DebugClearMemory ( >+ OUT VOID *Buffer, >+ IN UINTN Length >+ ) >+{ >+ ASSERT (Buffer !=3D NULL); >+ >+ return SetMem (Buffer, Length, PcdGet8 (PcdDebugClearMemoryValue)); >+} >+ >+ >+/** >+ Returns TRUE if ASSERT() macros are enabled. >+ >+ This function returns TRUE if the >DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of >+ PcdDebugProperyMask is set. Otherwise, FALSE is returned. >+ >+ @retval TRUE The DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of >PcdDebugProperyMask is set. >+ @retval FALSE The DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of >PcdDebugProperyMask is clear. >+ >+**/ >+BOOLEAN >+EFIAPI >+DebugAssertEnabled ( >+ VOID >+ ) >+{ >+ return (BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & >DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED) !=3D 0); >+} >+ >+ >+/** >+ Returns TRUE if DEBUG() macros are enabled. >+ >+ This function returns TRUE if the >DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of >+ PcdDebugProperyMask is set. Otherwise, FALSE is returned. >+ >+ @retval TRUE The DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of >PcdDebugProperyMask is set. >+ @retval FALSE The DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of >PcdDebugProperyMask is clear. >+ >+**/ >+BOOLEAN >+EFIAPI >+DebugPrintEnabled ( >+ VOID >+ ) >+{ >+ return (BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & >DEBUG_PROPERTY_DEBUG_PRINT_ENABLED) !=3D 0); >+} >+ >+ >+/** >+ Returns TRUE if DEBUG_CODE() macros are enabled. >+ >+ This function returns TRUE if the >DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of >+ PcdDebugProperyMask is set. Otherwise, FALSE is returned. >+ >+ @retval TRUE The DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of >PcdDebugProperyMask is set. >+ @retval FALSE The DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of >PcdDebugProperyMask is clear. >+ >+**/ >+BOOLEAN >+EFIAPI >+DebugCodeEnabled ( >+ VOID >+ ) >+{ >+ return (BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & >DEBUG_PROPERTY_DEBUG_CODE_ENABLED) !=3D 0); >+} >+ >+ >+/** >+ Returns TRUE if DEBUG_CLEAR_MEMORY() macro is enabled. >+ >+ This function returns TRUE if the >DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of >+ PcdDebugProperyMask is set. Otherwise, FALSE is returned. >+ >+ @retval TRUE The DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of >PcdDebugProperyMask is set. >+ @retval FALSE The DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of >PcdDebugProperyMask is clear. >+ >+**/ >+BOOLEAN >+EFIAPI >+DebugClearMemoryEnabled ( >+ VOID >+ ) >+{ >+ return (BOOLEAN) ((PcdGet8 (PcdDebugPropertyMask) & >DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED) !=3D 0); >+} >+ >+ >+/** >+ Returns TRUE if any one of the bit is set both in ErrorLevel and >PcdFixedDebugPrintErrorLevel. >+ >+ This function compares the bit mask of ErrorLevel and >PcdFixedDebugPrintErrorLevel. >+ >+ @retval TRUE Current ErrorLevel is supported. >+ @retval FALSE Current ErrorLevel is not supported. >+ >+**/ >+BOOLEAN >+EFIAPI >+DebugPrintLevelEnabled ( >+ IN CONST UINTN ErrorLevel >+ ) >+{ >+ return (BOOLEAN) ((ErrorLevel & >PcdGet32(PcdFixedDebugPrintErrorLevel)) !=3D 0); >+} >+ >diff --git >a/MdeModulePkg/Library/PeiDebugLibDebugPpi/PeiDebugLibDebugPpi.inf >b/MdeModulePkg/Library/PeiDebugLibDebugPpi/PeiDebugLibDebugPpi.inf >new file mode 100644 >index 0000000000..4ab21e577e >--- /dev/null >+++ >b/MdeModulePkg/Library/PeiDebugLibDebugPpi/PeiDebugLibDebugPpi.inf >@@ -0,0 +1,55 @@ >+## @file >+# Debug Lib instance through DebugServicePei for PEI phase >+# >+# This module installs gEdkiiFaultTolerantWriteGuid PPI to inform the ch= eck >for FTW last write data has been done. >+# >+# Copyright (c) 2019, Intel Corporation. All rights reserved.
>+# >+# This program and the accompanying materials >+# are licensed and made available under the terms and conditions of the >BSD License >+# which accompanies this distribution. The full text of the license may = be >found at >+# http://opensource.org/licenses/bsd-license.php >+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >BASIS, >+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER >EXPRESS OR IMPLIED. >+# >+## >+ >+[Defines] >+ INF_VERSION =3D 0x00010005 >+ BASE_NAME =3D PeiDebugLibDebugPpi >+ FILE_GUID =3D 2E08836C-4D1C-42F7-BBBE-EC5D25F1FDD4 >+ MODULE_TYPE =3D PEIM >+ VERSION_STRING =3D 1.0 >+ LIBRARY_CLASS =3D DebugLib|PEIM >+ >+# >+# The following information is for reference only and not required by the >build tools. >+# >+# VALID_ARCHITECTURES =3D IA32 X64 EBC >+# >+ >+[Sources] >+ DebugLib.c >+ >+[Packages] >+ MdePkg/MdePkg.dec >+ MdeModulePkg/MdeModulePkg.dec >+ >+[LibraryClasses] >+ PcdLib >+ BaseMemoryLib >+ DebugPrintErrorLevelLib >+ PeiServicesLib >+ PeiServicesTablePointerLib >+ >+[Ppis] >+ gEdkiiDebugPpiGuid ## CONSUMES >+ >+[Pcd] >+ gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue ## >SOMETIMES_CONSUMES >+ gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask ## CONSUMES >+ gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel ## >CONSUMES >+ >+[Depex] >+ gEdkiiDebugPpiGuid >+ >-- >2.16.2.windows.1