Re: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting it

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

This could avoid the "BootNext" set by PlatformBootManagerLib be consumed in *this* boot.

If I add "*" around "this", is it more clear?

Thanks/Ray

> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, September 28, 2017 2:00 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until
> booting it
> 
> I am ok with the code logic change, but a little confused by the new comment. It
> seems not match with the commit log.
> 
> " This could avoid the "BootNext" set by PlatformBootManagerLib be consumed
> in this boot. "
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Thursday, September 28, 2017 1:50 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [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.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Star Zeng <star.zeng@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