Re: [Patch V2 1/2] AdvancedFeaturePkg: Add SerialPortTerminalLib

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

Hey Heng,

SerialDebugFeaturePkg does not seem to describe this feature very well. This feature's function is routing ConIn/ConOut to the serial port, it has no impact on the debug log. I think SerialTerminalFeaturePkg would be a more appropriate name. Please see additional feedback inline.

On 10/23/20, 1:14 AM, Luo, Heng <heng.luo@intel.com> wrote:

> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3014
> 
> Add the serial port terminal library to get console redirect
> after the shell is loaded.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> 
> Signed-off-by: Heng Luo <heng.luo@intel.com>
> ---
>  Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc                                               |  4 +++-
>  Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeatures.dsc                                         |  3 +++
>  Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc                                      |  2 ++
>  Features/Intel/AdvancedFeaturePkg/Include/PostMemory.fdf                                               |  4 +++-
>  Features/Intel/AdvancedFeaturePkg/Include/PreMemory.fdf                                                |  4 +++-
>  Features/Intel/Debugging/SerialDebugFeaturePkg/Include/PostMemory.fdf                                  |  8 ++++++++
>  Features/Intel/Debugging/SerialDebugFeaturePkg/Include/PreMemory.fdf                                   |  8 ++++++++
>  Features/Intel/Debugging/SerialDebugFeaturePkg/Include/SerialDebugFeature.dsc                          | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  Features/Intel/Debugging/SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.c   | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  Features/Intel/Debugging/SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.h   | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  Features/Intel/Debugging/SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.inf | 35 +++++++++++++++++++++++++++++++++++
>  Features/Intel/Debugging/SerialDebugFeaturePkg/Readme.md                                               | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  Features/Intel/Debugging/SerialDebugFeaturePkg/SerialDebugFeaturePkg.dec                               | 31 +++++++++++++++++++++++++++++++
>  Features/Intel/Debugging/SerialDebugFeaturePkg/SerialDebugFeaturePkg.dsc                               | 31 +++++++++++++++++++++++++++++++
>  14 files changed, 438 insertions(+), 3 deletions(-)
> 
> diff --git a/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc b/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
> index ced0a97c37..28b2d82d26 100644
> --- a/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
> +++ b/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
> @@ -30,7 +30,8 @@
>    PEI_ARCH                            = IA32
>    DXE_ARCH                            = X64
>  
> -!include AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> +!include AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> +
>  
>  ################################################################################
>  #
> @@ -45,6 +46,7 @@
>    gS3FeaturePkgTokenSpaceGuid.PcdS3FeatureEnable                          |TRUE
>    gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable                  |TRUE
>    gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable            |TRUE
> +  gSerialDebugFeaturePkgTokenSpaceGuid.PcdSerialDebugFeatureEnable        |TRUE

Please rename to PcdSerialTerminalFeatureEnable

>    gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable    |TRUE
>    gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable                      |TRUE
>    gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable                              |FALSE
> diff --git a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeatures.dsc b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeatures.dsc
> index ac9ab80e8e..5f7b9ef33d 100644
> --- a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeatures.dsc
> +++ b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeatures.dsc
> @@ -19,6 +19,9 @@
>  !if gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable == TRUE
>    !include Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFeature.dsc
>  !endif
> +!if gSerialDebugFeaturePkgTokenSpaceGuid.PcdSerialDebugFeatureEnable == TRUE
> +  !include SerialDebugFeaturePkg/Include/SerialDebugFeature.dsc
> +!endif
>  
>  #
>  # Network Advanced Features
> diff --git a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> index ad248de800..07abd34d8f 100644
> --- a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> +++ b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> @@ -18,6 +18,7 @@
>    MdePkg/MdePkg.dec
>    AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dec
>    Usb3DebugFeaturePkg/Usb3DebugFeaturePkg.dec
> +  SerialDebugFeaturePkg/SerialDebugFeaturePkg.dec
>    NetworkFeaturePkg/NetworkFeaturePkg.dec
>    IpmiFeaturePkg/IpmiFeaturePkg.dec
>    S3FeaturePkg/S3FeaturePkg.dec
> @@ -38,6 +39,7 @@
>    gS3FeaturePkgTokenSpaceGuid.PcdS3FeatureEnable                          |FALSE
>    gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable                  |FALSE
>    gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable            |FALSE
> +  gSerialDebugFeaturePkgTokenSpaceGuid.PcdSerialDebugFeatureEnable        |FALSE
>    gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable    |FALSE
>    gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable                      |FALSE
>    gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable                              |FALSE
> diff --git a/Features/Intel/AdvancedFeaturePkg/Include/PostMemory.fdf b/Features/Intel/AdvancedFeaturePkg/Include/PostMemory.fdf
> index c49a0fd875..baa3b4f299 100644
> --- a/Features/Intel/AdvancedFeaturePkg/Include/PostMemory.fdf
> +++ b/Features/Intel/AdvancedFeaturePkg/Include/PostMemory.fdf
> @@ -20,7 +20,9 @@
>  !if gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable == TRUE
>    !include Debugging/Usb3DebugFeaturePkg/Include/PostMemory.fdf
>  !endif
> -
> +!if gSerialDebugFeaturePkgTokenSpaceGuid.PcdSerialDebugFeatureEnable == TRUE
> +  !include SerialDebugFeaturePkg/Include/PostMemory.fdf
> +!endif
>  #
>  # Network Advanced Features
>  #
> diff --git a/Features/Intel/AdvancedFeaturePkg/Include/PreMemory.fdf b/Features/Intel/AdvancedFeaturePkg/Include/PreMemory.fdf
> index 1b21e96a5f..1b56cdbd6e 100644
> --- a/Features/Intel/AdvancedFeaturePkg/Include/PreMemory.fdf
> +++ b/Features/Intel/AdvancedFeaturePkg/Include/PreMemory.fdf
> @@ -20,7 +20,9 @@
>  !if gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable == TRUE
>    !include Debugging/Usb3DebugFeaturePkg/Include/PreMemory.fdf
>  !endif
> -
> +!if gSerialDebugFeaturePkgTokenSpaceGuid.PcdSerialDebugFeatureEnable == TRUE
> +  !include SerialDebugFeaturePkg/Include/PreMemory.fdf
> +!endif
>  #
>  # Network Advanced Features
>  #
> diff --git a/Features/Intel/Debugging/SerialDebugFeaturePkg/Include/PostMemory.fdf b/Features/Intel/Debugging/SerialDebugFeaturePkg/Include/PostMemory.fdf
> new file mode 100644
> index 0000000000..202918511f
> --- /dev/null
> +++ b/Features/Intel/Debugging/SerialDebugFeaturePkg/Include/PostMemory.fdf
> @@ -0,0 +1,8 @@
> +## @file
> +#  FDF file for post-memory modules that enable Serial Debug

Change Serial Debug to Serial Terminal

> +#
> +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##

Please add the following drivers here:

INF  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
INF  MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf

> diff --git a/Features/Intel/Debugging/SerialDebugFeaturePkg/Include/PreMemory.fdf b/Features/Intel/Debugging/SerialDebugFeaturePkg/Include/PreMemory.fdf
> new file mode 100644
> index 0000000000..111bace4fc
> --- /dev/null
> +++ b/Features/Intel/Debugging/SerialDebugFeaturePkg/Include/PreMemory.fdf
> @@ -0,0 +1,8 @@
> +## @file
> +#  FDF file for pre-memory modules that enable Serial Debug.

Change Serial Debug to Serial Terminal

> +#
> +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> diff --git a/Features/Intel/Debugging/SerialDebugFeaturePkg/Include/SerialDebugFeature.dsc b/Features/Intel/Debugging/SerialDebugFeaturePkg/Include/SerialDebugFeature.dsc
> new file mode 100644
> index 0000000000..abed99ff8d
> --- /dev/null
> +++ b/Features/Intel/Debugging/SerialDebugFeaturePkg/Include/SerialDebugFeature.dsc

Please rename to SerialTerminalFeature.dsc

> @@ -0,0 +1,72 @@
> +## @file
> +# This is a build description file for the Serial Debug advanced feature.
> +# This file should be included into another package DSC file to build this feature.
> +#
> +# The DEC files are used by the utilities that parse DSC and
> +# INF files to generate AutoGen.c and AutoGen.h files
> +# for the build infrastructure.
> +#
> +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +################################################################################
> +#
> +# Defines Section - statements that will be processed to create a Makefile.
> +#
> +################################################################################
> +[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
> +
> +################################################################################
> +#
> +# Library Class section - list of all Library Classes needed by this feature.
> +#
> +################################################################################
> +[LibraryClasses]
> +  #######################################
> +  # Edk2 Packages
> +  #######################################
> +  #BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> +  #BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf

Remove commented out code.

> +  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> +  DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> +  UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
> +  UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
> +  UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +
> +#
> +# Feature DXE Components
> +#
> +[Components.X64]

This should be: [Components.$(DXE_ARCH)]

> +  #####################################
> +  # Serial Debug Feature Package
> +  #####################################
> +
> +  # Add library instances here that are not included in package components and should be tested
> +  # in the package build.
> +  SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.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]
> diff --git a/Features/Intel/Debugging/SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.c b/Features/Intel/Debugging/SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.c
> new file mode 100644
> index 0000000000..36c0e7110f
> --- /dev/null
> +++ b/Features/Intel/Debugging/SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.c
> @@ -0,0 +1,93 @@
> +/** @file
> +  Main file for NULL named library for Serial Port Deviceboard controller librarr.
> +
> +  INTEL CONFIDENTIAL

Remove INTEL CONFIDENTIAL

> +  Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include "SerialPortTerminalLib.h"
> +
> +GLOBAL_REMOVE_IF_UNREFERENCED SERIAL_DEVICE_PATH mSerialDevicePath = {
> +  {
> +    {
> +      HARDWARE_DEVICE_PATH,
> +      HW_VENDOR_DP,
> +      {
> +        (UINT8) sizeof (VENDOR_DEVICE_PATH),
> +        (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8)
> +      }
> +    },
> +    EDKII_SERIAL_PORT_LIB_VENDOR_GUID
> +  },
> +  {
> +    {
> +      MESSAGING_DEVICE_PATH,
> +      MSG_UART_DP,
> +      {
> +        (UINT8) sizeof (UART_DEVICE_PATH),
> +        (UINT8) ((sizeof (UART_DEVICE_PATH)) >> 8)
> +      }
> +    },
> +    0,                  // Reserved
> +    115200,             // BaudRate
> +    8,                  // DataBits
> +    1,                  // Parity
> +    1                   // StopBits

Please make PCDs to configure these values. Make sure to read the PCDs in SerialPortTerminalLibConstructor() not here in the global variable definition so that the PCDs can be made either Dynamic or DynamicEx type.

> +  },
> +  {
> +    {
> +      MESSAGING_DEVICE_PATH,
> +      MSG_VENDOR_DP,
> +      {
> +        (UINT8) (sizeof (VENDOR_DEVICE_PATH)),
> +        (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8),
> +      }
> +    },
> +    DEVICE_PATH_MESSAGING_PC_ANSI
> +  },
> +  gEndEntire
> +};
> +
> +/**
> +  Updates the ConOut, ConIn, ErrOut variables with the serial terminal device path
> +  @param                        none
> +  @retval                       none
> +**/
> +VOID
> +AddSerialTerminal (
> +  VOID
> +  )
> +{
> +  DEBUG ((DEBUG_INFO, "[AddSerialPortTerminal]\n"));
> +
> +  //
> +  // Append Serial Terminal into "ConIn"
> +  //
> +  EfiBootManagerUpdateConsoleVariable (ConOut, (EFI_DEVICE_PATH_PROTOCOL *) &mSerialDevicePath, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ConIn, (EFI_DEVICE_PATH_PROTOCOL *) &mSerialDevicePath, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ErrOut, (EFI_DEVICE_PATH_PROTOCOL *) &mSerialDevicePath, NULL);
> +}
> +
> +
> +/**
> +  Constructor for the Serial Port Device controller library.
> +
> +  @param ImageHandle    the image handle of the process
> +  @param SystemTable    the EFI System Table pointer
> +
> +  @retval EFI_SUCCESS        the shell command handlers were installed sucessfully
> +  @retval EFI_UNSUPPORTED    the shell level required was not found.
> +**/
> +EFI_STATUS
> +EFIAPI
> +SerialPortTerminalLibConstructor (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  AddSerialTerminal();
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/Features/Intel/Debugging/SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.h b/Features/Intel/Debugging/SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.h
> new file mode 100644
> index 0000000000..afb3eb2cb3
> --- /dev/null
> +++ b/Features/Intel/Debugging/SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.h
> @@ -0,0 +1,49 @@
> +/** @file
> +  Header file for NULL named library for Ps2 keyboard controller library.
> +
> +  INTEL CONFIDENTIAL

Remove INTEL CONFIDENTIAL

> +  Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef _PS2_KBC_LIB_H
> +#define _PS2_KBC_LIB_H

Should be _SERIAL_PORT_TERMINAL_LIB_H_

> +
> +#include <Uefi.h>
> +#include <Guid/SerialPortLibVendor.h>
> +#include <Library/UefiLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/UefiBootManagerLib.h>
> +
> +//
> +// Below is the platform console device path
> +//
> +typedef struct {
> +  VENDOR_DEVICE_PATH        Guid;
> +  UART_DEVICE_PATH          Uart;
> +  VENDOR_DEVICE_PATH        TerminalType;
> +  EFI_DEVICE_PATH_PROTOCOL  End;
> +} SERIAL_DEVICE_PATH;
> +
> +#define gPciRootBridge \
> +  { \
> +    { \
> +      ACPI_DEVICE_PATH, \
> +      ACPI_DP, \
> +      { \
> +        (UINT8) (sizeof (ACPI_HID_DEVICE_PATH)), \
> +        (UINT8) ((sizeof (ACPI_HID_DEVICE_PATH)) >> 8) \
> +      }, \
> +    }, \
> +    EISA_PNP_ID (0x0A03), \
> +    0 \
> +  }
> +
> +#define gEndEntire \
> +  { \
> +    END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE, { END_DEVICE_PATH_LENGTH, 0 } \
> +  }
> +
> +#endif
> diff --git a/Features/Intel/Debugging/SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.inf b/Features/Intel/Debugging/SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.inf
> new file mode 100644
> index 0000000000..4f63d1626f
> --- /dev/null
> +++ b/Features/Intel/Debugging/SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.inf
> @@ -0,0 +1,35 @@
> +## @file
> +# Component information file for Serial Port Terminal Redirection Library
> +#
> +# INTEL CONFIDENTIAL
> +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +[Defines]
> +  INF_VERSION                    = 0x00010006
> +  BASE_NAME                      = SerialPortTerminalLib
> +  FILE_GUID                      = E12BFA46-95F2-4ADC-9774-7E38DE78741E
> +  MODULE_TYPE                    = UEFI_DRIVER
> +  VERSION_STRING                 = 1.2
> +  LIBRARY_CLASS                  = NULL|UEFI_DRIVER

This should be: LIBRARY_CLASS                  = NULL|UEFI_DRIVER DXE_DRIVER DXE_RUNTIME_DRIVER

> +  CONSTRUCTOR                    = SerialPortTerminalLibConstructor
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  BoardModulePkg/BoardModulePkg.dec
> +
> +[Sources]
> +  SerialPortTerminalLib.c
> +  SerialPortTerminalLib.h
> +
> +[LibraryClasses]
> +  DevicePathLib
> +  DebugLib
> +  UefiDriverEntryPoint
> +  UefiBootManagerLib
> +  UefiLib
> +
> +[Pcd]
> diff --git a/Features/Intel/Debugging/SerialDebugFeaturePkg/Readme.md b/Features/Intel/Debugging/SerialDebugFeaturePkg/Readme.md
> new file mode 100644
> index 0000000000..5745be0f3e
> --- /dev/null
> +++ b/Features/Intel/Debugging/SerialDebugFeaturePkg/Readme.md
> @@ -0,0 +1,97 @@
> +# Overview
> +* **Feature Name:** Serial Debug
> +* **PI Phase(s) Supported:** DXE
> +* **SMM Required?** No
> +
> +## Purpose
> +The Serial Debug feature enables EDK II compliant firmware to use a serial port on the system for firmware debug.

This package should be just about the serial terminal, not the more general topic of serial debug. Accordingly, this should mention terminal and ConIn/ConOut not debug.

> +
> +SerialPortTerminalLib.inf provider NULL named library for Serial Port Terminal Redirection Library, it can be used by BdsDxe to redirect the console to serial port after the shell is loaded.
> +
> +# High-Level Theory of Operation
> +
> +Not available.
> +
> +## Firmware Volumes
> +
> +* N/A
> +
> +## Modules
> +
> +A bulleted list of the modules that make up the feature.
> +
> +* SerialPortTerminalLib
> +
> +## SerialPortTerminalLib
> +
> +This library can be used by BdsDxe to redirect the console to serial port.
> +
> +## Key Functions
> +
> +* Updates the ConOut, ConIn, ErrOut variables with the serial terminal device path.
> +
> +## Configuration
> +
> +* Add this library to BdsDxe driver, to add the serial device to ConIn and ConOut variables.
> +
> +## Data Flows
> +
> +Architecturally defined data structures and flows for the feature.
> +
> +### Data flows of SerialPortTerminalLib
> +
> +N/A
> +
> +## Control Flows
> +
> +```txt
> +| BdsDxe |                      | SerialPortTerminalLib |
> +------------                     -----------------------
> +      |                                      |
> +      |---SerialPortTerminalLibConstructor-->|---------
> +      |                                      |         |
> +      |                                      | AddSerialTerminal
> +      |                                      |         |
> +      |                                      |<--------
> +```
> +
> +## Build Flows
> +
> +No any special build flows is needed.
> +
> +## Test Point Results
> +
> +Not available.
> +
> +## Functional Exit Criteria
> +
> +Not available.
> +
> +## Feature Enabling Checklist
> +
> +1. Add this library to BdsDxe driver in OpenBoardPkg.dsc under section [Components.X64], to add the serial device to ConIn and ConOut variables:
> +
> +```dsc
> +MdeModulePkg/Universal/BdsDxe/BdsDxe.inf {
> +<LibraryClasses>
> +  NULL|SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.inf
> +}
> +```
> +
> +2. Include these 2 drivers to the OpenBoardPkg.dsc:
> +
> +```dsc
> + MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> + MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
> +```
> +
> +3. Include these 2 drivers to OpenBoardPkg.fdf under section [FV.FvUefiBootUncompact]:
> +
> +```fdf
> +INF  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> +INF  MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
> +```
> +
> +## Performance Impact
> +
> +Not available.
> diff --git a/Features/Intel/Debugging/SerialDebugFeaturePkg/SerialDebugFeaturePkg.dec b/Features/Intel/Debugging/SerialDebugFeaturePkg/SerialDebugFeaturePkg.dec
> new file mode 100644
> index 0000000000..1ad36565d0
> --- /dev/null
> +++ b/Features/Intel/Debugging/SerialDebugFeaturePkg/SerialDebugFeaturePkg.dec
> @@ -0,0 +1,31 @@
> +## @file
> +# This package provides advanced feature functionality for Serial Debug support.
> +# This package should only depend on EDK II Core packages, IntelSiliconPkg, and MinPlatformPkg.
> +#
> +# The DEC files are used by the utilities that parse DSC and
> +# INF files to generate AutoGen.c and AutoGen.h files
> +# for the build infrastructure.
> +#
> +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  DEC_SPECIFICATION = 0x00010017
> +  PACKAGE_NAME      = SerialDebugFeaturePkg
> +  PACKAGE_GUID      = A4BF76C6-6190-4B2C-B1C8-D872F60629B2
> +  PACKAGE_VERSION   = 0.1
> +
> +[Includes]
> +  Include
> +
> +[Guids]
> +  gSerialDebugFeaturePkgTokenSpaceGuid  =  {0xebda878b, 0xa5f1, 0x4b0f, {0x89, 0x2d, 0x47, 0x56, 0x97, 0xd4, 0x1a, 0x05}}
> +
> +[PcdsFeatureFlag]
> +  gSerialDebugFeaturePkgTokenSpaceGuid.PcdSerialDebugFeatureEnable|FALSE|BOOLEAN|0xA0000001
> +
> +
> +
> diff --git a/Features/Intel/Debugging/SerialDebugFeaturePkg/SerialDebugFeaturePkg.dsc b/Features/Intel/Debugging/SerialDebugFeaturePkg/SerialDebugFeaturePkg.dsc
> new file mode 100644
> index 0000000000..4247566a14
> --- /dev/null
> +++ b/Features/Intel/Debugging/SerialDebugFeaturePkg/SerialDebugFeaturePkg.dsc
> @@ -0,0 +1,31 @@
> +## @file
> +# This package provides advanced feature functionality for Serial Debug support.
> +# This package should only depend on EDK II Core packages, IntelSiliconPkg, and MinPlatformPkg.
> +#
> +# The DEC files are used by the utilities that parse DSC and
> +# INF files to generate AutoGen.c and AutoGen.h files
> +# for the build infrastructure.
> +#
> +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  PLATFORM_NAME                  = SerialDebugFeaturePkg
> +  PLATFORM_GUID                  = 378D0A55-A83D-4B99-B85D-D78703A28783
> +  PLATFORM_VERSION               = 0.1
> +  DSC_SPECIFICATION              = 0x00010005
> +  OUTPUT_DIRECTORY               = Build/$(PLATFORM_NAME)
> +  SUPPORTED_ARCHITECTURES        = IA32|X64
> +  BUILD_TARGETS                  = DEBUG|RELEASE|NOOPT
> +  SKUID_IDENTIFIER               = DEFAULT
> +  PEI_ARCH                       = IA32
> +  DXE_ARCH                       = X64
> +
> +
> +#
> +# This package always builds the feature.
> +#
> +!include Include/SerialDebugFeature.dsc
> -- 
> 2.24.0.windows.2

Thanks,
Nate