[edk2] [PATCH 1/5] MdeModulePkg/BdsDxe: fall back to a Boot Manager Menu loop before hanging

Laszlo Ersek posted 5 patches 8 years, 2 months ago
[edk2] [PATCH 1/5] MdeModulePkg/BdsDxe: fall back to a Boot Manager Menu loop before hanging
Posted by Laszlo Ersek 8 years, 2 months ago
Under the following scenario:

- no UEFI bootable application available anywhere in the system,
- ... not even for the default platform recovery option,
- no shell is built into the firmware image,
- but UiApp is available in the firmware image,

we should preferably not just hang in BdsEntry() with:

   DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n"));
   CpuDeadLoop ();

while the user sits at the TianoCore logo page, wondering what's going on.
Print an informative message to the console, wait for a keypress, and then
return to the Boot Manager Menu forever.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1515418
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=513
Suggested-by: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 60 ++++++++++++++++++--
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index dccc49090219..2b24755ac368 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -676,24 +676,73 @@ BdsAllocateMemoryForPerformanceData (
     //
     // Mark L"PerfDataMemAddr" variable to read-only if the Variable Lock protocol exists
     // Still lock it even the variable cannot be saved to prevent it's set by 3rd party code.
     //
     Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL, (VOID **) &VariableLock);
     if (!EFI_ERROR (Status)) {
       Status = VariableLock->RequestToLock (VariableLock, L"PerfDataMemAddr", &gPerformanceProtocolGuid);
       ASSERT_EFI_ERROR (Status);
     }
   }
 }
 
+/**
+  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
   consoles are initialized, and the boot options are tried.
 
   @param This             Protocol Instance structure.
 
 **/
 VOID
 EFIAPI
 BdsEntry (
   IN EFI_BDS_ARCH_PROTOCOL  *This
@@ -1079,34 +1128,37 @@ BdsEntry (
     }
 
     do {
       //
       // Retry to boot if any of the boot succeeds
       //
       LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOptionTypeBoot);
       BootSuccess = BootBootOptions (LoadOptions, LoadOptionCount, (BootManagerMenuStatus != EFI_NOT_FOUND) ? &BootManagerMenu : NULL);
       EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
     } while (BootSuccess);
   }
 
-  if (BootManagerMenuStatus != EFI_NOT_FOUND) {
-    EfiBootManagerFreeLoadOption (&BootManagerMenu);
-  }
-
   if (!BootSuccess) {
     LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOptionTypePlatformRecovery);
     ProcessLoadOptions (LoadOptions, LoadOptionCount);
     EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
   }
 
+  //
+  // If BootManagerMenu is available, fall back to it indefinitely.
+  //
+  if (BootManagerMenuStatus != EFI_NOT_FOUND) {
+    BdsBootManagerMenuLoop (&BootManagerMenu);
+  }
+
   DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n"));
   CpuDeadLoop ();
 }
 
 /**
   Set the variable and report the error through status code upon failure.
 
   @param  VariableName           A Null-terminated string that is the name of the vendor's variable.
                                  Each VariableName is unique for each VendorGuid. VariableName must
                                  contain 1 or more characters. If VariableName is an empty string,
                                  then EFI_INVALID_PARAMETER is returned.
   @param  VendorGuid             A unique identifier for the vendor.
-- 
2.14.1.3.gb7cf6e02401b


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/5] MdeModulePkg/BdsDxe: fall back to a Boot Manager Menu loop before hanging
Posted by Ni, Ruiyu 8 years, 2 months ago
Thanks for the patch. It follows the idea described in:
https://lists.01.org/pipermail/edk2-devel/2017-April/010294.html.

Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

Thanks/Ray

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, November 23, 2017 7:59 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH 1/5] MdeModulePkg/BdsDxe: fall back to a Boot Manager
> Menu loop before hanging
> 
> Under the following scenario:
> 
> - no UEFI bootable application available anywhere in the system,
> - ... not even for the default platform recovery option,
> - no shell is built into the firmware image,
> - but UiApp is available in the firmware image,
> 
> we should preferably not just hang in BdsEntry() with:
> 
>    DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n"));
>    CpuDeadLoop ();
> 
> while the user sits at the TianoCore logo page, wondering what's going on.
> Print an informative message to the console, wait for a keypress, and then
> return to the Boot Manager Menu forever.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1515418
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=513
> Suggested-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 60 ++++++++++++++++++-
> -
>  1 file changed, 56 insertions(+), 4 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> index dccc49090219..2b24755ac368 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> @@ -676,24 +676,73 @@ BdsAllocateMemoryForPerformanceData (
>      //
>      // Mark L"PerfDataMemAddr" variable to read-only if the Variable Lock
> protocol exists
>      // Still lock it even the variable cannot be saved to prevent it's set by 3rd
> party code.
>      //
>      Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL,
> (VOID **) &VariableLock);
>      if (!EFI_ERROR (Status)) {
>        Status = VariableLock->RequestToLock (VariableLock,
> L"PerfDataMemAddr", &gPerformanceProtocolGuid);
>        ASSERT_EFI_ERROR (Status);
>      }
>    }
>  }
> 
> +/**
> +  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
>    consoles are initialized, and the boot options are tried.
> 
>    @param This             Protocol Instance structure.
> 
>  **/
>  VOID
>  EFIAPI
>  BdsEntry (
>    IN EFI_BDS_ARCH_PROTOCOL  *This
> @@ -1079,34 +1128,37 @@ BdsEntry (
>      }
> 
>      do {
>        //
>        // Retry to boot if any of the boot succeeds
>        //
>        LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount,
> LoadOptionTypeBoot);
>        BootSuccess = BootBootOptions (LoadOptions, LoadOptionCount,
> (BootManagerMenuStatus != EFI_NOT_FOUND) ? &BootManagerMenu :
> NULL);
>        EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
>      } while (BootSuccess);
>    }
> 
> -  if (BootManagerMenuStatus != EFI_NOT_FOUND) {
> -    EfiBootManagerFreeLoadOption (&BootManagerMenu);
> -  }
> -
>    if (!BootSuccess) {
>      LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount,
> LoadOptionTypePlatformRecovery);
>      ProcessLoadOptions (LoadOptions, LoadOptionCount);
>      EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
>    }
> 
> +  //
> +  // If BootManagerMenu is available, fall back to it indefinitely.
> +  //
> +  if (BootManagerMenuStatus != EFI_NOT_FOUND) {
> +    BdsBootManagerMenuLoop (&BootManagerMenu);  }
> +
>    DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n"));
>    CpuDeadLoop ();
>  }
> 
>  /**
>    Set the variable and report the error through status code upon failure.
> 
>    @param  VariableName           A Null-terminated string that is the name of
> the vendor's variable.
>                                   Each VariableName is unique for each VendorGuid.
> VariableName must
>                                   contain 1 or more characters. If VariableName is an empty
> string,
>                                   then EFI_INVALID_PARAMETER is returned.
>    @param  VendorGuid             A unique identifier for the vendor.
> --
> 2.14.1.3.gb7cf6e02401b
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/5] MdeModulePkg/BdsDxe: fall back to a Boot Manager Menu loop before hanging
Posted by Laszlo Ersek 8 years, 2 months ago
On 11/23/17 04:43, Ni, Ruiyu wrote:
> Thanks for the patch. It follows the idea described in:
> https://lists.01.org/pipermail/edk2-devel/2017-April/010294.html.
> 
> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

Thank you, Ray! I might commit this first patch in isolation (to close
#513) while we continue discussing the rest. Not sure yet. (Depends on
my workload.)

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/5] MdeModulePkg/BdsDxe: fall back to a Boot Manager Menu loop before hanging
Posted by Laszlo Ersek 8 years, 2 months ago
Hi Ray,

On 11/23/17 14:09, Laszlo Ersek wrote:
> On 11/23/17 04:43, Ni, Ruiyu wrote:
>> Thanks for the patch. It follows the idea described in:
>> https://lists.01.org/pipermail/edk2-devel/2017-April/010294.html.
>>
>> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
> 
> Thank you, Ray! I might commit this first patch in isolation (to close
> #513) while we continue discussing the rest. Not sure yet. (Depends on
> my workload.)

thanks again for your original suggestion and your review; I've now
committed this single patch, isolated the rest of the series. Commit
d1de487dd2e7.

Cheers,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel