Re: [PATCH v2 2/2] MdeModulePkg/BdsDxe: Call *UnableToBoot() for ultimate boot failure

[Laszlo Ersek <lersek@redhat.com>]

Hi Ray,

again I'm only requesting a comment update:

On 06/29/18 08:03, Ruiyu Ni wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=982
> 
> The patch changes the default core behavior of ultimate boot failure
> to launch Boot Manager Menu.
> 
> 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/Universal/BdsDxe/BdsEntry.c | 65 +++++++-------------------------
>  1 file changed, 13 insertions(+), 52 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> index 49e403e181..db2a4cf208 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> @@ -634,55 +634,6 @@ BdsFormalizeEfiGlobalVariable (
>    BdsFormalizeOSIndicationVariable ();
>  }
>  
> -/**
> -  Enter an infinite loop of calling the Boot Manager Menu.
> -
> -  This is a last resort alternative to BdsEntry() giving up for good. This
> -  function never returns.
> -
> -  @param[in] BootManagerMenu  The EFI_BOOT_MANAGER_LOAD_OPTION located and/or
> -                              created by the EfiBootManagerGetBootManagerMenu()
> -                              call in BdsEntry().
> -**/
> -VOID
> -BdsBootManagerMenuLoop (
> -  IN EFI_BOOT_MANAGER_LOAD_OPTION *BootManagerMenu
> -  )
> -{
> -  EFI_INPUT_KEY Key;
> -
> -  //
> -  // 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
> -      );
> -    BdsWaitForSingleEvent (gST->ConIn->WaitForKey, 0);
> -
> -    //
> -    // Drain any queued keys.
> -    //
> -    while (!EFI_ERROR (gST->ConIn->ReadKeyStroke (gST->ConIn, &Key))) {
> -      //
> -      // just throw away Key
> -      //
> -    }
> -  }
> -
> -  for (;;) {
> -    EfiBootManagerBoot (BootManagerMenu);
> -  }
> -}
> -
>  /**
>  
>    Service routine for BdsInstance->Entry(). Devices are connected, the
> @@ -1088,10 +1039,20 @@ BdsEntry (
>    }
>  
>    //
> -  // If BootManagerMenu is available, fall back to it indefinitely.
> +  // Inform the platform that we're unable to boot. The platform may enter
> +  // BootManagerMenu with the public EfiBootManagerBoot() interface, if so
> +  // desired.

Please replace

  with the public EfiBootManagerBoot() interface

with

  with the EfiBootManagerGetBootManagerMenu() and EfiBootManagerBoot()
  interfaces

With that update:

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

Thank you!
Laszlo

>    //
> -  if (BootManagerMenuStatus != EFI_NOT_FOUND) {
> -    BdsBootManagerMenuLoop (&BootManagerMenu);
> +  Status = EfiBootManagerUnableToBoot ();
> +  if (EFI_ERROR (Status) && (BootManagerMenuStatus != EFI_NOT_FOUND)) {
> +    //
> +    // The platform didn't register a callback; fall back to BootManagerMenu
> +    // internally, indefinitely.
> +    //
> +    
> +    while (TRUE) {
> +      EfiBootManagerBoot (&BootManagerMenu);
> +    }
>    }
>  
>    DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n"));
>