The change Looks good to me. 
Also, Good to see you adding the comment about why we need to cache BootNext 
before all the platform hook function calls. :)

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

Regards,
Sunny Wang

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Thursday, September 28, 2017 1:50 PM
To: edk2-devel@lists.01.org
Cc: Eric Dong <eric.d...@intel.com>; Star Zeng <star.z...@intel.com>
Subject: [edk2] [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until 
booting it

Current implementation deletes the "BootNext" before calling any 
PlatformBootManagerLib APIs, but if system resets in PlatformBootManagerLib 
APIs, "BootNext" is not consumed but lost.

The patch defers the deletion of "BootNext" to before booting it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
Cc: Eric Dong <eric.d...@intel.com>
Cc: Star Zeng <star.z...@intel.com>
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35 ++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index ac5f9088dd..a6fe617b56 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -808,7 +808,8 @@ BdsEntry (
   ASSERT_EFI_ERROR (Status);
 
   //
-  // Cache and remove the "BootNext" NV variable.
+  // Cache the "BootNext" NV variable before calling any 
+ PlatformBootManagerLib APIs  // This could avoid the "BootNext" set by 
PlatformBootManagerLib be consumed in this boot.
   //
   GetEfiGlobalVariable2 (EFI_BOOT_NEXT_VARIABLE_NAME, (VOID **) &BootNext, 
&DataSize);
   if (DataSize != sizeof (UINT16)) {
@@ -817,17 +818,6 @@ BdsEntry (
     }
     BootNext = NULL;
   }
-  Status = gRT->SetVariable (
-                  EFI_BOOT_NEXT_VARIABLE_NAME,
-                  &gEfiGlobalVariableGuid,
-                  0,
-                  0,
-                  NULL
-                  );
-  //
-  // Deleting NV variable shouldn't fail unless it doesn't exist.
-  //
-  ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND);
 
   //
   // Initialize the platform language variables @@ -1052,10 +1042,25 @@ 
BdsEntry (
 
     EfiBootManagerHotkeyBoot ();
 
-    //
-    // Boot to "BootNext"
-    //
     if (BootNext != NULL) {
+      //
+      // Delete "BootNext" NV variable before transferring control to it to 
prevent loops.
+      //
+      Status = gRT->SetVariable (
+                      EFI_BOOT_NEXT_VARIABLE_NAME,
+                      &gEfiGlobalVariableGuid,
+                      0,
+                      0,
+                      NULL
+                      );
+      //
+      // Deleting NV variable shouldn't fail unless it doesn't exist.
+      //
+      ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND);
+
+      //
+      // Boot to "BootNext"
+      //
       UnicodeSPrint (BootNextVariableName, sizeof (BootNextVariableName), 
L"Boot%04x", *BootNext);
       Status = EfiBootManagerVariableToLoadOption (BootNextVariableName, 
&LoadOption);
       if (!EFI_ERROR (Status)) {
--
2.12.2.windows.2

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

Reply via email to