From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id DE2681A1E46 for ; Wed, 26 Oct 2016 18:48:28 -0700 (PDT) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP; 26 Oct 2016 18:48:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,404,1473145200"; d="scan'208,217";a="24162803" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga005.fm.intel.com with ESMTP; 26 Oct 2016 18:48:28 -0700 Received: from fmsmsx126.amr.corp.intel.com (10.18.125.43) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 26 Oct 2016 18:48:28 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX126.amr.corp.intel.com (10.18.125.43) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 26 Oct 2016 18:48:27 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.206]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.139]) with mapi id 14.03.0248.002; Thu, 27 Oct 2016 09:48:25 +0800 From: "Yao, Jiewen" To: "Kinney, Michael D" , "edk2-devel@lists.01.org" CC: "Tian, Feng" , "Gao, Liming" , "Zeng, Star" , "Zhang, Chao B" Thread-Topic: [edk2] [PATCH V4 6/8] QuarkPlatformPkg/PlatformBootManager: Add capsule/recovery handling. Thread-Index: AQHSLNWqxZFcS/UIi0aSz5SwrM2xhKC647oAgACqY8A= Date: Thu, 27 Oct 2016 01:48:25 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C50386B82CD@shsmsx102.ccr.corp.intel.com> References: <1477189908-8336-1-git-send-email-jiewen.yao@intel.com> <1477189908-8336-7-git-send-email-jiewen.yao@intel.com> In-Reply-To: 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 X-Content-Filtered-By: Mailman/MimeDel 2.1.21 Subject: Re: [PATCH V4 6/8] QuarkPlatformPkg/PlatformBootManager: Add capsule/recovery handling. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Oct 2016 01:48:29 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Yes, Mike. You are right. We do have plan to make it generic. As we discussed before, we will enhance= the tool to detect such info and set a PCD at build time. Then we can incl= ude that in the UiApp driver to report such error directly. The tool is not ready yet. So current platform solution is just a temporary= solution. We do have plan to migrate to the new solution after the tool is ready. The= n we can clean up the platform BDS code. Thank you Yao Jiewen From: Kinney, Michael D Sent: Thursday, October 27, 2016 7:36 AM To: Yao, Jiewen ; edk2-devel@lists.01.org; Kinney, Mi= chael D Cc: Tian, Feng ; Gao, Liming ; Z= eng, Star ; Zhang, Chao B Subject: RE: [edk2] [PATCH V4 6/8] QuarkPlatformPkg/PlatformBootManager: Ad= d capsule/recovery handling. Jiewen, Can the code that checks for the use of a test key be moved into a common B= DS lib or module? Maybe in MdeModulePkg\Universal\BdsDxe\BdsEntry.c right before the call to PlatformBootManagerAfterConsole()? The logic in BdsEntry.c can do the chec= k and set the PcdTestKeyUsed PCD and can go a DEBUG() message for the use of a test key. With the current design, you depend on a platform specific BDS library to i= nclude the test key check, and we want to make sure the check for the use of a test key is = always performed. Also, the test key check against PcdRsa2048Sha256PublicKeyBuffer is incompl= ete. The DEC file description of this PCD is as follows: ## Provides one or more SHA 256 Hashes of the RSA 2048 public keys used t= o verify Recovery and Capsule Update images # WARNING: The default value is treated as test key. Please do not use d= efault value in the production. # @Prompt One or more SHA 256 Hashes of RSA 2048 bit public keys used to = verify Recovery and Capsule Update images # gEfiSecurityPkgTokenSpaceGuid.PcdRsa2048Sha256PublicKeyBuffer|{0x91, 0x29= , 0xc4, 0xbd, 0xea, 0x6d, 0xda, 0xb3, 0xaa, 0x6f, 0x50, 0x16, 0xfc, 0xdb, 0= x4b, 0x7e, 0x3c, 0xd6, 0xdc, 0xa4, 0x7a, 0x0e, 0xdd, 0xe6, 0x15, 0x8c, 0x73= , 0x96, 0xa2, 0xd4, 0xa6, 0x4d}|VOID*|0x00010013 Since this PCD provides one or more SHA 256 Hashes, the check for the use o= f a test key needs to get the Size, determine how many hashes are in this PCD, and compare the test key v= alue against each entry in this array. Thanks, Mike > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ji= ewen Yao > Sent: Saturday, October 22, 2016 7:32 PM > To: edk2-devel@lists.01.org > Cc: Tian, Feng >; Gao, Li= ming >; Zeng, Star > >; Kinney, Michael D >; Zhang, Chao B > > > Subject: [edk2] [PATCH V4 6/8] QuarkPlatformPkg/PlatformBootManager: Add > capsule/recovery handling. > > 1) Add capsule and recovery boot path handling in platform BDS. > 2) Add check if the platform is using default test key for recovery or up= date. > Produce PcdTestKeyUsed to indicate if there is any > test key used in current BIOS, such as recovery key, > or capsule update key. > Then the generic UI may consume this PCD to show warning information. > > Cc: Michael D Kinney > > Cc: Kelly Steele > > Cc: Feng Tian > > Cc: Star Zeng > > Cc: Liming Gao > > Cc: Chao Zhang > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiewen Yao > > --- > QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c = | 131 > +++++++++++++++++++- > QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.h = | 9 +- > QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.i= nf | 16 ++- > 3 files changed, 151 insertions(+), 5 deletions(-) > > diff --git a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBoot= Manager.c > b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c > index 19ff3d0..f327c89 100644 > --- a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager= .c > +++ b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager= .c > @@ -2,7 +2,7 @@ > This file include all platform action which can be customized > by IBV/OEM. > > -Copyright (c) 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BS= D License > which accompanies this distribution. The full text of the license may b= e found at > @@ -205,6 +205,8 @@ PlatformBootManagerBeforeConsole ( > EFI_INPUT_KEY Enter; > EFI_INPUT_KEY F2; > EFI_BOOT_MANAGER_LOAD_OPTION BootOption; > + ESRT_MANAGEMENT_PROTOCOL *EsrtManagement; > + EFI_BOOT_MODE BootMode; > EFI_ACPI_S3_SAVE_PROTOCOL *AcpiS3Save; > EFI_HANDLE Handle; > EFI_EVENT EndOfDxeEvent; > @@ -246,6 +248,40 @@ PlatformBootManagerBeforeConsole ( > // > PlatformRegisterFvBootOption (&mUefiShellFileGuid, L"UEFI Shell", > LOAD_OPTION_ACTIVE); > > + Status =3D gBS->LocateProtocol(&gEsrtManagementProtocolGuid, NULL, (VO= ID > **)&EsrtManagement); > + if (EFI_ERROR(Status)) { > + EsrtManagement =3D NULL; > + } > + > + BootMode =3D GetBootModeHob(); > + switch (BootMode) { > + case BOOT_ON_FLASH_UPDATE: > + DEBUG((EFI_D_INFO, "ProcessCapsules Before EndOfDxe ......\n")); > + Status =3D ProcessCapsules (); > + DEBUG((EFI_D_INFO, "ProcessCapsules %r\n", Status)); > + break; > + case BOOT_IN_RECOVERY_MODE: > + break; > + case BOOT_ASSUMING_NO_CONFIGURATION_CHANGES: > + case BOOT_WITH_MINIMAL_CONFIGURATION: > + case BOOT_ON_S4_RESUME: > + if (EsrtManagement !=3D NULL) { > + // > + // Lock ESRT cache repository before EndofDxe if ESRT sync is not = needed > + // > + EsrtManagement->LockEsrtRepository(); > + } > + break; > + default: > + // > + // Require to sync ESRT from FMP in a new boot > + // > + if (EsrtManagement !=3D NULL) { > + EsrtManagement->SyncEsrtFmp(); > + } > + break; > + } > + > // > // Prepare for S3 > // > @@ -303,7 +339,64 @@ PlatformBootManagerAfterConsole ( > VOID > ) > { > - EFI_STATUS Status; > + EFI_STATUS Status; > + EFI_BOOT_MODE BootMode; > + ESRT_MANAGEMENT_PROTOCOL *EsrtManagement; > + VOID *Buffer; > + UINTN Size; > + > + Status =3D gBS->LocateProtocol(&gEsrtManagementProtocolGuid, NULL, (VO= ID > **)&EsrtManagement); > + if (EFI_ERROR(Status)) { > + EsrtManagement =3D NULL; > + } > + > + BootMode =3D GetBootModeHob(); > + switch (BootMode) { > + case BOOT_ON_FLASH_UPDATE: > + DEBUG((EFI_D_INFO, "Capsule Mode detected\n")); > + if (FeaturePcdGet(PcdSupportUpdateCapsuleReset)) { > + EfiBootManagerConnectAll (); > + EfiBootManagerRefreshAllBootOption (); > + > + // > + // Always sync ESRT Cache from FMP Instances after connect all and= before > capsule process > + // > + if (EsrtManagement !=3D NULL) { > + EsrtManagement->SyncEsrtFmp(); > + } > + > + DEBUG((EFI_D_INFO, "ProcessCapsules After ConnectAll ......\n")); > + Status =3D ProcessCapsules(); > + DEBUG((EFI_D_INFO, "ProcessCapsules %r\n", Status)); > + } > + break; > + > + case BOOT_IN_RECOVERY_MODE: > + DEBUG((EFI_D_INFO, "Recovery Mode detected\n")); > + // Passthrough > + > + case BOOT_ASSUMING_NO_CONFIGURATION_CHANGES: > + case BOOT_WITH_MINIMAL_CONFIGURATION: > + case BOOT_WITH_FULL_CONFIGURATION: > + case BOOT_WITH_FULL_CONFIGURATION_PLUS_DIAGNOSTICS: > + case BOOT_WITH_DEFAULT_SETTINGS: > + default: > + EfiBootManagerConnectAll (); > + EfiBootManagerRefreshAllBootOption (); > + > + // > + // Sync ESRT Cache from FMP Instance on demand after Connect All > + // > + if ((BootMode !=3D BOOT_ASSUMING_NO_CONFIGURATION_CHANGES) && > + (BootMode !=3D BOOT_WITH_MINIMAL_CONFIGURATION) && > + (BootMode !=3D BOOT_ON_S4_RESUME)) { > + if (EsrtManagement !=3D NULL) { > + EsrtManagement->SyncEsrtFmp(); > + } > + } > + > + break; > + } > > Print ( > L"\n" > @@ -313,6 +406,40 @@ PlatformBootManagerAfterConsole ( > ); > > // > + // Check if the platform is using test key. > + // > + Status =3D GetSectionFromAnyFv( > + PcdGetPtr(PcdEdkiiRsa2048Sha256TestPublicKeyFileGuid), > + EFI_SECTION_RAW, > + 0, > + &Buffer, > + &Size > + ); > + if (!EFI_ERROR(Status)) { > + if ((Size =3D=3D PcdGetSize(PcdRsa2048Sha256PublicKeyBuffer)) && > + (CompareMem(Buffer, PcdGetPtr(PcdRsa2048Sha256PublicKeyBuffer), = Size) =3D=3D 0)) { > + Print(L"WARNING: Recovery Test Key is used.\n"); > + PcdSetBoolS(PcdTestKeyUsed, TRUE); > + } > + FreePool(Buffer); > + } > + Status =3D GetSectionFromAnyFv( > + PcdGetPtr(PcdEdkiiPkcs7TestPublicKeyFileGuid), > + EFI_SECTION_RAW, > + 0, > + &Buffer, > + &Size > + ); > + if (!EFI_ERROR(Status)) { > + if ((Size =3D=3D PcdGetSize(PcdPkcs7CertBuffer)) && > + (CompareMem(Buffer, PcdGetPtr(PcdPkcs7CertBuffer), Size) =3D=3D = 0)) { > + Print(L"WARNING: Capsule Test Key is used.\n"); > + PcdSetBoolS(PcdTestKeyUsed, TRUE); > + } > + FreePool(Buffer); > + } > + > + // > // Use a DynamicHii type pcd to save the boot status, which is used to > // control configuration mode, such as FULL/MINIMAL/NO_CHANGES configu= ration. > // > diff --git a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBoot= Manager.h > b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.h > index 7413883..395f78b 100644 > --- a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager= .h > +++ b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager= .h > @@ -1,7 +1,7 @@ > /** @file > Head file for BDS Platform specific code > > -Copyright (c) 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BS= D License > which accompanies this distribution. The full text of the license may b= e found at > @@ -21,6 +21,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITH= ER EXPRESS OR > IMPLIED. > #include > #include > #include > +#include > #include > #include > #include > @@ -32,9 +33,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EIT= HER EXPRESS OR > IMPLIED. > #include > #include > #include > +#include > #include > #include > - > +#include > +#include > +#include > +#include > > typedef struct { > EFI_DEVICE_PATH_PROTOCOL *DevicePath; > diff --git a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBoot= ManagerLib.inf > b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.= inf > index d59f14a..eadf1fe 100644 > --- a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager= Lib.inf > +++ b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager= Lib.inf > @@ -1,7 +1,7 @@ > ## @file > # Include all platform action which can be customized by IBV/OEM. > # > -# Copyright (c) 2012 - 2015, Intel Corporation. All rights reserved. > +# Copyright (c) 2012 - 2016, Intel Corporation. All rights reserved. > # This program and the accompanying materials > # are licensed and made available under the terms and conditions of the= BSD License > # which accompanies this distribution. The full text of the license ma= y be found at > @@ -38,6 +38,8 @@ > IntelFrameworkPkg/IntelFrameworkPkg.dec > IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec > SourceLevelDebugPkg/SourceLevelDebugPkg.dec > + QuarkPlatformPkg/QuarkPlatformPkg.dec > + SecurityPkg/SecurityPkg.dec > > [LibraryClasses] > BaseLib > @@ -49,11 +51,16 @@ > UefiBootServicesTableLib > UefiLib > UefiBootManagerLib > + PrintLib > + HobLib > + CapsuleLib > + DxeServicesLib > > [Protocols] > gEfiFirmwareVolume2ProtocolGuid > gEfiAcpiS3SaveProtocolGuid > gEfiDxeSmmReadyToLockProtocolGuid > + gEsrtManagementProtocolGuid > > [Guids] > gEfiPcAnsiGuid > @@ -70,3 +77,10 @@ > gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits > gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType > gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdBootState > + gEfiMdeModulePkgTokenSpaceGuid.PcdSupportUpdateCapsuleReset > + gQuarkPlatformTokenSpaceGuid.PcdEdkiiRsa2048Sha256TestPublicKeyFileGui= d > + gQuarkPlatformTokenSpaceGuid.PcdEdkiiPkcs7TestPublicKeyFileGuid > + gEfiSecurityPkgTokenSpaceGuid.PcdRsa2048Sha256PublicKeyBuffer > + gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer > + gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed > + > -- > 2.7.4.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel