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

["Nhi Pham" <nhi@os.amperecomputing.com>]

On 6/8/21 06:46, Leif Lindholm wrote:
> 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?

Thanks Leif for the info.

The issue happened since below change  037d86dd7a (20/06/06 14:49) 
ArmPkg/PlatformBootManagerLib: don't connect all devices on each boot 
<Ard Biesheuvel>

 From that commit, System with a NVRAM cleared might try to boot with 
default boot option in Platform Recovery mode. As ReadyToBoot event is 
not triggered in this mode, services like ACPI and PCIe can't complete 
theirs finalizing configurations and cause the OS fails to boot. This 
patch is to prevent the system from entering Platform Recovery mode by 
recovering boot options list each time NVRAM cleared.

Best regards,

Nhi

>
> /
>      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
>>