Re: [PATCH 4/4] ArmPlatformPkg: add PL011 UART runtime debug driver

[Leif Lindholm <leif.lindholm@linaro.org>]

On Thu, Mar 01, 2018 at 06:11:42PM +0000, Ard Biesheuvel wrote:
> Implement the new runtime debug output protocol on top of a PL011 UART.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

LGTM
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.c   | 144 ++++++++++++++++++++
>  ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.inf |  62 +++++++++
>  2 files changed, 206 insertions(+)
> 
> diff --git a/ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.c b/ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.c
> new file mode 100644
> index 000000000000..155b2c50d463
> --- /dev/null
> +++ b/ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.c
> @@ -0,0 +1,144 @@
> +/** @file
> +  Runtime driver to produce debug output on a PL011 UART
> +
> +  Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <PiDxe.h>
> +#include <Library/DxeServicesTableLib.h>
> +#include <Library/PL011UartLib.h>
> +#include <Library/UefiDriverEntryPoint.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiRuntimeLib.h>
> +#include <Protocol/RuntimeDebugOutput.h>
> +
> +STATIC UINTN        mUartBase;
> +STATIC EFI_EVENT    mVirtualAddressChangeEvent;
> +
> +/**
> +  Write data from buffer to debug output device
> +
> +  Writes NumberOfBytes data bytes from Buffer to the debug output device.
> +  The number of bytes actually written to the device is returned.
> +  If the return value is less than NumberOfBytes, then the write operation
> +  failed.
> +  If NumberOfBytes is zero, then return 0.
> +
> +  @param  Buffer           Pointer to the data buffer to be written.
> +  @param  NumberOfBytes    Number of bytes to written to the device.
> +
> +  @retval 0                NumberOfBytes is 0.
> +  @retval >0               The number of bytes written to the serial device.
> +                           If this value is less than NumberOfBytes, then the
> +                           write operation failed.
> +
> +**/
> +STATIC
> +UINTN
> +PL011RuntimeDebugOutputWrite (
> +  IN  EDK2_RUNTIME_DEBUG_OUTPUT_PROTOCOL  *This,
> +  IN  UINT8                               *Buffer,
> +  IN  UINTN                               NumberOfBytes
> +  )
> +{
> +  return PL011UartWrite (mUartBase, Buffer, NumberOfBytes);
> +}
> +
> +STATIC EDK2_RUNTIME_DEBUG_OUTPUT_PROTOCOL mRuntimeDebugOutput = {
> +  PL011RuntimeDebugOutputWrite
> +};
> +
> +/**
> +  Fixup internal data so that EFI can be called in virtual mode.
> +
> +  @param[in]    Event   The Event that is being processed
> +  @param[in]    Context Event Context
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +VirtualNotifyEvent (
> +  IN EFI_EVENT        Event,
> +  IN VOID             *Context
> +  )
> +{
> +  EfiConvertPointer (0x0, (VOID **)&mUartBase);
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +PL011RuntimeDebugOutputDxeEntry (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  EFI_STATUS          Status;
> +  EFI_HANDLE          Handle;
> +  UINT64              BaudRate;
> +  UINT32              ReceiveFifoDepth;
> +  EFI_PARITY_TYPE     Parity;
> +  UINT8               DataBits;
> +  EFI_STOP_BITS_TYPE  StopBits;
> +
> +  mUartBase        = (UINTN)FixedPcdGet64 (PcdSerialRegisterBase);
> +  BaudRate         = FixedPcdGet64 (PcdUartDefaultBaudRate);
> +  ReceiveFifoDepth = 0;         // Use default FIFO depth
> +  Parity           = (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity);
> +  DataBits         = FixedPcdGet8 (PcdUartDefaultDataBits);
> +  StopBits         = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);
> +
> +  Status = PL011UartInitializePort (mUartBase, FixedPcdGet32 (PL011UartClkInHz),
> +             &BaudRate, &ReceiveFifoDepth, &Parity, &DataBits, &StopBits);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  //
> +  // Register for the virtual address change event
> +  //
> +  Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_NOTIFY,
> +                  VirtualNotifyEvent, NULL, &gEfiEventVirtualAddressChangeGuid,
> +                  &mVirtualAddressChangeEvent);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  //
> +  // Declare the UART MMIO region as EFI_MEMORY_RUNTIME
> +  //
> +  Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, mUartBase,
> +                  SIZE_4KB, EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
> +  if (EFI_ERROR (Status)) {
> +    goto CloseEvent;
> +  }
> +
> +  Status = gDS->SetMemorySpaceAttributes (mUartBase, SIZE_4KB,
> +                  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
> +  if (EFI_ERROR (Status)) {
> +    goto CloseEvent;
> +  }
> +
> +  Handle = NULL;
> +  Status = gBS->InstallMultipleProtocolInterfaces (&Handle,
> +                  &gEdkiiRuntimeDebugOutputProtocolGuid, &mRuntimeDebugOutput,
> +                  NULL);
> +  if (EFI_ERROR (Status)) {
> +    goto CloseEvent;
> +  }
> +
> +  return EFI_SUCCESS;
> +
> +CloseEvent:
> +  gBS->CloseEvent (mVirtualAddressChangeEvent);
> +
> +  return Status;
> +}
> diff --git a/ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.inf b/ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.inf
> new file mode 100644
> index 000000000000..28a8e514552e
> --- /dev/null
> +++ b/ArmPlatformPkg/Drivers/PL011RuntimeDebugOutputDxe/PL011RuntimeDebugOutputDxe.inf
> @@ -0,0 +1,62 @@
> +#/** @file
> +#  Runtime driver to produce debug output on a PL011 UART
> +#
> +#  Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> +#
> +#  This program and the accompanying materials are licensed and made available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution. The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      = PL011RuntimeDebugOutputDxe
> +  FILE_GUID                      = 494297ca-9205-463a-aae5-215ffd067cbb
> +  MODULE_TYPE                    = DXE_RUNTIME_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = PL011RuntimeDebugOutputDxeEntry
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +#  VALID_ARCHITECTURES           = AARCH64 ARM
> +#
> +
> +[Sources.common]
> +  PL011RuntimeDebugOutputDxe.c
> +
> +[Packages]
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  DxeServicesTableLib
> +  PcdLib
> +  PL011UartLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +  UefiRuntimeLib
> +
> +[Guids]
> +  gEfiEventVirtualAddressChangeGuid             ## CONSUMES # Event
> +
> +[Protocols]
> +  gEdkiiRuntimeDebugOutputProtocolGuid          ## PROTOCOL ALWAYS_PRODUCED
> +
> +[FixedPcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
> +  gArmPlatformTokenSpaceGuid.PL011UartClkInHz
> +
> +[Depex]
> +  TRUE
> -- 
> 2.11.0
>