Hi,
I think this patch is functional, but there are a few things that can be
improved.
1) We should not use of the hard coded constants for the ImageCapsuleSupport
values.
The UEFI spec has #defines for these values, and we need to figure out how
to add
those define values in the scope of the FMP Capsule Image Header. Perhaps
add the
values to FmpCapsuleImageHeaderClass in FmpCapsuleHeader.py:
class FmpCapsuleImageHeaderClass (object):
CAPSULE_SUPPORT_AUTHENTICATION = 0x0000000000000001
CAPSULE_SUPPORT_DEPENDENCY = 0x0000000000000002
From GenerateCapsule.py, you can use these values to set ImageCapsuleSupport.
Value 0 is not defined by the UEFI Spec. Though that appears to be the
value used
for a raw payload without authentication or dependency information.
Since GenerateCapsule does not import FmpCapsuleImageHeaderClass, these
defines
may have to be added to the FmpCapsuleHeaderClass instead.
2) The ImageCapsuleSupport parameter is added in the middle of the python
function arguments.
Since this is a new field added at the end of the V3 structure, I recommend
we add arguments
at the end of the argument list. This will be an optional argument, so any
existing use
of these methods for V2 use cases will not be impacted. These python
methods can be
potentially used by other consumers, so we should always consider backwards
compatibility
when updating BaseTools/Source/Python/Common areas.
Thanks,
Mike
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sughosh Ganu
> Sent: Monday, April 19, 2021 4:40 AM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.f...@intel.com>; Liming Gao
> <gaolim...@byosoft.com.cn>; Chen, Christine <yuwei.c...@intel.com>;
> Sughosh Ganu <sughosh.g...@linaro.org>
> Subject: [edk2-devel] [PATCH] BaseTools: GenerateCapsule.py: Add support for
> version 3 of FMP Image Header structure
>
> Add support for the ImageCapsuleSupport field, introduced in version 3
> of the EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER structure. This
> structure member is used to indicate if the corresponding payload has
> support for authentication and dependency.
>
> Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org>
> ---
> .../Source/Python/Capsule/GenerateCapsule.py | 5 +++-
> .../Common/Uefi/Capsule/FmpCapsuleHeader.py | 26 +++++++++++++------
> 2 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/BaseTools/Source/Python/Capsule/GenerateCapsule.py
> b/BaseTools/Source/Python/Capsule/GenerateCapsule.py
> index a8de988253..6419a2ac6f 100644
> --- a/BaseTools/Source/Python/Capsule/GenerateCapsule.py
> +++ b/BaseTools/Source/Python/Capsule/GenerateCapsule.py
> @@ -561,6 +561,7 @@ if __name__ == '__main__':
> print ('GenerateCapsule: error:' + str(Msg))
> sys.exit (1)
> for SinglePayloadDescriptor in PayloadDescriptorList:
> + ImageCapsuleSupport = 0x0000000000000000
> Result = SinglePayloadDescriptor.Payload
> try:
> FmpPayloadHeader.FwVersion =
> SinglePayloadDescriptor.FwVersion
> @@ -575,6 +576,7 @@ if __name__ == '__main__':
> if SinglePayloadDescriptor.UseDependency:
> CapsuleDependency.Payload = Result
> CapsuleDependency.DepexExp = SinglePayloadDescriptor.DepexExp
> + ImageCapsuleSupport |= 0x0000000000000002
> Result = CapsuleDependency.Encode ()
> if args.Verbose:
> CapsuleDependency.DumpInfo ()
> @@ -607,13 +609,14 @@ if __name__ == '__main__':
> FmpAuthHeader.MonotonicCount =
> SinglePayloadDescriptor.MonotonicCount
> FmpAuthHeader.CertData = CertData
> FmpAuthHeader.Payload = Result
> + ImageCapsuleSupport |= 0x0000000000000001
> Result = FmpAuthHeader.Encode ()
> if args.Verbose:
> FmpAuthHeader.DumpInfo ()
> except:
> print ('GenerateCapsule: error: can not encode FMP Auth
> Header')
> sys.exit (1)
> - FmpCapsuleHeader.AddPayload (SinglePayloadDescriptor.Guid,
> Result, HardwareInstance =
> SinglePayloadDescriptor.HardwareInstance, UpdateImageIndex =
> SinglePayloadDescriptor.UpdateImageIndex)
> + FmpCapsuleHeader.AddPayload (SinglePayloadDescriptor.Guid,
> Result, ImageCapsuleSupport, HardwareInstance =
> SinglePayloadDescriptor.HardwareInstance, UpdateImageIndex =
> SinglePayloadDescriptor.UpdateImageIndex)
> try:
> for EmbeddedDriver in EmbeddedDriverDescriptorList:
> FmpCapsuleHeader.AddEmbeddedDriver(EmbeddedDriver)
> diff --git a/BaseTools/Source/Python/Common/Uefi/Capsule/FmpCapsuleHeader.py
> b/BaseTools/Source/Python/Common/Uefi/Capsule/FmpCapsuleHeader.py
> index 91d24919c4..a2a5cf0db8 100644
> --- a/BaseTools/Source/Python/Common/Uefi/Capsule/FmpCapsuleHeader.py
> +++ b/BaseTools/Source/Python/Common/Uefi/Capsule/FmpCapsuleHeader.py
> @@ -47,14 +47,19 @@ class FmpCapsuleImageHeaderClass (object):
> # /// therefore can be modified without changing the Auth data.
> # ///
> # UINT64 UpdateHardwareInstance;
> + #
> + # ///
> + # /// Bits which indicate authentication and depex information for the
> image that follows this structure
> + # ///
> + # UINT64 ImageCapsuleSupport
> # } EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER;
> #
> - # #define EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION
> 0x00000002
> + # #define EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION
> 0x00000003
>
> - _StructFormat = '<I16sB3BIIQ'
> + _StructFormat = '<I16sB3BIIQQ'
> _StructSize = struct.calcsize (_StructFormat)
>
> - EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION = 0x00000002
> + EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION = 0x00000003
>
> def __init__ (self):
> self._Valid = False
> @@ -64,6 +69,7 @@ class FmpCapsuleImageHeaderClass (object):
> self.UpdateImageSize = 0
> self.UpdateVendorCodeSize = 0
> self.UpdateHardwareInstance = 0x0000000000000000
> + self.ImageCapsuleSupport = 0x0000000000000000
> self.Payload = b''
> self.VendorCodeBytes = b''
>
> @@ -78,7 +84,8 @@ class FmpCapsuleImageHeaderClass (object):
> 0,0,0,
> self.UpdateImageSize,
> self.UpdateVendorCodeSize,
> - self.UpdateHardwareInstance
> + self.UpdateHardwareInstance,
> + self.ImageCapsuleSupport
> )
> self._Valid = True
> return FmpCapsuleImageHeader + self.Payload + self.VendorCodeBytes
> @@ -86,7 +93,7 @@ class FmpCapsuleImageHeaderClass (object):
> def Decode (self, Buffer):
> if len (Buffer) < self._StructSize:
> raise ValueError
> - (Version, UpdateImageTypeId, UpdateImageIndex, r0, r1, r2,
> UpdateImageSize, UpdateVendorCodeSize,
> UpdateHardwareInstance) = \
> + (Version, UpdateImageTypeId, UpdateImageIndex, r0, r1, r2,
> UpdateImageSize, UpdateVendorCodeSize,
> UpdateHardwareInstance, ImageCapsuleSupport) = \
> struct.unpack (
> self._StructFormat,
> Buffer[0:self._StructSize]
> @@ -105,6 +112,7 @@ class FmpCapsuleImageHeaderClass (object):
> self.UpdateImageSize = UpdateImageSize
> self.UpdateVendorCodeSize = UpdateVendorCodeSize
> self.UpdateHardwareInstance = UpdateHardwareInstance
> + self.ImageCapsuleSupport = ImageCapsuleSupport
> self.Payload =
> Buffer[self._StructSize:self._StructSize + UpdateImageSize]
> self.VendorCodeBytes = Buffer[self._StructSize +
> UpdateImageSize:]
> self._Valid = True
> @@ -119,6 +127,7 @@ class FmpCapsuleImageHeaderClass (object):
> print ('EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER.UpdateImageSize
> = {UpdateImageSize:08X}'.format
> (UpdateImageSize = self.UpdateImageSize))
> print
> ('EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER.UpdateVendorCodeSize =
> {UpdateVendorCodeSize:08X}'.format
> (UpdateVendorCodeSize = self.UpdateVendorCodeSize))
> print
> ('EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER.UpdateHardwareInstance =
> {UpdateHardwareInstance:016X}'.format (UpdateHardwareInstance =
> self.UpdateHardwareInstance))
> + print
> ('EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER.ImageCapsuleSupport =
> {ImageCapsuleSupport:016X}'.format
> (ImageCapsuleSupport = self.ImageCapsuleSupport))
> print ('sizeof (Payload)
> = {Size:08X}'.format (Size = len
> (self.Payload)))
> print ('sizeof (VendorCodeBytes)
> = {Size:08X}'.format (Size = len
> (self.VendorCodeBytes)))
>
> @@ -172,8 +181,8 @@ class FmpCapsuleHeaderClass (object):
> raise ValueError
> return self._EmbeddedDriverList[Index]
>
> - def AddPayload (self, UpdateImageTypeId, Payload = b'', VendorCodeBytes
> = b'', HardwareInstance = 0, UpdateImageIndex
> = 1):
> - self._PayloadList.append ((UpdateImageTypeId, Payload,
> VendorCodeBytes, HardwareInstance, UpdateImageIndex))
> + def AddPayload (self, UpdateImageTypeId, Payload = b'',
> ImageCapsuleSupport = 0, VendorCodeBytes = b'',
> HardwareInstance = 0, UpdateImageIndex = 1):
> + self._PayloadList.append ((UpdateImageTypeId, Payload,
> ImageCapsuleSupport, VendorCodeBytes, HardwareInstance,
> UpdateImageIndex))
>
> def GetFmpCapsuleImageHeader (self, Index):
> if Index >= len (self._FmpCapsuleImageHeaderList):
> @@ -198,13 +207,14 @@ class FmpCapsuleHeaderClass (object):
> self._ItemOffsetList.append (Offset)
> Offset = Offset + len (EmbeddedDriver)
> Index = 1
> - for (UpdateImageTypeId, Payload, VendorCodeBytes, HardwareInstance,
> UpdateImageIndex) in self._PayloadList:
> + for (UpdateImageTypeId, Payload, ImageCapsuleSupport,
> VendorCodeBytes, HardwareInstance, UpdateImageIndex) in
> self._PayloadList:
> FmpCapsuleImageHeader = FmpCapsuleImageHeaderClass ()
> FmpCapsuleImageHeader.UpdateImageTypeId = UpdateImageTypeId
> FmpCapsuleImageHeader.UpdateImageIndex = UpdateImageIndex
> FmpCapsuleImageHeader.Payload = Payload
> FmpCapsuleImageHeader.VendorCodeBytes = VendorCodeBytes
> FmpCapsuleImageHeader.UpdateHardwareInstance = HardwareInstance
> + FmpCapsuleImageHeader.ImageCapsuleSupport =
> ImageCapsuleSupport
> FmpCapsuleImage = FmpCapsuleImageHeader.Encode ()
> FmpCapsuleData = FmpCapsuleData + FmpCapsuleImage
>
> --
> 2.17.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74301): https://edk2.groups.io/g/devel/message/74301
Mute This Topic: https://groups.io/mt/82206170/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
