Re: [PATCH] ArmPkg: Enable boot discovery policy for ARM package.

["Ard Biesheuvel" <ardb@kernel.org>]

On Fri, 6 Aug 2021 at 10:30, Grzegorz Bernacki <gjb@semihalf.com> wrote:
>
> This commit adds code which check BootDiscoveryPolicy variable and
> calls Boot Policy Manager Protocol to connect device specified by
> the variable. To enable that mechanism for platform
> EfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy PCD must be
> added to DSC file and BootDiscoveryPolicyUiLib should be added to
> UiApp libraries.
>

... or the platform will be broken once we apply this patch, right? If
so, please propose patches for all platforms in edk2-platforms that
use this library - we can't just break them.

> Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
> ---
>  ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  5 +
>  ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c               | 96 +++++++++++++++++++-
>  2 files changed, 100 insertions(+), 1 deletion(-)
>
> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 353d7a967b..86751b45f8 100644
> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -65,11 +65,15 @@
>
>  [Pcd]
>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy
>
>  [Guids]
> +  gBootDiscoveryPolicyMgrFormsetGuid
>    gEdkiiNonDiscoverableEhciDeviceGuid
>    gEdkiiNonDiscoverableUhciDeviceGuid
>    gEdkiiNonDiscoverableXhciDeviceGuid
> +  gEfiBootManagerPolicyNetworkGuid
> +  gEfiBootManagerPolicyConnectAllGuid
>    gEfiFileInfoGuid
>    gEfiFileSystemInfoGuid
>    gEfiFileSystemVolumeLabelInfoIdGuid
> @@ -79,6 +83,7 @@
>
>  [Protocols]
>    gEdkiiNonDiscoverableDeviceProtocolGuid
> +  gEfiBootManagerPolicyProtocolGuid
>    gEfiDevicePathProtocolGuid
>    gEfiGraphicsOutputProtocolGuid
>    gEfiLoadedImageProtocolGuid
> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> index 5ceb23d822..4332c45bb7 100644
> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -2,9 +2,10 @@
>    Implementation for PlatformBootManagerLib library class interfaces.
>
>    Copyright (C) 2015-2016, Red Hat, Inc.
> -  Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2021, ARM Ltd. All rights reserved.<BR>
>    Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
>    Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> +  Copyright (c) 2021, Semihalf All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -19,6 +20,7 @@
>  #include <Library/UefiBootManagerLib.h>
>  #include <Library/UefiLib.h>
>  #include <Library/UefiRuntimeServicesTableLib.h>
> +#include <Protocol/BootManagerPolicy.h>
>  #include <Protocol/DevicePath.h>
>  #include <Protocol/EsrtManagement.h>
>  #include <Protocol/GraphicsOutput.h>
> @@ -27,6 +29,7 @@
>  #include <Protocol/PciIo.h>
>  #include <Protocol/PciRootBridgeIo.h>
>  #include <Protocol/PlatformBootManager.h>
> +#include <Guid/BootDiscoveryPolicy.h>
>  #include <Guid/EventGroup.h>
>  #include <Guid/NonDiscoverableDevice.h>
>  #include <Guid/TtyTerm.h>
> @@ -703,6 +706,91 @@ HandleCapsules (
>
>  #define VERSION_STRING_PREFIX    L"Tianocore/EDK2 firmware version "
>
> +/**
> +  This functions checks the value of BootDiscoverPolicy variable and
> +  connect devices of class specified by that variable. Then it refreshes
> +  Boot order for newly discovered boot device.
> +
> +  @retval  EFI_SUCCESS  Devices connected succesfully or connection
> +                        not required.
> +  @retval  others       Return values from GetVariable(), LocateProtocol()
> +                        and ConnectDeviceClass().
> +--*/
> +STATIC
> +EFI_STATUS
> +BootDiscoveryPolicyHandler (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                       Status;
> +  UINT32                           DiscoveryPolicy;
> +  UINTN                            Size;
> +  EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy;
> +  EFI_GUID                         *Class;
> +
> +  Size = sizeof (DiscoveryPolicy);
> +  Status = gRT->GetVariable (
> +                  BOOT_DISCOVERY_POLICY_VAR,
> +                  &gBootDiscoveryPolicyMgrFormsetGuid,
> +                  NULL,
> +                  &Size,
> +                  &DiscoveryPolicy
> +                  );
> +  if (Status == EFI_NOT_FOUND) {
> +    Status = PcdSet32S (PcdBootDiscoveryPolicy, PcdGet32 (PcdBootDiscoveryPolicy));
> +    if (Status == EFI_NOT_FOUND) {
> +      return EFI_SUCCESS;
> +    } else if (EFI_ERROR (Status)) {
> +      return Status;
> +    }
> +    DiscoveryPolicy = PcdGet32 (PcdBootDiscoveryPolicy);
> +  } else if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  if (DiscoveryPolicy == BDP_CONNECT_MINIMAL) {
> +    return EFI_SUCCESS;
> +  }
> +
> +  switch (DiscoveryPolicy) {
> +    case BDP_CONNECT_NET:
> +      Class = &gEfiBootManagerPolicyNetworkGuid;
> +      break;
> +    case BDP_CONNECT_ALL:
> +      Class = &gEfiBootManagerPolicyConnectAllGuid;
> +      break;
> +    default:
> +      DEBUG ((
> +        DEBUG_INFO,
> +        "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discovery Policy\n",
> +        __FUNCTION__,
> +        DiscoveryPolicy
> +        ));
> +      return EFI_SUCCESS;
> +  }
> +
> +  Status = gBS->LocateProtocol (
> +                  &gEfiBootManagerPolicyProtocolGuid,
> +                  NULL,
> +                  (VOID **)&BMPolicy
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_INFO, "%a - Failed to locate gEfiBootManagerPolicyProtocolGuid."
> +      "Driver connect will be skipped.\n", __FUNCTION__));
> +    return Status;
> +  }
> +
> +  Status = BMPolicy->ConnectDeviceClass (BMPolicy, Class);
> +  if (EFI_ERROR (Status)){
> +    DEBUG ((DEBUG_ERROR, "%a - ConnectDeviceClass returns - %r\n", __FUNCTION__, Status));
> +    return Status;
> +  }
> +
> +  EfiBootManagerRefreshAllBootOption();
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>    Do the platform specific action after the console is ready
>    Possible things that can be done in PlatformBootManagerAfterConsole:
> @@ -753,6 +841,12 @@ PlatformBootManagerAfterConsole (
>      }
>    }
>
> +  //
> +  // Connect device specified by BootDiscoverPolicy variable and
> +  // refresh Boot order for newly discovered boot devices
> +  //
> +  BootDiscoveryPolicyHandler ();
> +
>    //
>    // On ARM, there is currently no reason to use the phased capsule
>    // update approach where some capsules are dispatched before EndOfDxe
> --
> 2.25.1
>