From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.151; helo=mga17.intel.com; envelope-from=hao.a.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) (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 17045211DF238 for ; Wed, 20 Mar 2019 22:22:28 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Mar 2019 22:22:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,251,1549958400"; d="scan'208";a="142522782" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by FMSMGA003.fm.intel.com with ESMTP; 20 Mar 2019 22:22:28 -0700 Received: from fmsmsx125.amr.corp.intel.com (10.18.125.40) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 20 Mar 2019 22:22:28 -0700 Received: from shsmsx107.ccr.corp.intel.com (10.239.4.96) by FMSMSX125.amr.corp.intel.com (10.18.125.40) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 20 Mar 2019 22:22:28 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.74]) by SHSMSX107.ccr.corp.intel.com ([169.254.9.252]) with mapi id 14.03.0415.000; Thu, 21 Mar 2019 13:22:25 +0800 From: "Wu, Hao A" To: "Gao, Zhichao" , "edk2-devel@lists.01.org" CC: "Gao, Liming" , "Wang, Jian J" , "Ni, Ray" , "Zeng, Star" , Sean Brogan , "Michael Turner" , Bret Barkelew Thread-Topic: [PATCH V3 16/17] MdeModulePkg/PeiDebugLibDebugPpi: Add PEI debug lib Thread-Index: AQHU3mg6wb7i0nCt3Uqr57v3IfjkuaYVhtLw Date: Thu, 21 Mar 2019 05:22:25 +0000 Message-ID: References: <20190319152549.16104-1-zhichao.gao@intel.com> <20190319152549.16104-17-zhichao.gao@intel.com> In-Reply-To: <20190319152549.16104-17-zhichao.gao@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH V3 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, 21 Mar 2019 05:22:29 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hello, Besides the comments in patch 14/17 to remove the 'PeiServices' & 'This' parameters from the PPI services, some minor comments below: > -----Original Message----- > From: Gao, Zhichao > Sent: Tuesday, March 19, 2019 11:26 PM > To: edk2-devel@lists.01.org > Cc: Gao, Liming; Wang, Jian J; Wu, Hao A; Ni, Ray; Zeng, Star; Sean Broga= n; > Michael Turner; Bret Barkelew > Subject: [PATCH V3 16/17] MdeModulePkg/PeiDebugLibDebugPpi: Add PEI > debug lib >=20 > From: Liming Gao >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1395 >=20 > 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. >=20 > 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 | 469 > +++++++++++++++++++++ > .../PeiDebugLibDebugPpi/PeiDebugLibDebugPpi.inf | 55 +++ > 2 files changed, 524 insertions(+) > create mode 100644 > MdeModulePkg/Library/PeiDebugLibDebugPpi/DebugLib.c > create mode 100644 > MdeModulePkg/Library/PeiDebugLibDebugPpi/PeiDebugLibDebugPpi.inf >=20 > diff --git a/MdeModulePkg/Library/PeiDebugLibDebugPpi/DebugLib.c > b/MdeModulePkg/Library/PeiDebugLibDebugPpi/DebugLib.c > new file mode 100644 > index 0000000000..839dff5268 > --- /dev/null > +++ b/MdeModulePkg/Library/PeiDebugLibDebugPpi/DebugLib.c > @@ -0,0 +1,469 @@ > +/** @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 = 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. > + > +**/ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +EDKII_DEBUG_PPI *mDebugPpi =3D NULL; > +CONST EFI_PEI_SERVICES **mPeiServicesTablePointer =3D NULL; If the EDKII_DEBUG_PPI interface changes, the above variable can be removed. > + > +/** > + 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 functi= on > + 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 > +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 functi= on > + 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 lis= t. > + > +**/ > +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 > + ); The indention seems not consistent for the above several lines. > + if (EFI_ERROR (Status)) { > + CpuDeadLoop(); > + } > + } > + > + if (mPeiServicesTablePointer =3D=3D NULL) { > + mPeiServicesTablePointer =3D GetPeiServicesTablePointer (); > + } If the EDKII_DEBUG_PPI interface changes, the above lines can be removed. > + > + mDebugPpi->DebugBPrint ( > + mPeiServicesTablePointer, > + mDebugPpi, > + ErrorLevel, > + Format, > + BaseListMarker > + ); The indention seems not consistent for the above several lines. > +} > + > + > +/** > + 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 consume= d > + by processing Format. > + @param Size The size, in bytes, of the BaseListMarker buff= er. > + > + @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 speci= fied 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 BaseListMar= ker, > 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 functi= on > + 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, = if > + DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED bit of > PcdDebugProperyMask is set then > + CpuDeadLoop() is called. If neither of these bits are set, then this = function > + returns immediately after the message is printed to the debug output > device. > + DebugAssert() must actively prevent recursion. If DebugAssert() is ca= lled > 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) Descrip= tion" 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 cond= ition. > + > +**/ > +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 > + ); The indention seems not consistent for the above several lines. > + if (EFI_ERROR (Status)) { > + CpuDeadLoop(); > + } > + } > + > + if (mPeiServicesTablePointer =3D=3D NULL) { > + mPeiServicesTablePointer =3D GetPeiServicesTablePointer (); > + } If the EDKII_DEBUG_PPI interface changes, the above lines can be removed. > + > + mDebugPpi->DebugAssert ( > + mPeiServicesTablePointer, > + mDebugPpi, > + FileName, > + LineNumber, > + Description > + ); The indention seems not consistent for the above several lines. > +} > + > + > +/** > + 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 c= heck > for FTW last write data has been done. Please help to correct the above description for the INF file. Best Regards, Hao Wu > +# > +# 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-EC5D25F1FDD= 4 > + 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 th= e > 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