Re: [edk2-platforms][PATCH v2 29/32] JadePkg: Recover boot options when NVRAM cleared

["Leif Lindholm" <leif@nuviainc.com>]

On Wed, May 26, 2021 at 17:07:21 +0700, Nhi Pham wrote:
> From: Vu Nguyen <vunguyen@os.amperecomputing.com>
> 
> Due to recent changes in EDK2, system without a valid boot options list
> in NVRAM will enter Platform Recovery mode. As ReadyToBoot event is not
> triggered in this mode, services like ACPI and PCIe won't be able to do
> some finalization settings and cause OS fails to boot.
> This change is to prevent the issue by recovery boot options list each
> time NVRAM is cleared.

Urgh.
Ultimately this is a bug in EDK2. And it should be fixed in edk2:
https://bugzilla.tianocore.org/show_bug.cgi?id=2831

But could you let me know which specific edk2 change broke this for
you, since it is not obvious to me?

/
    Leif

> Cc: Thang Nguyen <thang@os.amperecomputing.com>
> Cc: Chuong Tran <chuong@os.amperecomputing.com>
> Cc: Phong Vo <phong@os.amperecomputing.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> 
> Signed-off-by: Vu Nguyen <vunguyen@os.amperecomputing.com>
> ---
>  Platform/Ampere/JadePkg/Jade.dsc                                                  |  5 ++
>  Platform/Ampere/JadePkg/Jade.fdf                                                  |  5 ++
>  Platform/Ampere/JadePkg/Drivers/BootOptionsRecoveryDxe/BootOptionsRecoveryDxe.inf | 39 +++++++++++++
>  Platform/Ampere/JadePkg/Drivers/BootOptionsRecoveryDxe/BootOptionsRecoveryDxe.c   | 58 ++++++++++++++++++++
>  4 files changed, 107 insertions(+)
> 
> diff --git a/Platform/Ampere/JadePkg/Jade.dsc b/Platform/Ampere/JadePkg/Jade.dsc
> index 9d787113e3b5..023f2e898d7f 100755
> --- a/Platform/Ampere/JadePkg/Jade.dsc
> +++ b/Platform/Ampere/JadePkg/Jade.dsc
> @@ -185,3 +185,8 @@ [Components.common]
>    Silicon/Ampere/AmpereAltraPkg/Drivers/RasConfigDxe/RasConfigDxe.inf
>    Silicon/Ampere/AmpereAltraPkg/Drivers/WatchdogConfigDxe/WatchdogConfigDxe.inf
>    Silicon/Ampere/AmpereAltraPkg/Drivers/PcieDeviceConfigDxe/PcieDeviceConfigDxe.inf
> +
> +  #
> +  # Misc
> +  #
> +  Platform/Ampere/JadePkg/Drivers/BootOptionsRecoveryDxe/BootOptionsRecoveryDxe.inf
> diff --git a/Platform/Ampere/JadePkg/Jade.fdf b/Platform/Ampere/JadePkg/Jade.fdf
> index b0c2894d00f8..edbead046572 100755
> --- a/Platform/Ampere/JadePkg/Jade.fdf
> +++ b/Platform/Ampere/JadePkg/Jade.fdf
> @@ -361,4 +361,9 @@ [FV.FvMain]
>    INF Silicon/Ampere/AmpereAltraPkg/Drivers/WatchdogConfigDxe/WatchdogConfigDxe.inf
>    INF Silicon/Ampere/AmpereAltraPkg/Drivers/PcieDeviceConfigDxe/PcieDeviceConfigDxe.inf
>  
> +  #
> +  # Misc
> +  #
> +  INF Platform/Ampere/JadePkg/Drivers/BootOptionsRecoveryDxe/BootOptionsRecoveryDxe.inf
> +
>  !include Platform/Ampere/AmperePlatformPkg/FvRules.fdf.inc
> diff --git a/Platform/Ampere/JadePkg/Drivers/BootOptionsRecoveryDxe/BootOptionsRecoveryDxe.inf b/Platform/Ampere/JadePkg/Drivers/BootOptionsRecoveryDxe/BootOptionsRecoveryDxe.inf
> new file mode 100644
> index 000000000000..624332339beb
> --- /dev/null
> +++ b/Platform/Ampere/JadePkg/Drivers/BootOptionsRecoveryDxe/BootOptionsRecoveryDxe.inf
> @@ -0,0 +1,39 @@
> +#
> +# Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001B
> +  BASE_NAME                      = BootOptionsRecoveryDxe
> +  FILE_GUID                      = FDCDDC91-4F9E-400C-9BB4-1FE4BE9565B3
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = BootOptionsRecoveryDxeEntry
> +
> +[Sources]
> +  BootOptionsRecoveryDxe.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  Silicon/Ampere/AmpereSiliconPkg/AmpereSiliconPkg.dec
> +
> +[LibraryClasses]
> +  DebugLib
> +  PcdLib
> +  PrintLib
> +  UefiBootManagerLib
> +  UefiDriverEntryPoint
> +  UefiLib
> +
> +[Guids]
> +  gEfiEndOfDxeEventGroupGuid
> +
> +[Pcd]
> +  gAmpereTokenSpaceGuid.PcdNvramErased
> +
> +[Depex]
> +  gEfiFaultTolerantWriteProtocolGuid
> diff --git a/Platform/Ampere/JadePkg/Drivers/BootOptionsRecoveryDxe/BootOptionsRecoveryDxe.c b/Platform/Ampere/JadePkg/Drivers/BootOptionsRecoveryDxe/BootOptionsRecoveryDxe.c
> new file mode 100644
> index 000000000000..cd9637ec704e
> --- /dev/null
> +++ b/Platform/Ampere/JadePkg/Drivers/BootOptionsRecoveryDxe/BootOptionsRecoveryDxe.c
> @@ -0,0 +1,58 @@
> +/** @file
> +
> +  Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi.h>
> +
> +#include <Library/DebugLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/UefiBootManagerLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +
> +VOID
> +EFIAPI
> +RecoveryCallback (
> +  IN EFI_EVENT Event,
> +  IN VOID      *Context
> +  )
> +{
> +  DEBUG ((DEBUG_INFO, "%a: Do recover boot options\n", __FUNCTION__));
> +
> +  EfiBootManagerConnectAll ();
> +  EfiBootManagerRefreshAllBootOption ();
> +
> +  gBS->CloseEvent (Event);
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +BootOptionsRecoveryDxeEntry (
> +  IN EFI_HANDLE       ImageHandle,
> +  IN EFI_SYSTEM_TABLE *SystemTable
> +  )
> +{
> +  EFI_STATUS Status;
> +  EFI_EVENT  EndOfDxeEvent;
> +
> +  DEBUG ((DEBUG_INFO, "%a: NVRAM Clear is %d\n", PcdGetBool (PcdNvramErased), __FUNCTION__));
> +
> +  if (PcdGetBool (PcdNvramErased)) {
> +    DEBUG ((DEBUG_INFO, "%a: Register event to recover boot options\n", __FUNCTION__));
> +    Status = gBS->CreateEventEx (
> +                    EVT_NOTIFY_SIGNAL,
> +                    TPL_CALLBACK,
> +                    RecoveryCallback,
> +                    NULL,
> +                    &gEfiEndOfDxeEventGroupGuid,
> +                    &EndOfDxeEvent
> +                    );
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  return Status;
> +}
> -- 
> 2.17.1
>