Re: [edk2-devel] [PATCH 3/6] SimicsOpenBoardPkg: Enable serial terminal

["Ni, Ray" <ray.ni@intel.com>]

Sending to correct address of Zhiguang.

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Wednesday, May 24, 2023 8:29 PM
> To: devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zhiguang Liu
> <zhiguang@liu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [edk2-devel] [PATCH 3/6] SimicsOpenBoardPkg: Enable serial
> terminal
> 
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Zhiguang Liu <zhiguang@liu@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  .../BoardX58Ich10/OpenBoardPkg.fdf            |  1 -
>  .../BoardX58Ich10/OpenBoardPkgPcd.dsc         |  1 +
>  .../Library/BoardBdsHookLib/BoardBdsHookLib.c | 56 ++++++++-----------
>  3 files changed, 24 insertions(+), 34 deletions(-)
> 
> diff --git
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
> index 6ecff76362..aa44c01784 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
> @@ -198,7 +198,6 @@ INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
>  INF  MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
> 
>  INF
> MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDx
> e.inf
> 
>  INF  UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
> 
> -INF  MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
> 
>  INF  MdeModulePkg/Universal/PrintDxe/PrintDxe.inf
> 
>  INF  MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
> 
>  INF  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
> 
> diff --git
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.ds
> c
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.ds
> c
> index 732d95e44f..b6672d1a78 100644
> ---
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.ds
> c
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.ds
> c
> @@ -69,6 +69,7 @@
>    ######################################
> 
>    gNetworkFeaturePkgTokenSpaceGuid.PcdNetworkFeatureEnable|TRUE
> 
>    gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable|TRUE
> 
> +  gMinPlatformPkgTokenSpaceGuid.PcdSerialTerminalEnable|TRUE
> 
> 
> 
>  [PcdsFeatureFlag.X64]
> 
>    ######################################
> 
> diff --git
> a/Platform/Intel/SimicsOpenBoardPkg/Library/BoardBdsHookLib/BoardBdsH
> ookLib.c
> b/Platform/Intel/SimicsOpenBoardPkg/Library/BoardBdsHookLib/BoardBdsH
> ookLib.c
> index 3c6385c12f..238513f666 100644
> ---
> a/Platform/Intel/SimicsOpenBoardPkg/Library/BoardBdsHookLib/BoardBdsH
> ookLib.c
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/Library/BoardBdsHookLib/BoardBdsH
> ookLib.c
> @@ -837,45 +837,35 @@ PlatformInitializeConsole (
>    )
> 
>  {
> 
>    UINTN                              Index;
> 
> -  EFI_DEVICE_PATH_PROTOCOL           *VarConout;
> 
> -  EFI_DEVICE_PATH_PROTOCOL           *VarConin;
> 
> 
> 
>    //
> 
> -  // Connect RootBridge
> 
> +  // Do platform specific PCI Device check and add them to ConOut, ConIn,
> ErrOut
> 
> +  // Note: Why perform the ConIn/ConOut/ErrOut variables update without
> checking
> 
> +  //          whether "ConIn", "ConOut" or "ErrOut" is present in the variable
> store?
> 
> +  //       Because SerialPortTerminalLib.constructor() adds the serial terminal
> device
> 
> +  //          to "ConIn", "ConOut" and "ErrOut" variables always, checking
> presence of
> 
> +  //          the three variables will lead to the following update logic never runs.
> 
>    //
> 
> -  GetEfiGlobalVariable2 (EFI_CON_OUT_VARIABLE_NAME, (VOID **)
> &VarConout, NULL);
> 
> -  GetEfiGlobalVariable2 (EFI_CON_IN_VARIABLE_NAME, (VOID **)
> &VarConin, NULL);
> 
> -
> 
> -  if (VarConout == NULL || VarConin == NULL) {
> 
> -    //
> 
> -    // Do platform specific PCI Device check and add them to ConOut, ConIn,
> ErrOut
> 
> -    //
> 
> -    DetectAndPreparePlatformPciDevicePaths (FALSE);
> 
> -    DetectAndPreparePlatformPciDevicePaths(TRUE);
> 
> +  DetectAndPreparePlatformPciDevicePaths (FALSE);
> 
> +  DetectAndPreparePlatformPciDevicePaths(TRUE);
> 
> +  //
> 
> +  // Have chance to connect the platform default console,
> 
> +  // the platform default console is the minimue device group
> 
> +  // the platform should support
> 
> +  //
> 
> +  for (Index = 0; PlatformConsole[Index].DevicePath != NULL; ++Index) {
> 
>      //
> 
> -    // Have chance to connect the platform default console,
> 
> -    // the platform default console is the minimue device group
> 
> -    // the platform should support
> 
> +    // Update the console variable with the connect type
> 
>      //
> 
> -    for (Index = 0; PlatformConsole[Index].DevicePath != NULL; ++Index) {
> 
> -      //
> 
> -      // Update the console variable with the connect type
> 
> -      //
> 
> -      if ((PlatformConsole[Index].ConnectType & CONSOLE_IN) ==
> CONSOLE_IN) {
> 
> -        EfiBootManagerUpdateConsoleVariable (ConIn,
> PlatformConsole[Index].DevicePath, NULL);
> 
> -      }
> 
> -      if ((PlatformConsole[Index].ConnectType & CONSOLE_OUT) ==
> CONSOLE_OUT) {
> 
> -        EfiBootManagerUpdateConsoleVariable (ConOut,
> PlatformConsole[Index].DevicePath, NULL);
> 
> -      }
> 
> -      if ((PlatformConsole[Index].ConnectType & STD_ERROR) == STD_ERROR) {
> 
> -        EfiBootManagerUpdateConsoleVariable (ErrOut,
> PlatformConsole[Index].DevicePath, NULL);
> 
> -      }
> 
> +    if ((PlatformConsole[Index].ConnectType & CONSOLE_IN) == CONSOLE_IN)
> {
> 
> +      EfiBootManagerUpdateConsoleVariable (ConIn,
> PlatformConsole[Index].DevicePath, NULL);
> 
> +    }
> 
> +    if ((PlatformConsole[Index].ConnectType & CONSOLE_OUT) ==
> CONSOLE_OUT) {
> 
> +      EfiBootManagerUpdateConsoleVariable (ConOut,
> PlatformConsole[Index].DevicePath, NULL);
> 
> +    }
> 
> +    if ((PlatformConsole[Index].ConnectType & STD_ERROR) == STD_ERROR) {
> 
> +      EfiBootManagerUpdateConsoleVariable (ErrOut,
> PlatformConsole[Index].DevicePath, NULL);
> 
>      }
> 
> -  } else {
> 
> -    //
> 
> -    // Only detect VGA device and add them to ConOut
> 
> -    //
> 
> -    DetectAndPreparePlatformPciDevicePaths (TRUE);
> 
>    }
> 
>  }
> 
> 
> 
> --
> 2.39.1.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#105232):
> https://edk2.groups.io/g/devel/message/105232
> Mute This Topic: https://groups.io/mt/99107258/1712937
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe:
> https://edk2.groups.io/g/devel/leave/3759105/1712937/893644498/xyzzy
> [ray.ni@intel.com]
> -=-=-=-=-=-=
>