Thanks for helping update the related version check.
Please see my comment below.
With these two places updated, Reviewed-by: Wei6 Xu <wei6...@intel.com>
>-----Original Message-----
>From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Oleksiy
>Yakovlev
>Sent: Wednesday, May 13, 2020 6:06 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: [edk2-devel] [PATCH V4 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>
>---
> MdeModulePkg/Application/CapsuleApp/CapsuleDump.c | 1 +
> .../Library/DxeCapsuleLibFmp/DxeCapsuleLib.c | 20
>++++++++++++++++----
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
>diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
>b/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
>index 7e3e072..e3ab199 100644
>--- a/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
>+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
>@@ -98,6 +98,7 @@ DumpFmpCapsule (
> 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);
>+ Print(L" ImageCapsuleSupport - 0x%lx\n",
>+ FmpImageHeader->ImageCapsuleSupport);
> }
> }
> }
Wei: I think here the version check should be:
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));
}
}
>diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
>b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
>index 5dda561..f9819a7 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);
> }
> if (FmpImageSize < FmpImageHeaderSize) {
> DEBUG((DEBUG_ERROR, "FmpImageSize(0x%lx) <
>FmpImageHeaderSize(0x%x)\n", FmpImageSize, FmpImageHeaderSize)); @@
>-521,6 +523,7 @@ DumpFmpCapsule (
> 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));
>+ DEBUG((DEBUG_VERBOSE, " ImageCapsuleSupport - 0x%lx\n",
>ImageHeader->ImageCapsuleSupport));
> }
> }
> }
Wei: I think here the version check should be:
if (ImageHeader->Version >= 1) {
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 +931,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 +953,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 +1248,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 (#59467): https://edk2.groups.io/g/devel/message/59467
Mute This Topic: https://groups.io/mt/74169596/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
