Re: [PATCH v3 3/7] OvmfPkg/PlatformBds: Implement PlatformBootManagerUnableToBoot

[Laszlo Ersek <lersek@redhat.com>]

On 07/03/18 08:37, Ruiyu Ni wrote:
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien.grall@linaro.org>
> ---
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 61 +++++++++++++++++++++-
>  1 file changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index 57870cb856..e56ffc141a 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Platform BDS customizations.
>  
> -  Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2004 - 2018, Intel Corporation. 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
> @@ -1676,3 +1676,62 @@ PlatformBootManagerWaitCallback (
>      );
>  }
>  
> +/**
> +  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 attempts to enter an infinite loop.
> +**/
> +VOID
> +EFIAPI
> +PlatformBootManagerUnableToBoot (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                   Status;
> +  EFI_INPUT_KEY                Key;
> +  EFI_BOOT_MANAGER_LOAD_OPTION BootManagerMenu;
> +  UINTN                        Index;
> +
> +  //
> +  // BootManagerMenu doesn't contain the correct information when return status is EFI_NOT_FOUND.
> +  //

Before you commit this patch, can you please rewrap this long comment, like this:

  //
  // BootManagerMenu doesn't contain the correct information when return status
  // is EFI_NOT_FOUND.
  //

With that:

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

Thank you!
Laszlo

> +  Status = EfiBootManagerGetBootManagerMenu (&BootManagerMenu);
> +  if (EFI_ERROR (Status)) {
> +    return;
> +  }
> +  //
> +  // Normally BdsDxe does not print anything to the system console, but this is
> +  // a last resort -- the end-user will likely not see any DEBUG messages
> +  // logged in this situation.
> +  //
> +  // AsciiPrint() will NULL-check gST->ConOut internally. We check gST->ConIn
> +  // here to see if it makes sense to request and wait for a keypress.
> +  //
> +  if (gST->ConIn != NULL) {
> +    AsciiPrint (
> +      "%a: No bootable option or device was found.\n"
> +      "%a: Press any key to enter the Boot Manager Menu.\n",
> +      gEfiCallerBaseName,
> +      gEfiCallerBaseName
> +      );
> +    Status = gBS->WaitForEvent (1, &gST->ConIn->WaitForKey, &Index);
> +    ASSERT_EFI_ERROR (Status);
> +    ASSERT (Index == 0);
> +
> +    //
> +    // Drain any queued keys.
> +    //
> +    while (!EFI_ERROR (gST->ConIn->ReadKeyStroke (gST->ConIn, &Key))) {
> +      //
> +      // just throw away Key
> +      //
> +    }
> +  }
> +
> +  for (;;) {
> +    EfiBootManagerBoot (&BootManagerMenu);
> +  }
> +}
> +
>