* [PATCH 0/2] Use a pcd to control Platform Recovery behavior @ 2019-06-03 6:43 Gao, Zhichao 2019-06-03 6:43 ` [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit Gao, Zhichao 2019-06-03 6:43 ` [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control PlatformRecovery Gao, Zhichao 0 siblings, 2 replies; 9+ messages in thread From: Gao, Zhichao @ 2019-06-03 6:43 UTC (permalink / raw) To: devel Cc: Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao, Sean Brogan, Michael Turner, Bret Barkelew V1: Add a pcd PcdPlatformRecoverySupport to control the variable PlatformRecovery#### and the EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY bit of variable "OsIndicationsSupported". V2: While PcdPlatformRecoverySupport is FALSE, do not set a PlatformRecovery#### Variable. But remain boot from a default file path(such as \EFI\BOOT\BOOTX64.EFI). Add memory check before build platform default boot option. Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Hao Wu <hao.a.wu@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Star Zeng <star.zeng@intel.com> Cc: Liming Gao <liming.gao@intel.com> Cc: Sean Brogan <sean.brogan@microsoft.com> Cc: Michael Turner <Michael.Turner@microsoft.com> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> Bret Barkelew (1): MdeModulePkg: Add a pcd to set the OS indications bit Zhichao Gao (1): MdeModulePkg/BdsDxe: Use a pcd to control PlatformRecovery MdeModulePkg/MdeModulePkg.dec | 6 ++ MdeModulePkg/Universal/BdsDxe/BdsDxe.inf | 3 +- MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 89 ++++++++++++++---------- 3 files changed, 61 insertions(+), 37 deletions(-) -- 2.21.0.windows.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit 2019-06-03 6:43 [PATCH 0/2] Use a pcd to control Platform Recovery behavior Gao, Zhichao @ 2019-06-03 6:43 ` Gao, Zhichao 2019-06-03 6:59 ` Ni, Ray 2019-06-03 6:43 ` [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control PlatformRecovery Gao, Zhichao 1 sibling, 1 reply; 9+ messages in thread From: Gao, Zhichao @ 2019-06-03 6:43 UTC (permalink / raw) To: devel Cc: Bret Barkelew, Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao, Sean Brogan, Michael Turner From: Bret Barkelew <Bret.Barkelew@microsoft.com> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1678 Add a pcd PcdPlatformRecoverySupport to conditionally set an OS indications bit and set the 'PlatformRecovery####' variable. This pcd would also control whether the BDS supports platform recovery or not. Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Hao Wu <hao.a.wu@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Star Zeng <star.zeng@intel.com> Cc: Liming Gao <liming.gao@intel.com> Cc: Sean Brogan <sean.brogan@microsoft.com> Cc: Michael Turner <Michael.Turner@microsoft.com> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com> --- MdeModulePkg/MdeModulePkg.dec | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 0a9fcddecc..da2b85770c 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -1660,6 +1660,12 @@ # @Prompt Reset on memory type information change. gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|TRUE|BOOLEAN|0x00010056 + ## Indicates if the BDS supports Platform Recovery.<BR><BR> + # TRUE - BDS supports Platform Recovery.<BR> + # FALSE - BDS does not support Platform Recovery.<BR> + # @Prompt Support Platform Recovery. + gEfiMdeModulePkgTokenSpaceGuid.PcdPlatformRecoverySupport|TRUE|BOOLEAN|0x00010078 + ## Specify the foreground color for Subtile text in HII Form Browser. The default value is EFI_BLUE. # Only following values defined in UEFI specification are valid:<BR><BR> # 0x00 (EFI_BLACK)<BR> -- 2.21.0.windows.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit 2019-06-03 6:43 ` [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit Gao, Zhichao @ 2019-06-03 6:59 ` Ni, Ray 0 siblings, 0 replies; 9+ messages in thread From: Ni, Ray @ 2019-06-03 6:59 UTC (permalink / raw) To: Gao, Zhichao, devel@edk2.groups.io Cc: Bret Barkelew, Wang, Jian J, Wu, Hao A, Zeng, Star, Gao, Liming, Sean Brogan, Michael Turner Reviewed-by: Ray Ni <ray.ni@intel.com> > -----Original Message----- > From: Gao, Zhichao <zhichao.gao@intel.com> > Sent: Monday, June 3, 2019 2:44 PM > To: devel@edk2.groups.io > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J > <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray > <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming > <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; > Michael Turner <Michael.Turner@microsoft.com> > Subject: [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit > > From: Bret Barkelew <Bret.Barkelew@microsoft.com> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1678 > > Add a pcd PcdPlatformRecoverySupport to conditionally set an OS indications > bit and set the 'PlatformRecovery####' > variable. This pcd would also control whether the BDS supports platform > recovery or not. > > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Hao Wu <hao.a.wu@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Star Zeng <star.zeng@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > Cc: Sean Brogan <sean.brogan@microsoft.com> > Cc: Michael Turner <Michael.Turner@microsoft.com> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com> > --- > MdeModulePkg/MdeModulePkg.dec | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/MdeModulePkg/MdeModulePkg.dec > b/MdeModulePkg/MdeModulePkg.dec index 0a9fcddecc..da2b85770c > 100644 > --- a/MdeModulePkg/MdeModulePkg.dec > +++ b/MdeModulePkg/MdeModulePkg.dec > @@ -1660,6 +1660,12 @@ > # @Prompt Reset on memory type information change. > > gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationC > hange|TRUE|BOOLEAN|0x00010056 > > + ## Indicates if the BDS supports Platform Recovery.<BR><BR> > + # TRUE - BDS supports Platform Recovery.<BR> > + # FALSE - BDS does not support Platform Recovery.<BR> > + # @Prompt Support Platform Recovery. > + > + > gEfiMdeModulePkgTokenSpaceGuid.PcdPlatformRecoverySupport|TRUE|B > OOLEAN > + |0x00010078 > + > ## Specify the foreground color for Subtile text in HII Form Browser. The > default value is EFI_BLUE. > # Only following values defined in UEFI specification are valid:<BR><BR> > # 0x00 (EFI_BLACK)<BR> > -- > 2.21.0.windows.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control PlatformRecovery 2019-06-03 6:43 [PATCH 0/2] Use a pcd to control Platform Recovery behavior Gao, Zhichao 2019-06-03 6:43 ` [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit Gao, Zhichao @ 2019-06-03 6:43 ` Gao, Zhichao 2019-06-03 6:59 ` Ni, Ray 1 sibling, 1 reply; 9+ messages in thread From: Gao, Zhichao @ 2019-06-03 6:43 UTC (permalink / raw) To: devel Cc: Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao, Sean Brogan, Michael Turner, Bret Barkelew REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1678 Use the PcdPlatformRecoverySupport to control the function of platform recovery in BDS. First, set the variable's ("OsIndicationsSupported") EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY bit base on the pcd. It would affect the variable "OsIndications". While the platform does not support the platform recovery, it is inappropriate to set a PlatformRecovery#### variable. So skip setting the variable. But it should remain the behavior of booting from a default file path (such as \EFI\BOOT\BOOTX64.EFI) to be compatible with the previous version UEFI spec. Add memory check before build platform default boot option. Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Hao Wu <hao.a.wu@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Star Zeng <star.zeng@intel.com> Cc: Liming Gao <liming.gao@intel.com> Cc: Sean Brogan <sean.brogan@microsoft.com> Cc: Michael Turner <Michael.Turner@microsoft.com> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com> --- MdeModulePkg/Universal/BdsDxe/BdsDxe.inf | 3 +- MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 89 ++++++++++++++---------- 2 files changed, 55 insertions(+), 37 deletions(-) diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf index 6913389d34..7f94ca17df 100644 --- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf +++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf @@ -5,7 +5,7 @@ # gEfiBdsArchProtocolGuid. After DxeCore finish dispatching, DxeCore will invoke Entry # interface of protocol gEfiBdsArchProtocolGuid, then BDS phase is entered. # -# Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR> +# Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.<BR> # SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -95,6 +95,7 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdConInConnectOnDemand ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdErrorCodeSetVariable ## SOMETIMES_CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdPlatformRecoverySupport ## CONSUMES [Depex] TRUE diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c index 9d312bd982..3d84d5a8aa 100644 --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c @@ -546,10 +546,14 @@ BdsFormalizeOSIndicationVariable ( // Status = EfiBootManagerGetBootManagerMenu (&BootManagerMenu); if (Status != EFI_NOT_FOUND) { - OsIndicationSupport = EFI_OS_INDICATIONS_BOOT_TO_FW_UI | EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY; + OsIndicationSupport = EFI_OS_INDICATIONS_BOOT_TO_FW_UI; EfiBootManagerFreeLoadOption (&BootManagerMenu); } else { - OsIndicationSupport = EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY; + OsIndicationSupport = 0; + } + + if (PcdGetBool (PcdPlatformRecoverySupport)) { + OsIndicationSupport |= EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY; } Status = gRT->SetVariable ( @@ -662,6 +666,7 @@ BdsEntry ( BOOLEAN BootSuccess; EFI_DEVICE_PATH_PROTOCOL *FilePath; EFI_STATUS BootManagerMenuStatus; + EFI_BOOT_MANAGER_LOAD_OPTION PlatformDefaultBootOption; HotkeyTriggered = NULL; Status = EFI_SUCCESS; @@ -763,41 +768,45 @@ BdsEntry ( // InitializeLanguage (TRUE); - // - // System firmware must include a PlatformRecovery#### variable specifying - // a short-form File Path Media Device Path containing the platform default - // file path for removable media - // FilePath = FileDevicePath (NULL, EFI_REMOVABLE_MEDIA_FILE_NAME); - Status = EfiBootManagerInitializeLoadOption ( - &LoadOption, - LoadOptionNumberUnassigned, - LoadOptionTypePlatformRecovery, - LOAD_OPTION_ACTIVE, - L"Default PlatformRecovery", - FilePath, - NULL, - 0 - ); - ASSERT_EFI_ERROR (Status); - LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOptionTypePlatformRecovery); - if (EfiBootManagerFindLoadOption (&LoadOption, LoadOptions, LoadOptionCount) == -1) { - for (Index = 0; Index < LoadOptionCount; Index++) { - // - // The PlatformRecovery#### options are sorted by OptionNumber. - // Find the the smallest unused number as the new OptionNumber. - // - if (LoadOptions[Index].OptionNumber != Index) { - break; + if (FilePath != NULL) { + Status = EfiBootManagerInitializeLoadOption ( + &PlatformDefaultBootOption, + LoadOptionNumberUnassigned, + LoadOptionTypePlatformRecovery, + LOAD_OPTION_ACTIVE, + L"Default PlatformRecovery", + FilePath, + NULL, + 0 + ); + ASSERT_EFI_ERROR (Status); + + // + // System firmware must include a PlatformRecovery#### variable specifying + // a short-form File Path Media Device Path containing the platform default + // file path for removable media if the platform supports Platform Recovery. + // + if (PcdGetBool (PcdPlatformRecoverySupport)) { + LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOptionTypePlatformRecovery); + if (EfiBootManagerFindLoadOption (&LoadOption, LoadOptions, LoadOptionCount) == -1) { + for (Index = 0; Index < LoadOptionCount; Index++) { + // + // The PlatformRecovery#### options are sorted by OptionNumber. + // Find the the smallest unused number as the new OptionNumber. + // + if (LoadOptions[Index].OptionNumber != Index) { + break; + } + } + PlatformDefaultBootOption.OptionNumber = Index; + Status = EfiBootManagerLoadOptionToVariable (&PlatformDefaultBootOption); + ASSERT_EFI_ERROR (Status); } + EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); } - LoadOption.OptionNumber = Index; - Status = EfiBootManagerLoadOptionToVariable (&LoadOption); - ASSERT_EFI_ERROR (Status); + FreePool (FilePath); } - EfiBootManagerFreeLoadOption (&LoadOption); - FreePool (FilePath); - EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); // // Report Status Code to indicate connecting drivers will happen @@ -1043,10 +1052,18 @@ BdsEntry ( } if (!BootSuccess) { - LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOptionTypePlatformRecovery); - ProcessLoadOptions (LoadOptions, LoadOptionCount); - EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); + if (PlatformRecovery) { + LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOptionTypePlatformRecovery); + ProcessLoadOptions (LoadOptions, LoadOptionCount); + EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); + } else { + // + // When platform recovery is not enabled, still boot to platform default file path. + // + EfiBootManagerProcessLoadOption (&PlatformDefaultBootOption); + } } + EfiBootManagerFreeLoadOption (&PlatformDefaultBootOption); DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n")); PlatformBootManagerUnableToBoot (); -- 2.21.0.windows.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control PlatformRecovery 2019-06-03 6:43 ` [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control PlatformRecovery Gao, Zhichao @ 2019-06-03 6:59 ` Ni, Ray 2019-06-03 7:13 ` Gao, Zhichao 0 siblings, 1 reply; 9+ messages in thread From: Ni, Ray @ 2019-06-03 6:59 UTC (permalink / raw) To: Gao, Zhichao, devel@edk2.groups.io Cc: Wang, Jian J, Wu, Hao A, Zeng, Star, Gao, Liming, Sean Brogan, Michael Turner, Bret Barkelew > + // > + // When platform recovery is not enabled, still boot to platform default > file path. > + // > + EfiBootManagerProcessLoadOption (&PlatformDefaultBootOption); > + } > } > + EfiBootManagerFreeLoadOption (&PlatformDefaultBootOption); PlatformDefaultBootOption might be uninitialized if FilePath is NULL. FilePath is NULL when memory allocation fails. So I am thinking maybe ASSERT (FilePath != NULL) is enough when EfiBootManagerInitializeLoadOption(). > > DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n")); > PlatformBootManagerUnableToBoot (); > -- > 2.21.0.windows.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control PlatformRecovery 2019-06-03 6:59 ` Ni, Ray @ 2019-06-03 7:13 ` Gao, Zhichao 2019-06-03 7:26 ` Ni, Ray 0 siblings, 1 reply; 9+ messages in thread From: Gao, Zhichao @ 2019-06-03 7:13 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io Cc: Wang, Jian J, Wu, Hao A, Zeng, Star, Gao, Liming, Sean Brogan, Michael Turner, Bret Barkelew > -----Original Message----- > From: Ni, Ray > Sent: Monday, June 3, 2019 2:59 PM > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; > Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; > Sean Brogan <sean.brogan@microsoft.com>; Michael Turner > <Michael.Turner@microsoft.com>; Bret Barkelew > <Bret.Barkelew@microsoft.com> > Subject: RE: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control > PlatformRecovery > > > + // > > + // When platform recovery is not enabled, still boot to > > + platform default > > file path. > > + // > > + EfiBootManagerProcessLoadOption (&PlatformDefaultBootOption); > > + } > > } > > + EfiBootManagerFreeLoadOption (&PlatformDefaultBootOption); > > PlatformDefaultBootOption might be uninitialized if FilePath is NULL. EfiBootManagerFreeLoadOption would check the pointer before free. So it is safe regardless of the PlatformDefaultBootOption is initialized or not. > > FilePath is NULL when memory allocation fails. > So I am thinking maybe ASSERT (FilePath != NULL) is enough when > EfiBootManagerInitializeLoadOption(). Maybe it is not enough. The ASSERT only work in DEBUG build. Did I take a mistake? Thanks, Zhichao > > > > > > > DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n")); > > PlatformBootManagerUnableToBoot (); > > -- > > 2.21.0.windows.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control PlatformRecovery 2019-06-03 7:13 ` Gao, Zhichao @ 2019-06-03 7:26 ` Ni, Ray 2019-06-03 7:42 ` Gao, Zhichao 0 siblings, 1 reply; 9+ messages in thread From: Ni, Ray @ 2019-06-03 7:26 UTC (permalink / raw) To: Gao, Zhichao, devel@edk2.groups.io Cc: Wang, Jian J, Wu, Hao A, Zeng, Star, Gao, Liming, Sean Brogan, Michael Turner, Bret Barkelew > -----Original Message----- > From: Gao, Zhichao <zhichao.gao@intel.com> > Sent: Monday, June 3, 2019 3:13 PM > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; > Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; > Sean Brogan <sean.brogan@microsoft.com>; Michael Turner > <Michael.Turner@microsoft.com>; Bret Barkelew > <Bret.Barkelew@microsoft.com> > Subject: RE: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control > PlatformRecovery > > > > > -----Original Message----- > > From: Ni, Ray > > Sent: Monday, June 3, 2019 2:59 PM > > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A > > <hao.a.wu@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming > > <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; > > Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew > > <Bret.Barkelew@microsoft.com> > > Subject: RE: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control > > PlatformRecovery > > > > > + // > > > + // When platform recovery is not enabled, still boot to > > > + platform default > > > file path. > > > + // > > > + EfiBootManagerProcessLoadOption (&PlatformDefaultBootOption); > > > + } > > > } > > > + EfiBootManagerFreeLoadOption (&PlatformDefaultBootOption); > > > > PlatformDefaultBootOption might be uninitialized if FilePath is NULL. > > EfiBootManagerFreeLoadOption would check the pointer before free. So it is > safe regardless of the PlatformDefaultBootOption is initialized or not. > > > > > FilePath is NULL when memory allocation fails. > > So I am thinking maybe ASSERT (FilePath != NULL) is enough when > > EfiBootManagerInitializeLoadOption(). > > Maybe it is not enough. The ASSERT only work in DEBUG build. Did I take a > mistake? You are right. How about do a DEBUG print followed by a CpuDeadLoop() when FilePath is NULL? Then all code below that point doesn't need to take care this error any more. > > Thanks, > Zhichao > > > > > > > > > > > > > DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n")); > > > PlatformBootManagerUnableToBoot (); > > > -- > > > 2.21.0.windows.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control PlatformRecovery 2019-06-03 7:26 ` Ni, Ray @ 2019-06-03 7:42 ` Gao, Zhichao 0 siblings, 0 replies; 9+ messages in thread From: Gao, Zhichao @ 2019-06-03 7:42 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io Cc: Wang, Jian J, Wu, Hao A, Zeng, Star, Gao, Liming, Sean Brogan, Michael Turner, Bret Barkelew > -----Original Message----- > From: Ni, Ray > Sent: Monday, June 3, 2019 3:27 PM > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; > Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; > Sean Brogan <sean.brogan@microsoft.com>; Michael Turner > <Michael.Turner@microsoft.com>; Bret Barkelew > <Bret.Barkelew@microsoft.com> > Subject: RE: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control > PlatformRecovery > > > > > -----Original Message----- > > From: Gao, Zhichao <zhichao.gao@intel.com> > > Sent: Monday, June 3, 2019 3:13 PM > > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A > > <hao.a.wu@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming > > <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; > > Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew > > <Bret.Barkelew@microsoft.com> > > Subject: RE: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control > > PlatformRecovery > > > > > > > > > -----Original Message----- > > > From: Ni, Ray > > > Sent: Monday, June 3, 2019 2:59 PM > > > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A > > > <hao.a.wu@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming > > > <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; > > > Michael Turner <Michael.Turner@microsoft.com>; Bret Barkelew > > > <Bret.Barkelew@microsoft.com> > > > Subject: RE: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control > > > PlatformRecovery > > > > > > > + // > > > > + // When platform recovery is not enabled, still boot to > > > > + platform default > > > > file path. > > > > + // > > > > + EfiBootManagerProcessLoadOption (&PlatformDefaultBootOption); > > > > + } > > > > } > > > > + EfiBootManagerFreeLoadOption (&PlatformDefaultBootOption); > > > > > > PlatformDefaultBootOption might be uninitialized if FilePath is NULL. > > > > EfiBootManagerFreeLoadOption would check the pointer before free. So > > it is safe regardless of the PlatformDefaultBootOption is initialized or not. > > > > > > > > FilePath is NULL when memory allocation fails. > > > So I am thinking maybe ASSERT (FilePath != NULL) is enough when > > > EfiBootManagerInitializeLoadOption(). > > > > Maybe it is not enough. The ASSERT only work in DEBUG build. Did I > > take a mistake? > > You are right. > How about do a DEBUG print followed by a CpuDeadLoop() when FilePath is > NULL? > > Then all code below that point doesn't need to take care this error any more. I am OK with that because fail to allocate memory should be a serious error that would make the boot flow work incorrect. And putting the system into a dead loop to indicate the failure of boot is fine. > > > > > Thanks, > > Zhichao > > > > > > > > > > > > > > > > > > > DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n")); > > > > PlatformBootManagerUnableToBoot (); > > > > -- > > > > 2.21.0.windows.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/2] Use a pcd to control Platform Recovery behavior @ 2019-04-02 5:49 Zhichao Gao 2019-04-02 5:49 ` [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit Zhichao Gao 0 siblings, 1 reply; 9+ messages in thread From: Zhichao Gao @ 2019-04-02 5:49 UTC (permalink / raw) To: edk2-devel Add a pcd PcdPlatformRecoverySupport to control the variable PlatformRecovery#### and the EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY bit of variable "OsIndicationsSupported". Bret Barkelew (2): MdeModulePkg: Add a pcd to set the OS indications bit MdeModulePkg/BdsDxe: Use a pcd to set OS indications bit MdeModulePkg/MdeModulePkg.dec | 6 ++ MdeModulePkg/Universal/BdsDxe/BdsDxe.inf | 1 + MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 74 +++++++++++++----------- 3 files changed, 47 insertions(+), 34 deletions(-) -- 2.21.0.windows.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit 2019-04-02 5:49 [PATCH 0/2] Use a pcd to control Platform Recovery behavior Zhichao Gao @ 2019-04-02 5:49 ` Zhichao Gao 0 siblings, 0 replies; 9+ messages in thread From: Zhichao Gao @ 2019-04-02 5:49 UTC (permalink / raw) To: edk2-devel Cc: Bret Barkelew, Jian J Wang, Ray Ni, Star Zeng, Liming Gao, Sean Brogan, Michael Turner From: Bret Barkelew <Bret.Barkelew@microsoft.com> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1678 Add a pcd PcdPlatformRecoverySupport to conditionally set an OS indications bit. Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Star Zeng <star.zeng@intel.com> Cc: Liming Gao <liming.gao@intel.com> Cc: Sean Brogan <sean.brogan@microsoft.com> Cc: Michael Turner <Michael.Turner@microsoft.com> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Zhichao Gao <zhichao.gao@intel.com> --- MdeModulePkg/MdeModulePkg.dec | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 43992b7f65..870f49749a 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -1669,6 +1669,12 @@ # @Prompt Reset on memory type information change. gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|TRUE|BOOLEAN|0x00010056 + ## Indicates if the BDS supports Platform Recovery.<BR><BR> + # TRUE - Platform supports Platform Recovery.<BR> + # FALSE - Does not add Boot Manager to boot order.<BR> + # @Prompt Support Platform Recovery. + gEfiMdeModulePkgTokenSpaceGuid.PcdPlatformRecoverySupport|TRUE|BOOLEAN|0x00010078 + ## Specify the foreground color for Subtile text in HII Form Browser. The default value is EFI_BLUE. # Only following values defined in UEFI specification are valid:<BR><BR> # 0x00 (EFI_BLACK)<BR> -- 2.21.0.windows.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-06-03 7:42 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-06-03 6:43 [PATCH 0/2] Use a pcd to control Platform Recovery behavior Gao, Zhichao 2019-06-03 6:43 ` [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit Gao, Zhichao 2019-06-03 6:59 ` Ni, Ray 2019-06-03 6:43 ` [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to control PlatformRecovery Gao, Zhichao 2019-06-03 6:59 ` Ni, Ray 2019-06-03 7:13 ` Gao, Zhichao 2019-06-03 7:26 ` Ni, Ray 2019-06-03 7:42 ` Gao, Zhichao -- strict thread matches above, loose matches on Subject: below -- 2019-04-02 5:49 [PATCH 0/2] Use a pcd to control Platform Recovery behavior Zhichao Gao 2019-04-02 5:49 ` [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit Zhichao Gao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox