From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 1B7D481DB2 for ; Mon, 7 Nov 2016 14:22:40 -0800 (PST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga101.jf.intel.com with ESMTP; 07 Nov 2016 14:22:42 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,459,1473145200"; d="scan'208";a="28472449" Received: from orsmsx102.amr.corp.intel.com ([10.22.225.129]) by fmsmga006.fm.intel.com with ESMTP; 07 Nov 2016 14:22:42 -0800 Received: from orsmsx160.amr.corp.intel.com (10.22.226.43) by ORSMSX102.amr.corp.intel.com (10.22.225.129) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 7 Nov 2016 14:22:42 -0800 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.250]) by ORSMSX160.amr.corp.intel.com ([10.22.226.43]) with mapi id 14.03.0248.002; Mon, 7 Nov 2016 14:22:41 -0800 From: "Kinney, Michael D" To: "Yao, Jiewen" , "edk2-devel@lists.01.org" , "Kinney, Michael D" CC: "Tian, Feng" , "Zeng, Star" , "Gao, Liming" , "Zhang, Chao B" , "Fan, Jeff" Thread-Topic: [PATCH V9 0/7] Add capsule support for Quark. Thread-Index: AQHSOPRKzAeWagIHnkWQ4sRb3+AVZ6DOGOPg Date: Mon, 7 Nov 2016 22:22:40 +0000 Message-ID: References: <1478522495-9248-1-git-send-email-jiewen.yao@intel.com> In-Reply-To: <1478522495-9248-1-git-send-email-jiewen.yao@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOGI5OWQ1NjctNjdlOS00ODQyLWI0MDItMGVmNjdlNTJjODczIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6InBZdWtzaThUVGtXejROb2FnU0tuczhmOE5KKzZ5RmRqXC9ZdDlyQkRGYVdVPSJ9 x-originating-ip: [10.22.254.138] MIME-Version: 1.0 Subject: Re: [PATCH V9 0/7] Add capsule support for Quark. 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: Mon, 07 Nov 2016 22:22:40 -0000 Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Jiewen, Thank you for all the updates through the versions of these patch series. I have tested this patch series on Galileo platforms in the QuarkPlatformPk= g. Series: Reviewed-by: Michael Kinney Tested-by: Michael Kinney Thanks, Mike > -----Original Message----- > From: Yao, Jiewen > Sent: Monday, November 7, 2016 4:41 AM > To: edk2-devel@lists.01.org > Cc: Tian, Feng ; Zeng, Star ; K= inney, Michael > D ; Gao, Liming ; Zhang= , Chao B > ; Fan, Jeff > Subject: [PATCH V9 0/7] Add capsule support for Quark. >=20 > =3D=3DBelow is V9 description=3D=3D > 1) SignedCapsulePkg: Add more detail description in EdkiiSystemFmpCapsule= .h > 2) SignedCapsulePkg: Force FileGuid in INI file to be mandatory. > 3) SignedCapsulePkg: Fix FV alignment issue in RecoveryPeim. > (Thanks Mike Kinney's great help to narrow down the issue) > 4) PlatformPkg: Descriptor use sizeof(string) instead of hardcode 16. > 5) QuarkPkg: Add PayloadFv to be 2nd FV for recovery. > 6) Vlv2Pkg: Sync to latest codebase and resolve conflict. > 7) All: Update some NULL pointer check. > 8) All: Clean up commit message. >=20 > =3D=3DBelow is V8 description=3D=3D > 1) MdeModulePkg/dec: > set gEfiMdeModulePkgTokenSpaceGuid.PcdSystemFmpCapsuleImageTypeIdGuid > to 0 as default. > 2) SignedCapsulePkg/dec: > set gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareFileGuid > to 0 as default. > 3) QuarkPlatformPkg: Set CAPSULE_ENABLE/RECOVERY_ENABLE to FALSE as defau= lt. > 4) All: rename EFI_D_INFO =3D> DEBUG_INFO >=20 > =3D=3DBelow is V7 description=3D=3D > 1) MdeModulePkg/MdeModulePkg.dec: refine status code comment. > 2) UefiCpuPkg: Move Microcode capsule related conent to Feature/capsule d= ir. > 3) Vlv2TbltDevicePkg: Add MICOCODE_CAPSULE_ENABLE macro. >=20 > Only series 1, 3, 5 are sent for update review. > The other series are unchanged. >=20 > =3D=3DBelow is V6 description=3D=3D > 1) MdeModulePkg/CapsuleApp: Fix -D issue. > 2) MdeModulePkg/DEC: Cleanup Capsule related StatusCode. > 3) UefiCpuPkg: Remove MicrocodeUpdateApp > 4) UefiCpuPkg: Add Microcode FMP build sample >=20 > Only series 1 and 3 are sent for update review. > The other series are unchanged. >=20 > =3D=3DBelow is V5 description=3D=3D > 1) MdeModulePkg/CapsuleApp: Remove [NR]. Add more description. > 2) MdeModulePkg/DEC: Update StatusCode to OEM region. > 3) MdeModulePkg/DxeCapsuleLib: Use NULL ProcessCapsules() > for runtime lib, because it is not needed for runtime. > 4) MdeModulePkg/FmpAuthenticationLib: Add more description. > 5) SignedCapsulePkg/DEC: Add data structure description > for PcdEdkiiSystemFirmwareImageDescriptor. > 6) SignedCapsulePkg/DEC: Add Pkcs7 and Rsa2048 Key file PCD. > These 2 PCD are moved from platform pkg to SignedCapsulePkg. > 7) QuarkPlatformPkg/FDF: Refine order of capsule section. > 8) Fix typo and coding style issue. >=20 > Below items are defered to other patch series, because > the tool and library are not ready yet. >=20 > A) MdeModulePkg/DxeCapsuleLib: separate BMP parsing logic > to another library. > That is very good suggestion, and we agree it is a right direction. > I discussed with the owner of image decoder. > We prefer adding a generic library class to convert > the image data to GOP BLT buffer. It supports *any* image format, > not only BMP. The owner of image decoder will drive the new design. > I filed https://bugzilla.tianocore.org/show_bug.cgi?id=3D175 to track tha= t. > I suggest we just keep the current solution as a temp solution and > migrate to the new one once it is ready later. >=20 > B) PlatformPkg/Bds: Move test key check logic to generic part. > This is very good suggestion and we are discussing with Tool > team to add such detection at build time and set a PCD to indicate that. > The generic code can use this PCD to know if there is a test key. > I filed https://bugzilla.tianocore.org/show_bug.cgi?id=3D185 to track tha= t. > Adding such check in the generic code is very complicated, so current > temporary solution is to let platform BDS do such check. > The platform BDS will be cleaned up, once the tool is ready. >=20 > =3D=3DBelow is V4 description=3D=3D > 1) SecurityPkg - Refine AuthenticateFmpImage() API to let caller > input PublicKeyData and PublicKeyDataLength, instead of PCD. > The benefit is that then this API can be used for a platform > which stores PublicKeyData in anywhere other than PCD. > 2) SecurityPkg - Use OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData) > for better understanding the code. > 3) MdeModulePkg - Update CapsuleApp to let it consume > ShellParameters protocol to get Argc and Argv. > 4) UefiCpuPkg - Update MicrocodeCapsuleApp to let it consume > ShellParameters protocol to get Argc and Argv. > 5) QuarkPlatformPkg - Merge QuarkCapsule.fdf to Quark.fdf. >=20 > =3D=3DBelow is V3 description=3D=3D > 1) We move all EDKII related capsule definition to SignedCapsulePkg. > MdeModulePkg only contains FmAuthenticationLib and CapsuleApp, > because they are generic and follow UEFI specification on FMP/ESRT > and Microsoft platform firmware update document. > Any capsule implementation can use them. >=20 > Here is full library classes: > MdeModulePkg: > FmpAuthenticationLib.h: new lib - follow UEFI spec. (*) > Verify FMP signature of FMP Capsule > CapsuleLib.h: new API =A8C ProcessCapsules() > It processes all the capsules. Remove duplicated code in platform BDS. > UefiCpuPkg: > MicrocodeFlashAccessLib.h: Update Microcode region. > SignedCapsulePkg: > EdkiiSystemCapsuleLib.h =A8C Library for EDKII system FMP. > IniParsingLib.h =A8C Library for INI file parsing. > PlatformFlashAccessLib.h =A8C Library for write flash. >=20 > 2) We will submit 5 series. > Series 1: Generic Update (MdeModulePkg/SecurityPkg) > DxeCapsuleLib > FmAuthenticationLib (*) > CapsuleApp (*) > Series 2: EDKII Capsule (SignedCapsulePkg) > IniParsingLib > EdkiiSystemCapsuleLib > PlatformFlashAccessLib > SystemFirmwareUpdate driver > RecoveryModuleLoadPei driver > Series 3: Microcode Update (UefiCpuPkg) > MicrocodeFlashAccessLib > MicrocodeUpdate driver. > Series 4: Quark update > Series 5: Vlv2 update >=20 > 3) DxeCapsuleLib: Move code that performs authentication and parsing of > the capsule format into the implementation of the FMP Protocol. > We move the dispatch FV code from CapsuleLib to SystemFirmwareReport.efi. > SystemFirmwareReport.efi supports SetImage() to verify and dispatch the > SystemFirmwareUpdate.efi, then pass thru SetImage() request to > SystemFirmwareUpdate.efi. >=20 > Now the DxeCapsuleLib is very clean and it does not have any EDKII > capsule format knowledge. >=20 > 4) DxeCapsuleLib: Fix issue where a reset may be too soon. > Defer reset to 2nd pass. >=20 > 5) DxeCapsuleLib: Boot mode check is removed. > Capsule should be populated to system table even boot mode is not BIOS_UP= DATE. >=20 > 5) FmAuthenticationLib: Add zero ImageSize check. >=20 > 6) FmAuthenticationLib: Remove Authentication Library Registration. > Each FMP Producer needs to carry its own auth algoritms(s). > Now we have FmpAuthenticationLibPkcs7 and FmpAuthenticationLibRsa2048Sha2= 56. > No registration is needed. >=20 > 7) FmAuthenticationLib: Move MonotonicCount handling after Payload > We confirmed with USWG to process MonotonicCount after PayLoad. >=20 > =3D=3DBelow is V2 description=3D=3D > The V2 series patch incorporated the feedback for V1. >=20 > There are 3 major updates. > 1) BDS is update to display a warning message if TEST key > is used to sign recovery image or capsule image. > So a production BIOS should always use its own production singing > key for the capsule image generation. A production BIOS should > never use test key. > 2) IniParsingLib is enhanced to do more sanity check for invalid > input. The detail data format is added in IniParsingLib.h header > file. If there is any vialation, the OpenInitFile() API will > return failure. > 3) The *Bios* keyword is renamed to *SystemFirmware* in any > header file or c file data structure definition. >=20 > The rest is minor update, such as add help info, clean > up debug message, coding style. >=20 > =3D=3DBelow is V1 description=3D=3D > This series patch provides sample on how to do signed capsule update > and recovery in EDKII. >=20 > This series patch is also checked into git@github.com:jyao1/edk2.git. >=20 > The feature includes: > 1) Define EDKII signed system BIOS capsule format. > 2) Provide EDKII signed system BIOS update sample. > 3) Provide EDKII signed recovery sample. > 4) Provide Microcode update sample for X86 system. > 5) Update Quark to use new capsule/recovery solution. > 6) Update Vlv2(MinnowMax) to use new capsule/recovery solution. >=20 > The signed capsule/recovery solution is in MdeModulePkg. > The capsule in IntelFrameworkModulePkg is deprecated. > The Microcode update solution is in UefiCpuPkg. >=20 > Cc: Feng Tian > Cc: Star Zeng > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Chao Zhang > Cc: Jeff Fan > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiewen Yao >=20 > Jiewen Yao (7): > QuarkPlatformPkg/PlatformFlashAccessLib: Add instance for update. > QuarkPlatformPkg/SystemFirmwareDescriptor: Add Descriptor for capsule. > QuarkPlatformPkg/SystemFirmwareUpdateConfig: Add capsule config file. > QuarkPlatformPkg/PlatformInit: Remove recovery PPI installation. > QuarkPlatformPkg/PlatformBootManager: Add capsule/recovery handling. > QuarkPlatformPkg/dsc/fdf: Add capsule/recovery support. > QuarkPlatformPkg/Readme: add capsule/recovery related content. >=20 >=20 > QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/PlatformF= lashAccessLibD > xe.c | 206 ++++++++++++ >=20 > QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/PlatformF= lashAccessLibD > xe.inf | 53 +++ > QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/SpiFlash= Device.c > | 336 ++++++++++++++++++++ > QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/SpiFlash= Device.h > | 186 +++++++++++ >=20 > QuarkPlatformPkg/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareD= escriptor.aslc > | 89 ++++++ > QuarkPlatformPkg/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmware= Descriptor.inf > | 46 +++ >=20 > QuarkPlatformPkg/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareD= escriptorPei.c > | 66 ++++ >=20 > QuarkPlatformPkg/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwar= eUpdateConfig. > ini | 63 ++++ > QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c > | 131 +++++++- > QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.h > | 9 +- > QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.i= nf > | 17 +- > QuarkPlatformPkg/Platform/Pei/PlatformInit/MemoryCallback.c > | 3 +- > QuarkPlatformPkg/Quark.dsc > | 73 ++++- > QuarkPlatformPkg/Quark.fdf > | 137 ++++++++ > QuarkPlatformPkg/QuarkMin.dsc > | 7 +- > QuarkPlatformPkg/Readme.md > | 18 ++ > 16 files changed, 1418 insertions(+), 22 deletions(-) > create mode 100644 > QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/PlatformF= lashAccessLibD > xe.c > create mode 100644 > QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/PlatformF= lashAccessLibD > xe.inf > create mode 100644 > QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/SpiFlashD= evice.c > create mode 100644 > QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/SpiFlashD= evice.h > create mode 100644 > QuarkPlatformPkg/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareD= escriptor.aslc > create mode 100644 > QuarkPlatformPkg/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareD= escriptor.inf > create mode 100644 > QuarkPlatformPkg/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareD= escriptorPei.c > create mode 100644 > QuarkPlatformPkg/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwar= eUpdateConfig. > ini >=20 > -- > 2.7.4.windows.1