Re: [edk2-devel][edk2-platforms][PATCH V1 01/27] BeepDebugFeaturePkg: Use MinPlatformPkg build include files

["Nate DeSimone" <nathaniel.l.desimone@intel.com>]

Hi Isaac,

Comments inline.

Thanks,
Nate

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Oram,
> Isaac W
> Sent: Tuesday, January 11, 2022 6:20 PM
> To: devel@edk2.groups.io
> Cc: Oram, Isaac W <isaac.w.oram@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Tan, Ming
> <ming.tan@intel.com>
> Subject: [edk2-devel][edk2-platforms][PATCH V1 01/27]
> BeepDebugFeaturePkg: Use MinPlatformPkg build include files
> 
> The BeepDebugFeaturePkg was malformed and did not build anything and
> thus wasn't testable in standalone build.
> 
> Added
> gBeepDebugFeaturePkgTokenSpaceGuid.PcdBeepDebugFeatureEnable
> to properly control the advanced feature inclusion.
> 
> Use the standard MinPlatform includes for core libraries. This increases the
> build time, but reduces complexity when integrating into a board build. Board
> build optimization is more valuable than standalone build optimization.
> 
> Enable standalone build to test the library build and the component builds
> using edk2 StatusCodeHandler drivers.
> 
> Add unique file names for the StatusCodeHandler components to avoid
> collisions with any existing board handlers when incorporated in board DSC
> and FDF files.
> 
> Add PreMemory.fdf and PostMemory.fdf content for including in board FDF
> or AdvancedFeaturePkg in the future.
> 
> Removed unused sections in DSC and FDF to reduce file clutter.
> 
> Removed stale file header content from prior proprietary licensing.
> 
> Moved feature customization and common include into the package DSC file
> to reduce board complexity when BeepDebugFeature.dsc is included.
> 
> Removed the FixedAtBuild build control for beep feature as it would be
> redundant with the FeatureFlag control of the whole feature. Build control is
> via PcdBeepDebugFeatureEnable and dynamic control is via
> PcdStatusCodeUseBeep.
> 
> Improved Readme content, format, and coding style use.
> 
> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ming Tan <ming.tan@intel.com>
> 
> Signed-off-by: Isaac Oram <isaac.w.oram@intel.com>
> ---
> 
> Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg.d
> ec                                                 |   7 +-
> 
> Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg.d
> sc                                                 |  28 +++
> 
> Features/Intel/Debugging/BeepDebugFeaturePkg/Include/BeepDebugFeat
> ure.dsc                                            | 222 ++++++--------------
> 
> Features/Intel/Debugging/BeepDebugFeaturePkg/Include/Library/BeepLib.
> h                                               |   6 +-
>  Features/Intel/Debugging/BeepDebugFeaturePkg/Include/PostMemory.fdf
> |  14 ++
>  Features/Intel/Debugging/BeepDebugFeaturePkg/Include/PreMemory.fdf
> |  13 ++
> 
> Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCode
> HandlerLib/PeiBeepStatusCodeHandlerLib.inf        |   5 +-
> 
> Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCode
> HandlerLib/RuntimeDxeBeepStatusCodeHandlerLib.inf |   3 -
> 
> Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCode
> HandlerLib/SmmBeepStatusCodeHandlerLib.inf        |   3 -
>  Features/Intel/Debugging/BeepDebugFeaturePkg/Readme.md
> |  91 +++++---
>  10 files changed, 186 insertions(+), 206 deletions(-)
> 
> diff --git
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg
> .dec
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg
> .dec
> index 4f4b36b091..d90611da51 100644
> ---
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg
> .dec
> +++
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg
> .d
> +++ ec
> @@ -31,6 +31,9 @@
>  [Guids]
>    gBeepDebugFeaturePkgTokenSpaceGuid  =  {0x54f56fb5, 0xea0e, 0x4518,
> {0xa0, 0x3e, 0x1b, 0xeb, 0x56, 0x94, 0xd2, 0x16}}
> 
> -[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> +[PcdsFeatureFlag]
> +
> +gBeepDebugFeaturePkgTokenSpaceGuid.PcdBeepDebugFeatureEnable|FA
> LSE|BOOL
> +EAN|0x00000000
> +
> +[PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
>    # Beep is a legacy feature, disabled it by default
> -
> gBeepDebugFeaturePkgTokenSpaceGuid.PcdStatusCodeUseBeep|FALSE|B
> OOLEAN|0x00000001
> +
> +
> gBeepDebugFeaturePkgTokenSpaceGuid.PcdStatusCodeUseBeep|TRUE|BO
> OLEAN|0
> + x00000001
> diff --git
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg
> .dsc
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg
> .dsc
> index 47254f9974..56cad020bf 100644
> ---
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg
> .dsc
> +++
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg
> .d
> +++ sc
> @@ -24,6 +24,34 @@
>    PEI_ARCH                       = IA32
>    DXE_ARCH                       = X64
> 
> +[Packages]
> +  MinPlatformPkg/MinPlatformPkg.dec
> +  BeepDebugFeaturePkg/BeepDebugFeaturePkg.dec
> +
> +[PcdsFeatureFlag]
> +  #
> +  # PCD needed for MinPlatform build includes
> +  #
> +  gMinPlatformPkgTokenSpaceGuid.PcdSmiHandlerProfileEnable
> |FALSE
> +  gMinPlatformPkgTokenSpaceGuid.PcdUefiSecureBootEnable
> |FALSE
> +  gMinPlatformPkgTokenSpaceGuid.PcdPerformanceEnable
> |FALSE
> +
> +[PcdsDynamicExDefault]
> +  gBeepDebugFeaturePkgTokenSpaceGuid.PcdStatusCodeUseBeep|TRUE
> +
> +#
> +# Include common libraries
> +#
> +!include MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> +!include MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
> +!include MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
> +
> +[LibraryClasses.Common]
> +  #
> +  # Required by common status code handler infrastructure
> +  #
> +
> +PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePl
> atfo
> +rmHookLibNull.inf
> +
>  #
>  # This package always builds the feature.
>  #
> diff --git
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/BeepDebugFe
> ature.dsc
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/BeepDebugFe
> ature.dsc
> index b24b3e458f..251de6932b 100644
> ---
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/BeepDebugFe
> ature.dsc
> +++
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/BeepDebugFe
> at
> +++ ure.dsc
> @@ -18,97 +18,16 @@
>  #
> 
> ##########################################################
> ######################
>  [Defines]
> -!ifndef $(PEI_ARCH)
> -  !error "PEI_ARCH must be specified to build this feature!"
> -!endif
> -!ifndef $(DXE_ARCH)
> -  !error "DXE_ARCH must be specified to build this feature!"
> -!endif
> -
> -
> ##########################################################
> ######################
> -#
> -# PCD Section - list of PCD Entries modified by the feature.
> -#
> -
> ##########################################################
> ######################
> -
> -# Unmark the following and StatusCodeHandler.efi to build the .dsc file
> directly -#[PcdsDynamicDefault] -#
> gBeepDebugFeaturePkgTokenSpaceGuid.PcdStatusCodeUseBeep|TRUE
> -
> -
> ##########################################################
> ######################
> -#
> -# Library Class section - list of all Library Classes needed by this feature.
> -#
> -
> ##########################################################
> ######################
> -
> -!include MdePkg/MdeLibs.dsc.inc
> -
> -[LibraryClasses]
> -  #######################################
> -  # Edk2 Packages
> -  #######################################
> -  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> -  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> -  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> -  DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> -  DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> -  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> -  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> -  PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> -  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> -
> TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTem
> plate.inf
> -
> UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBo
> otServicesTableLib.inf
> -
> UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntry
> Point.inf
> -  UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
> -
> UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib
> /UefiRuntimeServicesTableLib.inf
> -  PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
> -
> -[LibraryClasses.common.PEIM]
> -  #######################################
> -  # Edk2 Packages
> -  #######################################
> -  HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
> -
> MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemory
> AllocationLib.inf
> -
> PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/P
> eiServicesTablePointerLibIdt.inf
> -
> -
> [LibraryClasses.IA32.PEIM,LibraryClasses.IA32.PEI_CORE,LibraryClasses.IA32.
> SEC]
> -  #######################################
> -  # Edk2 Packages
> -  #######################################
> -
> ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiR
> eportStatusCodeLib.inf
> -
> -[LibraryClasses.common.DXE_DRIVER]
> -  #######################################
> -  # Edk2 Packages
> -  #######################################
> -  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> -
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemo
> ryAllocationLib.inf
> -
> -[LibraryClasses.common.DXE_RUNTIME_DRIVER]
> -  #######################################
> -  # Edk2 Packages
> -  #######################################
> -  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> -
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemo
> ryAllocationLib.inf
> -  UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> -
> ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCod
> eLib/RuntimeDxeReportStatusCodeLib.inf
> -
> -[LibraryClasses.common.UEFI_DRIVER]
> -  #######################################
> -  # Edk2 Packages
> -  #######################################
> -  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> -
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemo
> ryAllocationLib.inf
> -
> -[LibraryClasses.X64.DXE_SMM_DRIVER]
> -  #######################################
> -  # Edk2 Packages
> -  #######################################
> -
> SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesT
> ableLib.inf
> -
> MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMe
> moryAllocationLib.inf
> -  SmmIoLib|MdePkg/Library/SmmIoLib/SmmIoLib.inf
> -  SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf
> -
> ReportStatusCodeLib|MdeModulePkg/Library/SmmReportStatusCodeLib/S
> mmReportStatusCodeLib.inf
> +  !ifndef $(PEI_ARCH)
> +    !error "PEI_ARCH must be specified to build this feature!"
> +  !endif
> +  !ifndef $(DXE_ARCH)
> +    !error "DXE_ARCH must be specified to build this feature!"
> +  !endif
> +
> +  DEFINE BEEP_PEIM_FILENAME  = b2356504-8ea3-42bd-912a-4b331990644a
> +  DEFINE BEEP_DXE_FILENAME   = f1211fa9-d83d-4c79-8726-3afaebba1070
> +  DEFINE BEEP_SMM_FILENAME   = a82cd452-0f17-4417-b8be-bb8cfdf9fa26
> 
> 
> ##########################################################
> ######################
>  #
> @@ -128,77 +47,60 @@
>  # Feature PEI Components
>  #
> 
> -# @todo: Change below line to [Components.$(PEI_ARCH)] after
> https://bugzilla.tianocore.org/show_bug.cgi?id=2308
> -#        is completed.
> +#
> +# By default, make the functional control a patcheable in module PCD #
> +[PcdsPatchableInModule]
> +  gBeepDebugFeaturePkgTokenSpaceGuid.PcdStatusCodeUseBeep
> +
> +[LibraryClasses.Common]
> +
> BeepLib|Debugging/BeepDebugFeaturePkg/Library/BeepLib/BeepLibNull.inf
> +
> +BeepMapLib|Debugging/BeepDebugFeaturePkg/Library/BeepMapLib/Bee
> pMapLib.
> +inf
> +
> +[LibraryClasses.PEIM, LibraryClasses.PEI_CORE]
> +
> +StatusCodeHandlerLib|Debugging/BeepDebugFeaturePkg/Library/BeepSta
> tusCo
> +deHandlerLib/PeiBeepStatusCodeHandlerLib.inf
> +
> +[LibraryClasses.DXE_RUNTIME_DRIVER]
> +
> +StatusCodeHandlerLib|Debugging/BeepDebugFeaturePkg/Library/BeepSta
> tusCo
> +deHandlerLib/RuntimeDxeBeepStatusCodeHandlerLib.inf
> +
> +[LibraryClasses.DXE_SMM_DRIVER]
> +
> +StatusCodeHandlerLib|Debugging/BeepDebugFeaturePkg/Library/BeepSta
> tusCo
> +deHandlerLib/SmmBeepStatusCodeHandlerLib.inf
> +
>  [Components.IA32]

This should be changed to [Components.$(PEI_ARCH)]

> -  #####################################
> -  # Beep Debug Feature Package
> -  #####################################
> 
> -  # Add library instances here that are not included in package components
> and should be tested
> -  # in the package build.
> -
> BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/PeiBeepStatusC
> odeHandlerLib.inf
> +
> MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.in
> f {
> +    <Defines>
> +      #
> +      # Many boards already have StatusCodeHandler components built from
> the common core code
> +      # Providing a unique name avoids collisions.  Both status code handler
> components will
> +      # install their listeners with the status code routers.
> +      #
> +      FILE_GUID = $(BEEP_PEIM_FILENAME)
> +    <LibraryClasses>
> +
> +
> NULL|Debugging/BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLi
> b/Pe
> + iBeepStatusCodeHandlerLib.inf
> +  }
> 
> -  # The following is an example for used with StatusCodeHandler:
> -#
> MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.in
> f {
> -#   <LibraryClasses>
> -#
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibN
> ull/OemHookStatusCodeLibNull.inf
> -#
> SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.in
> f
> -#     BeepLib|BeepDebugFeaturePkg/Library/BeepLib/BeepLibNull.inf
> -#
> BeepMapLib|BeepDebugFeaturePkg/Library/BeepMapLib/BeepMapLib.inf
> -#
> NULL|BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/PeiBeepSt
> atusCodeHandlerLib.inf
> -# }
> -
> -  # Add components here that should be included in the package build.
> -
> -#
> -# Feature DXE Components
> -#
> -
> -# @todo: Change below line to [Components.$(DXE_ARCH)] after
> https://bugzilla.tianocore.org/show_bug.cgi?id=2308
> -#        is completed.
>  [Components.X64]

This should be changed to [Components.$(DXE_ARCH)]

> -  #####################################
> -  # Beep Debug Feature Package
> -  #####################################
> +
> MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHan
> dlerRuntimeDxe.inf {
> +    <Defines>
> +      #
> +      # Many boards already have StatusCodeHandler components built from
> the common core code
> +      # Providing a unique name avoids collisions.  Both status code handler
> components will
> +      # install their listeners with the status code routers.
> +      #
> +      FILE_GUID = $(BEEP_DXE_FILENAME)
> +    <LibraryClasses>
> +
> +
> NULL|Debugging/BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLi
> b/Ru
> + ntimeDxeBeepStatusCodeHandlerLib.inf
> +  }
> 
> -  # Add library instances here that are not included in package components
> and should be tested
> -  # in the package build.
> -
> BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/RuntimeDxeBee
> pStatusCodeHandlerLib.inf
> -
> BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/SmmBeepStatus
> CodeHandlerLib.inf
> -
> -  # The following is an example for used with StatusCodeHandler:
> -#
> MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHan
> dlerRuntimeDxe.inf {
> -#   <LibraryClasses>
> -#
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibN
> ull/OemHookStatusCodeLibNull.inf
> -#
> SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.in
> f
> -#     BeepLib|BeepDebugFeaturePkg/Library/BeepLib/BeepLibNull.inf
> -#
> BeepMapLib|BeepDebugFeaturePkg/Library/BeepMapLib/BeepMapLib.inf
> -#
> NULL|BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/RuntimeD
> xeBeepStatusCodeHandlerLib.inf
> -# }
> -
> -#
> MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSm
> m.inf {
> -#   <LibraryClasses>
> -#
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibN
> ull/OemHookStatusCodeLibNull.inf
> -#
> SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.in
> f
> -#     BeepLib|BeepDebugFeaturePkg/Library/BeepLib/BeepLibNull.inf
> -#
> BeepMapLib|BeepDebugFeaturePkg/Library/BeepMapLib/BeepMapLib.inf
> -#
> NULL|BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/SmmBeep
> StatusCodeHandlerLib.inf
> -# }
> -
> -  # Add components here that should be included in the package build.
> -
> -
> ##########################################################
> #########################################
> -#
> -# BuildOptions Section - Define the module specific tool chain flags that
> should be used as
> -#                        the default flags for a module. These flags are appended to any
> -#                        standard flags that are defined by the build process. They can
> be
> -#                        applied for any modules or only those modules with the
> specific
> -#                        module style (EDK or EDKII) specified in [Components] section.
> -#
> -#                        For advanced features, it is recommended to enable
> [BuildOptions] in
> -#                        the applicable INF file so it does not affect the whole board
> package
> -#                        build when this DSC file is active.
> -#
> -
> ##########################################################
> #########################################
> -[BuildOptions]
> +
> MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSm
> m.inf {
> +    <Defines>
> +      #
> +      # Many boards already have StatusCodeHandler components built from
> the common core code
> +      # Providing a unique name avoids collisions.  Both status code handler
> components will
> +      # install their listeners with the status code routers.
> +      #
> +      FILE_GUID = $(BEEP_SMM_FILENAME)
> +    <LibraryClasses>
> +
> +
> NULL|Debugging/BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLi
> b/Sm
> + mBeepStatusCodeHandlerLib.inf
> +  }
> diff --git
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/Library/BeepLi
> b.h
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/Library/BeepLi
> b.h
> index f768acc557..0b8f71b097 100644
> ---
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/Library/BeepLi
> b.h
> +++
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/Library/BeepL
> +++ ib.h
> @@ -15,12 +15,12 @@
>    Sends the 32-bit value specified by Value to Beep device, and returns
> Value.
>    Some implementations of this library function may perform I/O operations
>    directly to Beep device.  Other implementations may send Value to
> -  ReportStatusCode(), and the status code reporting mechanism will
> eventually
> +  ReportStatusCode (), and the status code reporting mechanism will
> + eventually
>    display the 32-bit value on the status reporting device.
> 
> -  Beep() must actively prevent recursion.  If Beep() is called while
> +  Beep () must actively prevent recursion.  If Beep () is called while
>    processing another Post Code Library function, then
> -  Beep() must return Value immediately.
> +  Beep () must return Value immediately.
> 
>    @param  Value  Beep count.
>  **/
> diff --git
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/PostMemory.f
> df
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/PostMemory.f
> df
> new file mode 100644
> index 0000000000..447fb88235
> --- /dev/null
> +++
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/PostMemory.f
> d
> +++ f
> @@ -0,0 +1,14 @@
> +## @file
> +#  FDF file for post-memory BeepDebugFeature advanced feature.
> +#
> +#  This file is intended to be included into another package so
> +advanced features #  can be conditionally included in the flash image
> +by enabling the respective #  feature via its FeaturePCD.
> +#
> +# Copyright (C) 2022 Intel Corporation.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent ##
> +
> +INF FILE_GUID = $(BEEP_DXE_FILENAME)
> +MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRu
> +ntimeDxe.inf INF FILE_GUID = $(BEEP_SMM_FILENAME)
> +MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerS
> mm.inf
> diff --git
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/PreMemory.fd
> f
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/PreMemory.fd
> f
> new file mode 100644
> index 0000000000..160f5e3ffd
> --- /dev/null
> +++
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/PreMemory.fd
> f
> @@ -0,0 +1,13 @@
> +## @file
> +#  FDF file for pre-memory BeepDebugFeature advanced feature.
> +#
> +#  This file is intended to be included into another package so
> +advanced features #  can be conditionally included in the flash image
> +by enabling the respective #  feature via its FeaturePCD.
> +#
> +# Copyright (C) 2022 Intel Corporation.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent ##
> +
> +INF FILE_GUID = $(BEEP_PEIM_FILENAME)
> +MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.i
> nf
> diff --git
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCod
> eHandlerLib/PeiBeepStatusCodeHandlerLib.inf
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCod
> eHandlerLib/PeiBeepStatusCodeHandlerLib.inf
> index 421b246663..5be78a72e8 100644
> ---
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCod
> eHandlerLib/PeiBeepStatusCodeHandlerLib.inf
> +++
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCod
> +++ eHandlerLib/PeiBeepStatusCodeHandlerLib.inf
> @@ -4,9 +4,6 @@
>  # Copyright (c) 2010 - 2020, Intel Corporation. All rights reserved.<BR>  #
> SPDX-License-Identifier: BSD-2-Clause-Patent  # -# vendor.  This file may not
> be modified, except as allowed by -# additional terms of your license
> agreement.
> -#
>  ##
> 
>  [Defines]
> @@ -15,7 +12,7 @@
>    FILE_GUID                      = C4210E71-0A38-4728-8D25-4876348AA380
>    MODULE_TYPE                    = PEIM
>    CONSTRUCTOR                    = PeiBeepStatusCodeHandlerLibConstructor
> -  LIBRARY_CLASS                  = StatusCodeHandlerLib|SEC PEIM PEI_COR
> +  LIBRARY_CLASS                  = StatusCodeHandlerLib|SEC PEIM PEI_CORE
> 
>  #
>  # The following information is for reference only and not required by the
> build tools.
> diff --git
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCod
> eHandlerLib/RuntimeDxeBeepStatusCodeHandlerLib.inf
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCod
> eHandlerLib/RuntimeDxeBeepStatusCodeHandlerLib.inf
> index b9aae39128..2343e35a78 100644
> ---
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCod
> eHandlerLib/RuntimeDxeBeepStatusCodeHandlerLib.inf
> +++
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCod
> +++ eHandlerLib/RuntimeDxeBeepStatusCodeHandlerLib.inf
> @@ -4,9 +4,6 @@
>  # Copyright (c) 2010 - 2020, Intel Corporation. All rights reserved.<BR>  #
> SPDX-License-Identifier: BSD-2-Clause-Patent  # -# vendor.  This file may not
> be modified, except as allowed by -# additional terms of your license
> agreement.
> -#
>  ##
> 
>  [Defines]
> diff --git
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCod
> eHandlerLib/SmmBeepStatusCodeHandlerLib.inf
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCod
> eHandlerLib/SmmBeepStatusCodeHandlerLib.inf
> index caa82264ae..4db1748fe3 100644
> ---
> a/Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCod
> eHandlerLib/SmmBeepStatusCodeHandlerLib.inf
> +++
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCod
> +++ eHandlerLib/SmmBeepStatusCodeHandlerLib.inf
> @@ -4,9 +4,6 @@
>  # Copyright (c) 2010 - 2020, Intel Corporation. All rights reserved.<BR>  #
> SPDX-License-Identifier: BSD-2-Clause-Patent  # -# vendor.  This file may not
> be modified, except as allowed by -# additional terms of your license
> agreement.
> -#
>  ##
> 
>  [Defines]
> diff --git a/Features/Intel/Debugging/BeepDebugFeaturePkg/Readme.md
> b/Features/Intel/Debugging/BeepDebugFeaturePkg/Readme.md
> index ee254d0f99..6626c9b764 100644
> --- a/Features/Intel/Debugging/BeepDebugFeaturePkg/Readme.md
> +++ b/Features/Intel/Debugging/BeepDebugFeaturePkg/Readme.md
> @@ -6,25 +6,29 @@
>  More Information:
> 
>  ## Purpose
> -The BeepDebugFeaturePkg include some useful beep debug libraries, such
> as get beep value from status code and beep.
> -This is an important capability in firmware development to get and analyze
> the early error when there is not serial port.
> +Very often it is necessary to debug very close to the reset vector or in
> production systems that lack serial ports, seven segment displays, or useful
> LED that are typically used to output useful debug messages.
> 
> +The BeepDebugFeaturePkg includes some useful beep focused debug
> libraries.
> +
> +This isn't intended for production use.
> +
> +There is not currently seamless integration into the SecCore component
> that handles the reset vector. In order to debug that early, it will be
> necessary to use the BeepLib directly in SEC code.
> 
>  # High-Level Theory of Operation
> -It provide a library BeepStatusCodeHandlerLib used by edk2
> StatusCodeHandler.efi, used to do beep if needed.
> -It also provide a library of BeepMap lib, it map the status code to beep
> value.
> +It provides a library, BeepStatusCodeHandlerLib, used by edk2
> StatusCodeHandler.efi, used to do beep if needed.
> +It also provide a library of BeepMap lib which maps the status code to a
> beep value.
>  A library of Beep lib is needed by platform, and this pkg has a Null
> implementation.
> 
> -In the library contstructor function, BeepStatusCodeHandlerLib register the
> call back function for ReportStatusCode.
> -When called, it call GetBeepFromStatusCode() in BeepMapLib to get beep
> value from status code, and call Beep() in BeepLib to beep.
> +In the library contstructor function, BeepStatusCodeHandlerLib registers
> the call back function for ReportStatusCode. When called, it calls
> GetBeepFromStatusCode (); in BeepMapLib to get beep value from status
> code, and calls Beep () in BeepLib to beep a speaker.
> 
> -BeepStatusCodeHandlerLib include 3 libraries for PEI, RuntimeDxe, SMM:
> +BeepStatusCodeHandlerLib includes three libraries for PEI, RuntimeDxe,
> and SMM:
>  * PeiBeepStatusCodeHandlerLib
>  * RuntimeDxeBeepStatusCodeHandlerLib
>  * SmmBeepStatusCodeHandlerLib
> 
>  ## Firmware Volumes
> -Linked with StatusCodeHandler.efi, and make sure put the
> StatusCodeHandler.efi after the ReportStatusCodeRouter.efi.
> +These libraries need to be linked into StatusCodeHandler components.
> +Make sure one puts the StatusCodeHandler.efi after the
> ReportStatusCodeRouter.efi.
> 
>  ## Modules
>  * BeepStatusCodeHandlerLib
> @@ -32,13 +36,17 @@ Linked with StatusCodeHandler.efi, and make sure
> put the StatusCodeHandler.efi a
>  * BeepLibNull
> 
>  ## BeepStatusCodeHandlerLib
> -This library register the call back function for ReportStatusCode, and get
> beep valude from status code, and do beep.
> +This library registers the callback function for ReportStatusCode, gets beep
> value from status code, and does the beep.
> 
>  ## BeepMapLib
> -This library provide a function to get beep value from status code.
> +This library provides a function to get a beep value for a status code.
> +
> +## BeepLibNull
> +This library provide a function to perform the beep.
> 
>  ## Key Functions
>  * In PeiBeepStatusCodeHandlerLib:
> +```
>    EFI_STATUS
>    EFIAPI
>    BeepStatusCodeReportWorker (
> @@ -49,8 +57,10 @@ This library provide a function to get beep value from
> status code.
>      IN CONST EFI_GUID                 *CallerId,
>      IN CONST EFI_STATUS_CODE_DATA     *Data OPTIONAL
>    )
> +```
> 
>  * In RuntimeDxeBeepStatusCodeHandlerLib:
> +```
>    EFI_STATUS
>    EFIAPI
>    BeepStatusCodeReportWorker (
> @@ -60,8 +70,10 @@ This library provide a function to get beep value from
> status code.
>      IN EFI_GUID                       *CallerId,
>      IN EFI_STATUS_CODE_DATA           *Data OPTIONAL
>    )
> +```
> 
>  * In SmmBeepStatusCodeHandlerLib:
> +```
>    EFI_STATUS
>    EFIAPI
>    BeepStatusCodeReportWorker (
> @@ -71,55 +83,72 @@ This library provide a function to get beep value from
> status code.
>      IN EFI_GUID                       *CallerId,
>      IN EFI_STATUS_CODE_DATA           *Data OPTIONAL
>      )
> +```
> 
>  * In BeepMapLib:
> +```
>    UINT32
>    EFIAPI
>    GetBeepValueFromStatusCode (
>      IN EFI_STATUS_CODE_TYPE           CodeType,
>      IN EFI_STATUS_CODE_VALUE          Value
> -  )
> +    )
> +```
> 
>  * In BeepLib:
> +```
>    VOID
>    EFIAPI
>    Beep (
>      IN UINT32  Value
> -  )
> +    )
> +```
> 
>  ## Configuration
> -* Link the library to StatusCodeHandler.efi.
> -  Example:
> -
> MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHan
> dlerRuntimeDxe.inf {
> -    <LibraryClasses>
> -
> NULL|BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/RuntimeD
> xeBeepStatusCodeHandlerLib.inf
> -    }
> -  Refer to BeepDebugFeature.dsc for other example.
> -* Config PCD
> gBeepDebugFeaturePkgTokenSpaceGuid.PcdStatusCodeUseBeep.
> -  In platform .dsc file, need to config the type of
> gBeepDebugFeaturePkgTokenSpaceGuid.PcdStatusCodeUseBeep.
> -  Use PcdsFixedAtBuild to save binary size, and use PcdsDynamic if want to
> enable/disable in runtime.
> -* Implemented platform's special BeepMapLib if needed.
> -* Provide the platform's special BeepLib.
> -* Make sure put the StatusCodeHandler.efi after the
> ReportStatusCodeRouter.efi.
> +* Configure PCD
> gBeepDebugFeaturePkgTokenSpaceGuid.PcdStatusCodeUseBeep.
> +  In board DSC file, the board developer needs to configure the type of
> gBeepDebugFeaturePkgTokenSpaceGuid.PcdStatusCodeUseBeep control
> desired.
> +  [PcdsFixedAtBuild] is the feature default value as this has lowest size.
> +  [PcdsDynamicExDefault] is the most common configuration as it provides
> dynamic control during debugging.
> +* Implemented board specific BeepMapLib if custom status code to beep
> code mapping as needed.
> +* Provide the board specific BeepLib to perform beeps on the board specific
> hardware.
> +```The default library does not cause any hardware to beep```
> 
>  ## Data Flows
>  Status Code (ReportStatusCode) -> Beep Value
> (GetBeepValueFromStatusCode).
> 
>  ## Control Flows
> -ReportStatusCode() -> BeepStatusCodeReportWorker() ->
> GetBeepValueFromStatusCode() -> Beep()
> +ReportStatusCode () -> BeepStatusCodeReportWorker () ->
> +GetBeepValueFromStatusCode () -> Beep ()
> 
>  ## Build Flows
> -There is not special build flows.
> +Standalone build
> +* build -a IA32 -a X64 -p
> +Debugging\BeepDebugFeaturePkg\BeepDebugFeaturePkg.dsc
> 
> +AdvanceFeaturePkg build
> +* build -a IA32 -a X64 -p AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
>  ## Test Point Results
> -Verify the post code shown is correct.
> +None
> 
>  ## Functional Exit Criteria
>  N/A
> 
>  ## Feature Enabling Checklist
> -* Set the PCD
> gBeepDebugFeaturePkgTokenSpaceGuid.PcdStatusCodeUseBeep to TRUE.
> -* Plug out all the memory, check can here the beep.
> +* Verify in board DSC file that
> +gBeepDebugFeaturePkgTokenSpaceGuid.PcdBeepDebugFeatureEnable set
> to
> +TRUE
> +* Verify board specific BeepLib implemented and included in board DSC file.
> +* Verify that the board has a PlatformHookLib instance.  There is a
> +null library implementation if needed ```
> +
> +PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePl
> atfo
> +rmHookLibNull.inf
> +```
> +* Verify that your board has the StatusCodeHandler components (PEIM or
> driver) desired.
> +```
> +  Example:
> +
> MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHan
> dlerRuntimeDxe.inf {
> +    <LibraryClasses>
> +
> NULL|BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/RuntimeD
> xeBeepStatusCodeHandlerLib.inf
> +    }
> +  There are default StatusCodeHandlers for PEI, RT, and SMM in
> BeepDebugFeaturePkg/Include in PreMemory.fdf and PostMemory.fdf for
> use.  But most boards will already have these components and you will just
> want to add the appropriate *StatusCodeHandlerLib.inf to each component.
> +```
> +* Build
> +* Remove all the memory from the system and verify audible beep is heard
> when attempting to boot.
> 
>  ## Common Optimizations
> -* Implemented platform's special BeepMapLib if needed.
> +N/A
> --
> 2.27.0.windows.1
> 
> 
> 
> 
>