Re: [PATCH v3 10/15] ArmVirtPkg: 16550 UART Platform hook library

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 6/24/20 3:34 PM, Sami Mujawar wrote:
> The BaseSerialPort16550 library invokes the
> PlatformHookSerialPortInitialize() implemented as
> part of the PlatformHook library, to perform platform
> specific initialization required to enable use of the
> 16550 device. The BaseSerialPort16550 library uses
> the PcdSerialRegisterBase to obtain the base address
> of the UART for MMIO operations.
> 
> Some VMMs like Kvmtool provide the base address of
> the console serial port in the platform device tree.
> 
> This patch introduces two instances of the Platform
> Hook library:
> 1. EarlyFdt16550SerialPortHookLib - parses the
>     platform device tree to extract the base
>     address of the 16550 UART and update the PCD
>     PcdSerialRegisterBase.
> 2. Fdt16550SerialPortHookLib - reads the GUID
>     Hob gEarly16550UartBaseAddressGuid (that caches
>     the base address of the 16550 UART discovered
>     during early stages) and updates the PCD
>     PcdSerialRegisterBase.
> 
> Note:
>    a. The PCD PcdSerialRegisterBase is configured
>       as PatchableInModule.
>    b. A separate patch introduces a PlatformPeiLib
>       that trampolines the 16550 UART base address
>       from the Pcd PcdSerialRegisterBase to the
>       GUID Hob gEarly16550UartBaseAddressGuid.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

> ---
>   ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.c   | 138 ++++++++++++++++++++
>   ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf |  36 +++++
>   ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.c        |  57 ++++++++
>   ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf      |  39 ++++++
>   ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.uni      |  14 ++
>   5 files changed, 284 insertions(+)
> 
> diff --git a/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.c b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..4e97302d01dc5692e046fb901663373828b532ee
> --- /dev/null
> +++ b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.c
> @@ -0,0 +1,138 @@
> +/** @file
> +  Early Platform Hook Library instance for 16550 Uart.
> +
> +  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Base.h>
> +#include <Uefi.h>
> +
> +#include <Pi/PiBootMode.h>
> +#include <Pi/PiHob.h>
> +
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/PlatformHookLib.h>
> +#include <libfdt.h>
> +
> +/** Get the UART base address of the console serial-port from the DT.
> +
> +  This function fetches the node referenced in the "stdout-path"
> +  property of the "chosen" node and returns the base address of
> +  the console UART.
> +
> +  @param [in]   Fdt                   Pointer to a Flattened Device Tree (Fdt).
> +  @param [out]  SerialConsoleAddress  If success, contains the base address
> +                                      of the console serial-port.
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_NOT_FOUND           Console serial-port info not found in DT.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GetSerialConsolePortAddress (
> +  IN  CONST VOID    *Fdt,
> +  OUT       UINT64  *SerialConsoleAddress
> +  )
> +{
> +  CONST CHAR8   *Prop;
> +  INT32         PropSize;
> +  CONST CHAR8   *Path;
> +  INT32         PathLen;
> +  INT32         ChosenNode;
> +  INT32         SerialConsoleNode;
> +  INT32         Len;
> +  CONST CHAR8   *NodeStatus;
> +  CONST UINT64  *RegProperty;
> +
> +  if ((Fdt == NULL) || (fdt_check_header (Fdt) != 0)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // The "chosen" node resides at the the root of the DT. Fetch it.
> +  ChosenNode = fdt_path_offset (Fdt, "/chosen");
> +  if (ChosenNode < 0) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  Prop = fdt_getprop (Fdt, ChosenNode, "stdout-path", &PropSize);
> +  if (PropSize < 0) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  // Determine the actual path length, as a colon terminates the path.
> +  Path = ScanMem8 (Prop, ':', PropSize);
> +  if (Path == NULL) {
> +    PathLen = AsciiStrLen (Prop);
> +  } else {
> +    PathLen = Path - Prop;
> +  }
> +
> +  // Aliases cannot start with a '/', so it must be the actual path.
> +  if (Prop[0] == '/') {
> +    SerialConsoleNode = fdt_path_offset_namelen (Fdt, Prop, PathLen);
> +  } else {
> +    // Lookup the alias, as this contains the actual path.
> +    Path = fdt_get_alias_namelen (Fdt, Prop, PathLen);
> +    if (Path == NULL) {
> +      return EFI_NOT_FOUND;
> +    }
> +    SerialConsoleNode = fdt_path_offset (Fdt, Path);
> +  }
> +
> +  NodeStatus = fdt_getprop (Fdt, SerialConsoleNode, "status", &Len);
> +  if ((NodeStatus != NULL) && (AsciiStrCmp (NodeStatus, "okay") != 0)) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  RegProperty = fdt_getprop (Fdt, SerialConsoleNode, "reg", &Len);
> +  if (Len != 16) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *SerialConsoleAddress = fdt64_to_cpu (ReadUnaligned64 (RegProperty));
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/** Platform hook to retrieve the 16550 UART base address from the platform
> +    Device tree and store it in PcdSerialRegisterBase.
> +
> +  @retval RETURN_SUCCESS            Success.
> +  @retval RETURN_INVALID_PARAMETER  A parameter was invalid.
> +  @retval RETURN_NOT_FOUND          Serial port information not found.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +PlatformHookSerialPortInitialize (
> +  VOID
> +  )
> +{
> +  RETURN_STATUS   Status;
> +  VOID            *DeviceTreeBase;
> +  UINT64          SerialConsoleAddress;
> +
> +  if (PcdGet64 (PcdSerialRegisterBase) != 0) {
> +    return RETURN_SUCCESS;
> +  }
> +
> +  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
> +  if (DeviceTreeBase == NULL) {
> +    return RETURN_NOT_FOUND;
> +  }
> +
> +  Status = GetSerialConsolePortAddress (DeviceTreeBase, &SerialConsoleAddress);
> +  if (RETURN_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  return (EFI_STATUS)PcdSet64S (PcdSerialRegisterBase, SerialConsoleAddress);
> +}
> +
> diff --git a/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf
> new file mode 100644
> index 0000000000000000000000000000000000000000..007a45eca2a622118044b527a5bc6403b706608a
> --- /dev/null
> +++ b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.inf
> @@ -0,0 +1,36 @@
> +## @file
> +#  Early Platform Hook Library instance for 16550 Uart.
> +#
> +#  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001B
> +  BASE_NAME                      = EarlyFdt16550SerialPortHookLib
> +  MODULE_UNI_FILE                = Fdt16550SerialPortHookLib.uni
> +  FILE_GUID                      = FFB19961-79CC-4684-84A8-C31B0A2BBE82
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = PlatformHookLib|SEC PEI_CORE PEIM
> +
> +[Sources]
> +  EarlyFdt16550SerialPortHookLib.c
> +
> +[LibraryClasses]
> +  BaseLib
> +  PcdLib
> +  FdtLib
> +  HobLib
> +
> +[Packages]
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[Pcd]
> +  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
> diff --git a/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.c b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..1100c12aef361e39cf78f0c3e4e14cc2feecfcee
> --- /dev/null
> +++ b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.c
> @@ -0,0 +1,57 @@
> +/** @file
> +  Platform Hook Library instance for 16550 Uart.
> +
> +  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Base.h>
> +#include <Uefi.h>
> +
> +#include <Pi/PiBootMode.h>
> +#include <Pi/PiHob.h>
> +
> +#include <Guid/Early16550UartBaseAddress.h>
> +#include <Guid/Fdt.h>
> +#include <Guid/FdtHob.h>
> +
> +#include <Library/BaseLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/PlatformHookLib.h>
> +
> +/** Platform hook to retrieve the 16550 UART base address from the GUID Hob
> +    that caches the UART base address from early boot stage and store it in
> +    PcdSerialRegisterBase.
> +
> +  @retval RETURN_SUCCESS    Success.
> +  @retval RETURN_NOT_FOUND  Serial Port information not found.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +PlatformHookSerialPortInitialize (
> +  VOID
> +  )
> +{
> +  VOID            *Hob;
> +  UINT64          *UartBase;
> +
> +  if (PcdGet64 (PcdSerialRegisterBase) != 0) {
> +    return RETURN_SUCCESS;
> +  }
> +
> +  Hob = GetFirstGuidHob (&gEarly16550UartBaseAddressGuid);
> +  if ((Hob == NULL) || (GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (*UartBase))) {
> +    return RETURN_NOT_FOUND;
> +  }
> +
> +  UartBase = GET_GUID_HOB_DATA (Hob);
> +  if ((UINTN)*UartBase == 0) {
> +    return RETURN_NOT_FOUND;
> +  }
> +
> +  return (RETURN_STATUS)PcdSet64S (PcdSerialRegisterBase, (UINTN)*UartBase);
> +}
> +
> diff --git a/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
> new file mode 100644
> index 0000000000000000000000000000000000000000..ba789ca69041b647a4a881b03110ff853df89f65
> --- /dev/null
> +++ b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
> @@ -0,0 +1,39 @@
> +## @file
> +#  Platform Hook Library instance for 16550 Uart.
> +#
> +#  Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001B
> +  BASE_NAME                      = Fdt16550SerialPortHookLib
> +  MODULE_UNI_FILE                = Fdt16550SerialPortHookLib.uni
> +  FILE_GUID                      = C6DFD3F0-179D-4376-89A5-F641A2E7EFB5
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = PlatformHookLib|DXE_CORE DXE_DRIVER UEFI_DRIVER DXE_RUNTIME_DRIVER UEFI_APPLICATION
> +  CONSTRUCTOR                    = PlatformHookSerialPortInitialize
> +
> +[Sources]
> +  Fdt16550SerialPortHookLib.c
> +
> +[LibraryClasses]
> +  BaseLib
> +  PcdLib
> +  HobLib
> +
> +[Packages]
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
> +
> +[Guids]
> +  gEarly16550UartBaseAddressGuid
> +
> diff --git a/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.uni b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.uni
> new file mode 100644
> index 0000000000000000000000000000000000000000..19a61a4a03244321bda7c88cd26128b44bfd46b4
> --- /dev/null
> +++ b/ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.uni
> @@ -0,0 +1,14 @@
> +// /** @file
> +// Platform Hook Library instance for 16550 Uart.
> +//
> +//
> +// Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
> +//
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +// **/
> +
> +#string STR_MODULE_ABSTRACT     #language en-US "Platform Hook Library instance for 16550 Uart."
> +
> +#string STR_MODULE_DESCRIPTION  #language en-US "Platform Hook Library instance for 16550 Uart."
> +
>