Oleksiy:
  When create PR, I find some issue in this patch. The same issue is also in 
SignedCapsulePkg. Because original comments from Xu Wei6, I would like Wei to 
provide the updated patch for MdeModulePkg and SignedCapsulePkg.

Thanks
Liming
> -----Original Message-----
> From: Oleksiy Yakovlev <oleks...@ami.com>
> Sent: Friday, May 15, 2020 4:52 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <liming....@intel.com>; Kinney, Michael D 
> <michael.d.kin...@intel.com>; Wang, Jian J <jian.j.w...@intel.com>; Wu,
> Hao A <hao.a...@intel.com>; Yao, Jiewen <jiewen....@intel.com>; Zhang, Chao B 
> <chao.b.zh...@intel.com>; fel...@ami.com;
> oleks...@ami.com
> Subject: [PATCH V7 5/6] MdeModulePkg: Add FMP Capsule Image Header extension
> 
> Add bitmask to structure which gives a binary-inspectable mechanism to
> determine if a capsule contains an authentication section or depex section.
> (UEFI 2.8 errata a, mantis 2026)
> 
> Signed-off-by: Oleksiy Yakovlev <oleks...@ami.com>
> 
> Reviewed-by: Wei6 Xu <wei6...@intel.com>
> ---
>  MdeModulePkg/Application/CapsuleApp/CapsuleDump.c  |  7 ++++--
>  .../Library/DxeCapsuleLibFmp/DxeCapsuleLib.c       | 26 
> +++++++++++++++++-----
>  2 files changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c 
> b/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
> index 7e3e072..057bfa8 100644
> --- a/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
> +++ b/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
> @@ -96,8 +96,11 @@ DumpFmpCapsule (
>      Print(L"  UpdateImageIndex       - 0x%x\n", 
> FmpImageHeader->UpdateImageIndex);
>      Print(L"  UpdateImageSize        - 0x%x\n", 
> FmpImageHeader->UpdateImageSize);
>      Print(L"  UpdateVendorCodeSize   - 0x%x\n", 
> FmpImageHeader->UpdateVendorCodeSize);
> -    if (FmpImageHeader->Version >= 
> EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION) {
> -      Print(L"  UpdateHardwareInstance - 0x%lx\n", 
> FmpImageHeader->UpdateHardwareInstance);
> +    if (ImageHeader->Version >= 1) {
> +    DEBUG((DEBUG_VERBOSE, "    UpdateHardwareInstance - 0x%lx\n", 
> ImageHeader->UpdateHardwareInstance));
> +      if (ImageHeader->Version >= 
> EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION) {
> +         DEBUG((DEBUG_VERBOSE, "    ImageCapsuleSupport - 0x%lx\n", 
> ImageHeader->ImageCapsuleSupport));
> +      }
>      }
>    }
[Liming] Here should be FmpImageHeader

>  }
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c 
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> index 5dda561..68cece6 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> @@ -285,8 +285,10 @@ ValidateFmpCapsule (
>        DEBUG((DEBUG_ERROR, "ImageHeader->Version(0x%x) Unknown\n", 
> ImageHeader->Version));
>        return EFI_INVALID_PARAMETER;
>      }
> -    if (ImageHeader->Version < 
> EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION) {
> +    if (ImageHeader->Version == 1) {
>        FmpImageHeaderSize = 
> OFFSET_OF(EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER, 
> UpdateHardwareInstance);
> +    } else {
> +      FmpImageHeaderSize = 
> OFFSET_OF(EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER, ImageCapsuleSupport);
>      }

[Liming] Here should have one condition for ImageHeader->Version == 2.

>      if (FmpImageSize < FmpImageHeaderSize) {
>        DEBUG((DEBUG_ERROR, "FmpImageSize(0x%lx) < 
> FmpImageHeaderSize(0x%x)\n", FmpImageSize, FmpImageHeaderSize));
> @@ -519,8 +521,11 @@ DumpFmpCapsule (
>      DEBUG((DEBUG_VERBOSE, "    UpdateImageIndex       - 0x%x\n", 
> ImageHeader->UpdateImageIndex));
>      DEBUG((DEBUG_VERBOSE, "    UpdateImageSize        - 0x%x\n", 
> ImageHeader->UpdateImageSize));
>      DEBUG((DEBUG_VERBOSE, "    UpdateVendorCodeSize   - 0x%x\n", 
> ImageHeader->UpdateVendorCodeSize));
> -    if (ImageHeader->Version >= 
> EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION) {
> -      DEBUG((DEBUG_VERBOSE, "    UpdateHardwareInstance - 0x%lx\n", 
> ImageHeader->UpdateHardwareInstance));
> +    if (ImageHeader->Version >= 1) {
[Liming] Here should be ImageHeader->Version >= 2.

Thanks
Liming
> +      DEBUG((DEBUG_INFO, "(UpdateHardwareInstance - 0x%x)", 
> ImageHeader->UpdateHardwareInstance));
> +      if (ImageHeader->Version >= 
> EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION) {
> +        DEBUG((DEBUG_INFO, "(ImageCapsuleSupport - 0x%x)",  
> ImageHeader->ImageCapsuleSupport));
> +      }
>      }
>    }
>  }
> @@ -928,9 +933,14 @@ SetFmpImageData (
>    } else {
>      //
>      // If the EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER is version 1,
> -    // Header should exclude UpdateHardwareInstance field
> +    // Header should exclude UpdateHardwareInstance field, and
> +    // ImageCapsuleSupport field if version is 2.
>      //
> -    Image = (UINT8 *)ImageHeader + 
> OFFSET_OF(EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER,
> UpdateHardwareInstance);
> +    if (ImageHeader->Version == 1) {
> +      Image = (UINT8 *)ImageHeader + 
> OFFSET_OF(EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER,
> UpdateHardwareInstance);
> +    } else {
> +      Image = (UINT8 *)ImageHeader + 
> OFFSET_OF(EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER, ImageCapsuleSupport);
> +    }
>    }
> 
>    if (ImageHeader->UpdateVendorCodeSize == 0) {
> @@ -945,6 +955,7 @@ SetFmpImageData (
>    DEBUG((DEBUG_INFO, "ImageIndex - 0x%x ", ImageHeader->UpdateImageIndex));
>    if (ImageHeader->Version >= 
> EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION) {
>      DEBUG((DEBUG_INFO, "(UpdateHardwareInstance - 0x%x)", 
> ImageHeader->UpdateHardwareInstance));
> +    DEBUG((DEBUG_INFO, "(ImageCapsuleSupport - 0x%x)", 
> ImageHeader->ImageCapsuleSupport));
>    }
>    DEBUG((DEBUG_INFO, "\n"));
> 
> @@ -1239,7 +1250,10 @@ ProcessFmpCapsuleImage (
>      ImageHeader  = (EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER *)((UINT8 
> *)FmpCapsuleHeader +
> ItemOffsetList[Index]);
> 
>      UpdateHardwareInstance = 0;
> -    if (ImageHeader->Version >= 
> EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION) {
> +    ///
> +    /// UpdateHardwareInstance field was added in Version 2
> +    ///
> +    if (ImageHeader->Version >= 2) {
>        UpdateHardwareInstance = ImageHeader->UpdateHardwareInstance;
>      }
> 
> --
> 2.9.0.windows.1
> 
> 
> Please consider the environment before printing this email.
> 
> The information contained in this message may be confidential and proprietary 
> to American Megatrends (AMI).  This communication
> is intended to be read only by the individual or entity to whom it is 
> addressed or by their designee. If the reader of this message is
> not the intended recipient, you are on notice that any distribution of this 
> message, in any form, is strictly prohibited.  Please
> promptly notify the sender by reply e-mail or by telephone at 770-246-8600, 
> and then delete or destroy all copies of the transmission.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#59648): https://edk2.groups.io/g/devel/message/59648
Mute This Topic: https://groups.io/mt/74214498/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to