public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib
@ 2019-03-15  5:17 Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api " Zhichao Gao
                   ` (16 more replies)
  0 siblings, 17 replies; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Michael D Kinney, Leif Lindholm, Ard Biesheuvel, Jordan Justen,
	Laszlo Ersek, Chasel Chiu, Nate DeSimone, Star Zeng, Jian J Wang,
	Hao Wu, Ray Ni, Liming Gao, Sean Brogan, Michael Turner,
	Bret Barkelew

Add a new API DebugVPrint to all the instances of DebugLib.
This API is added to provide a function who want to implement
special debug function with '...' parameter.
Add a PEIM to install gEdkiiDebugPpiGuid, and implement a PEI
debug library instance base on it. All PEIMs except pei core
type can use the PeiDebugLibDebugPpi to reduce its image size.

V2: 
Remove redundant code in DebugPrint.
Fix some coding sytle issues.
Remove some unenforced descirption in the comments of DebugVPrint.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>

Liming Gao (1):
  MdeModulePkg/PeiDebugLibDebugPpi: Add PEI debug lib

Zhichao Gao (16):
  MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
  MdePkg/BaseDebugLibNull: Add a new api DebugVPrint for DebugLib
  MdePkg/BaseDebugLibSerialPort: Add a new api DebugVPrint
  MdePkg/UefidebugLibConOut: Add a new api DebugVPrint
  MdePkg/UefiDebugLibStdErr: Add a new api DebugVPrint
  MdePkg/DxeRuntimeDebugLibSerialPort: Add a new api DebugVPrint
  MdePkg/UefiDebuglibDebugPortProtocol: Add a new api DebugVPrint
  ArmPkg/SemiHostingDebugLib: Add a new api DebugVPrint
  OvmfPkg/PlatformDebugLibIoPort: Add a new api DebugVPrint
  IntelFsp2Pkg/BaseFspDebugLibSerialPort: Add a new api DebugVPrint
  IntelFspPkg/BaseFspDebugLibSerialPort: Add a new api DebugVPrint
  IntelFramworkModulePkg/PeiDxeDebugLibReportStatusCode: Add a new api
  MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add a new api
  MdeModulePkg: Add definitions for EDKII DEBUG PPI
  MdeModulePkg: Add a PEIM to install Debug PPI
  MdeModulePkg: Add PEIM and lib to dsc file

 ArmPkg/Library/SemiHostingDebugLib/DebugLib.c      |  38 ++-
 .../PeiDxeDebugLibReportStatusCode/DebugLib.c      |  40 ++-
 .../Library/BaseFspDebugLibSerialPort/DebugLib.c   |  42 ++-
 .../Library/BaseFspDebugLibSerialPort/DebugLib.c   |  42 ++-
 MdeModulePkg/Include/Ppi/Debug.h                   |  90 ++++++
 .../Library/PeiDebugLibDebugPpi/DebugLib.c         | 302 +++++++++++++++++++++
 .../PeiDebugLibDebugPpi/PeiDebugLibDebugPpi.inf    |  55 ++++
 .../PeiDxeDebugLibReportStatusCode/DebugLib.c      |  38 ++-
 MdeModulePkg/MdeModulePkg.dec                      |   3 +
 MdeModulePkg/MdeModulePkg.dsc                      |   3 +
 .../Universal/DebugServicePei/DebugService.c       |  68 +++++
 .../Universal/DebugServicePei/DebugService.h       |  64 +++++
 .../Universal/DebugServicePei/DebugServicePei.c    |  54 ++++
 .../Universal/DebugServicePei/DebugServicePei.inf  |  51 ++++
 .../Universal/DebugServicePei/DebugServicePei.uni  |  20 ++
 MdePkg/Include/Library/DebugLib.h                  |  33 ++-
 MdePkg/Library/BaseDebugLibNull/DebugLib.c         |  28 +-
 MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c   |  39 ++-
 .../DxeRuntimeDebugLibSerialPort/DebugLib.c        |  40 ++-
 MdePkg/Library/UefiDebugLibConOut/DebugLib.c       |  38 ++-
 .../UefiDebugLibDebugPortProtocol/DebugLib.c       |  39 ++-
 MdePkg/Library/UefiDebugLibStdErr/DebugLib.c       |  39 ++-
 OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c  |  38 ++-
 23 files changed, 1145 insertions(+), 59 deletions(-)
 create mode 100644 MdeModulePkg/Include/Ppi/Debug.h
 create mode 100644 MdeModulePkg/Library/PeiDebugLibDebugPpi/DebugLib.c
 create mode 100644 MdeModulePkg/Library/PeiDebugLibDebugPpi/PeiDebugLibDebugPpi.inf
 create mode 100644 MdeModulePkg/Universal/DebugServicePei/DebugService.c
 create mode 100644 MdeModulePkg/Universal/DebugServicePei/DebugService.h
 create mode 100644 MdeModulePkg/Universal/DebugServicePei/DebugServicePei.c
 create mode 100644 MdeModulePkg/Universal/DebugServicePei/DebugServicePei.inf
 create mode 100644 MdeModulePkg/Universal/DebugServicePei/DebugServicePei.uni

-- 
2.16.2.windows.1



^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  6:15   ` Jordan Justen
  2019-03-15  5:17 ` [PATCH V2 02/17] MdePkg/BaseDebugLibNull: " Zhichao Gao
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Michael D Kinney, Liming Gao, Sean Brogan, Michael Turner,
	Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a new api DebugVPrint prototype definition in the
DebugLib header file. This api would expose a print
routine with VaList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 MdePkg/Include/Library/DebugLib.h | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h
index e6a7a357b2..51d89bbd52 100644
--- a/MdePkg/Include/Library/DebugLib.h
+++ b/MdePkg/Include/Library/DebugLib.h
@@ -8,7 +8,7 @@
   of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is
   defined, then debug and assert related macros wrapped by it are the NULL implementations.
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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 that accompanies this distribution.
 The full text of the license may be found at
@@ -101,6 +101,29 @@ DebugPrint (
   );
 
 
+/**
+  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
+  );
+
+
 /**
   Prints an assert message containing a filename, line number, and description.
   This may be followed by a breakpoint or a dead loop.
@@ -221,6 +244,7 @@ DebugClearMemoryEnabled (
   VOID
   );
 
+
 /**
   Returns TRUE if any one of the bit is set both in ErrorLevel and PcdFixedDebugPrintErrorLevel.
 
@@ -236,6 +260,7 @@ DebugPrintLevelEnabled (
   IN  CONST UINTN        ErrorLevel
   );
 
+
 /**
   Internal worker macro that calls DebugAssert().
 
@@ -273,6 +298,7 @@ DebugPrintLevelEnabled (
 #define _DEBUG(Expression)   DebugPrint Expression
 #endif
 
+
 /**
   Macro that calls DebugAssert() if an expression evaluates to FALSE.
 
@@ -299,6 +325,7 @@ DebugPrintLevelEnabled (
   #define ASSERT(Expression)
 #endif
 
+
 /**
   Macro that calls DebugPrint().
 
@@ -322,6 +349,7 @@ DebugPrintLevelEnabled (
   #define DEBUG(Expression)
 #endif
 
+
 /**
   Macro that calls DebugAssert() if an EFI_STATUS evaluates to an error code.
 
@@ -348,6 +376,7 @@ DebugPrintLevelEnabled (
   #define ASSERT_EFI_ERROR(StatusParameter)
 #endif
 
+
 /**
   Macro that calls DebugAssert() if a RETURN_STATUS evaluates to an error code.
 
@@ -375,6 +404,7 @@ DebugPrintLevelEnabled (
   #define ASSERT_RETURN_ERROR(StatusParameter)
 #endif
 
+
 /**
   Macro that calls DebugAssert() if a protocol is already installed in the
   handle database.
@@ -418,6 +448,7 @@ DebugPrintLevelEnabled (
   #define ASSERT_PROTOCOL_ALREADY_INSTALLED(Handle, Guid)
 #endif
 
+
 /**
   Macro that marks the beginning of debug source code.
 
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 02/17] MdePkg/BaseDebugLibNull: Add a new api DebugVPrint for DebugLib
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api " Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 03/17] MdePkg/BaseDebugLibSerialPort: Add a new api DebugVPrint Zhichao Gao
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Michael D Kinney, Liming Gao, Sean Brogan, Michael Turner,
	Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a new api DebugVPrint implementation in the
DebugLib instance. This api would expose a print
routine with VaList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 MdePkg/Library/BaseDebugLibNull/DebugLib.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseDebugLibNull/DebugLib.c b/MdePkg/Library/BaseDebugLibNull/DebugLib.c
index 1a7d4aba79..cc8b79ef6f 100644
--- a/MdePkg/Library/BaseDebugLibNull/DebugLib.c
+++ b/MdePkg/Library/BaseDebugLibNull/DebugLib.c
@@ -1,7 +1,7 @@
 /** @file
   Null Base Debug Library instance with empty functions.
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 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
@@ -41,6 +41,31 @@ DebugPrint (
 }
 
 
+/**
+  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
+  )
+{
+}
+
+
 /**
   Prints an assert message containing a filename, line number, and description.
   This may be followed by a breakpoint or a dead loop.
@@ -178,6 +203,7 @@ DebugClearMemoryEnabled (
   return FALSE;
 }
 
+
 /**
   Returns TRUE if any one of the bit is set both in ErrorLevel and PcdFixedDebugPrintErrorLevel.
 
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 03/17] MdePkg/BaseDebugLibSerialPort: Add a new api DebugVPrint
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api " Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 02/17] MdePkg/BaseDebugLibNull: " Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 04/17] MdePkg/UefidebugLibConOut: " Zhichao Gao
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Michael D Kinney, Liming Gao, Sean Brogan, Michael Turner,
	Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a new api DebugVPrint implementation in the
DebugLib instance. This api would expose a print
routine with VaList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c | 39 +++++++++++++++++++++---
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
index ffb84b39e5..0321e9c955 100644
--- a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
+++ b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
@@ -7,7 +7,7 @@
   being blocked.  This may occur if a key(s) are pressed in a terminal emulator
   used to monitor the DEBUG() and ASSERT() messages.
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 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
@@ -47,6 +47,7 @@ BaseDebugLibSerialPortConstructor (
   return SerialPortInitialize ();
 }
 
+
 /**
   Prints a debug message to the debug output device if the specified error level is enabled.
 
@@ -70,9 +71,38 @@ DebugPrint (
   ...
   )
 {
-  CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
   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.
+
+  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
+  )
+{
+  CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
+
   //
   // If Format is NULL, then ASSERT().
   //
@@ -88,9 +118,7 @@ DebugPrint (
   //
   // Convert the DEBUG() message to an ASCII String
   //
-  VA_START (Marker, Format);
-  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
-  VA_END (Marker);
+  AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker);
 
   //
   // Send the print string to a Serial Port
@@ -264,6 +292,7 @@ DebugClearMemoryEnabled (
   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.
 
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 04/17] MdePkg/UefidebugLibConOut: Add a new api DebugVPrint
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (2 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 03/17] MdePkg/BaseDebugLibSerialPort: Add a new api DebugVPrint Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 05/17] MdePkg/UefiDebugLibStdErr: " Zhichao Gao
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Michael D Kinney, Liming Gao, Sean Brogan, Michael Turner,
	Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a new api DebugVPrint implementation in the
DebugLib instance. This api would expose a print
routine with VaList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 MdePkg/Library/UefiDebugLibConOut/DebugLib.c | 38 ++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
index f04207c93f..a3c342f91d 100644
--- a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
+++ b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
@@ -1,7 +1,7 @@
 /** @file
   UEFI Debug Library that sends messages to the Console Output Device in the EFI System Table.
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 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
@@ -50,9 +50,38 @@ DebugPrint (
   ...
   )
 {
-  CHAR16   Buffer[MAX_DEBUG_MESSAGE_LENGTH];
   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.
+
+  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
+  )
+{
+  CHAR16   Buffer[MAX_DEBUG_MESSAGE_LENGTH];
+
   //
   // If Format is NULL, then ASSERT().
   //
@@ -68,9 +97,7 @@ DebugPrint (
   //
   // Convert the DEBUG() message to a Unicode String
   //
-  VA_START (Marker, Format);
-  UnicodeVSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH,  Format, Marker);
-  VA_END (Marker);
+  UnicodeVSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH,  Format, VaListMarker);
 
 
   //
@@ -259,6 +286,7 @@ DebugClearMemoryEnabled (
   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.
 
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 05/17] MdePkg/UefiDebugLibStdErr: Add a new api DebugVPrint
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (3 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 04/17] MdePkg/UefidebugLibConOut: " Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 06/17] MdePkg/DxeRuntimeDebugLibSerialPort: " Zhichao Gao
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Michael D Kinney, Liming Gao, Sean Brogan, Michael Turner,
	Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a new api DebugVPrint implementation in the
DebugLib instance. This api would expose a print
routine with VaList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 MdePkg/Library/UefiDebugLibStdErr/DebugLib.c | 39 +++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/MdePkg/Library/UefiDebugLibStdErr/DebugLib.c b/MdePkg/Library/UefiDebugLibStdErr/DebugLib.c
index 6830a3caa1..6fd68beae2 100644
--- a/MdePkg/Library/UefiDebugLibStdErr/DebugLib.c
+++ b/MdePkg/Library/UefiDebugLibStdErr/DebugLib.c
@@ -1,7 +1,7 @@
 /** @file
   UEFI Debug Lib that sends messages to the Standard Error Device in the EFI System Table.
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 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
@@ -51,9 +51,38 @@ DebugPrint (
   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.
+
+  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
+  )
 {
   CHAR16   Buffer[MAX_DEBUG_MESSAGE_LENGTH];
-  VA_LIST  Marker;
 
   //
   // If Format is NULL, then ASSERT().
@@ -70,9 +99,7 @@ DebugPrint (
   //
   // Convert the DEBUG() message to a Unicode String
   //
-  VA_START (Marker, Format);
-  UnicodeVSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH, Format, Marker);
-  VA_END (Marker);
+  UnicodeVSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH, Format, VaListMarker);
 
   //
   // Send the print string to the Standard Error device
@@ -82,7 +109,6 @@ DebugPrint (
   }
 }
 
-
 /**
   Prints an assert message containing a filename, line number, and description.
   This may be followed by a breakpoint or a dead loop.
@@ -260,6 +286,7 @@ DebugClearMemoryEnabled (
   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.
 
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 06/17] MdePkg/DxeRuntimeDebugLibSerialPort: Add a new api DebugVPrint
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (4 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 05/17] MdePkg/UefiDebugLibStdErr: " Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 07/17] MdePkg/UefiDebuglibDebugPortProtocol: " Zhichao Gao
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Michael D Kinney, Liming Gao, Sean Brogan, Michael Turner,
	Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a new api DebugVPrint implementation in the
DebugLib instance. This api would expose a print
routine with VaList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 .../DxeRuntimeDebugLibSerialPort/DebugLib.c        | 40 +++++++++++++++++++---
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
index e1266f77fa..a0489b762f 100644
--- a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
+++ b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
@@ -4,7 +4,7 @@
   been called, to prevent touching hardware that is no longer owned by the
   firmware.
 
-  Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
   Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
 
   This program and the accompanying materials
@@ -52,6 +52,7 @@ ExitBootServicesEvent (
   mEfiAtRuntime = TRUE;
 }
 
+
 /**
   The constructor function to initialize the Serial Port library and
   register a callback for the ExitBootServices event.
@@ -83,6 +84,7 @@ DxeRuntimeDebugLibSerialPortConstructor (
                                       &mEfiExitBootServicesEvent);
 }
 
+
 /**
   If a runtime driver exits with an error, it must call this routine
   to free the allocated resource before the exiting.
@@ -103,6 +105,7 @@ DxeRuntimeDebugLibSerialPortDestructor (
   return SystemTable->BootServices->CloseEvent (mEfiExitBootServicesEvent);
 }
 
+
 /**
   Prints a debug message to the debug output device if the specified error level is enabled.
 
@@ -125,9 +128,37 @@ DebugPrint (
   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.
+
+  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
+  )
 {
   CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
-  VA_LIST  Marker;
 
   if (mEfiAtRuntime) {
     return;
@@ -148,9 +179,7 @@ DebugPrint (
   //
   // Convert the DEBUG() message to an ASCII String
   //
-  VA_START (Marker, Format);
-  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
-  VA_END (Marker);
+  AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker);
 
   //
   // Send the print string to a Serial Port
@@ -327,6 +356,7 @@ DebugClearMemoryEnabled (
   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.
 
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 07/17] MdePkg/UefiDebuglibDebugPortProtocol: Add a new api DebugVPrint
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (5 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 06/17] MdePkg/DxeRuntimeDebugLibSerialPort: " Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 08/17] ArmPkg/SemiHostingDebugLib: " Zhichao Gao
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Michael D Kinney, Liming Gao, Sean Brogan, Michael Turner,
	Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a new api DebugVPrint implementation in the
DebugLib instance. This api would expose a print
routine with VaList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 .../UefiDebugLibDebugPortProtocol/DebugLib.c       | 39 +++++++++++++++++++---
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLib.c b/MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLib.c
index c2209f4123..a068d45b50 100644
--- a/MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLib.c
+++ b/MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLib.c
@@ -1,7 +1,7 @@
 /** @file
   UEFI Debug Library that sends messages to EFI_DEBUGPORT_PROTOCOL.Write.
 
-  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2015 - 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
@@ -84,6 +84,7 @@ UefiDebugLibDebugPortProtocolWrite (
   }
 }
 
+
 /**
   Prints a debug message to the debug output device if the specified error level is enabled.
 
@@ -106,9 +107,38 @@ DebugPrint (
   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.
+
+  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
+  )
 {
   CHAR8      Buffer[MAX_DEBUG_MESSAGE_LENGTH];
-  VA_LIST    Marker;
 
   //
   // If Format is NULL, then ASSERT().
@@ -125,9 +155,7 @@ DebugPrint (
   //
   // Convert the DEBUG() message to an ASCII String
   //
-  VA_START (Marker, Format);
-  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
-  VA_END (Marker);
+  AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker);
 
   //
   // Send the print string to EFI_DEBUGPORT_PROTOCOL.Write.
@@ -311,6 +339,7 @@ DebugClearMemoryEnabled (
   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.
 
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 08/17] ArmPkg/SemiHostingDebugLib: Add a new api DebugVPrint
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (6 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 07/17] MdePkg/UefiDebuglibDebugPortProtocol: " Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 09/17] OvmfPkg/PlatformDebugLibIoPort: " Zhichao Gao
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Leif Lindholm, Ard Biesheuvel, Liming Gao, Sean Brogan,
	Michael Turner, Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a new api DebugVPrint implementation in the
DebugLib instance. This api would expose a print
routine with VaList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 ArmPkg/Library/SemiHostingDebugLib/DebugLib.c | 38 ++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/ArmPkg/Library/SemiHostingDebugLib/DebugLib.c b/ArmPkg/Library/SemiHostingDebugLib/DebugLib.c
index ec03edb774..b3455fa21c 100644
--- a/ArmPkg/Library/SemiHostingDebugLib/DebugLib.c
+++ b/ArmPkg/Library/SemiHostingDebugLib/DebugLib.c
@@ -1,7 +1,7 @@
 /** @file
   UEFI Debug Library that uses PrintLib to send messages to STDERR.
 
-  Copyright (c) 2006 - 2007, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
   Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
@@ -28,7 +28,6 @@
 #define MAX_DEBUG_MESSAGE_LENGTH  0x100
 
 /**
-
   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 PcdDebugPrintErrorLevel, then print
@@ -48,9 +47,38 @@ DebugPrint (
   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.
+
+  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
+  )
 {
   CHAR8    AsciiBuffer[MAX_DEBUG_MESSAGE_LENGTH];
-  VA_LIST  Marker;
 
   //
   // If Format is NULL, then ASSERT().
@@ -67,9 +95,7 @@ DebugPrint (
   //
   // Convert the DEBUG() message to a Unicode String
   //
-  VA_START (Marker, Format);
-  AsciiVSPrint (AsciiBuffer, sizeof (AsciiBuffer), Format, Marker);
-  VA_END (Marker);
+  AsciiVSPrint (AsciiBuffer, sizeof (AsciiBuffer), Format, VaListMarker);
 
   SemihostWriteString (AsciiBuffer);
 }
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 09/17] OvmfPkg/PlatformDebugLibIoPort: Add a new api DebugVPrint
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (7 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 08/17] ArmPkg/SemiHostingDebugLib: " Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-20 16:13   ` Laszlo Ersek
  2019-03-15  5:17 ` [PATCH V2 10/17] IntelFsp2Pkg/BaseFspDebugLibSerialPort: " Zhichao Gao
                   ` (7 subsequent siblings)
  16 siblings, 1 reply; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Liming Gao,
	Sean Brogan, Michael Turner, Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a new api DebugVPrint implementation in the
DebugLib instance. This api would expose a print
routine with VaList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c | 38 ++++++++++++++++++++---
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
index 36cde54976..74013c28a2 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
@@ -2,7 +2,7 @@
   Base Debug library instance for QEMU debug port.
   It uses PrintLib to send debug messages to a fixed I/O port.
 
-  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
   Copyright (c) 2012, Red Hat, Inc.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
@@ -51,9 +51,38 @@ DebugPrint (
   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.
+
+  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
+  )
 {
   CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
-  VA_LIST  Marker;
   UINTN    Length;
 
   //
@@ -72,9 +101,7 @@ DebugPrint (
   //
   // Convert the DEBUG() message to an ASCII String
   //
-  VA_START (Marker, Format);
-  Length = AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
-  VA_END (Marker);
+  Length = AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker);
 
   //
   // Send the print string to the debug I/O port
@@ -270,6 +297,7 @@ DebugPrintLevelEnabled (
   return (BOOLEAN) ((ErrorLevel & PcdGet32(PcdFixedDebugPrintErrorLevel)) != 0);
 }
 
+
 /**
   Return the result of detecting the debug I/O port device.
 
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 10/17] IntelFsp2Pkg/BaseFspDebugLibSerialPort: Add a new api DebugVPrint
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (8 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 09/17] OvmfPkg/PlatformDebugLibIoPort: " Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  7:31   ` Chiu, Chasel
  2019-03-15  5:17 ` [PATCH V2 11/17] IntelFspPkg/BaseFspDebugLibSerialPort: " Zhichao Gao
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Chasel Chiu, Nate DeSimone, Star Zeng, Liming Gao, Sean Brogan,
	Michael Turner, Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a new api DebugVPrint implementation in the
DebugLib instance. This api would expose a print
routine with VaList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 .../Library/BaseFspDebugLibSerialPort/DebugLib.c   | 42 +++++++++++++++++++---
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/DebugLib.c b/IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
index 73bb08e357..57b6020a58 100644
--- a/IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
+++ b/IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2014 - 2015, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2014 - 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
@@ -62,9 +62,38 @@ DebugPrint (
   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.
+
+  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
+  )
 {
   CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
-  VA_LIST  Marker;
 
   //
   // If Format is NULL, then ASSERT().
@@ -88,9 +117,7 @@ DebugPrint (
   //
   // Convert the DEBUG() message to an ASCII String
   //
-  VA_START (Marker, Format);
-  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
-  VA_END (Marker);
+  AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker);
 
   //
   // Send the print string to a Serial Port
@@ -98,6 +125,7 @@ DebugPrint (
   SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer));
 }
 
+
 /**
   Convert an UINT32 value into HEX string sepcified by Buffer.
 
@@ -118,6 +146,7 @@ FillHex (
   }
 }
 
+
 /**
   Prints an assert message containing a filename, line number, and description.
   This may be followed by a breakpoint or a dead loop.
@@ -172,6 +201,7 @@ DebugAssertInternal (
   CpuDeadLoop ();
 }
 
+
 /**
   Prints an assert message containing a filename, line number, and description.
   This may be followed by a breakpoint or a dead loop.
@@ -270,6 +300,7 @@ DebugPrintEnabled (
   return (BOOLEAN) ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_DEBUG_PRINT_ENABLED) != 0);
 }
 
+
 /**
   Returns TRUE if DEBUG_CODE() macros are enabled.
 
@@ -309,6 +340,7 @@ DebugClearMemoryEnabled (
   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.
 
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 11/17] IntelFspPkg/BaseFspDebugLibSerialPort: Add a new api DebugVPrint
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (9 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 10/17] IntelFsp2Pkg/BaseFspDebugLibSerialPort: " Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  7:32   ` Chiu, Chasel
  2019-03-15  5:17 ` [PATCH V2 12/17] IntelFramworkModulePkg/PeiDxeDebugLibReportStatusCode: Add a new api Zhichao Gao
                   ` (5 subsequent siblings)
  16 siblings, 1 reply; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Chasel Chiu, Nate DeSimone, Star Zeng, Liming Gao, Sean Brogan,
	Michael Turner, Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a new api DebugVPrint implementation in the
DebugLib instance. This api would expose a print
routine with VaList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 .../Library/BaseFspDebugLibSerialPort/DebugLib.c   | 42 +++++++++++++++++++---
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/IntelFspPkg/Library/BaseFspDebugLibSerialPort/DebugLib.c b/IntelFspPkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
index 73bb08e357..57b6020a58 100644
--- a/IntelFspPkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
+++ b/IntelFspPkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2014 - 2015, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2014 - 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
@@ -62,9 +62,38 @@ DebugPrint (
   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.
+
+  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
+  )
 {
   CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
-  VA_LIST  Marker;
 
   //
   // If Format is NULL, then ASSERT().
@@ -88,9 +117,7 @@ DebugPrint (
   //
   // Convert the DEBUG() message to an ASCII String
   //
-  VA_START (Marker, Format);
-  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
-  VA_END (Marker);
+  AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker);
 
   //
   // Send the print string to a Serial Port
@@ -98,6 +125,7 @@ DebugPrint (
   SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer));
 }
 
+
 /**
   Convert an UINT32 value into HEX string sepcified by Buffer.
 
@@ -118,6 +146,7 @@ FillHex (
   }
 }
 
+
 /**
   Prints an assert message containing a filename, line number, and description.
   This may be followed by a breakpoint or a dead loop.
@@ -172,6 +201,7 @@ DebugAssertInternal (
   CpuDeadLoop ();
 }
 
+
 /**
   Prints an assert message containing a filename, line number, and description.
   This may be followed by a breakpoint or a dead loop.
@@ -270,6 +300,7 @@ DebugPrintEnabled (
   return (BOOLEAN) ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_DEBUG_PRINT_ENABLED) != 0);
 }
 
+
 /**
   Returns TRUE if DEBUG_CODE() macros are enabled.
 
@@ -309,6 +340,7 @@ DebugClearMemoryEnabled (
   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.
 
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 12/17] IntelFramworkModulePkg/PeiDxeDebugLibReportStatusCode: Add a new api
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (10 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 11/17] IntelFspPkg/BaseFspDebugLibSerialPort: " Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 13/17] MdeModulePkg/PeiDxeDebugLibReportStatusCode: " Zhichao Gao
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel; +Cc: Liming Gao, Sean Brogan, Michael Turner, Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a new api DebugVPrint implementation in the
DebugLib instance. This api would expose a print
routine with VaList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 .../PeiDxeDebugLibReportStatusCode/DebugLib.c      | 40 +++++++++++++++++++---
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c b/IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c
index b0445115a9..2e4a41d2d8 100644
--- a/IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c
+++ b/IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c
@@ -4,7 +4,7 @@
   Note that if the debug message length is larger than the maximum allowable
   record length, then the debug message will be ignored directly.
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 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
@@ -52,12 +52,43 @@ DebugPrint (
   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.
+
+  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().
+  If the length of the message string specificed by Format is larger than the maximum allowable
+  record length, then directly return and not print it.
+
+  @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          Buffer[(EFI_STATUS_CODE_DATA_MAX_SIZE / sizeof (UINT64)) + 1];
   EFI_DEBUG_INFO  *DebugInfo;
   UINTN           TotalSize;
   UINTN           DestBufferSize;
-  VA_LIST         VaListMarker;
   BASE_LIST       BaseListMarker;
   CHAR8           *FormatString;
   BOOLEAN         Long;
@@ -129,7 +160,6 @@ DebugPrint (
   // of format in DEBUG string, which is followed by the DEBUG format string.
   // Here we will process the variable arguments and pack them in this area.
   //
-  VA_START (VaListMarker, Format);
   for (; *Format != '\0'; Format++) {
     //
     // Only format with prefix % is processed.
@@ -214,11 +244,9 @@ DebugPrint (
     // If the converted BASE_LIST is larger than the 12 * sizeof (UINT64) allocated bytes, then return
     //
     if ((CHAR8 *)BaseListMarker > FormatString) {
-      VA_END (VaListMarker);
       return;
     }
   }
-  VA_END (VaListMarker);
 
   //
   // Send the DebugInfo record
@@ -234,6 +262,7 @@ DebugPrint (
     );
 }
 
+
 /**
   Prints an assert message containing a filename, line number, and description.
   This may be followed by a breakpoint or a dead loop.
@@ -470,6 +499,7 @@ DebugClearMemoryEnabled (
   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.
 
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 13/17] MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add a new api
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (11 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 12/17] IntelFramworkModulePkg/PeiDxeDebugLibReportStatusCode: Add a new api Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 14/17] MdeModulePkg: Add definitions for EDKII DEBUG PPI Zhichao Gao
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao, Sean Brogan,
	Michael Turner, Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a new api DebugVPrint implementation in the
DebugLib instance. This api would expose a print
routine with VaList parameter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 .../PeiDxeDebugLibReportStatusCode/DebugLib.c      | 38 +++++++++++++++++++---
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c
index 6f0f416273..1afbfb6c15 100644
--- a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c
+++ b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c
@@ -4,7 +4,7 @@
   Note that if the debug message length is larger than the maximum allowable
   record length, then the debug message will be ignored directly.
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 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
@@ -52,11 +52,40 @@ DebugPrint (
   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.
+
+  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          Buffer[(EFI_STATUS_CODE_DATA_MAX_SIZE / sizeof (UINT64)) + 1];
   EFI_DEBUG_INFO  *DebugInfo;
   UINTN           TotalSize;
-  VA_LIST         VaListMarker;
   BASE_LIST       BaseListMarker;
   CHAR8           *FormatString;
   BOOLEAN         Long;
@@ -125,7 +154,6 @@ DebugPrint (
   // of format in DEBUG string, which is followed by the DEBUG format string.
   // Here we will process the variable arguments and pack them in this area.
   //
-  VA_START (VaListMarker, Format);
 
   //
   // Use the actual format string.
@@ -215,11 +243,9 @@ DebugPrint (
     // If the converted BASE_LIST is larger than the 12 * sizeof (UINT64) allocated bytes, then return
     //
     if ((CHAR8 *)BaseListMarker > FormatString) {
-      VA_END (VaListMarker);
       return;
     }
   }
-  VA_END (VaListMarker);
 
   //
   // Send the DebugInfo record
@@ -235,6 +261,7 @@ DebugPrint (
     );
 }
 
+
 /**
   Prints an assert message containing a filename, line number, and description.
   This may be followed by a breakpoint or a dead loop.
@@ -471,6 +498,7 @@ DebugClearMemoryEnabled (
   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.
 
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 14/17] MdeModulePkg: Add definitions for EDKII DEBUG PPI
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (12 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 13/17] MdeModulePkg/PeiDxeDebugLibReportStatusCode: " Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 15/17] MdeModulePkg: Add a PEIM to install Debug PPI Zhichao Gao
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao, Sean Brogan,
	Michael Turner, Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a debug PPI for PEI phase. This PPI will provide basic
services of debug. PEI debug lib instance can use these
services to implement debug function to reduce the PEIM
which consume the debug lib.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 MdeModulePkg/Include/Ppi/Debug.h | 90 ++++++++++++++++++++++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec    |  3 ++
 2 files changed, 93 insertions(+)
 create mode 100644 MdeModulePkg/Include/Ppi/Debug.h

diff --git a/MdeModulePkg/Include/Ppi/Debug.h b/MdeModulePkg/Include/Ppi/Debug.h
new file mode 100644
index 0000000000..deb03a4319
--- /dev/null
+++ b/MdeModulePkg/Include/Ppi/Debug.h
@@ -0,0 +1,90 @@
+/** @file
+  Define the EDKII_DEBUG_PPI that PEIMs can use to dump info to debug port.
+
+  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.
+
+**/
+
+#ifndef __EDKII_DEBUG_PPI_H__
+#define __EDKII_DEBUG_PPI_H__
+
+#include <Pi/PiPeiCis.h>
+
+//
+// Global ID for the EDKII_DEBUG_PPI
+//
+#define EDKII_DEBUG_PPI_GUID \
+  { \
+    0x999e699c, 0xb013, 0x475e, {0xb1, 0x7b, 0xf3, 0xa8, 0xae, 0x5c, 0x48, 0x75} \
+  }
+
+///
+/// Forward declaration for the PEI_DEBUG_LIB_DEBUG_PPI EDKII_DEBUG_PPI
+///
+typedef struct _EDKII_DEBUG_PPI EDKII_DEBUG_PPI;
+
+/**
+  Print a debug message to debug output device if the specified error level
+  is enabled.
+
+  @param[in] PeiServices              The pointer to the PEI Services Table.
+  @param[in] This                     The pointer to this instance of EDKII_DEBUG_PPI
+  @param[in] ErrorLevel               The error level of the debug message.
+  @param[in] Format                   Format string for the debug message to print.
+  @param[in] VaListMarker             VA_LIST marker for the variable argument list.
+
+**/
+typedef
+VOID
+(EFIAPI *EDKII_DEBUG_VPRINT)(
+  IN CONST EFI_PEI_SERVICES         **PeiServices,
+  IN EDKII_DEBUG_PPI                *This,
+  IN UINTN                          ErrorLevel,
+  IN CONST CHAR8                    *Format,
+  IN VA_LIST                        Marker
+  );
+
+/**
+  Print an assert message containing a filename, line number, and description.
+  This may be followed by a breakpoint or a dead loop.
+
+  @param[in] PeiServices              The pointer to the PEI Services Table.
+  @param[in] This                     The pointer to this instance of EDKII_DEBUG_PPI
+  @param[in] FileName                 The pointer to the name of the source file that
+                                      generated the assert condition.
+  @param[in] LineNumber               The line number in the source file that generated
+                                      the assert condition
+  @param[in] Description              The pointer to the description of the assert condition.
+
+**/
+typedef
+VOID
+(EFIAPI *EDKII_DEBUG_ASSERT)(
+  IN CONST EFI_PEI_SERVICES         **PeiServices,
+  IN EDKII_DEBUG_PPI                *This,
+  IN CONST CHAR8                    *FileName,
+  IN UINTN                          LineNumber,
+  IN CONST CHAR8                    *Description
+  );
+
+///
+/// This PPI contains a set of services to print message to debug output device
+///
+struct _EDKII_DEBUG_PPI {
+  EDKII_DEBUG_VPRINT                DebugVPrint;
+  EDKII_DEBUG_ASSERT                DebugAssert;
+};
+
+extern EFI_GUID gEdkiiDebugPpiGuid;
+
+#endif
+
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index a2130bc439..9bbd0572f5 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -492,6 +492,9 @@
   ## Include/Ppi/AtaPassThru.h
   gEdkiiPeiAtaPassThruPpiGuid               = { 0xa16473fd, 0xd474, 0x4c89, { 0xae, 0xc7, 0x90, 0xb8, 0x3c, 0x73, 0x86, 0x9  } }
 
+  ## Include/Ppi/Debug.h
+  gEdkiiDebugPpiGuid                        = { 0x999e699c, 0xb013, 0x475e, { 0xb1, 0x7b, 0xf3, 0xa8, 0xae, 0x5c, 0x48, 0x75 } }
+
 [Protocols]
   ## Load File protocol provides capability to load and unload EFI image into memory and execute it.
   #  Include/Protocol/LoadPe32Image.h
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 15/17] MdeModulePkg: Add a PEIM to install Debug PPI
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (13 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 14/17] MdeModulePkg: Add definitions for EDKII DEBUG PPI Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 16/17] MdeModulePkg/PeiDebugLibDebugPpi: Add PEI debug lib Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 17/17] MdeModulePkg: Add PEIM and lib to dsc file Zhichao Gao
  16 siblings, 0 replies; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao, Sean Brogan,
	Michael Turner, Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add a PEIM to install Debug PPI so that PEI debug library
instance can locate gEdkiiDebugPpiGuid to implement the
debug functions. Using this PPI can reduce the size of
PEIMs which consume the debug library.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 .../Universal/DebugServicePei/DebugService.c       | 68 ++++++++++++++++++++++
 .../Universal/DebugServicePei/DebugService.h       | 64 ++++++++++++++++++++
 .../Universal/DebugServicePei/DebugServicePei.c    | 54 +++++++++++++++++
 .../Universal/DebugServicePei/DebugServicePei.inf  | 51 ++++++++++++++++
 .../Universal/DebugServicePei/DebugServicePei.uni  | 20 +++++++
 5 files changed, 257 insertions(+)
 create mode 100644 MdeModulePkg/Universal/DebugServicePei/DebugService.c
 create mode 100644 MdeModulePkg/Universal/DebugServicePei/DebugService.h
 create mode 100644 MdeModulePkg/Universal/DebugServicePei/DebugServicePei.c
 create mode 100644 MdeModulePkg/Universal/DebugServicePei/DebugServicePei.inf
 create mode 100644 MdeModulePkg/Universal/DebugServicePei/DebugServicePei.uni

diff --git a/MdeModulePkg/Universal/DebugServicePei/DebugService.c b/MdeModulePkg/Universal/DebugServicePei/DebugService.c
new file mode 100644
index 0000000000..24bc2df797
--- /dev/null
+++ b/MdeModulePkg/Universal/DebugServicePei/DebugService.c
@@ -0,0 +1,68 @@
+/** @file
+  Debug services instances for PEI phase.
+
+  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 <Ppi/Debug.h>
+#include <Library/DebugLib.h>
+
+/**
+  Print a debug message to debug output device if the specified error level
+  is enabled.
+
+  @param[in] PeiServices              The pointer to the PEI Services Table.
+  @param[in] This                     The pointer to this instance of EDKII_DEBUG_PPI
+  @param[in] ErrorLevel               The error level of the debug message.
+  @param[in] Format                   Format string for the debug message to print.
+  @param[in] VaListMarker             VA_LIST marker for the variable argument list.
+
+**/
+VOID
+EFIAPI
+PeiDebugVPrint(
+  IN CONST EFI_PEI_SERVICES         **PeiServices,
+  IN EDKII_DEBUG_PPI                *This,
+  IN UINTN                          ErrorLevel,
+  IN CONST CHAR8                    *Format,
+  IN VA_LIST                        Marker
+  )
+{
+  DebugVPrint(ErrorLevel, Format, Marker);
+}
+
+/**
+  Print an assert message containing a filename, line number, and description.
+  This may be followed by a breakpoint or a dead loop.
+
+  @param[in] PeiServices              The pointer to the PEI Services Table.
+  @param[in] This                     The pointer to this instance of EDKII_DEBUG_PPI
+  @param[in] FileName                 The pointer to the name of the source file that
+                                      generated the assert condition.
+  @param[in] LineNumber               The line number in the source file that generated
+                                      the assert condition
+  @param[in] Description              The pointer to the description of the assert condition.
+
+**/
+VOID
+EFIAPI
+PeiDebugAssert(
+  IN CONST EFI_PEI_SERVICES         **PeiServices,
+  IN EDKII_DEBUG_PPI                *This,
+  IN CONST CHAR8                    *FileName,
+  IN UINTN                          LineNumber,
+  IN CONST CHAR8                    *Description
+  )
+{
+  DebugAssert(FileName, LineNumber, Description);
+}
+
diff --git a/MdeModulePkg/Universal/DebugServicePei/DebugService.h b/MdeModulePkg/Universal/DebugServicePei/DebugService.h
new file mode 100644
index 0000000000..f7a5c4320c
--- /dev/null
+++ b/MdeModulePkg/Universal/DebugServicePei/DebugService.h
@@ -0,0 +1,64 @@
+/** @file
+  Header file of Debug services instances.
+
+  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.
+
+**/
+#ifndef __DEBUG_SERVICE_H__
+#define __DEBUG_SERVICE_H__
+
+#include <Ppi/Debug.h>
+
+/**
+  Print a debug message to debug output device if the specified error level
+  is enabled.
+
+  @param[in] PeiServices              The pointer to the PEI Services Table.
+  @param[in] This                     The pointer to this instance of EDKII_DEBUG_PPI
+  @param[in] ErrorLevel               The error level of the debug message.
+  @param[in] Format                   Format string for the debug message to print.
+  @param[in] VaListMarker             VA_LIST marker for the variable argument list.
+
+**/
+VOID
+EFIAPI
+PeiDebugVPrint(
+  IN CONST EFI_PEI_SERVICES         **PeiServices,
+  IN EDKII_DEBUG_PPI                *This,
+  IN UINTN                          ErrorLevel,
+  IN CONST CHAR8                    *Format,
+  IN VA_LIST                        Marker
+  );
+
+/**
+  Prints an assert message containing a filename, line number, and description.
+  This may be followed by a breakpoint or a dead loop.
+
+  @param[in] PeiServices              The pointer to the PEI Services Table.
+  @param[in] This                     The pointer to this instance of EDKII_DEBUG_PPI
+  @param[in] FileName                 The pointer to the name of the source file that
+                                      generated the assert condition.
+  @param[in] LineNumber               The line number in the source file that generated
+                                      the assert condition
+  @param[in] Description              The pointer to the description of the assert condition.
+
+**/
+VOID
+EFIAPI
+PeiDebugAssert(
+  IN CONST EFI_PEI_SERVICES         **PeiServices,
+  IN EDKII_DEBUG_PPI                *This,
+  IN CONST CHAR8                    *FileName,
+  IN UINTN                          LineNumber,
+  IN CONST CHAR8                    *Description
+  );
+
+#endif
diff --git a/MdeModulePkg/Universal/DebugServicePei/DebugServicePei.c b/MdeModulePkg/Universal/DebugServicePei/DebugServicePei.c
new file mode 100644
index 0000000000..9c6f89faef
--- /dev/null
+++ b/MdeModulePkg/Universal/DebugServicePei/DebugServicePei.c
@@ -0,0 +1,54 @@
+/** @file
+  This driver installs gEdkiiPeiDebugLibDebugGuid PPI to provide
+  debug services for PEIMs.
+
+  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 <Uefi/UefiBaseType.h>
+#include <Library/PeimEntryPoint.h>
+#include <Library/PeiServicesLib.h>
+#include "DebugService.h"
+
+EDKII_DEBUG_PPI mDebugPpi = {
+  PeiDebugVPrint,
+  PeiDebugAssert
+};
+
+EFI_PEI_PPI_DESCRIPTOR mDebugServicePpi = {
+  (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+  &gEdkiiDebugPpiGuid,
+  (VOID *)&mDebugPpi
+};
+
+/**
+  Entry point of Debug Service PEIM
+
+  This funciton installs EDKII DEBUG PPI
+
+  @param  FileHandle  Handle of the file being invoked.
+  @param  PeiServices Describes the list of possible PEI Services.
+
+  @retval EFI_SUCESS  The entry point of Debug Service PEIM executes successfully.
+  @retval Others      Some error occurs during the execution of this function.
+
+**/
+EFI_STATUS
+EFIAPI
+DebugSerivceInitialize (
+  IN EFI_PEI_FILE_HANDLE        FileHandle,
+  IN CONST EFI_PEI_SERVICES     **PeiServices
+  )
+{
+  return PeiServicesInstallPpi (&mDebugServicePpi);
+}
+
diff --git a/MdeModulePkg/Universal/DebugServicePei/DebugServicePei.inf b/MdeModulePkg/Universal/DebugServicePei/DebugServicePei.inf
new file mode 100644
index 0000000000..f9563ed1b3
--- /dev/null
+++ b/MdeModulePkg/Universal/DebugServicePei/DebugServicePei.inf
@@ -0,0 +1,51 @@
+## @file
+#  Debug services for PEI phase
+#
+#  This module installs gEdkiiFaultTolerantWriteGuid PPI to inform the check for FTW last write data has been done.
+#
+#  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                      = DebugServicePei
+  MODULE_UNI_FILE                = DebugServicePei.uni
+  FILE_GUID                      = B73F81B9-1DFC-487C-824C-0509EE2B0128
+  MODULE_TYPE                    = PEIM
+  VERSION_STRING                 = 1.0
+
+  ENTRY_POINT                    = DebugSerivceInitialize
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 EBC
+#
+
+[Sources]
+  DebugServicePei.c
+  DebugService.c
+  DebugService.h
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  PeimEntryPoint
+  PeiServicesLib
+  DebugLib
+
+[Ppis]
+  gEdkiiDebugPpiGuid                    ## PRODUCE
+
+[Depex]
+  TRUE
+
diff --git a/MdeModulePkg/Universal/DebugServicePei/DebugServicePei.uni b/MdeModulePkg/Universal/DebugServicePei/DebugServicePei.uni
new file mode 100644
index 0000000000..2ac1d997d0
--- /dev/null
+++ b/MdeModulePkg/Universal/DebugServicePei/DebugServicePei.uni
@@ -0,0 +1,20 @@
+///** @file
+//  This driver installs gEdkiiPeiDebugLibDebugGuid PPI to provide
+//  debug services for PEIMs.
+//
+//  Copyright (c) 2013 - 2018, 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.
+//
+//**/
+
+#string STR_MODULE_ABSTRACT             #language en-US "Provide debug services at PEI phase."
+
+#string STR_MODULE_DESCRIPTION          #language en-US "It produces gEdkiiPeiDebugLibDebugGuid to print message to debug output device"
+
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 16/17] MdeModulePkg/PeiDebugLibDebugPpi: Add PEI debug lib
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (14 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 15/17] MdeModulePkg: Add a PEIM to install Debug PPI Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  5:17 ` [PATCH V2 17/17] MdeModulePkg: Add PEIM and lib to dsc file Zhichao Gao
  16 siblings, 0 replies; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Liming Gao, Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Sean Brogan,
	Michael Turner, Bret Barkelew

From: Liming Gao <liming.gao@intel.com>

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 <zhichao.gao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 .../Library/PeiDebugLibDebugPpi/DebugLib.c         | 302 +++++++++++++++++++++
 .../PeiDebugLibDebugPpi/PeiDebugLibDebugPpi.inf    |  55 ++++
 2 files changed, 357 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..d51fe6e4ee
--- /dev/null
+++ b/MdeModulePkg/Library/PeiDebugLibDebugPpi/DebugLib.c
@@ -0,0 +1,302 @@
+/** @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>
+#include <Library/PeiServicesTablePointerLib.h>
+
+EDKII_DEBUG_PPI             *mDebugPpi = NULL;
+CONST EFI_PEI_SERVICES      **mPeiServicesTablePointer = 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      The format string for the debug message to print.
+  @param  ...         The 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.
+
+  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
+  )
+{
+  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
+                );
+    if (EFI_ERROR (Status)) {
+      CpuDeadLoop();
+    }
+  }
+
+  if (mPeiServicesTablePointer == NULL) {
+    mPeiServicesTablePointer = GetPeiServicesTablePointer ();
+  }
+
+  mDebugPpi->DebugVPrint (
+              mPeiServicesTablePointer,
+              mDebugPpi,
+              ErrorLevel,
+              Format,
+              VaListMarker
+              );
+}
+
+
+/**
+  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
+                );
+    if (EFI_ERROR (Status)) {
+      CpuDeadLoop();
+    }
+  }
+
+  if (mPeiServicesTablePointer == NULL) {
+    mPeiServicesTablePointer = GetPeiServicesTablePointer ();
+  }
+
+  mDebugPpi->DebugAssert (
+              mPeiServicesTablePointer,
+              mDebugPpi,
+              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 != 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.
+#
+#  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



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH V2 17/17] MdeModulePkg: Add PEIM and lib to dsc file
  2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
                   ` (15 preceding siblings ...)
  2019-03-15  5:17 ` [PATCH V2 16/17] MdeModulePkg/PeiDebugLibDebugPpi: Add PEI debug lib Zhichao Gao
@ 2019-03-15  5:17 ` Zhichao Gao
  2019-03-15  5:54   ` Andrew Fish
  16 siblings, 1 reply; 32+ messages in thread
From: Zhichao Gao @ 2019-03-15  5:17 UTC (permalink / raw)
  To: edk2-devel
  Cc: Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao, Sean Brogan,
	Michael Turner, Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395

Add the new PEIM DebugServicePei and library instance
PeiDebugLibDebugPpi to dsc file to verify it can build
correctly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 MdeModulePkg/MdeModulePkg.dsc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 6cd1727a0d..dec441e23e 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -297,6 +297,7 @@
   MdeModulePkg/Library/PlatformHookLibSerialPortPpi/PlatformHookLibSerialPortPpi.inf
   MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
   MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
+  MdeModulePkg/Library/PeiDebugLibDebugPpi/PeiDebugLibDebugPpi.inf
   MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
   MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManagerLibNull.inf
   MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
@@ -423,6 +424,8 @@
   MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
   MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.inf
 
+  MdeModulePkg/Universal/DebugServicePei/DebugServicePei.inf
+
   MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
   MdeModulePkg/Library/FmpAuthenticationLibNull/FmpAuthenticationLibNull.inf
   MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 17/17] MdeModulePkg: Add PEIM and lib to dsc file
  2019-03-15  5:17 ` [PATCH V2 17/17] MdeModulePkg: Add PEIM and lib to dsc file Zhichao Gao
@ 2019-03-15  5:54   ` Andrew Fish
  2019-04-01  6:17     ` Jordan Justen
  0 siblings, 1 reply; 32+ messages in thread
From: Andrew Fish @ 2019-03-15  5:54 UTC (permalink / raw)
  To: Zhichao Gao, Mike Kinney, Gao, Liming
  Cc: edk2-devel, Bret Barkelew, Hao Wu, Michael Turner, Star Zeng

I understand the motivation for this change as I've done something much less portable that looks a lot like this to save the PEI XIP space....

I seem to remember a long time ago we add a public VA_LIST to an API and we ran into an issue due to the marker format being compiler specific. You don't tend to see this on VC++ as it mostly uses a pointer to the frame, but GCC and clang can use a data structure especially not on IA32 (i386) for a VA_LIST (this is part of the reason there are so many #ifdefs in the Base.h VA_LIST section).

In the past to fix this Mike Kinney added BASE_LIST.  I seem to remember MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode passes a BASE_LIST into ReportStatusCode vs. a VA_LIST for this reason. Was this VA_LIST portability concern taken into consideration for this new API?

In general VA_LIST is not an issue as long as all the code is compiled with the same compiler but if binaries are in play then you can have issues. So things like FSB, Option ROMs, OS Loaders, EFI Shell, EFI Apps. are when you can see the issue. 

Thanks,

Andrew Fish

> On Mar 14, 2019, at 10:17 PM, Zhichao Gao <zhichao.gao@intel.com> wrote:
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> 
> Add the new PEIM DebugServicePei and library instance
> PeiDebugLibDebugPpi to dsc file to verify it can build
> correctly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> ---
> MdeModulePkg/MdeModulePkg.dsc | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
> index 6cd1727a0d..dec441e23e 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -297,6 +297,7 @@
>   MdeModulePkg/Library/PlatformHookLibSerialPortPpi/PlatformHookLibSerialPortPpi.inf
>   MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
>   MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
> +  MdeModulePkg/Library/PeiDebugLibDebugPpi/PeiDebugLibDebugPpi.inf
>   MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
>   MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManagerLibNull.inf
>   MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> @@ -423,6 +424,8 @@
>   MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
>   MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.inf
> 
> +  MdeModulePkg/Universal/DebugServicePei/DebugServicePei.inf
> +
>   MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
>   MdeModulePkg/Library/FmpAuthenticationLibNull/FmpAuthenticationLibNull.inf
>   MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> -- 
> 2.16.2.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
  2019-03-15  5:17 ` [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api " Zhichao Gao
@ 2019-03-15  6:15   ` Jordan Justen
  2019-03-18  0:14     ` Gao, Zhichao
  0 siblings, 1 reply; 32+ messages in thread
From: Jordan Justen @ 2019-03-15  6:15 UTC (permalink / raw)
  To: Zhichao Gao, edk2-devel
  Cc: Michael D Kinney, Bret Barkelew, Michael Turner, Liming Gao

On 2019-03-14 22:17:33, Zhichao Gao wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> 
> Add a new api DebugVPrint prototype definition in the
> DebugLib header file. This api would expose a print
> routine with VaList parameter.

These lines seem to be fairly short, with the longest be 54 chars. I
guess not a problem, but by the recommendation says they could be up
to 75 in length.

https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format

> 
> diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h
> index e6a7a357b2..51d89bbd52 100644
> --- a/MdePkg/Include/Library/DebugLib.h
> +++ b/MdePkg/Include/Library/DebugLib.h
> @@ -8,7 +8,7 @@
>    of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is
>    defined, then debug and assert related macros wrapped by it are the NULL implementations.
>  
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 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 that accompanies this distribution.
>  The full text of the license may be found at
> @@ -101,6 +101,29 @@ DebugPrint (
>    );
>  
> +/**
> +  Prints a debug message to the debug output device if the specified error level is enabled.

According to the style guide:

  "Preferably, limit line lengths to 80 columns or less."

https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C

https://github.com/tianocore-docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf

But, this line uses 92 columns.

I think there are similar cases in other patches.

> +
> +  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
> +  );
> +
> +
>  /**
>    Prints an assert message containing a filename, line number, and description.
>    This may be followed by a breakpoint or a dead loop.
> @@ -221,6 +244,7 @@ DebugClearMemoryEnabled (
>    VOID
>    );
>  
> +

What's going on here? It seems like maybe extra lines are added that
have nothing to do with this patch.

-Jordan

>  /**
>    Returns TRUE if any one of the bit is set both in ErrorLevel and PcdFixedDebugPrintErrorLevel.
>  
> @@ -236,6 +260,7 @@ DebugPrintLevelEnabled (
>    IN  CONST UINTN        ErrorLevel
>    );
>  
> +
>  /**
>    Internal worker macro that calls DebugAssert().
>  
> @@ -273,6 +298,7 @@ DebugPrintLevelEnabled (
>  #define _DEBUG(Expression)   DebugPrint Expression
>  #endif
>  
> +
>  /**
>    Macro that calls DebugAssert() if an expression evaluates to FALSE.
>  
> @@ -299,6 +325,7 @@ DebugPrintLevelEnabled (
>    #define ASSERT(Expression)
>  #endif
>  
> +
>  /**
>    Macro that calls DebugPrint().
>  
> @@ -322,6 +349,7 @@ DebugPrintLevelEnabled (
>    #define DEBUG(Expression)
>  #endif
>  
> +
>  /**
>    Macro that calls DebugAssert() if an EFI_STATUS evaluates to an error code.
>  
> @@ -348,6 +376,7 @@ DebugPrintLevelEnabled (
>    #define ASSERT_EFI_ERROR(StatusParameter)
>  #endif
>  
> +
>  /**
>    Macro that calls DebugAssert() if a RETURN_STATUS evaluates to an error code.
>  
> @@ -375,6 +404,7 @@ DebugPrintLevelEnabled (
>    #define ASSERT_RETURN_ERROR(StatusParameter)
>  #endif
>  
> +
>  /**
>    Macro that calls DebugAssert() if a protocol is already installed in the
>    handle database.
> @@ -418,6 +448,7 @@ DebugPrintLevelEnabled (
>    #define ASSERT_PROTOCOL_ALREADY_INSTALLED(Handle, Guid)
>  #endif
>  
> +
>  /**
>    Macro that marks the beginning of debug source code.
>  
> -- 
> 2.16.2.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 10/17] IntelFsp2Pkg/BaseFspDebugLibSerialPort: Add a new api DebugVPrint
  2019-03-15  5:17 ` [PATCH V2 10/17] IntelFsp2Pkg/BaseFspDebugLibSerialPort: " Zhichao Gao
@ 2019-03-15  7:31   ` Chiu, Chasel
  0 siblings, 0 replies; 32+ messages in thread
From: Chiu, Chasel @ 2019-03-15  7:31 UTC (permalink / raw)
  To: Gao, Zhichao, edk2-devel@lists.01.org
  Cc: Desimone, Nathaniel L, Zeng, Star, Gao, Liming, Sean Brogan,
	Michael Turner, Bret Barkelew


Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

> -----Original Message-----
> From: Gao, Zhichao
> Sent: Friday, March 15, 2019 1:18 PM
> To: edk2-devel@lists.01.org
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao,
> Liming <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: [PATCH V2 10/17] IntelFsp2Pkg/BaseFspDebugLibSerialPort: Add a new
> api DebugVPrint
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> 
> Add a new api DebugVPrint implementation in the DebugLib instance. This api
> would expose a print routine with VaList parameter.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> ---
>  .../Library/BaseFspDebugLibSerialPort/DebugLib.c   | 42
> +++++++++++++++++++---
>  1 file changed, 37 insertions(+), 5 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> b/IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> index 73bb08e357..57b6020a58 100644
> --- a/IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> +++ b/IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -  Copyright (c) 2014 - 2015, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 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 @@ -62,9 +62,38 @@ DebugPrint (
>    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.
> +
> +  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
> +  )
>  {
>    CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> -  VA_LIST  Marker;
> 
>    //
>    // If Format is NULL, then ASSERT().
> @@ -88,9 +117,7 @@ DebugPrint (
>    //
>    // Convert the DEBUG() message to an ASCII String
>    //
> -  VA_START (Marker, Format);
> -  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
> -  VA_END (Marker);
> +  AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker);
> 
>    //
>    // Send the print string to a Serial Port @@ -98,6 +125,7 @@ DebugPrint (
>    SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer));  }
> 
> +
>  /**
>    Convert an UINT32 value into HEX string sepcified by Buffer.
> 
> @@ -118,6 +146,7 @@ FillHex (
>    }
>  }
> 
> +
>  /**
>    Prints an assert message containing a filename, line number, and description.
>    This may be followed by a breakpoint or a dead loop.
> @@ -172,6 +201,7 @@ DebugAssertInternal (
>    CpuDeadLoop ();
>  }
> 
> +
>  /**
>    Prints an assert message containing a filename, line number, and description.
>    This may be followed by a breakpoint or a dead loop.
> @@ -270,6 +300,7 @@ DebugPrintEnabled (
>    return (BOOLEAN) ((PcdGet8(PcdDebugPropertyMask) &
> DEBUG_PROPERTY_DEBUG_PRINT_ENABLED) != 0);  }
> 
> +
>  /**
>    Returns TRUE if DEBUG_CODE() macros are enabled.
> 
> @@ -309,6 +340,7 @@ DebugClearMemoryEnabled (
>    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.
> 
> --
> 2.16.2.windows.1



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 11/17] IntelFspPkg/BaseFspDebugLibSerialPort: Add a new api DebugVPrint
  2019-03-15  5:17 ` [PATCH V2 11/17] IntelFspPkg/BaseFspDebugLibSerialPort: " Zhichao Gao
@ 2019-03-15  7:32   ` Chiu, Chasel
  0 siblings, 0 replies; 32+ messages in thread
From: Chiu, Chasel @ 2019-03-15  7:32 UTC (permalink / raw)
  To: Gao, Zhichao, edk2-devel@lists.01.org
  Cc: Desimone, Nathaniel L, Zeng, Star, Gao, Liming, Sean Brogan,
	Michael Turner, Bret Barkelew


Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

> -----Original Message-----
> From: Gao, Zhichao
> Sent: Friday, March 15, 2019 1:18 PM
> To: edk2-devel@lists.01.org
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao,
> Liming <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: [PATCH V2 11/17] IntelFspPkg/BaseFspDebugLibSerialPort: Add a new
> api DebugVPrint
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> 
> Add a new api DebugVPrint implementation in the DebugLib instance. This api
> would expose a print routine with VaList parameter.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> ---
>  .../Library/BaseFspDebugLibSerialPort/DebugLib.c   | 42
> +++++++++++++++++++---
>  1 file changed, 37 insertions(+), 5 deletions(-)
> 
> diff --git a/IntelFspPkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> b/IntelFspPkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> index 73bb08e357..57b6020a58 100644
> --- a/IntelFspPkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> +++ b/IntelFspPkg/Library/BaseFspDebugLibSerialPort/DebugLib.c
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -  Copyright (c) 2014 - 2015, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 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 @@ -62,9 +62,38 @@ DebugPrint (
>    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.
> +
> +  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
> +  )
>  {
>    CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> -  VA_LIST  Marker;
> 
>    //
>    // If Format is NULL, then ASSERT().
> @@ -88,9 +117,7 @@ DebugPrint (
>    //
>    // Convert the DEBUG() message to an ASCII String
>    //
> -  VA_START (Marker, Format);
> -  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
> -  VA_END (Marker);
> +  AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker);
> 
>    //
>    // Send the print string to a Serial Port @@ -98,6 +125,7 @@ DebugPrint (
>    SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer));  }
> 
> +
>  /**
>    Convert an UINT32 value into HEX string sepcified by Buffer.
> 
> @@ -118,6 +146,7 @@ FillHex (
>    }
>  }
> 
> +
>  /**
>    Prints an assert message containing a filename, line number, and description.
>    This may be followed by a breakpoint or a dead loop.
> @@ -172,6 +201,7 @@ DebugAssertInternal (
>    CpuDeadLoop ();
>  }
> 
> +
>  /**
>    Prints an assert message containing a filename, line number, and description.
>    This may be followed by a breakpoint or a dead loop.
> @@ -270,6 +300,7 @@ DebugPrintEnabled (
>    return (BOOLEAN) ((PcdGet8(PcdDebugPropertyMask) &
> DEBUG_PROPERTY_DEBUG_PRINT_ENABLED) != 0);  }
> 
> +
>  /**
>    Returns TRUE if DEBUG_CODE() macros are enabled.
> 
> @@ -309,6 +340,7 @@ DebugClearMemoryEnabled (
>    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.
> 
> --
> 2.16.2.windows.1



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
  2019-03-15  6:15   ` Jordan Justen
@ 2019-03-18  0:14     ` Gao, Zhichao
  2019-03-18  6:20       ` Jordan Justen
  0 siblings, 1 reply; 32+ messages in thread
From: Gao, Zhichao @ 2019-03-18  0:14 UTC (permalink / raw)
  To: Justen, Jordan L, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Bret Barkelew, Michael Turner, Gao, Liming



> -----Original Message-----
> From: Justen, Jordan L
> Sent: Friday, March 15, 2019 2:16 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api
> DebugVPrint for DebugLib
> 
> On 2019-03-14 22:17:33, Zhichao Gao wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> >
> > Add a new api DebugVPrint prototype definition in the DebugLib header
> > file. This api would expose a print routine with VaList parameter.
> 
> These lines seem to be fairly short, with the longest be 54 chars. I guess not a
> problem, but by the recommendation says they could be up to 75 in length.
> 
> https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-
> Format
> 

It is hard for someone to control the characters in a line. Making the text massage readable base on the rules make more sense. It is easier to control the chars number within 75. Line length less than 76 characters is legal.

> >
> > diff --git a/MdePkg/Include/Library/DebugLib.h
> > b/MdePkg/Include/Library/DebugLib.h
> > index e6a7a357b2..51d89bbd52 100644
> > --- a/MdePkg/Include/Library/DebugLib.h
> > +++ b/MdePkg/Include/Library/DebugLib.h
> > @@ -8,7 +8,7 @@
> >    of size reduction when compiler optimization is disabled. If
> MDEPKG_NDEBUG is
> >    defined, then debug and assert related macros wrapped by it are the
> NULL implementations.
> >
> > -Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > reserved.<BR>
> > +Copyright (c) 2006 - 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 that
> accompanies this distribution.
> >  The full text of the license may be found at @@ -101,6 +101,29 @@
> > DebugPrint (
> >    );
> >
> > +/**
> > +  Prints a debug message to the debug output device if the specified error
> level is enabled.
> 
> According to the style guide:
> 
>   "Preferably, limit line lengths to 80 columns or less."
> 
> https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C
> 
> https://github.com/tianocore-
> docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf
> 
> But, this line uses 92 columns.
> 
> I think there are similar cases in other patches.

Right. This sentence I copied from the comment of DebugPrint function which might be designed for a long time. The CCS might not be designed as such style at that time. For this, it's better to change both of the two sentences.
By the way, the whole edk2 repo may contain a lot of lines violated this rule. It takes effort to fix them all.

> 
> > +
> > +  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
> > +  );
> > +
> > +
> >  /**
> >    Prints an assert message containing a filename, line number, and
> description.
> >    This may be followed by a breakpoint or a dead loop.
> > @@ -221,6 +244,7 @@ DebugClearMemoryEnabled (
> >    VOID
> >    );
> >
> > +
> 
> What's going on here? It seems like maybe extra lines are added that have
> nothing to do with this patch.
> 
> -Jordan
> 

For this library DebugLib, seems it prefers to put two blank lines between functions. I just want to make it tidy. Maybe it is an inappropriate behavior. But there are no rules for how much blank lines should be putted between functions. For my own, one single blank line is enough. Both adding blank lines and deleting blank lines seem inappropriate. Or just stay the original style?

Thanks,
Zhichao

> >  /**
> >    Returns TRUE if any one of the bit is set both in ErrorLevel and
> PcdFixedDebugPrintErrorLevel.
> >
> > @@ -236,6 +260,7 @@ DebugPrintLevelEnabled (
> >    IN  CONST UINTN        ErrorLevel
> >    );
> >
> > +
> >  /**
> >    Internal worker macro that calls DebugAssert().
> >
> > @@ -273,6 +298,7 @@ DebugPrintLevelEnabled (
> >  #define _DEBUG(Expression)   DebugPrint Expression
> >  #endif
> >
> > +
> >  /**
> >    Macro that calls DebugAssert() if an expression evaluates to FALSE.
> >
> > @@ -299,6 +325,7 @@ DebugPrintLevelEnabled (
> >    #define ASSERT(Expression)
> >  #endif
> >
> > +
> >  /**
> >    Macro that calls DebugPrint().
> >
> > @@ -322,6 +349,7 @@ DebugPrintLevelEnabled (
> >    #define DEBUG(Expression)
> >  #endif
> >
> > +
> >  /**
> >    Macro that calls DebugAssert() if an EFI_STATUS evaluates to an error
> code.
> >
> > @@ -348,6 +376,7 @@ DebugPrintLevelEnabled (
> >    #define ASSERT_EFI_ERROR(StatusParameter)  #endif
> >
> > +
> >  /**
> >    Macro that calls DebugAssert() if a RETURN_STATUS evaluates to an error
> code.
> >
> > @@ -375,6 +404,7 @@ DebugPrintLevelEnabled (
> >    #define ASSERT_RETURN_ERROR(StatusParameter)
> >  #endif
> >
> > +
> >  /**
> >    Macro that calls DebugAssert() if a protocol is already installed in the
> >    handle database.
> > @@ -418,6 +448,7 @@ DebugPrintLevelEnabled (
> >    #define ASSERT_PROTOCOL_ALREADY_INSTALLED(Handle, Guid)  #endif
> >
> > +
> >  /**
> >    Macro that marks the beginning of debug source code.
> >
> > --
> > 2.16.2.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
  2019-03-18  0:14     ` Gao, Zhichao
@ 2019-03-18  6:20       ` Jordan Justen
  2019-03-18 15:09         ` Gao, Liming
  0 siblings, 1 reply; 32+ messages in thread
From: Jordan Justen @ 2019-03-18  6:20 UTC (permalink / raw)
  To: Gao, Zhichao, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Bret Barkelew, Michael Turner, Gao, Liming

On 2019-03-17 17:14:40, Gao, Zhichao wrote:
> > -----Original Message-----
> > From: Justen, Jordan L
> > Sent: Friday, March 15, 2019 2:16 PM
> > 
> > On 2019-03-14 22:17:33, Zhichao Gao wrote:
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> > >
> > > Add a new api DebugVPrint prototype definition in the
> > > DebugLib header file. This api would expose a print
> > > routine with VaList parameter.
> > 
> > These lines seem to be fairly short, with the longest be 54 chars. I guess not a
> > problem, but by the recommendation says they could be up to 75 in length.
> > 
> > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-
> > Format
> > 
> 
> It is hard for someone to control the characters in a line.

I guess that depends on your editor. :)

> Making the text massage readable base on the rules make more sense.
> It is easier to control the chars number within 75. Line length less
> than 76 characters is legal.

Yes, I mentioned it is legal, but it is noticably short. I don't think
it is so short that it is really a problem, so feel free to leave it
if you prefer.

> > 
> > According to the style guide:
> > 
> >   "Preferably, limit line lengths to 80 columns or less."
> > 
> > https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C
> > 
> > https://github.com/tianocore-
> > docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf
> > 
> > But, this line uses 92 columns.
> > 
> > I think there are similar cases in other patches.
> 
> Right. This sentence I copied from the comment of DebugPrint
> function which might be designed for a long time. The CCS might not
> be designed as such style at that time. For this, it's better to
> change both of the two sentences.
> By the way, the whole edk2 repo may contain a lot of lines violated
> this rule. It takes effort to fix them all.

I agree there are many violations of this in edk2.

Usually, it's not too important to go around fixing the issues that
already happened, but if you are changing or adding some code, then
you can try to make sure the new lines you add are good.

-Jordan


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
  2019-03-18  6:20       ` Jordan Justen
@ 2019-03-18 15:09         ` Gao, Liming
  2019-03-18 18:34           ` Jordan Justen
  0 siblings, 1 reply; 32+ messages in thread
From: Gao, Liming @ 2019-03-18 15:09 UTC (permalink / raw)
  To: Justen, Jordan L, Gao, Zhichao, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Bret Barkelew, Michael Turner, Gao, Liming

Jordan:
  Coding style document CCS_2_1_Draft.pdf 5.1 General Rules, 5.1.1 Lines shall be 120 columns, or less. Preferably, limit line lengths to 80 columns or less. So, I don't think the line length bigger than 80 and less than 120 violates coding style documents. We should update wiki https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C to match the documentation. Limit line length to 120 characters instead of 80 characters. 

Thanks
Liming
> -----Original Message-----
> From: Justen, Jordan L
> Sent: Monday, March 18, 2019 2:21 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
> 
> On 2019-03-17 17:14:40, Gao, Zhichao wrote:
> > > -----Original Message-----
> > > From: Justen, Jordan L
> > > Sent: Friday, March 15, 2019 2:16 PM
> > >
> > > On 2019-03-14 22:17:33, Zhichao Gao wrote:
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> > > >
> > > > Add a new api DebugVPrint prototype definition in the
> > > > DebugLib header file. This api would expose a print
> > > > routine with VaList parameter.
> > >
> > > These lines seem to be fairly short, with the longest be 54 chars. I guess not a
> > > problem, but by the recommendation says they could be up to 75 in length.
> > >
> > > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-
> > > Format
> > >
> >
> > It is hard for someone to control the characters in a line.
> 
> I guess that depends on your editor. :)
> 
> > Making the text massage readable base on the rules make more sense.
> > It is easier to control the chars number within 75. Line length less
> > than 76 characters is legal.
> 
> Yes, I mentioned it is legal, but it is noticably short. I don't think
> it is so short that it is really a problem, so feel free to leave it
> if you prefer.
> 
> > >
> > > According to the style guide:
> > >
> > >   "Preferably, limit line lengths to 80 columns or less."
> > >
> > > https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C
> > >
> > > https://github.com/tianocore-
> > > docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf
> > >
> > > But, this line uses 92 columns.
> > >
> > > I think there are similar cases in other patches.
> >
> > Right. This sentence I copied from the comment of DebugPrint
> > function which might be designed for a long time. The CCS might not
> > be designed as such style at that time. For this, it's better to
> > change both of the two sentences.
> > By the way, the whole edk2 repo may contain a lot of lines violated
> > this rule. It takes effort to fix them all.
> 
> I agree there are many violations of this in edk2.
> 
> Usually, it's not too important to go around fixing the issues that
> already happened, but if you are changing or adding some code, then
> you can try to make sure the new lines you add are good.
> 
> -Jordan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
  2019-03-18 15:09         ` Gao, Liming
@ 2019-03-18 18:34           ` Jordan Justen
  2019-03-19 14:45             ` Gao, Liming
  0 siblings, 1 reply; 32+ messages in thread
From: Jordan Justen @ 2019-03-18 18:34 UTC (permalink / raw)
  To: Gao, Liming, Gao, Zhichao, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Bret Barkelew, Michael Turner, Gao, Liming

On 2019-03-18 08:09:36, Gao, Liming wrote:
> Jordan:
>   Coding style document CCS_2_1_Draft.pdf 5.1 General Rules, 5.1.1
>   Lines shall be 120 columns, or less. Preferably, limit line
>   lengths to 80 columns or less. So, I don't think the line length
>   bigger than 80 and less than 120 violates coding style documents.

I think this is clarified as an "exception" base: "When this doesn’t
leave sufficient space for a good postfix style comment, extend the
line to a total of 120 columns."

That doesn't apply to this case.

I have never seen a case where I thought this exception was useful in
EDK II. I think the exception should be removed, because if a true
exceptional case ever did arise, it would be so obvious, no one would
need to question it.

The only case I've seen (not in EDK II) where going beyond 80 columns
seemed to make some sense was a table processed by a build tool. In
that case the table has so many columns that line went beyond 80
columns.

There a good code-readability reasons for this limit. For one, we can
be almost certain that everyone will have 80 columns visible in their
editor. But, just as importantly, as the line gets too long, it
becomes more challenging to track back to the start of the next line.
(Think of newspaper columns, and imagine if they ran all the way
across the page.) I'm not sure 80 columns is the "perfect" number, but
it does seem that most agree it is probably close.

>   We should update wiki
>   https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C
>   to match the documentation. Limit line length to 120 characters
>   instead of 80 characters.

Since this is an "exception", and not normally helpful, I don't think
we should add it do the wiki. It will only add confusion and lead to
more code that doesn't follow the preferred limit.

-Jordan

> 
> > -----Original Message-----
> > From: Justen, Jordan L
> > Sent: Monday, March 18, 2019 2:21 PM
> > To: Gao, Zhichao <zhichao.gao@intel.com>; edk2-devel@lists.01.org
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Michael Turner
> > <Michael.Turner@microsoft.com>; Gao, Liming <liming.gao@intel.com>
> > Subject: RE: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
> > 
> > On 2019-03-17 17:14:40, Gao, Zhichao wrote:
> > > > -----Original Message-----
> > > > From: Justen, Jordan L
> > > > Sent: Friday, March 15, 2019 2:16 PM
> > > >
> > > > On 2019-03-14 22:17:33, Zhichao Gao wrote:
> > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> > > > >
> > > > > Add a new api DebugVPrint prototype definition in the
> > > > > DebugLib header file. This api would expose a print
> > > > > routine with VaList parameter.
> > > >
> > > > These lines seem to be fairly short, with the longest be 54 chars. I guess not a
> > > > problem, but by the recommendation says they could be up to 75 in length.
> > > >
> > > > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-
> > > > Format
> > > >
> > >
> > > It is hard for someone to control the characters in a line.
> > 
> > I guess that depends on your editor. :)
> > 
> > > Making the text massage readable base on the rules make more sense.
> > > It is easier to control the chars number within 75. Line length less
> > > than 76 characters is legal.
> > 
> > Yes, I mentioned it is legal, but it is noticably short. I don't think
> > it is so short that it is really a problem, so feel free to leave it
> > if you prefer.
> > 
> > > >
> > > > According to the style guide:
> > > >
> > > >   "Preferably, limit line lengths to 80 columns or less."
> > > >
> > > > https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C
> > > >
> > > > https://github.com/tianocore-
> > > > docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf
> > > >
> > > > But, this line uses 92 columns.
> > > >
> > > > I think there are similar cases in other patches.
> > >
> > > Right. This sentence I copied from the comment of DebugPrint
> > > function which might be designed for a long time. The CCS might not
> > > be designed as such style at that time. For this, it's better to
> > > change both of the two sentences.
> > > By the way, the whole edk2 repo may contain a lot of lines violated
> > > this rule. It takes effort to fix them all.
> > 
> > I agree there are many violations of this in edk2.
> > 
> > Usually, it's not too important to go around fixing the issues that
> > already happened, but if you are changing or adding some code, then
> > you can try to make sure the new lines you add are good.
> > 
> > -Jordan


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
  2019-03-18 18:34           ` Jordan Justen
@ 2019-03-19 14:45             ` Gao, Liming
  2019-03-19 15:44               ` stephano
  0 siblings, 1 reply; 32+ messages in thread
From: Gao, Liming @ 2019-03-19 14:45 UTC (permalink / raw)
  To: Justen, Jordan L, Gao, Zhichao, edk2-devel@lists.01.org,
	Cetola, Stephano
  Cc: Kinney, Michael D, Bret Barkelew, Michael Turner, Gao, Liming

Jordan:
  I suggest to add the line length discussion in edk2 community meeting. Which length is better, 80 or 120? If we expect every patch follows this rule, do we need update PatchCheck to add the checker, and notify the developer know this requirement?

  For now, I suggest that the developer bases on current process, pass PatchCheck and code review. 

Thanks
Liming
>-----Original Message-----
>From: Justen, Jordan L
>Sent: Tuesday, March 19, 2019 2:34 AM
>To: Gao, Liming <liming.gao@intel.com>; Gao, Zhichao
><zhichao.gao@intel.com>; edk2-devel@lists.01.org
>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Bret Barkelew
><Bret.Barkelew@microsoft.com>; Michael Turner
><Michael.Turner@microsoft.com>; Gao, Liming <liming.gao@intel.com>
>Subject: RE: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api
>DebugVPrint for DebugLib
>
>On 2019-03-18 08:09:36, Gao, Liming wrote:
>> Jordan:
>>   Coding style document CCS_2_1_Draft.pdf 5.1 General Rules, 5.1.1
>>   Lines shall be 120 columns, or less. Preferably, limit line
>>   lengths to 80 columns or less. So, I don't think the line length
>>   bigger than 80 and less than 120 violates coding style documents.
>
>I think this is clarified as an "exception" base: "When this doesn’t
>leave sufficient space for a good postfix style comment, extend the
>line to a total of 120 columns."
>
>That doesn't apply to this case.
>
>I have never seen a case where I thought this exception was useful in
>EDK II. I think the exception should be removed, because if a true
>exceptional case ever did arise, it would be so obvious, no one would
>need to question it.
>
>The only case I've seen (not in EDK II) where going beyond 80 columns
>seemed to make some sense was a table processed by a build tool. In
>that case the table has so many columns that line went beyond 80
>columns.
>
>There a good code-readability reasons for this limit. For one, we can
>be almost certain that everyone will have 80 columns visible in their
>editor. But, just as importantly, as the line gets too long, it
>becomes more challenging to track back to the start of the next line.
>(Think of newspaper columns, and imagine if they ran all the way
>across the page.) I'm not sure 80 columns is the "perfect" number, but
>it does seem that most agree it is probably close.
>
>>   We should update wiki
>>   https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C
>>   to match the documentation. Limit line length to 120 characters
>>   instead of 80 characters.
>
>Since this is an "exception", and not normally helpful, I don't think
>we should add it do the wiki. It will only add confusion and lead to
>more code that doesn't follow the preferred limit.
>
>-Jordan
>
>>
>> > -----Original Message-----
>> > From: Justen, Jordan L
>> > Sent: Monday, March 18, 2019 2:21 PM
>> > To: Gao, Zhichao <zhichao.gao@intel.com>; edk2-devel@lists.01.org
>> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Bret Barkelew
><Bret.Barkelew@microsoft.com>; Michael Turner
>> > <Michael.Turner@microsoft.com>; Gao, Liming <liming.gao@intel.com>
>> > Subject: RE: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api
>DebugVPrint for DebugLib
>> >
>> > On 2019-03-17 17:14:40, Gao, Zhichao wrote:
>> > > > -----Original Message-----
>> > > > From: Justen, Jordan L
>> > > > Sent: Friday, March 15, 2019 2:16 PM
>> > > >
>> > > > On 2019-03-14 22:17:33, Zhichao Gao wrote:
>> > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
>> > > > >
>> > > > > Add a new api DebugVPrint prototype definition in the
>> > > > > DebugLib header file. This api would expose a print
>> > > > > routine with VaList parameter.
>> > > >
>> > > > These lines seem to be fairly short, with the longest be 54 chars. I
>guess not a
>> > > > problem, but by the recommendation says they could be up to 75 in
>length.
>> > > >
>> > > > https://github.com/tianocore/tianocore.github.io/wiki/Commit-
>Message-
>> > > > Format
>> > > >
>> > >
>> > > It is hard for someone to control the characters in a line.
>> >
>> > I guess that depends on your editor. :)
>> >
>> > > Making the text massage readable base on the rules make more sense.
>> > > It is easier to control the chars number within 75. Line length less
>> > > than 76 characters is legal.
>> >
>> > Yes, I mentioned it is legal, but it is noticably short. I don't think
>> > it is so short that it is really a problem, so feel free to leave it
>> > if you prefer.
>> >
>> > > >
>> > > > According to the style guide:
>> > > >
>> > > >   "Preferably, limit line lengths to 80 columns or less."
>> > > >
>> > > > https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C
>> > > >
>> > > > https://github.com/tianocore-
>> > > > docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf
>> > > >
>> > > > But, this line uses 92 columns.
>> > > >
>> > > > I think there are similar cases in other patches.
>> > >
>> > > Right. This sentence I copied from the comment of DebugPrint
>> > > function which might be designed for a long time. The CCS might not
>> > > be designed as such style at that time. For this, it's better to
>> > > change both of the two sentences.
>> > > By the way, the whole edk2 repo may contain a lot of lines violated
>> > > this rule. It takes effort to fix them all.
>> >
>> > I agree there are many violations of this in edk2.
>> >
>> > Usually, it's not too important to go around fixing the issues that
>> > already happened, but if you are changing or adding some code, then
>> > you can try to make sure the new lines you add are good.
>> >
>> > -Jordan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
  2019-03-19 14:45             ` Gao, Liming
@ 2019-03-19 15:44               ` stephano
  0 siblings, 0 replies; 32+ messages in thread
From: stephano @ 2019-03-19 15:44 UTC (permalink / raw)
  To: Gao, Liming, Justen, Jordan L, Gao, Zhichao,
	edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Michael Turner, Bret Barkelew

On 3/19/2019 7:45 AM, Gao, Liming wrote:
> Jordan:
>    I suggest to add the line length discussion in edk2 community meeting. Which length is better, 80 or 120? If we expect every patch follows this rule, do we need update PatchCheck to add the checker, and notify the developer know this requirement?
> 

I've added this discussion to the list for next month's community meeting:

https://www.tianocore.org/community-meeting/

Cheers,
Stephano


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 09/17] OvmfPkg/PlatformDebugLibIoPort: Add a new api DebugVPrint
  2019-03-15  5:17 ` [PATCH V2 09/17] OvmfPkg/PlatformDebugLibIoPort: " Zhichao Gao
@ 2019-03-20 16:13   ` Laszlo Ersek
  0 siblings, 0 replies; 32+ messages in thread
From: Laszlo Ersek @ 2019-03-20 16:13 UTC (permalink / raw)
  To: Zhichao Gao, edk2-devel
  Cc: Jordan Justen, Michael Turner, Bret Barkelew, Liming Gao

On 03/15/19 06:17, Zhichao Gao wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> 
> Add a new api DebugVPrint implementation in the
> DebugLib instance. This api would expose a print
> routine with VaList parameter.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> ---
>  OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c | 38 ++++++++++++++++++++---
>  1 file changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> index 36cde54976..74013c28a2 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> @@ -2,7 +2,7 @@
>    Base Debug library instance for QEMU debug port.
>    It uses PrintLib to send debug messages to a fixed I/O port.
>  
> -  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>    Copyright (c) 2012, Red Hat, Inc.<BR>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD License
> @@ -51,9 +51,38 @@ DebugPrint (
>    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.
> +
> +  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
> +  )
>  {
>    CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> -  VA_LIST  Marker;
>    UINTN    Length;
>  
>    //
> @@ -72,9 +101,7 @@ DebugPrint (
>    //
>    // Convert the DEBUG() message to an ASCII String
>    //
> -  VA_START (Marker, Format);
> -  Length = AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
> -  VA_END (Marker);
> +  Length = AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker);
>  
>    //
>    // Send the print string to the debug I/O port
> @@ -270,6 +297,7 @@ DebugPrintLevelEnabled (
>    return (BOOLEAN) ((ErrorLevel & PcdGet32(PcdFixedDebugPrintErrorLevel)) != 0);
>  }
>  
> +
>  /**
>    Return the result of detecting the debug I/O port device.
>  
> 

Thanks for addressing my v1 comments.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Laszlo


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 17/17] MdeModulePkg: Add PEIM and lib to dsc file
  2019-03-15  5:54   ` Andrew Fish
@ 2019-04-01  6:17     ` Jordan Justen
  2019-04-01  6:33       ` Jordan Justen
  0 siblings, 1 reply; 32+ messages in thread
From: Jordan Justen @ 2019-04-01  6:17 UTC (permalink / raw)
  To: Gao, Liming, Andrew Fish, Andrew Fish via edk2-devel, Mike Kinney,
	Zhichao Gao
  Cc: Hao Wu, edk2-devel, Bret Barkelew, Michael Turner, Star Zeng,
	Ard Biesheuvel, Laszlo Ersek

On 2019-03-14 22:54:24, Andrew Fish via edk2-devel wrote:
> I understand the motivation for this change as I've done something
> much less portable that looks a lot like this to save the PEI XIP
> space....
> 
> I seem to remember a long time ago we add a public VA_LIST to an API
> and we ran into an issue due to the marker format being compiler
> specific. You don't tend to see this on VC++ as it mostly uses a
> pointer to the frame, but GCC and clang can use a data structure
> especially not on IA32 (i386) for a VA_LIST (this is part of the
> reason there are so many #ifdefs in the Base.h VA_LIST section).
> 
> In the past to fix this Mike Kinney added BASE_LIST. I seem to
> remember MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode passes
> a BASE_LIST into ReportStatusCode vs. a VA_LIST for this reason. Was
> this VA_LIST portability concern taken into consideration for this
> new API?
> 
> In general VA_LIST is not an issue as long as all the code is
> compiled with the same compiler but if binaries are in play then you
> can have issues. So things like FSB, Option ROMs, OS Loaders, EFI
> Shell, EFI Apps. are when you can see the issue.
> 

I didn't see a reply to Andrew's concern. Was it considered, and
addressed?

I remember having several issues in the past with var-args. I haven't
seen it show up in quite a while.

I know we have to make sure to consistently apply EFIAPI to such
function declarations.

I did think we still considered it forbidden to use var-args across a
binary interface. (PPI, Protocol, Core functions)

I'm not sure if EFIAPI is enough to make it safe as long as all code
is compiled by the same compiler, or if it truely makes it safe for a
binary compiled with one compiler to be called from a binary compiled
with any other compiler.

I'll add Ard and Laszlo to the Cc, as I seem to recall they may have
looked at it before. (Although, I think they've been Cc'd on the
series, so maybe they've already considered this, and don't expect
there is a problem.)

-Jordan


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 17/17] MdeModulePkg: Add PEIM and lib to dsc file
  2019-04-01  6:17     ` Jordan Justen
@ 2019-04-01  6:33       ` Jordan Justen
  2019-04-01  6:37         ` Gao, Zhichao
  0 siblings, 1 reply; 32+ messages in thread
From: Jordan Justen @ 2019-04-01  6:33 UTC (permalink / raw)
  To: Gao, Liming, Andrew Fish, Andrew Fish via edk2-devel, Mike Kinney,
	Zhichao Gao
  Cc: Hao Wu, edk2-devel, Bret Barkelew, Michael Turner, Star Zeng,
	Ard Biesheuvel, Laszlo Ersek

Whoops. It looks like this was specifically addressed in v3 of the
series. I just didn't see a follow up email discussion to Andrew's
email.

Please disregard.

-Jordan

On 2019-03-31 23:17:12, Jordan Justen wrote:
> On 2019-03-14 22:54:24, Andrew Fish via edk2-devel wrote:
> > I understand the motivation for this change as I've done something
> > much less portable that looks a lot like this to save the PEI XIP
> > space....
> > 
> > I seem to remember a long time ago we add a public VA_LIST to an API
> > and we ran into an issue due to the marker format being compiler
> > specific. You don't tend to see this on VC++ as it mostly uses a
> > pointer to the frame, but GCC and clang can use a data structure
> > especially not on IA32 (i386) for a VA_LIST (this is part of the
> > reason there are so many #ifdefs in the Base.h VA_LIST section).
> > 
> > In the past to fix this Mike Kinney added BASE_LIST. I seem to
> > remember MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode passes
> > a BASE_LIST into ReportStatusCode vs. a VA_LIST for this reason. Was
> > this VA_LIST portability concern taken into consideration for this
> > new API?
> > 
> > In general VA_LIST is not an issue as long as all the code is
> > compiled with the same compiler but if binaries are in play then you
> > can have issues. So things like FSB, Option ROMs, OS Loaders, EFI
> > Shell, EFI Apps. are when you can see the issue.
> > 
> 
> I didn't see a reply to Andrew's concern. Was it considered, and
> addressed?
> 
> I remember having several issues in the past with var-args. I haven't
> seen it show up in quite a while.
> 
> I know we have to make sure to consistently apply EFIAPI to such
> function declarations.
> 
> I did think we still considered it forbidden to use var-args across a
> binary interface. (PPI, Protocol, Core functions)
> 
> I'm not sure if EFIAPI is enough to make it safe as long as all code
> is compiled by the same compiler, or if it truely makes it safe for a
> binary compiled with one compiler to be called from a binary compiled
> with any other compiler.
> 
> I'll add Ard and Laszlo to the Cc, as I seem to recall they may have
> looked at it before. (Although, I think they've been Cc'd on the
> series, so maybe they've already considered this, and don't expect
> there is a problem.)
> 
> -Jordan


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH V2 17/17] MdeModulePkg: Add PEIM and lib to dsc file
  2019-04-01  6:33       ` Jordan Justen
@ 2019-04-01  6:37         ` Gao, Zhichao
  0 siblings, 0 replies; 32+ messages in thread
From: Gao, Zhichao @ 2019-04-01  6:37 UTC (permalink / raw)
  To: Justen, Jordan L, Gao, Liming, Andrew Fish,
	Andrew Fish via edk2-devel, Kinney, Michael D
  Cc: Wu, Hao A, edk2-devel, Bret Barkelew, Michael Turner, Zeng, Star,
	Ard Biesheuvel, Laszlo Ersek

Thanks for your clear check. I didn't reply all the comments.
The patch set is update to V7 base on the comments from the community.

Thanks,
Zhichao

> -----Original Message-----
> From: Justen, Jordan L
> Sent: Monday, April 1, 2019 2:33 PM
> To: Gao, Liming <liming.gao@intel.com>; Andrew Fish <afish@apple.com>;
> Andrew Fish via edk2-devel <edk2-devel@lists.01.org>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>; edk2-devel <edk2-
> devel@lists.01.org>; Bret Barkelew <Bret.Barkelew@microsoft.com>;
> Michael Turner <Michael.Turner@microsoft.com>; Zeng, Star
> <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: Re: [edk2] [PATCH V2 17/17] MdeModulePkg: Add PEIM and lib to
> dsc file
> 
> Whoops. It looks like this was specifically addressed in v3 of the series. I just
> didn't see a follow up email discussion to Andrew's email.
> 
> Please disregard.
> 
> -Jordan
> 
> On 2019-03-31 23:17:12, Jordan Justen wrote:
> > On 2019-03-14 22:54:24, Andrew Fish via edk2-devel wrote:
> > > I understand the motivation for this change as I've done something
> > > much less portable that looks a lot like this to save the PEI XIP
> > > space....
> > >
> > > I seem to remember a long time ago we add a public VA_LIST to an API
> > > and we ran into an issue due to the marker format being compiler
> > > specific. You don't tend to see this on VC++ as it mostly uses a
> > > pointer to the frame, but GCC and clang can use a data structure
> > > especially not on IA32 (i386) for a VA_LIST (this is part of the
> > > reason there are so many #ifdefs in the Base.h VA_LIST section).
> > >
> > > In the past to fix this Mike Kinney added BASE_LIST. I seem to
> > > remember MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode
> passes
> > > a BASE_LIST into ReportStatusCode vs. a VA_LIST for this reason. Was
> > > this VA_LIST portability concern taken into consideration for this
> > > new API?
> > >
> > > In general VA_LIST is not an issue as long as all the code is
> > > compiled with the same compiler but if binaries are in play then you
> > > can have issues. So things like FSB, Option ROMs, OS Loaders, EFI
> > > Shell, EFI Apps. are when you can see the issue.
> > >
> >
> > I didn't see a reply to Andrew's concern. Was it considered, and
> > addressed?
> >
> > I remember having several issues in the past with var-args. I haven't
> > seen it show up in quite a while.
> >
> > I know we have to make sure to consistently apply EFIAPI to such
> > function declarations.
> >
> > I did think we still considered it forbidden to use var-args across a
> > binary interface. (PPI, Protocol, Core functions)
> >
> > I'm not sure if EFIAPI is enough to make it safe as long as all code
> > is compiled by the same compiler, or if it truely makes it safe for a
> > binary compiled with one compiler to be called from a binary compiled
> > with any other compiler.
> >
> > I'll add Ard and Laszlo to the Cc, as I seem to recall they may have
> > looked at it before. (Although, I think they've been Cc'd on the
> > series, so maybe they've already considered this, and don't expect
> > there is a problem.)
> >
> > -Jordan

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2019-04-01  6:37 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-15  5:17 [PATCH V2 00/17] Add a new API DebugVPrint for DebugLib Zhichao Gao
2019-03-15  5:17 ` [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api " Zhichao Gao
2019-03-15  6:15   ` Jordan Justen
2019-03-18  0:14     ` Gao, Zhichao
2019-03-18  6:20       ` Jordan Justen
2019-03-18 15:09         ` Gao, Liming
2019-03-18 18:34           ` Jordan Justen
2019-03-19 14:45             ` Gao, Liming
2019-03-19 15:44               ` stephano
2019-03-15  5:17 ` [PATCH V2 02/17] MdePkg/BaseDebugLibNull: " Zhichao Gao
2019-03-15  5:17 ` [PATCH V2 03/17] MdePkg/BaseDebugLibSerialPort: Add a new api DebugVPrint Zhichao Gao
2019-03-15  5:17 ` [PATCH V2 04/17] MdePkg/UefidebugLibConOut: " Zhichao Gao
2019-03-15  5:17 ` [PATCH V2 05/17] MdePkg/UefiDebugLibStdErr: " Zhichao Gao
2019-03-15  5:17 ` [PATCH V2 06/17] MdePkg/DxeRuntimeDebugLibSerialPort: " Zhichao Gao
2019-03-15  5:17 ` [PATCH V2 07/17] MdePkg/UefiDebuglibDebugPortProtocol: " Zhichao Gao
2019-03-15  5:17 ` [PATCH V2 08/17] ArmPkg/SemiHostingDebugLib: " Zhichao Gao
2019-03-15  5:17 ` [PATCH V2 09/17] OvmfPkg/PlatformDebugLibIoPort: " Zhichao Gao
2019-03-20 16:13   ` Laszlo Ersek
2019-03-15  5:17 ` [PATCH V2 10/17] IntelFsp2Pkg/BaseFspDebugLibSerialPort: " Zhichao Gao
2019-03-15  7:31   ` Chiu, Chasel
2019-03-15  5:17 ` [PATCH V2 11/17] IntelFspPkg/BaseFspDebugLibSerialPort: " Zhichao Gao
2019-03-15  7:32   ` Chiu, Chasel
2019-03-15  5:17 ` [PATCH V2 12/17] IntelFramworkModulePkg/PeiDxeDebugLibReportStatusCode: Add a new api Zhichao Gao
2019-03-15  5:17 ` [PATCH V2 13/17] MdeModulePkg/PeiDxeDebugLibReportStatusCode: " Zhichao Gao
2019-03-15  5:17 ` [PATCH V2 14/17] MdeModulePkg: Add definitions for EDKII DEBUG PPI Zhichao Gao
2019-03-15  5:17 ` [PATCH V2 15/17] MdeModulePkg: Add a PEIM to install Debug PPI Zhichao Gao
2019-03-15  5:17 ` [PATCH V2 16/17] MdeModulePkg/PeiDebugLibDebugPpi: Add PEI debug lib Zhichao Gao
2019-03-15  5:17 ` [PATCH V2 17/17] MdeModulePkg: Add PEIM and lib to dsc file Zhichao Gao
2019-03-15  5:54   ` Andrew Fish
2019-04-01  6:17     ` Jordan Justen
2019-04-01  6:33       ` Jordan Justen
2019-04-01  6:37         ` Gao, Zhichao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox