[PATCH v2 0/2] Enhance BDS to handle ultimate boot failure

[PATCH v2 0/2] Enhance BDS to handle ultimate boot failure

[Ruiyu Ni]

Ruiyu Ni (2):
  MdeModulePkg/UefiBootManagerLib: new APIs for ultimate boot failure
  MdeModulePkg/BdsDxe: Call *UnableToBoot() for ultimate boot failure

 MdeModulePkg/Include/Library/UefiBootManagerLib.h | 48 +++++++++++++++++
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c  | 41 ++++++++++++++
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c          | 65 +++++------------------
 3 files changed, 102 insertions(+), 52 deletions(-)

-- 
2.16.1.windows.1

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

[Ruiyu Ni]

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.
+
+  @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;
+}
-- 
2.16.1.windows.1

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

[Ruiyu Ni]

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.
   //
-  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"));
-- 
2.16.1.windows.1

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

[Wang, Sunny (HPS SW)]

Looks good to me. Thanks for addressing my comment, Ray. 

Reviewed-by: Sunny Wang <sunnywang@hpe.com>

-----Original Message-----
From: Ruiyu Ni [mailto:ruiyu.ni@intel.com] 
Sent: Friday, June 29, 2018 2:04 PM
To: edk2-devel@lists.01.org
Cc: Sean Brogan <sean.brogan@microsoft.com>; Michael Turner <Michael.Turner@microsoft.com>; Laszlo Ersek <lersek@redhat.com>; Wang, Sunny (HPS SW) <sunnywang@hpe.com>
Subject: [PATCH v2 1/2] MdeModulePkg/UefiBootManagerLib: new APIs for ultimate boot failure

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.
+
+  @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;
+}
--
2.16.1.windows.1

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

[Wang, Sunny (HPS SW)]

Looks good to me. Thanks for addressing my comment, Ray. 
Reviewed-by: Sunny Wang <sunnywang@hpe.com>

-----Original Message-----
From: Ruiyu Ni [mailto:ruiyu.ni@intel.com] 
Sent: Friday, June 29, 2018 2:04 PM
To: edk2-devel@lists.01.org
Cc: Sean Brogan <sean.brogan@microsoft.com>; Michael Turner <Michael.Turner@microsoft.com>; Laszlo Ersek <lersek@redhat.com>; Wang, Sunny (HPS SW) <sunnywang@hpe.com>
Subject: [PATCH v2 2/2] MdeModulePkg/BdsDxe: Call *UnableToBoot() for ultimate boot failure

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.
   //
-  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"));
--
2.16.1.windows.1

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

[Laszlo Ersek]

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;
> +}
> 

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

[Laszlo Ersek]

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"));
> 

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

[Ni, Ruiyu]

Laszlo,
I will post a V3 version that removes the registration mechanism, back
to the solution letting platform provides PlatformBootManagerUnableToBoot(),
based on the feedbacks I received.


> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, June 29, 2018 8:41 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Subject: Re: [edk2] [PATCH v2 1/2] MdeModulePkg/UefiBootManagerLib: new
> APIs for ultimate boot failure
> 
> 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;
> > +}
> >