Re: [PATCH v2 1/2] MdeModulePkg/UefiBootManagerLib: new APIs for ultimate boot failure

[Laszlo Ersek <lersek@redhat.com>]

Hi Ray,

thanks for this patch; I only have a request for updating a comment:

On 06/29/18 08:03, Ruiyu Ni wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=982
> 
> When no boot option could be launched including platform recovery
> options and options pointing to applications built into firmware
> volumes, a platform callback registered through
> EfiBootManagerRegisterUnableToBootHandler() is called.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Sunny Wang <sunnywang@hpe.com>
> ---
>  MdeModulePkg/Include/Library/UefiBootManagerLib.h | 48 +++++++++++++++++++++++
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c  | 41 +++++++++++++++++++
>  2 files changed, 89 insertions(+)
> 
> diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> index bfc0cb86f8..453893e030 100644
> --- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> +++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> @@ -800,4 +800,52 @@ EFIAPI
>  EfiBootManagerDispatchDeferredImages (
>    VOID
>    );
> +
> +/**
> +  The function is called when no boot option could be launched,
> +  including platform recovery options and options pointing to applications
> +  built into firmware volumes.
> +
> +  The platform may register this function to inform the user about the above fact.
> +  If this function returns, BDS core attempts to enter an infinite loop of pulling
> +  up the Boot Manager Menu (if the platform includes the Boot Manager Menu).
> +  If the Boot Manager Menu is unavailable, BDS will hang.
> +**/
> +typedef
> +VOID
> +(EFIAPI *EFI_BOOT_MANAGER_UNABLE_TO_BOOT) (
> +  VOID
> +  );
> +
> +/**
> +  Register the callback function when no boot option could be launched,
> +  including platform recovery options and options pointing to applications
> +  built into firmware volumes.
> +
> +  @param Handler  The callback function.
> +**/
> +VOID
> +EFIAPI
> +EfiBootManagerRegisterUnableToBootHandler (
> +  EFI_BOOT_MANAGER_UNABLE_TO_BOOT  Handler
> +  );
> +
> +/**
> +  The function is called when no boot option could be launched,
> +  including platform recovery options and options pointing to applications
> +  built into firmware volumes.
> +
> +  If this function returns, BDS core attempts to enter an infinite loop of pulling
> +  up the Boot Manager Menu (if the platform includes the Boot Manager Menu).
> +  If the Boot Manager Menu is unavailable, BDS will hang.

This comment paragraph should state the following:

  If this function returns EFI_UNSUPPORTED, BDS core attempts to enter
  an infinite loop of pulling up the Boot Manager Menu (if the platform
  includes the Boot Manager Menu). If the Boot Manager Menu is
  unavailable, BDS will hang.

  If this function returns EFI_SUCCESS, BDS will hang immediately.


The same update should be done to the "BmBoot.c" file, below.

With this update:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

> +
> +  @retval EFI_SUCCESS     The unable-to-boot callback is called successfully.
> +  @retval EFI_UNSUPPORTED There is no unable-to-boot callback registered.
> +**/
> +EFI_STATUS
> +EFIAPI
> +EfiBootManagerUnableToBoot (
> +  VOID
> +  );
> +
>  #endif
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 6a23477eb8..e59d790122 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -19,6 +19,7 @@ EFI_RAM_DISK_PROTOCOL                        *mRamDisk                  = NULL;
>  
>  EFI_BOOT_MANAGER_REFRESH_LEGACY_BOOT_OPTION  mBmRefreshLegacyBootOption = NULL;
>  EFI_BOOT_MANAGER_LEGACY_BOOT                 mBmLegacyBoot              = NULL;
> +EFI_BOOT_MANAGER_UNABLE_TO_BOOT              mBmUnableToBoot            = NULL;
>  
>  ///
>  /// This GUID is used for an EFI Variable that stores the front device pathes
> @@ -2461,3 +2462,43 @@ EfiBootManagerGetBootManagerMenu (
>    }
>  }
>  
> +/**
> +  Register the callback function when no boot option could be launched,
> +  including platform recovery options and options pointing to applications
> +  built into firmware volumes.
> +
> +  @param Handler  The callback function.
> +**/
> +VOID
> +EFIAPI
> +EfiBootManagerRegisterUnableToBootHandler (
> +  EFI_BOOT_MANAGER_UNABLE_TO_BOOT  Handler
> +  )
> +{
> +  mBmUnableToBoot = Handler;
> +}
> +
> +/**
> +  The function is called when no boot option could be launched,
> +  including platform recovery options and options pointing to applications
> +  built into firmware volumes.
> +
> +  If this function returns, BDS core attempts to enter an infinite loop of pulling
> +  up the Boot Manager Menu (if the platform includes the Boot Manager Menu).
> +  If the Boot Manager Menu is unavailable, BDS will hang.
> +
> +  @retval EFI_SUCCESS     The unable-to-boot callback is called successfully.
> +  @retval EFI_UNSUPPORTED There is no unable-to-boot callback registered.
> +**/
> +EFI_STATUS
> +EFIAPI
> +EfiBootManagerUnableToBoot (
> +  VOID
> +  )
> +{
> +  if (mBmUnableToBoot != NULL) {
> +    mBmUnableToBoot ();
> +    return EFI_SUCCESS;
> +  }
> +  return EFI_UNSUPPORTED;
> +}
>