[edk2-devel] [PATCH] BaseTools: GenerateCapsule.py: Add support for version 3 of FMP Image Header structure
Sughosh Ganu
sughosh.ganu at linaro.org
Thu Apr 22 06:05:48 UTC 2021
hi Michael,
On Wed, 21 Apr 2021 at 21:04, Kinney, Michael D <michael.d.kinney at intel.com>
wrote:
> Sughosh,
>
>
>
> My feedback on compatibility was for the parameter list to the Python
> methods. Not the C structures. The python modules in the Common
> directory could be used by other tools.
>
Okay, understood. I will make the changes and submit a v2 shortly. Thanks
for the explanation.
-sughosh
>
>
> Mike
>
>
>
> *From:* Sughosh Ganu <sughosh.ganu at linaro.org>
> *Sent:* Wednesday, April 21, 2021 12:02 AM
> *To:* Kinney, Michael D <michael.d.kinney at intel.com>
> *Cc:* devel at edk2.groups.io; Feng, Bob C <bob.c.feng at intel.com>; Liming
> Gao <gaoliming at byosoft.com.cn>; Chen, Christine <yuwei.chen at intel.com>
> *Subject:* Re: [edk2-devel] [PATCH] BaseTools: GenerateCapsule.py: Add
> support for version 3 of FMP Image Header structure
>
>
>
> hi Michael,
>
> Thanks for your review.
>
>
>
> On Tue, 20 Apr 2021 at 21:26, Kinney, Michael D <
> michael.d.kinney at intel.com> wrote:
>
> 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.
>
>
>
> Okay. I will make the changes as per your suggestion.
>
>
>
>
> 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.
>
>
>
> I believe that adding support for compatibility with previous versions
> will be a more involved effort. If you look at the code, the way it is
> right now, the EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER structure
> currently supports version 2 -- there is no provision for generating a
> capsule with version 1 for example. So the way the code is structured right
> now, we can only support one version. So, if you are looking for having
> support for previous versions as well, I think that would require adding
> support for passing the structure version that we need, and then based on.
> this information, the structure would have certain fields present/absent.
> Would it be fine to add this field in the structure for now. If you think
> it necessary, support for multiple versions of the structure can be added
> as a separate exercise. Is my understanding of your comment correct, or am
> I missing something.
>
>
>
> -sughosh
>
>
>
>
> Thanks,
>
> Mike
>
> > -----Original Message-----
> > From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Sughosh
> Ganu
> > Sent: Monday, April 19, 2021 4:40 AM
> > To: devel at edk2.groups.io
> > Cc: Feng, Bob C <bob.c.feng at intel.com>; Liming Gao <
> gaoliming at byosoft.com.cn>; Chen, Christine <yuwei.chen at intel.com>;
> > Sughosh Ganu <sughosh.ganu at 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.ganu at 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 (#74346): https://edk2.groups.io/g/devel/message/74346
Mute This Topic: https://groups.io/mt/82206170/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20210422/6b10c33a/attachment.htm>
More information about the edk2-devel-archive
mailing list