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: ray.ni@intel.com) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by groups.io with SMTP; Mon, 03 Jun 2019 23:16:58 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Jun 2019 23:16:58 -0700 X-ExtLoop1: 1 Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga001.fm.intel.com with ESMTP; 03 Jun 2019 23:16:58 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 3 Jun 2019 23:16:57 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.137]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.10]) with mapi id 14.03.0415.000; Tue, 4 Jun 2019 14:16:56 +0800 From: "Ni, Ray" To: "devel@edk2.groups.io" , "Gao, Zhichao" CC: "Wang, Jian J" , "Wu, Hao A" , "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: AQHVGnGhO6Xd4Rf9W0SJMf6DiuF5t6aLBP6w Date: Tue, 4 Jun 2019 06:16:55 +0000 Deferred-Delivery: Tue, 4 Jun 2019 06:16:00 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C18E124@SHSMSX104.ccr.corp.intel.com> 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: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDAyMjRhNzktZTFmYi00NTEzLTg4OTItMTc0Y2UwNTFiNzMzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiYzRWSGVjSGgwZGt3Q0g0MzVzSWp0XC9RZlJnZ0k0U3REQ2xUNG9leGt6dEZ3NGcyRFJYYmZlOWRaVVU2NFFMMlYifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Ray Ni > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Gao, Zhichao > Sent: Tuesday, June 4, 2019 9:05 AM > To: devel@edk2.groups.io > Cc: Wang, Jian J ; Wu, Hao A = ; > Ni, Ray ; Zeng, Star ; Gao, Limin= g > ; Sean Brogan ; > 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 th= e > variable. But it should remain the behavior of booting from a default fi= le > 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 entered. > # > -# 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 sup= ports > Platform Recovery. > + // > + if (PcdGetBool (PcdPlatformRecoverySupport)) { > + 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; > + } > } > + 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