[PATCH v3 0/7] Add platform hook for ultimate boot failure.

[PATCH v3 0/7] Add platform hook for ultimate boot failure.

[Ruiyu Ni]

Ruiyu Ni (7):
  MdeModulePkg/PlatformBootManager: Add PlatformBootManagerUnableToBoot
  CorebootPayload/PlatformBDS: Impl PlatformBootManagerUnableToBoot
  OvmfPkg/PlatformBds: Implement PlatformBootManagerUnableToBoot
  Nt32Pkg/PlatformBDS: Implement PlatformBootManagerUnableToBoot
  QuarkPlatform/PlatformBDS: Implement PlatformBootManagerUnableToBoot
  MdeModulePkg/BdsDxe: Revert "fall back to UI loop before hanging"
  MdeModulePkg/BdsDxe: Call PlatformBootManagerUnableToBoot()

 .../PlatformBootManagerLib/PlatformBootManager.c   | 19 ++++++-
 .../Include/Library/PlatformBootManagerLib.h       | 13 +++++
 .../PlatformBootManager.c                          | 19 ++++++-
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c           | 61 ++--------------------
 .../PlatformBootManagerLib/PlatformBootManager.c   | 19 ++++++-
 .../Library/PlatformBootManagerLib/BdsPlatform.c   | 61 +++++++++++++++++++++-
 .../PlatformBootManagerLib/PlatformBootManager.c   | 19 ++++++-
 7 files changed, 150 insertions(+), 61 deletions(-)

-- 
2.16.1.windows.1

[PATCH v3 1/7] MdeModulePkg/PlatformBootManager: Add PlatformBootManagerUnableToBoot

[Ruiyu Ni]

The patch adds a new API PlatformBootManagerUnableToBoot()'
to PlatformBootManagerLib.
The new API is provided by platform bds library and is called when
no boot option could be launched.

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/PlatformBootManagerLib.h | 13 +++++++++++++
 .../PlatformBootManagerLibNull/PlatformBootManager.c  | 19 ++++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Include/Library/PlatformBootManagerLib.h b/MdeModulePkg/Include/Library/PlatformBootManagerLib.h
index 65630ce2bb..6e26329043 100644
--- a/MdeModulePkg/Include/Library/PlatformBootManagerLib.h
+++ b/MdeModulePkg/Include/Library/PlatformBootManagerLib.h
@@ -59,4 +59,17 @@ PlatformBootManagerWaitCallback (
   UINT16          TimeoutRemain
   );
 
+/**
+  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
+  );
+
 #endif
diff --git a/MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManager.c b/MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManager.c
index 1390e19097..5a4455ef23 100644
--- a/MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManager.c
+++ b/MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManager.c
@@ -2,7 +2,7 @@
   This file include all platform action which can be customized
   by IBV/OEM.
 
-Copyright (c) 2012 - 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2012 - 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
@@ -65,3 +65,20 @@ PlatformBootManagerWaitCallback (
 {
   return;
 }
+
+/**
+  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
+  )
+{
+  return;
+}
+
-- 
2.16.1.windows.1

[PATCH v3 2/7] CorebootPayload/PlatformBDS: Impl PlatformBootManagerUnableToBoot

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
---
 .../PlatformBootManagerLib/PlatformBootManager.c      | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c b/CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
index 7e92441da1..368e89d586 100644
--- a/CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
+++ b/CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -2,7 +2,7 @@
   This file include all platform action which can be customized
   by IBV/OEM.
 
-Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 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
@@ -252,3 +252,20 @@ PlatformBootManagerWaitCallback (
 {
   return;
 }
+
+/**
+  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
+  )
+{
+  return;
+}
+
-- 
2.16.1.windows.1

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

[Ruiyu Ni]

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

[PATCH v3 4/7] Nt32Pkg/PlatformBDS: Implement PlatformBootManagerUnableToBoot

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Hao A Wu <Hao.a.wu@intel.com>
---
 .../PlatformBootManagerLib/PlatformBootManager.c      | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c b/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
index 99f30f9ec2..cf8289faff 100644
--- a/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
+++ b/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -2,7 +2,7 @@
   This file include all platform action which can be customized
   by IBV/OEM.
 
-Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
@@ -403,3 +403,20 @@ PlatformBootManagerWaitCallback (
     0
     );
 }
+
+/**
+  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
+  )
+{
+  return;
+}
+
-- 
2.16.1.windows.1

[PATCH v3 5/7] QuarkPlatform/PlatformBDS: Implement PlatformBootManagerUnableToBoot

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Kelly Steele <kelly.steele@intel.com>
---
 .../PlatformBootManagerLib/PlatformBootManager.c      | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
index 53391c6077..8b25f55f1c 100644
--- a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
+++ b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -2,7 +2,7 @@
 This file include all platform action which can be customized
 by IBV/OEM.
 
-Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 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
@@ -459,3 +459,20 @@ PlatformBootManagerWaitCallback (
 {
   Print (L"\r%-2d seconds remained...", TimeoutRemain);
 }
+
+/**
+  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
+  )
+{
+  return;
+}
+
-- 
2.16.1.windows.1

[PATCH v3 6/7] MdeModulePkg/BdsDxe: Revert "fall back to UI loop before hanging"

[Ruiyu Ni]

Commit d1de487dd2e77f4741abcbd71d19a8c93971fda0
"MdeModulePkg/BdsDxe: fall back to a Boot Manager Menu loop before
 hanging"
changed BDS core to fall back to UI loop when no bootable option
can be launched.
Now since PlatformBootManagerUnableToBoot() is added, the commit
can be reverted.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 60 +++-----------------------------
 1 file changed, 4 insertions(+), 56 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 49e403e181..39b643c77a 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
@@ -1081,19 +1032,16 @@ BdsEntry (
     } 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 ();
 }
-- 
2.16.1.windows.1

[PATCH v3 7/7] MdeModulePkg/BdsDxe: Call PlatformBootManagerUnableToBoot()

[Ruiyu Ni]

When no boot option can be launched, BDS core calls
PlatformBootManagerUnableToBoot() to let platform BdsDxe handle it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Sunny Wang <sunnywang@hpe.com>
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 39b643c77a..a25663ea43 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -1043,6 +1043,7 @@ BdsEntry (
   }
 
   DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n"));
+  PlatformBootManagerUnableToBoot ();
   CpuDeadLoop ();
 }
 
-- 
2.16.1.windows.1

Re: [PATCH v3 1/7] MdeModulePkg/PlatformBootManager: Add PlatformBootManagerUnableToBoot

[Laszlo Ersek]

On 07/03/18 08:37, Ruiyu Ni wrote:
> The patch adds a new API PlatformBootManagerUnableToBoot()'

(1) Unbalanced single quote (').

> to PlatformBootManagerLib.
> The new API is provided by platform bds library and is called when
> no boot option could be launched.
> 
> 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/PlatformBootManagerLib.h | 13 +++++++++++++
>  .../PlatformBootManagerLibNull/PlatformBootManager.c  | 19 ++++++++++++++++++-
>  2 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Include/Library/PlatformBootManagerLib.h b/MdeModulePkg/Include/Library/PlatformBootManagerLib.h
> index 65630ce2bb..6e26329043 100644
> --- a/MdeModulePkg/Include/Library/PlatformBootManagerLib.h
> +++ b/MdeModulePkg/Include/Library/PlatformBootManagerLib.h
> @@ -59,4 +59,17 @@ PlatformBootManagerWaitCallback (
>    UINT16          TimeoutRemain
>    );
>  
> +/**
> +  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
> +  );
> +
>  #endif
> diff --git a/MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManager.c b/MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManager.c
> index 1390e19097..5a4455ef23 100644
> --- a/MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManager.c
> +++ b/MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManager.c
> @@ -2,7 +2,7 @@
>    This file include all platform action which can be customized
>    by IBV/OEM.
>  
> -Copyright (c) 2012 - 2015, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2012 - 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
> @@ -65,3 +65,20 @@ PlatformBootManagerWaitCallback (
>  {
>    return;
>  }
> +
> +/**
> +  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
> +  )
> +{
> +  return;
> +}
> +
> 

(2) Do we intentionally add such "return" statements? An empty function
body (possibly with a comment) would work as well.

I'm fine either way, I'd just like to highlight that there is a
TianoCore BZ:

https://bugzilla.tianocore.org/show_bug.cgi?id=843

about removing useless "return" statements.

I believe we should decide whether such return statements are a good
thing for edk2. If they are, we should close #843 as NOTEBUG. If they
aren't a good thing, we shouldn't add more of them.

Anyway, whatever the decision for (1) and (2) above, I don't think
reposting would be necessary just because of them; it should be OK to
update the patch before pushing.

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

Thanks!
Laszlo

Re: [PATCH v3 6/7] MdeModulePkg/BdsDxe: Revert "fall back to UI loop before hanging"

[Laszlo Ersek]

On 07/03/18 08:37, Ruiyu Ni wrote:
> Commit d1de487dd2e77f4741abcbd71d19a8c93971fda0
> "MdeModulePkg/BdsDxe: fall back to a Boot Manager Menu loop before
>  hanging"
> changed BDS core to fall back to UI loop when no bootable option
> can be launched.
> Now since PlatformBootManagerUnableToBoot() is added, the commit
> can be reverted.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
>  MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 60 +++-----------------------------
>  1 file changed, 4 insertions(+), 56 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> index 49e403e181..39b643c77a 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
> @@ -1081,19 +1032,16 @@ BdsEntry (
>      } 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 ();
>  }
> 

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

Re: [PATCH v3 7/7] MdeModulePkg/BdsDxe: Call PlatformBootManagerUnableToBoot()

[Laszlo Ersek]

On 07/03/18 08:37, Ruiyu Ni wrote:
> When no boot option can be launched, BDS core calls
> PlatformBootManagerUnableToBoot() to let platform BdsDxe handle it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Sunny Wang <sunnywang@hpe.com>
> ---
>  MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> index 39b643c77a..a25663ea43 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> @@ -1043,6 +1043,7 @@ BdsEntry (
>    }
>  
>    DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n"));
> +  PlatformBootManagerUnableToBoot ();
>    CpuDeadLoop ();
>  }
>  
> 

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

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

[Laszlo Ersek]

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

Re: [PATCH v3 0/7] Add platform hook for ultimate boot failure.

[Laszlo Ersek]

Hi Ray,

On 07/03/18 08:37, Ruiyu Ni wrote:
> Ruiyu Ni (7):
>   MdeModulePkg/PlatformBootManager: Add PlatformBootManagerUnableToBoot
>   CorebootPayload/PlatformBDS: Impl PlatformBootManagerUnableToBoot
>   OvmfPkg/PlatformBds: Implement PlatformBootManagerUnableToBoot
>   Nt32Pkg/PlatformBDS: Implement PlatformBootManagerUnableToBoot
>   QuarkPlatform/PlatformBDS: Implement PlatformBootManagerUnableToBoot
>   MdeModulePkg/BdsDxe: Revert "fall back to UI loop before hanging"
>   MdeModulePkg/BdsDxe: Call PlatformBootManagerUnableToBoot()
>
>  .../PlatformBootManagerLib/PlatformBootManager.c   | 19 ++++++-
>  .../Include/Library/PlatformBootManagerLib.h       | 13 +++++
>  .../PlatformBootManager.c                          | 19 ++++++-
>  MdeModulePkg/Universal/BdsDxe/BdsEntry.c           | 61 ++--------------------
>  .../PlatformBootManagerLib/PlatformBootManager.c   | 19 ++++++-
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 61 +++++++++++++++++++++-
>  .../PlatformBootManagerLib/PlatformBootManager.c   | 19 ++++++-
>  7 files changed, 150 insertions(+), 61 deletions(-)
>

thanks a lot for this version as well!

It seems we have the following PlatformBootManagerLib instances in the
edk2 tree:

  ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
  ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
  CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
  MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManagerLibNull.inf
  Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
  OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
  QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
  Vlv2TbltDevicePkg/Library/PlatformBdsLib/PlatformBdsLib.inf

This series updates all but ArmPkg, ArmVirtPkg and Vlv2TbltDevicePkg. So
I think three more patches would be necessary, before patch #6.

Originally, with the registration pattern, I offered to port the OvmfPkg
update to ArmVirtPkg myself, later. However, with the present pattern,
these three platforms would stop building, so I think they should be
updated before we commit patches #6 and #7.

For ArmPkg and Vlv2TbltDevicePkg, I *think* the Null implementation ("do
nothing") is appropriate. (I'm CC'ing those package maintainers). For
ArmVirtPkg, duplicating the OvmfPkg approach would be correct.

Can you take on updating these three platforms as well? Or can I help
somehow with it? (It's tricky because the three platform patches that I
could post should go in the middle of the series, neither before nor
after.)

Thank you!
Laszlo

Re: [PATCH v3 2/7] CorebootPayload/PlatformBDS: Impl PlatformBootManagerUnableToBoot

[You, Benjamin]

Reviewed-by: Benjamin You <benjamin.you@intel.com>

> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Tuesday, July 3, 2018 2:38 PM
> To: edk2-devel@lists.01.org
> Cc: Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
> <prince.agyeman@intel.com>; You, Benjamin <benjamin.you@intel.com>
> Subject: [PATCH v3 2/7] CorebootPayload/PlatformBDS: Impl
> PlatformBootManagerUnableToBoot
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Prince Agyeman <prince.agyeman@intel.com>
> Cc: Benjamin You <benjamin.you@intel.com>
> ---
>  .../PlatformBootManagerLib/PlatformBootManager.c      | 19
> ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git
> a/CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManage
> r.c
> b/CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManage
> r.c
> index 7e92441da1..368e89d586 100644
> ---
> a/CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManage
> r.c
> +++
> b/CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManage
> r.c
> @@ -2,7 +2,7 @@
>    This file include all platform action which can be customized
>    by IBV/OEM.
> 
> -Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2015 - 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
> @@ -252,3 +252,20 @@ PlatformBootManagerWaitCallback (
>  {
>    return;
>  }
> +
> +/**
> +  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
> +  )
> +{
> +  return;
> +}
> +
> --
> 2.16.1.windows.1

Re: [PATCH v3 0/7] Add platform hook for ultimate boot failure.

[Ni, Ruiyu]

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, July 3, 2018 11:53 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: edk2-devel@lists.01.org; Leif Lindholm (Linaro address)
> <leif.lindholm@linaro.org>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Wei,
> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
> Subject: Re: [edk2] [PATCH v3 0/7] Add platform hook for ultimate boot failure.
> 
> Hi Ray,
> 
> On 07/03/18 08:37, Ruiyu Ni wrote:
> > Ruiyu Ni (7):
> >   MdeModulePkg/PlatformBootManager: Add
> PlatformBootManagerUnableToBoot
> >   CorebootPayload/PlatformBDS: Impl PlatformBootManagerUnableToBoot
> >   OvmfPkg/PlatformBds: Implement PlatformBootManagerUnableToBoot
> >   Nt32Pkg/PlatformBDS: Implement PlatformBootManagerUnableToBoot
> >   QuarkPlatform/PlatformBDS: Implement
> PlatformBootManagerUnableToBoot
> >   MdeModulePkg/BdsDxe: Revert "fall back to UI loop before hanging"
> >   MdeModulePkg/BdsDxe: Call PlatformBootManagerUnableToBoot()
> >
> >  .../PlatformBootManagerLib/PlatformBootManager.c   | 19 ++++++-
> >  .../Include/Library/PlatformBootManagerLib.h       | 13 +++++
> >  .../PlatformBootManager.c                          | 19 ++++++-
> >  MdeModulePkg/Universal/BdsDxe/BdsEntry.c           | 61 ++--------------------
> >  .../PlatformBootManagerLib/PlatformBootManager.c   | 19 ++++++-
> >  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 61
> +++++++++++++++++++++-
> >  .../PlatformBootManagerLib/PlatformBootManager.c   | 19 ++++++-
> >  7 files changed, 150 insertions(+), 61 deletions(-)
> >
> 
> thanks a lot for this version as well!
> 
> It seems we have the following PlatformBootManagerLib instances in the
> edk2 tree:
> 
>   ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>   ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> 
> CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerL
> ib.inf
> 
> MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManagerLi
> bNull.inf
>   Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>   OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> 
> QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.i
> nf
>   Vlv2TbltDevicePkg/Library/PlatformBdsLib/PlatformBdsLib.inf
> 
> This series updates all but ArmPkg, ArmVirtPkg and Vlv2TbltDevicePkg. So I think
> three more patches would be necessary, before patch #6.
I tried to search all PlatformBootManagerAfterConsole() to locate all instances
of PlatformBootManagerLib.
Now it seems I still missed three.
I will make an updated patch and send V4.

> 
> Originally, with the registration pattern, I offered to port the OvmfPkg update to
> ArmVirtPkg myself, later. However, with the present pattern, these three
> platforms would stop building, so I think they should be updated before we
> commit patches #6 and #7.
> 
> For ArmPkg and Vlv2TbltDevicePkg, I *think* the Null implementation ("do
> nothing") is appropriate. (I'm CC'ing those package maintainers). For ArmVirtPkg,
> duplicating the OvmfPkg approach would be correct.
> 
> Can you take on updating these three platforms as well? Or can I help somehow
> with it? (It's tricky because the three platform patches that I could post should
> go in the middle of the series, neither before nor
> after.)
> 
> Thank you!
> Laszlo

Re: [PATCH v3 4/7] Nt32Pkg/PlatformBDS: Implement PlatformBootManagerUnableToBoot

[Wu, Hao A]

Reviewed-by: Hao Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu

> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Tuesday, July 03, 2018 2:38 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A
> Subject: [PATCH v3 4/7] Nt32Pkg/PlatformBDS: Implement
> PlatformBootManagerUnableToBoot
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Hao A Wu <Hao.a.wu@intel.com>
> ---
>  .../PlatformBootManagerLib/PlatformBootManager.c      | 19
> ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
> b/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
> index 99f30f9ec2..cf8289faff 100644
> --- a/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
> +++ b/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
> @@ -2,7 +2,7 @@
>    This file include all platform action which can be customized
>    by IBV/OEM.
> 
> -Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
>  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
> @@ -403,3 +403,20 @@ PlatformBootManagerWaitCallback (
>      0
>      );
>  }
> +
> +/**
> +  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
> +  )
> +{
> +  return;
> +}
> +
> --
> 2.16.1.windows.1

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

[Ni, Ruiyu]

On 7/3/2018 11:37 PM, Laszlo Ersek wrote:
> 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.
>    //
Sure.

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


-- 
Thanks,
Ray

Re: [PATCH v3 0/7] Add platform hook for ultimate boot failure.

[Ni, Ruiyu]

On 7/4/2018 9:08 AM, Ni, Ruiyu wrote:
> 
> 
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Tuesday, July 3, 2018 11:53 PM
>> To: Ni, Ruiyu <ruiyu.ni@intel.com>
>> Cc: edk2-devel@lists.01.org; Leif Lindholm (Linaro address)
>> <leif.lindholm@linaro.org>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Wei,
>> David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>
>> Subject: Re: [edk2] [PATCH v3 0/7] Add platform hook for ultimate boot failure.
>>
>> Hi Ray,
>>
>> On 07/03/18 08:37, Ruiyu Ni wrote:
>>> Ruiyu Ni (7):
>>>    MdeModulePkg/PlatformBootManager: Add
>> PlatformBootManagerUnableToBoot
>>>    CorebootPayload/PlatformBDS: Impl PlatformBootManagerUnableToBoot
>>>    OvmfPkg/PlatformBds: Implement PlatformBootManagerUnableToBoot
>>>    Nt32Pkg/PlatformBDS: Implement PlatformBootManagerUnableToBoot
>>>    QuarkPlatform/PlatformBDS: Implement
>> PlatformBootManagerUnableToBoot
>>>    MdeModulePkg/BdsDxe: Revert "fall back to UI loop before hanging"
>>>    MdeModulePkg/BdsDxe: Call PlatformBootManagerUnableToBoot()
>>>
>>>   .../PlatformBootManagerLib/PlatformBootManager.c   | 19 ++++++-
>>>   .../Include/Library/PlatformBootManagerLib.h       | 13 +++++
>>>   .../PlatformBootManager.c                          | 19 ++++++-
>>>   MdeModulePkg/Universal/BdsDxe/BdsEntry.c           | 61 ++--------------------
>>>   .../PlatformBootManagerLib/PlatformBootManager.c   | 19 ++++++-
>>>   .../Library/PlatformBootManagerLib/BdsPlatform.c   | 61
>> +++++++++++++++++++++-
>>>   .../PlatformBootManagerLib/PlatformBootManager.c   | 19 ++++++-
>>>   7 files changed, 150 insertions(+), 61 deletions(-)
>>>
>>
>> thanks a lot for this version as well!
>>
>> It seems we have the following PlatformBootManagerLib instances in the
>> edk2 tree:
>>
>>    ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>>    ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>>
>> CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerL
>> ib.inf
>>
>> MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManagerLi
>> bNull.inf
>>    Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>>    OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>>
>> QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.i
>> nf
>>    Vlv2TbltDevicePkg/Library/PlatformBdsLib/PlatformBdsLib.inf
>>
>> This series updates all but ArmPkg, ArmVirtPkg and Vlv2TbltDevicePkg. So I think
>> three more patches would be necessary, before patch #6.
> I tried to search all PlatformBootManagerAfterConsole() to locate all instances
> of PlatformBootManagerLib.
> Now it seems I still missed three.
> I will make an updated patch and send V4.
> 
>>
>> Originally, with the registration pattern, I offered to port the OvmfPkg update to
>> ArmVirtPkg myself, later. However, with the present pattern, these three
>> platforms would stop building, so I think they should be updated before we
>> commit patches #6 and #7.
>>
>> For ArmPkg and Vlv2TbltDevicePkg, I *think* the Null implementation ("do
>> nothing") is appropriate. (I'm CC'ing those package maintainers). For ArmVirtPkg,
>> duplicating the OvmfPkg approach would be correct.
Vlv2TbltDevicePkg is still using old IntelFrameworkModulePkg/BdsDxe.
So I will just update ArmPkg and ArmVirtPkg.

>>
>> Can you take on updating these three platforms as well? Or can I help somehow
>> with it? (It's tricky because the three platform patches that I could post should
>> go in the middle of the series, neither before nor
>> after.)
>>
>> Thank you!
>> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 


-- 
Thanks,
Ray