Re: [PATCH 00/12] Add EDKII signed capsule support.

["Kinney, Michael D" <michael.d.kinney@intel.com>]

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 QuarkPlatformPkg.

Series:

Reviewed-by: Michael Kinney <michael.d.kinney@intel.com>
Tested-by: Michael Kinney <michael.d.kinney@intel.com>

Thanks,

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiewen Yao
> Sent: Monday, November 7, 2016 4:40 AM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Gao, Liming <liming.gao@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff
> <jeff.fan@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: [edk2] [PATCH 00/12] Add EDKII signed capsule support.
> 
> ==Below is V9 description==
> 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.
> 
> ==Below is V8 description==
> 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 default.
> 4) All: rename EFI_D_INFO => DEBUG_INFO
> 
> ==Below is V7 description==
> 1) MdeModulePkg/MdeModulePkg.dec: refine status code comment.
> 2) UefiCpuPkg: Move Microcode capsule related conent to Feature/capsule dir.
> 3) Vlv2TbltDevicePkg: Add MICOCODE_CAPSULE_ENABLE macro.
> 
> Only series 1, 3, 5 are sent for update review.
> The other series are unchanged.
> 
> ==Below is V6 description==
> 1) MdeModulePkg/CapsuleApp: Fix -D issue.
> 2) MdeModulePkg/DEC: Cleanup Capsule related StatusCode.
> 3) UefiCpuPkg: Remove MicrocodeUpdateApp
> 4) UefiCpuPkg: Add Microcode FMP build sample
> 
> Only series 1 and 3 are sent for update review.
> The other series are unchanged.
> 
> ==Below is V5 description==
> 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.
> 
> Below items are defered to other patch series, because
> the tool and library are not ready yet.
> 
> 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=175 to track that.
> I suggest we just keep the current solution as a temp solution and
> migrate to the new one once it is ready later.
> 
> 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=185 to track that.
> 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.
> 
> ==Below is V4 description==
> 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.
> 
> ==Below is V3 description==
> 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.
> 
> Here is full library classes:
> MdeModulePkg:
> 	FmpAuthenticationLib.h: new lib - follow UEFI spec. (*)
> 		Verify FMP signature of FMP Capsule
> 	CapsuleLib.h: new API ¨C ProcessCapsules()
> 		It processes all the capsules. Remove duplicated code in platform BDS.
> UefiCpuPkg:
> 	MicrocodeFlashAccessLib.h: Update Microcode region.
> SignedCapsulePkg:
> 	EdkiiSystemCapsuleLib.h ¨C Library for EDKII system FMP.
> 	IniParsingLib.h ¨C Library for INI file parsing.
> 	PlatformFlashAccessLib.h ¨C Library for write flash.
> 
> 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
> 
> 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.
> 
> Now the DxeCapsuleLib is very clean and it does not have any EDKII
> capsule format knowledge.
> 
> 4) DxeCapsuleLib: Fix issue where a reset may be too soon.
> Defer reset to 2nd pass.
> 
> 5) DxeCapsuleLib: Boot mode check is removed.
> Capsule should be populated to system table even boot mode is not BIOS_UPDATE.
> 
> 5) FmAuthenticationLib: Add zero ImageSize check.
> 
> 6) FmAuthenticationLib: Remove Authentication Library Registration.
> Each FMP Producer needs to carry its own auth algoritms(s).
> Now we have FmpAuthenticationLibPkcs7 and FmpAuthenticationLibRsa2048Sha256.
> No registration is needed.
> 
> 7) FmAuthenticationLib: Move MonotonicCount handling after Payload
> We confirmed with USWG to process MonotonicCount after PayLoad.
> 
> ==Below is V2 description==
> The V2 series patch incorporated the feedback for V1.
> 
> 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.
> 
> The rest is minor update, such as add help info, clean
> up debug message, coding style.
> 
> ==Below is V1 description==
> This series patch provides sample on how to do signed capsule update
> and recovery in EDKII.
> 
> This series patch is also checked into git@github.com:jyao1/edk2.git.
> 
> 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.
> 
> The signed capsule/recovery solution is in MdeModulePkg.
> The capsule in IntelFrameworkModulePkg is deprecated.
> The Microcode update solution is in UefiCpuPkg.
> 
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> 
> Jiewen Yao (12):
>   SignedCapsulePkg: Add license file.
>   SignedCapsulePkg/Include: Add EDKII system FMP capsule header.
>   SignedCapsulePkg/Include: Add EdkiiSystemCapsuleLib definition.
>   SignedCapsulePkg/Include: Add IniParsingLib header.
>   SignedCapsulePkg/Include: Add PlatformFlashAccessLib header.
>   SignedCapsulePkg/CapsulePkg.dec: Add capsule related definition.
>   SignedCapsulePkg/IniParsingLib: Add InitParsingLib instance.
>   SignedCapsulePkg/EdkiiSystemCapsuleLib: Add EdkiiSystemCapsuleLib.
>   SignedCapsulePkg/PlatformFlashAccessLib: Add NULL instance.
>   SignedCapsulePkg/SystemFirmwareUpdate: Add SystemFirmwareUpdate.
>   SignedCapsulePkg/RecoveryModuleLoadPei: Add RecoveryModuleLoadPei.
>   SignedCapsulePkg/CapsulePkg.dsc: Add capsule related component.
> 
>  SignedCapsulePkg/Contributions.txt                                                 |
> 218 +++
>  SignedCapsulePkg/Include/Guid/EdkiiSystemFmpCapsule.h                              |
> 151 +++
>  SignedCapsulePkg/Include/Library/EdkiiSystemCapsuleLib.h                           |
> 154 +++
>  SignedCapsulePkg/Include/Library/IniParsingLib.h                                   |
> 166 +++
>  SignedCapsulePkg/Include/Library/PlatformFlashAccessLib.h                          |
> 57 +
>  SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.c             |
> 671 +++++++++
>  SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf           |
> 61 +
>  SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.uni           |
> 22 +
>  SignedCapsulePkg/Library/IniParsingLib/IniParsingLib.c                             |
> 1420 ++++++++++++++++++++
>  SignedCapsulePkg/Library/IniParsingLib/IniParsingLib.inf                           |
> 43 +
>  SignedCapsulePkg/Library/IniParsingLib/IniParsingLib.uni                           |
> 22 +
>  SignedCapsulePkg/Library/PlatformFlashAccessLibNull/PlatformFlashAccessLibNull.c   |
> 51 +
>  SignedCapsulePkg/Library/PlatformFlashAccessLibNull/PlatformFlashAccessLibNull.inf |
> 40 +
>  SignedCapsulePkg/Library/PlatformFlashAccessLibNull/PlatformFlashAccessLibNull.uni |
> 21 +
>  SignedCapsulePkg/License.txt                                                       |
> 25 +
>  SignedCapsulePkg/SignedCapsulePkg.dec                                              |
> 76 ++
>  SignedCapsulePkg/SignedCapsulePkg.dsc                                              |
> 210 +++
>  SignedCapsulePkg/Universal/RecoveryModuleLoadPei/ParseConfigProfile.c              |
> 163 +++
>  SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPei.c           |
> 806 +++++++++++
>  SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPei.h           |
> 44 +
>  SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPei.inf         |
> 71 +
>  SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPei.uni         |
> 21 +
>  SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPeiExtra.uni    |
> 20 +
>  SignedCapsulePkg/Universal/SystemFirmwareUpdate/ParseConfigProfile.c               |
> 213 +++
>  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c          |
> 385 ++++++
>  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareDxe.h                |
> 408 ++++++
>  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareReportDxe.c          |
> 262 ++++
>  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareReportDxe.inf        |
> 69 +
>  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareReportDxe.uni        |
> 21 +
>  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareReportDxeExtra.uni   |
> 20 +
>  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c          |
> 526 ++++++++
>  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf        |
> 72 +
>  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.uni        |
> 21 +
>  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxeExtra.uni   |
> 20 +
>  34 files changed, 6550 insertions(+)
>  create mode 100644 SignedCapsulePkg/Contributions.txt
>  create mode 100644 SignedCapsulePkg/Include/Guid/EdkiiSystemFmpCapsule.h
>  create mode 100644 SignedCapsulePkg/Include/Library/EdkiiSystemCapsuleLib.h
>  create mode 100644 SignedCapsulePkg/Include/Library/IniParsingLib.h
>  create mode 100644 SignedCapsulePkg/Include/Library/PlatformFlashAccessLib.h
>  create mode 100644
> SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.c
>  create mode 100644
> SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf
>  create mode 100644
> SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.uni
>  create mode 100644 SignedCapsulePkg/Library/IniParsingLib/IniParsingLib.c
>  create mode 100644 SignedCapsulePkg/Library/IniParsingLib/IniParsingLib.inf
>  create mode 100644 SignedCapsulePkg/Library/IniParsingLib/IniParsingLib.uni
>  create mode 100644
> SignedCapsulePkg/Library/PlatformFlashAccessLibNull/PlatformFlashAccessLibNull.c
>  create mode 100644
> SignedCapsulePkg/Library/PlatformFlashAccessLibNull/PlatformFlashAccessLibNull.inf
>  create mode 100644
> SignedCapsulePkg/Library/PlatformFlashAccessLibNull/PlatformFlashAccessLibNull.uni
>  create mode 100644 SignedCapsulePkg/License.txt
>  create mode 100644 SignedCapsulePkg/SignedCapsulePkg.dec
>  create mode 100644 SignedCapsulePkg/SignedCapsulePkg.dsc
>  create mode 100644
> SignedCapsulePkg/Universal/RecoveryModuleLoadPei/ParseConfigProfile.c
>  create mode 100644
> SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPei.c
>  create mode 100644
> SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPei.h
>  create mode 100644
> SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPei.inf
>  create mode 100644
> SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPei.uni
>  create mode 100644
> SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPeiExtra.uni
>  create mode 100644
> SignedCapsulePkg/Universal/SystemFirmwareUpdate/ParseConfigProfile.c
>  create mode 100644
> SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c
>  create mode 100644 SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareDxe.h
>  create mode 100644
> SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareReportDxe.c
>  create mode 100644
> SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareReportDxe.inf
>  create mode 100644
> SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareReportDxe.uni
>  create mode 100644
> SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareReportDxeExtra.uni
>  create mode 100644
> SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
>  create mode 100644
> SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf
>  create mode 100644
> SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.uni
>  create mode 100644
> SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxeExtra.uni
> 
> --
> 2.7.4.windows.1