Re: [PATCH 1/2] UefiPayloadPkg: Add a new DebugPrintErrorLevelLib instance

["Guo Dong" <guo.dong@intel.com>]

Hi Yuanhao,

The suggestion looks good to me.

Thanks,
Guo

-----Original Message-----
From: Xie, Yuanhao <yuanhao.xie@intel.com> 
Sent: Monday, March 28, 2022 11:54 PM
To: Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
Cc: You, Benjamin <benjamin.you@intel.com>; Rhodes, Sean <sean@starlabs.systems>; devel@edk2.groups.io
Subject: RE: [PATCH 1/2] UefiPayloadPkg: Add a new DebugPrintErrorLevelLib instance

Hi Guo,

Sorry for the late response.
Just as we discussed today, and also from the suggestions by Ray, in the new patch I will 

1. changed "UNIVERSAL_PAYLOAD_ DEBUG_PRINT_ERROR_LEVEL" to "UEFI_PAYLOAD_DEBUG_PRINT_ERROR_LEVEL", and 2. add a comment referencing Debuglib.h to declare bits for "ErrorLevel" parameters. The reason why better not just declare those common bits is because it can cause confusion if other developers still use the rest.

What do you think?

Thanks,
Yuanhao

-----Original Message-----
From: Dong, Guo <guo.dong@intel.com>
Sent: Tuesday, March 29, 2022 12:00 PM
To: Xie, Yuanhao <yuanhao.xie@intel.com>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; You, Benjamin <benjamin.you@intel.com>; Rhodes, Sean <sean@starlabs.systems>
Subject: RE: [PATCH 1/2] UefiPayloadPkg: Add a new DebugPrintErrorLevelLib instance


I replied in another email. It looks the comments are not addressed in this patch, especially on this comment:
In the DebugPrintErrorLevel.h, ErrorLevel is defined as UINT32, but its usage is not clear for bootloaders ( so need add more info in the header file)

Thanks,
Guo
-----Original Message-----
From: Xie, Yuanhao <yuanhao.xie@intel.com>
Sent: Sunday, March 27, 2022 10:59 PM
To: devel@edk2.groups.io
Cc: Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Maurice Ma <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>; Rhodes, Sean <sean@starlabs.systems>
Subject: [PATCH 1/2] UefiPayloadPkg: Add a new DebugPrintErrorLevelLib instance

It consumes the HOB defined in
UefiPayloadPkg/Include/Guid/DebugPrintErrorLevel.h, and allow bootloader  to config DebugPrintErrorLevel.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>

Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
 UefiPayloadPkg/Include/Guid/DebugPrintErrorLevel.h                               | 27 +++++++++++++++++++++++++++
 UefiPayloadPkg/Library/DebugPrintErrorLevelLibHob/DebugPrintErrorLevelLibHob.c   | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 UefiPayloadPkg/Library/DebugPrintErrorLevelLibHob/DebugPrintErrorLevelLibHob.inf | 39 +++++++++++++++++++++++++++++++++++++++
 UefiPayloadPkg/UefiPayloadPkg.dec                                                |  2 +-
 4 files changed, 144 insertions(+), 1 deletion(-)

diff --git a/UefiPayloadPkg/Include/Guid/DebugPrintErrorLevel.h b/UefiPayloadPkg/Include/Guid/DebugPrintErrorLevel.h
new file mode 100644
index 0000000000..9a3f4eb28e
--- /dev/null
+++ b/UefiPayloadPkg/Include/Guid/DebugPrintErrorLevel.h
@@ -0,0 +1,27 @@
+/** @file
+  Define the structure for Debug Print Error Level Guid Hob.
+
+Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef UNIVERSAL_PAYLOAD_DEBUG_PRINT_ERROR_LEVEL_H_
+#define UNIVERSAL_PAYLOAD_DEBUG_PRINT_ERROR_LEVEL_H_
+
+#include <Uefi.h>
+#include <UniversalPayload/UniversalPayload.h>
+
+#pragma pack (1)
+
+typedef struct {
+  UNIVERSAL_PAYLOAD_GENERIC_HEADER    Header;
+  UINT32                              ErrorLevel;
+} UNIVERSAL_PAYLOAD_DEBUG_PRINT_ERROR_LEVEL;
+
+#pragma pack()
+
+#define UNIVERSAL_PAYLOAD_DEBUG_PRINT_ERROR_LEVEL_REVISION  1
+
+extern GUID  gEdkiiDebugPrintErrorLevelGuid; #endif
diff --git a/UefiPayloadPkg/Library/DebugPrintErrorLevelLibHob/DebugPrintErrorLevelLibHob.c b/UefiPayloadPkg/Library/DebugPrintErrorLevelLibHob/DebugPrintErrorLevelLibHob.c
new file mode 100644
index 0000000000..18378249ab
--- /dev/null
+++ b/UefiPayloadPkg/Library/DebugPrintErrorLevelLibHob/DebugPrintErrorL
+++ evelLibHob.c
@@ -0,0 +1,77 @@
+/** @file
+  Debug Print Error Level library instance that retrieves
+  the DebugPrintErrorLevel from bootloader.
+
+  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <Uefi.h>
+#include <PiDxe.h>
+#include <Library/PcdLib.h>
+#include <Library/HobLib.h>
+#include <Guid/DebugPrintErrorLevel.h>
+#include <Library/DebugPrintErrorLevelLib.h>
+#include <UniversalPayload/UniversalPayload.h>
+
+STATIC UINT32  gDebugPrintErrorLevel;
+STATIC BOOLEAN gDebugPrintErrorLevelInitialized = FALSE;
+/**
+  Returns the debug print error level mask for the current module.
+
+  @return  Debug print error level mask for the current module.
+
+**/
+UINT32
+EFIAPI
+GetDebugPrintErrorLevel (
+  VOID
+  )
+{
+  VOID                                        *GuidHob;
+  UNIVERSAL_PAYLOAD_GENERIC_HEADER            *GenericHeader;
+  UNIVERSAL_PAYLOAD_DEBUG_PRINT_ERROR_LEVEL   *DebugPrintErrorLevel;
+
+  if (!gDebugPrintErrorLevelInitialized) {
+    gDebugPrintErrorLevelInitialized = TRUE;
+    gDebugPrintErrorLevel = PcdGet32(PcdDebugPrintErrorLevel);
+    GuidHob = GetFirstGuidHob (&gEdkiiDebugPrintErrorLevelGuid);
+    if (GuidHob != NULL) {
+      GenericHeader = (UNIVERSAL_PAYLOAD_GENERIC_HEADER *)GET_GUID_HOB_DATA (GuidHob);
+      if ((sizeof (UNIVERSAL_PAYLOAD_GENERIC_HEADER) < GET_GUID_HOB_DATA_SIZE (GuidHob)) 
+          && (GenericHeader->Length <= GET_GUID_HOB_DATA_SIZE (GuidHob))) {
+        if (GenericHeader->Revision == UNIVERSAL_PAYLOAD_DEBUG_PRINT_ERROR_LEVEL_REVISION) {
+          DebugPrintErrorLevel =  (UNIVERSAL_PAYLOAD_DEBUG_PRINT_ERROR_LEVEL *)GET_GUID_HOB_DATA (GuidHob);
+          if (DebugPrintErrorLevel->Header.Length > UNIVERSAL_PAYLOAD_SIZEOF_THROUGH_FIELD (UNIVERSAL_PAYLOAD_DEBUG_PRINT_ERROR_LEVEL, ErrorLevel)) {
+            gDebugPrintErrorLevel = DebugPrintErrorLevel->ErrorLevel;  
+          }
+        }
+      } 
+    }
+  }
+  return gDebugPrintErrorLevel;
+}
+
+/**
+  Sets the global debug print error level mask fpr the entire platform.
+
+  @param   ErrorLevel     Global debug print error level.
+
+  @retval  TRUE           The debug print error level mask was sucessfully set.
+  @retval  FALSE          The debug print error level mask could not be set.
+
+**/
+BOOLEAN
+EFIAPI
+SetDebugPrintErrorLevel (
+  UINT32  ErrorLevel
+  )
+{
+  //
+  // This library uinstance does not support setting the global debug 
+print error
+  // level mask.
+  //
+  return FALSE;
+}
diff --git a/UefiPayloadPkg/Library/DebugPrintErrorLevelLibHob/DebugPrintErrorLevelLibHob.inf b/UefiPayloadPkg/Library/DebugPrintErrorLevelLibHob/DebugPrintErrorLevelLibHob.inf
new file mode 100644
index 0000000000..0845b5a2f4
--- /dev/null
+++ b/UefiPayloadPkg/Library/DebugPrintErrorLevelLibHob/DebugPrintErrorL
+++ evelLibHob.inf
@@ -0,0 +1,39 @@
+## @file
+#  Debug Print Error Level library instance that retrieves #  the 
+DebugPrintErrorLevel from bootloader.
+#
+#  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR> # #
+SPDX-License-Identifier: BSD-2-Clause-Patent # ##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = DebugPrintErrorLevelLibHob
+  FILE_GUID                      = c3fead6d-bd4c-4131-bd5f-4bbceecc0fef
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = DebugPrintErrorLevelLib
+
+#
+#  VALID_ARCHITECTURES           = IA32 X64 EBC
+#
+
+[Sources]
+  DebugPrintErrorLevelLibHob.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  UefiPayloadPkg/UefiPayloadPkg.dec
+
+[LibraryClasses]
+  PcdLib
+  HobLib
+
+[Pcd]
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel
+
+[Guids]
+  gEdkiiDebugPrintErrorLevelGuid
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dec b/UefiPayloadPkg/UefiPayloadPkg.dec
index 4051172caf..5c1aeb8235 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dec
+++ b/UefiPayloadPkg/UefiPayloadPkg.dec
@@ -31,7 +31,7 @@
 
   ##include/Guid/BootManagerMenu.h
   gEdkiiBootManagerMenuFileGuid = { 0xdf939333, 0x42fc, 0x4b2a, { 0xa5, 0x9e, 0xbb, 0xae, 0x82, 0x81, 0xfe, 0xef }}
-
+  gEdkiiDebugPrintErrorLevelGuid = { 0xad82f436, 0x75c5, 0x4aa9, { 
+ 0x92, 0x93, 0xc5, 0x55, 0x0a, 0x7f, 0xf9, 0x71 }}
   gUefiAcpiBoardInfoGuid   = {0xad3d31b, 0xb3d8, 0x4506, {0xae, 0x71, 0x2e, 0xf1, 0x10, 0x6, 0xd9, 0xf}}
   gUefiSerialPortInfoGuid  = { 0x6c6872fe, 0x56a9, 0x4403, { 0xbb, 0x98, 0x95, 0x8d, 0x62, 0xde, 0x87, 0xf1 } }
   gLoaderMemoryMapInfoGuid = { 0xa1ff7424, 0x7a1a, 0x478e, { 0xa9, 0xe4, 0x92, 0xf3, 0x57, 0xd1, 0x28, 0x32 } }
--
2.30.0.windows.1