Re: [PATCH V9 0/7] Add capsule support for Quark.

["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: Yao, Jiewen
> Sent: Monday, November 7, 2016 4:41 AM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>; Kinney, Michael
> D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>; Fan, Jeff <jeff.fan@intel.com>
> Subject: [PATCH V9 0/7] Add capsule support for Quark.
> 
> ==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 (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.
> 
> 
> QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/PlatformFlashAccessLibD
> xe.c   | 206 ++++++++++++
> 
> QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/PlatformFlashAccessLibD
> xe.inf |  53 +++
>  QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/SpiFlashDevice.c
> | 336 ++++++++++++++++++++
>  QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/SpiFlashDevice.h
> | 186 +++++++++++
> 
> QuarkPlatformPkg/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc
> |  89 ++++++
>  QuarkPlatformPkg/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
> |  46 +++
> 
> QuarkPlatformPkg/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c
> |  66 ++++
> 
> QuarkPlatformPkg/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.
> ini    |  63 ++++
>  QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
> | 131 +++++++-
>  QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.h
> |   9 +-
>  QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> |  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/PlatformFlashAccessLibD
> xe.c
>  create mode 100644
> QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/PlatformFlashAccessLibD
> xe.inf
>  create mode 100644
> QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/SpiFlashDevice.c
>  create mode 100644
> QuarkPlatformPkg/Feature/Capsule/Library/PlatformFlashAccessLib/SpiFlashDevice.h
>  create mode 100644
> QuarkPlatformPkg/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc
>  create mode 100644
> QuarkPlatformPkg/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
>  create mode 100644
> QuarkPlatformPkg/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c
>  create mode 100644
> QuarkPlatformPkg/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.
> ini
> 
> --
> 2.7.4.windows.1