Re: [PATCH v2 10/12] ArmVirtPkg/FdtClientDxe: install DT as sysconfig table in protocol notify

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 17 March 2017 at 20:47, Laszlo Ersek <lersek@redhat.com> wrote:
> Replace the dependency on PcdPureAcpiBoot with a Platform Has Device Tree
> notification callback. Move the sysconfig table installation from the
> entry point function to the callback.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |   5 +-
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 105 ++++++++++++++++++--
>  2 files changed, 98 insertions(+), 12 deletions(-)
>
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> index 3a0cd37040eb..99dcbde59497 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> @@ -25,6 +25,7 @@ [Sources]
>    FdtClientDxe.c
>
>  [Packages]
> +  ArmPkg/ArmPkg.dec
>    ArmVirtPkg/ArmVirtPkg.dec
>    EmbeddedPkg/EmbeddedPkg.dec
>    MdeModulePkg/MdeModulePkg.dec
> @@ -39,14 +40,12 @@ [LibraryClasses]
>    UefiDriverEntryPoint
>
>  [Protocols]
> +  gEdkiiPlatformHasDeviceTreeProtocolGuid ## CONSUMES
>    gFdtClientProtocolGuid                  ## PRODUCES
>
>  [Guids]
>    gFdtHobGuid
>    gFdtTableGuid
>
> -[FeaturePcd]
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
> -
>  [Depex]
>    TRUE
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> index 547a29fce62c..9dda0d005d61 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> @@ -24,6 +24,7 @@
>  #include <Guid/FdtHob.h>
>
>  #include <Protocol/FdtClient.h>
> +#include <Protocol/PlatformHasDeviceTree.h>
>
>  STATIC VOID  *mDeviceTreeBase;
>
> @@ -306,6 +307,40 @@ STATIC FDT_CLIENT_PROTOCOL mFdtClientProtocol = {
>    GetOrInsertChosenNode,
>  };
>
> +STATIC
> +VOID
> +EFIAPI
> +OnPlatformHasDeviceTree (
> +  IN EFI_EVENT Event,
> +  IN VOID      *Context
> +  )
> +{
> +  EFI_STATUS Status;
> +  VOID       *Interface;
> +  VOID       *DeviceTreeBase;
> +
> +  Status = gBS->LocateProtocol (
> +                  &gEdkiiPlatformHasDeviceTreeProtocolGuid,
> +                  NULL,                                     // Registration
> +                  &Interface
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    return;
> +  }
> +
> +  DeviceTreeBase = Context;
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "%a: exposing DTB @ 0x%p to OS\n",
> +    __FUNCTION__,
> +    DeviceTreeBase
> +    ));
> +  Status = gBS->InstallConfigurationTable (&gFdtTableGuid, DeviceTreeBase);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  gBS->CloseEvent (Event);
> +}
> +
>  EFI_STATUS
>  EFIAPI
>  InitializeFdtClientDxe (
> @@ -316,6 +351,8 @@ InitializeFdtClientDxe (
>    VOID              *Hob;
>    VOID              *DeviceTreeBase;
>    EFI_STATUS        Status;
> +  EFI_EVENT         PlatformHasDeviceTreeEvent;
> +  VOID              *Registration;
>
>    Hob = GetFirstGuidHob (&gFdtHobGuid);
>    if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
> @@ -333,15 +370,65 @@ InitializeFdtClientDxe (
>
>    DEBUG ((EFI_D_INFO, "%a: DTB @ 0x%p\n", __FUNCTION__, mDeviceTreeBase));
>
> -  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> -    //
> -    // Only install the FDT as a configuration table if we want to leave it up
> -    // to the OS to decide whether it prefers ACPI over DT.
> -    //
> -    Status = gBS->InstallConfigurationTable (&gFdtTableGuid, DeviceTreeBase);
> -    ASSERT_EFI_ERROR (Status);
> +  //
> +  // Register a protocol notify for the EDKII Platform Has Device Tree
> +  // Protocol.
> +  //
> +  Status = gBS->CreateEvent (
> +                  EVT_NOTIFY_SIGNAL,
> +                  TPL_CALLBACK,
> +                  OnPlatformHasDeviceTree,
> +                  DeviceTreeBase,             // Context
> +                  &PlatformHasDeviceTreeEvent
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: CreateEvent(): %r\n", __FUNCTION__, Status));
> +    return Status;
>    }
>
> -  return gBS->InstallProtocolInterface (&ImageHandle, &gFdtClientProtocolGuid,
> -                EFI_NATIVE_INTERFACE, &mFdtClientProtocol);
> +  Status = gBS->RegisterProtocolNotify (
> +                  &gEdkiiPlatformHasDeviceTreeProtocolGuid,
> +                  PlatformHasDeviceTreeEvent,
> +                  &Registration
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: RegisterProtocolNotify(): %r\n",
> +      __FUNCTION__,
> +      Status
> +      ));
> +    goto CloseEvent;
> +  }
> +
> +  //
> +  // Kick the event; the protocol could be available already.
> +  //
> +  Status = gBS->SignalEvent (PlatformHasDeviceTreeEvent);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: SignalEvent(): %r\n", __FUNCTION__, Status));
> +    goto CloseEvent;
> +  }
> +
> +  Status = gBS->InstallProtocolInterface (
> +                  &ImageHandle,
> +                  &gFdtClientProtocolGuid,
> +                  EFI_NATIVE_INTERFACE,
> +                  &mFdtClientProtocol
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: InstallProtocolInterface(): %r\n",
> +      __FUNCTION__,
> +      Status
> +      ));
> +    goto CloseEvent;
> +  }
> +
> +  return Status;
> +
> +CloseEvent:
> +  gBS->CloseEvent (PlatformHasDeviceTreeEvent);
> +  return Status;
>  }
> --
> 2.9.3
>
>