Re: [Patch V2 2/2] UpXtreme: console redirect after the shell is loaded

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

Hey Heng,

Please see 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
> 
> Use SerialPortTerminalLib to get console redirect after the shell is
> loaded:
> 1. Add SerialPortTerminalLib to BdsDxe driver, to add the serial device to
>  ConIn and ConOut variables
> 2. Include SerialDxe and TerminalDxe to the Up Xtreme's OpenBoardPkg.dsc
>  and OpenBoardPkg.fdf
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Signed-off-by: Heng Luo <heng.luo@intel.com>
> ---
>  Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc | 9 ++++++++-
>  Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf | 2 ++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc
> index fb493973e2..4c472cf9ee 100644
> --- a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc
> +++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc
> @@ -39,7 +39,8 @@
>    #
>    # Include PCD configuration for this board.
>    #
> -  !include AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> +  !include AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> +
>    !include OpenBoardPkgPcd.dsc
>    !include AdvancedFeaturePkg/Include/AdvancedFeatures.dsc
>  
> @@ -82,6 +83,12 @@
>  [Components.X64]
>  !include $(PLATFORM_PACKAGE)/Include/Dsc/CoreDxeInclude.dsc
>  !include $(PLATFORM_SI_PACKAGE)/SiPkgDxe.dsc
> +MdeModulePkg/Universal/BdsDxe/BdsDxe.inf{
> +<LibraryClasses>
> +  NULL|SerialDebugFeaturePkg/Library/SerialPortTerminalLib/SerialPortTerminalLib.inf

Should only be included if PcdSerialTerminalFeatureEnable == TRUE

> +}
> +MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> +MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
>  
>  #######################################
>  # Build Option Includes
> diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
> index 0d99114961..e15cb7dfa9 100644
> --- a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
> +++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
> @@ -342,6 +342,8 @@ INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
>  INF  MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
>  INF  MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
>  INF  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> +INF  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> +INF  MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf

It would be better to put this into Features/Intel/Debugging/SerialTerminalFeaturePkg/Include/PostMemory.fdf
Then you don't need to modify the .fdf file for every BoardPkg to enable this advanced feature.

>  INF  MdeModulePkg/Universal/Console/GraphicsOutputDxe/GraphicsOutputDxe.inf
>  INF  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
>  INF  BoardModulePkg/LegacySioDxe/LegacySioDxe.inf
> -- 
> 2.24.0.windows.2

Thanks,
Nate