From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.93, mailfrom: hao.a.wu@intel.com) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by groups.io with SMTP; Thu, 13 Jun 2019 00:39:29 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Jun 2019 00:39:29 -0700 X-ExtLoop1: 1 Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga005.fm.intel.com with ESMTP; 13 Jun 2019 00:39:29 -0700 Received: from fmsmsx154.amr.corp.intel.com (10.18.116.70) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 13 Jun 2019 00:39:29 -0700 Received: from shsmsx108.ccr.corp.intel.com (10.239.4.97) by FMSMSX154.amr.corp.intel.com (10.18.116.70) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 13 Jun 2019 00:39:28 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.185]) by SHSMSX108.ccr.corp.intel.com ([169.254.8.188]) with mapi id 14.03.0415.000; Thu, 13 Jun 2019 15:39:26 +0800 From: "Wu, Hao A" To: "devel@edk2.groups.io" , "Gao, Zhichao" CC: "Wang, Jian J" , "Ni, Ray" , "Zeng, Star" , "Gao, Liming" , Sean Brogan , Michael Turner , Bret Barkelew Subject: Re: [edk2-devel] [PATCH V3 2/2] MdeModulePkg/BdsDxe: Use a pcd to control PlatformRecovery Thread-Topic: [edk2-devel] [PATCH V3 2/2] MdeModulePkg/BdsDxe: Use a pcd to control PlatformRecovery Thread-Index: AQHVGnGeZiLMVfjmKk2YfDMrLWK6W6aZPdnA Date: Thu, 13 Jun 2019 07:39:25 +0000 Message-ID: References: <20190604010515.14352-1-zhichao.gao@intel.com> <20190604010515.14352-3-zhichao.gao@intel.com> In-Reply-To: <20190604010515.14352-3-zhichao.gao@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: hao.a.wu@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable One comment below: > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Gao, Zhichao > Sent: Tuesday, June 04, 2019 9:05 AM > To: devel@edk2.groups.io > Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Zeng, Star; Gao, Liming; Sean Brog= an; > Michael Turner; Bret Barkelew > Subject: [edk2-devel] [PATCH V3 2/2] MdeModulePkg/BdsDxe: Use a pcd to > control PlatformRecovery >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1678 >=20 > 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. >=20 > Add memory check before build platform default boot option. If > fail to allocate memory for the defualt boot file path, put the > system into dead loop to indicate it is unable to boot. >=20 > Cc: Jian J Wang > Cc: Hao Wu > Cc: Ray Ni > Cc: Star Zeng > Cc: Liming Gao > Cc: Sean Brogan > Cc: Michael Turner > Cc: Bret Barkelew > Signed-off-by: Zhichao Gao > --- > MdeModulePkg/Universal/BdsDxe/BdsDxe.inf | 3 +- > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 71 +++++++++++++++------ > --- > 2 files changed, 47 insertions(+), 27 deletions(-) >=20 > 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 w= ill > invoke Entry > # interface of protocol gEfiBdsArchProtocolGuid, then BDS phase is ent= ered. > # > -# Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved. > +# Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved. > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > ## > @@ -95,6 +95,7 @@ > gEfiMdeModulePkgTokenSpaceGuid.PcdConInConnectOnDemand > ## CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdErrorCodeSetVariable #= # > SOMETIMES_CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed #= # > CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdPlatformRecoverySupport > ## CONSUMES >=20 > [Depex] > TRUE > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > index 9d312bd982..8c8a0b5236 100644 > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > @@ -5,7 +5,7 @@ > After DxeCore finish DXE phase, gEfiBdsArchProtocolGuid->BdsEntry wil= l be > invoked > to enter BDS phase. >=20 > -Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2004 - 2019, Intel Corporation. All rights reserved.
> (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
> SPDX-License-Identifier: BSD-2-Clause-Patent > @@ -546,10 +546,14 @@ BdsFormalizeOSIndicationVariable ( > // > Status =3D EfiBootManagerGetBootManagerMenu (&BootManagerMenu); > if (Status !=3D EFI_NOT_FOUND) { > - OsIndicationSupport =3D EFI_OS_INDICATIONS_BOOT_TO_FW_UI | > EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY; > + OsIndicationSupport =3D EFI_OS_INDICATIONS_BOOT_TO_FW_UI; > EfiBootManagerFreeLoadOption (&BootManagerMenu); > } else { > - OsIndicationSupport =3D > EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY; > + OsIndicationSupport =3D 0; > + } > + > + if (PcdGetBool (PcdPlatformRecoverySupport)) { > + OsIndicationSupport |=3D > EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY; > } >=20 > Status =3D gRT->SetVariable ( > @@ -662,6 +666,7 @@ BdsEntry ( > BOOLEAN BootSuccess; > EFI_DEVICE_PATH_PROTOCOL *FilePath; > EFI_STATUS BootManagerMenuStatus; > + EFI_BOOT_MANAGER_LOAD_OPTION PlatformDefaultBootOption; >=20 > HotkeyTriggered =3D NULL; > Status =3D EFI_SUCCESS; > @@ -763,14 +768,13 @@ BdsEntry ( > // > InitializeLanguage (TRUE); >=20 > - // > - // System firmware must include a PlatformRecovery#### variable > specifying > - // a short-form File Path Media Device Path containing the platform d= efault > - // file path for removable media > - // > FilePath =3D FileDevicePath (NULL, EFI_REMOVABLE_MEDIA_FILE_NAME); > + if (FilePath =3D=3D NULL) { > + DEBUG ((DEBUG_ERROR, "Fail to allocate memory for defualt boot file > path. Unable to boot.\n")); > + CpuDeadLoop (); > + } > Status =3D EfiBootManagerInitializeLoadOption ( > - &LoadOption, > + &PlatformDefaultBootOption, > LoadOptionNumberUnassigned, > LoadOptionTypePlatformRecovery, > LOAD_OPTION_ACTIVE, > @@ -780,24 +784,31 @@ BdsEntry ( > 0 > ); > ASSERT_EFI_ERROR (Status); > - LoadOptions =3D EfiBootManagerGetLoadOptions (&LoadOptionCount, > LoadOptionTypePlatformRecovery); > - if (EfiBootManagerFindLoadOption (&LoadOption, LoadOptions, > LoadOptionCount) =3D=3D -1) { > - for (Index =3D 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 !=3D Index) { > - break; > + > + // > + // 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 =3D EfiBootManagerGetLoadOptions (&LoadOptionCount, > LoadOptionTypePlatformRecovery); > + if (EfiBootManagerFindLoadOption (&LoadOption, LoadOptions, > LoadOptionCount) =3D=3D -1) { In the above 'if' statement, 'LoadOption' is not being initialized. Is this a typo here? Best Regards, Hao Wu > + for (Index =3D 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 !=3D Index) { > + break; > + } > } > + PlatformDefaultBootOption.OptionNumber =3D Index; > + Status =3D EfiBootManagerLoadOptionToVariable > (&PlatformDefaultBootOption); > + ASSERT_EFI_ERROR (Status); > } > - LoadOption.OptionNumber =3D Index; > - Status =3D EfiBootManagerLoadOptionToVariable (&LoadOption); > - ASSERT_EFI_ERROR (Status); > + EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); > } > - EfiBootManagerFreeLoadOption (&LoadOption); > FreePool (FilePath); > - EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); >=20 > // > // Report Status Code to indicate connecting drivers will happen > @@ -1043,10 +1054,18 @@ BdsEntry ( > } >=20 > if (!BootSuccess) { > - LoadOptions =3D EfiBootManagerGetLoadOptions (&LoadOptionCount, > LoadOptionTypePlatformRecovery); > - ProcessLoadOptions (LoadOptions, LoadOptionCount); > - EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); > + if (PlatformRecovery) { > + LoadOptions =3D 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); >=20 > DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n")); > PlatformBootManagerUnableToBoot (); > -- > 2.21.0.windows.1 >=20 >=20 >=20