For the sections you point. It seems good to watch the patch downloaded. The indention is one or two space behind the function name depend on tab expand(2 space). Is it need to adjust them?
Thanks, Zhichao > -----Original Message----- > From: Wu, Hao A > Sent: Friday, March 22, 2019 10:09 AM > To: Gao, Zhichao <[email protected]>; [email protected] > Cc: Gao, Liming <[email protected]>; Wang, Jian J > <[email protected]>; Ni, Ray <[email protected]>; Zeng, Star > <[email protected]>; Sean Brogan <[email protected]>; > Michael Turner <[email protected]>; Bret Barkelew > <[email protected]> > Subject: RE: [PATCH V4 16/17] MdeModulePkg/PeiDebugLibDebugPpi: Add > PEI debug lib > > > -----Original Message----- > > From: Gao, Zhichao > > Sent: Thursday, March 21, 2019 10:05 PM > > To: [email protected] > > 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 <[email protected]> > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395 > > > > 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 <[email protected]> > > Cc: Jian J Wang <[email protected]> > > Cc: Hao Wu <[email protected]> > > Cc: Ray Ni <[email protected]> > > Cc: Star Zeng <[email protected]> > > Cc: Liming Gao <[email protected]> > > Cc: Sean Brogan <[email protected]> > > Cc: Michael Turner <[email protected]> > > Cc: Bret Barkelew <[email protected]> > > --- > > .../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.<BR> > > + > > + 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 <PiPei.h> > > +#include <Ppi/Debug.h> > > +#include <Library/DebugLib.h> > > +#include <Library/BaseMemoryLib.h> > > +#include <Library/PcdLib.h> > > +#include <Library/PeiServicesLib.h> > > +#include <Library/DebugPrintErrorLevelLib.h> > > + > > +EDKII_DEBUG_PPI *mDebugPpi = 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 > > + function 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 accessed > > + 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 > > + function 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 != NULL); > > + > > + // > > + // Check driver Debug Level value and global debug level // if > > + ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) { > > + return; > > + } > > + > > + if (mDebugPpi == NULL) { > > + Status = PeiServicesLocatePpi ( > > ***** > > > + &gEdkiiDebugPpiGuid, > > + 0, > > + NULL, > > + (VOID **)&mDebugPpi > > + ); > > ***** > > The indention seems not consistent for the above several lines. > > > > + if (EFI_ERROR (Status)) { > > + CpuDeadLoop(); > > + } > > + } > > + > > + mDebugPpi->DebugBPrint ( > > ***** > > > + 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 > consumed > > + by processing Format. > > + @param Size The size, in bytes, of the BaseListMarker buffer. > > + > > + @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 != NULL); > > + > > + ASSERT (BaseListMarker != NULL); > > + > > + BaseListStart = BaseListMarker; > > + > > + for (; *Format != '\0'; Format++) { > > + // > > + // Only format with prefix % is processed. > > + // > > + if (*Format != '%') { > > + continue; > > + } > > + > > + Long = FALSE; > > + > > + // > > + // Parse Flags and Width > > + // > > + for (Format++; TRUE; Format++) { > > + if (*Format == '.' || *Format == '-' || *Format == '+' || *Format == > > ' ') { > > + // > > + // These characters in format field are omitted. > > + // > > + continue; > > + } > > + if (*Format >= '0' && *Format <= '9') { > > + // > > + // These characters in format field are omitted. > > + // > > + continue; > > + } > > + if (*Format == 'L' || *Format == 'l') { > > + // > > + // 'L" or "l" in format field means the number being printed is a > UINT64 > > + // > > + Long = TRUE; > > + continue; > > + } > > + if (*Format == '*') { > > + // > > + // '*' in format field means the precision of the field is > > specified by > > + // a UINTN argument in the argument list. > > + // > > + BASE_ARG (BaseListMarker, UINTN) = VA_ARG (VaListMarker, > UINTN); > > + continue; > > + } > > + if (*Format == '\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 == 'p') && (sizeof (VOID *) > 4)) { > > + Long = TRUE; > > + } > > + if (*Format == 'p' || *Format == 'X' || *Format == 'x' || *Format > > + == 'd' || > > *Format == 'u') { > > + if (Long) { > > + BASE_ARG (BaseListMarker, INT64) = VA_ARG (VaListMarker, INT64); > > + } else { > > + BASE_ARG (BaseListMarker, int) = VA_ARG (VaListMarker, int); > > + } > > + } else if (*Format == 's' || *Format == 'S' || *Format == 'a' || > > + *Format == > > 'g' || *Format == 't') { > > + BASE_ARG (BaseListMarker, VOID *) = VA_ARG (VaListMarker, VOID > *); > > + } else if (*Format == 'c') { > > + BASE_ARG (BaseListMarker, UINTN) = VA_ARG (VaListMarker, UINTN); > > + } else if (*Format == 'r') { > > + BASE_ARG (BaseListMarker, RETURN_STATUS) = VA_ARG > > + (VaListMarker, > > RETURN_STATUS); > > + } > > + > > + // > > + // If the converted BASE_LIST is larger than the size of > > + BaseListMarker, 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 > > + function 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 = 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 <FileName>(<LineNumber>): > > <Description>\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 > > + called > > while > > + processing another DebugAssert(), then DebugAssert() must return > > immediately. > > + > > + If FileName is NULL, then a <FileName> string of "(NULL) Filename" > > + is > > printed. > > + If Description is NULL, then a <Description> string of "(NULL) > > + Description" 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 > condition. > > + > > +**/ > > +VOID > > +EFIAPI > > +DebugAssert ( > > + IN CONST CHAR8 *FileName, > > + IN UINTN LineNumber, > > + IN CONST CHAR8 *Description > > + ) > > +{ > > + EFI_STATUS Status; > > + > > + if (mDebugPpi == NULL) { > > + Status = PeiServicesLocatePpi ( > > ***** > > > + &gEdkiiDebugPpiGuid, > > + 0, > > + NULL, > > + (VOID **)&mDebugPpi > > + ); > > ***** > > The indention seems not consistent for the above several lines. > > > > + if (EFI_ERROR (Status)) { > > + CpuDeadLoop(); > > + } > > + } > > + > > + mDebugPpi->DebugAssert ( > > ***** > > > + 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 != 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) != 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) != 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) != 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) != 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)) != 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 check > > for FTW last write data has been done. > > Please help to correct the above description for the INF file. > > With the these minor comments handled: > Reviewed-by: Hao Wu <[email protected]> > > Best Regards, > Hao Wu > > > > +# > > +# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> # > > +# 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 = 0x00010005 > > + BASE_NAME = PeiDebugLibDebugPpi > > + FILE_GUID = 2E08836C-4D1C-42F7-BBBE-EC5D25F1FDD4 > > + MODULE_TYPE = PEIM > > + VERSION_STRING = 1.0 > > + LIBRARY_CLASS = DebugLib|PEIM > > + > > +# > > +# The following information is for reference only and not required by > > +the build > > tools. > > +# > > +# VALID_ARCHITECTURES = 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 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
