From: "Michael Kubacki" <michael.kubacki@outlook.com>
To: Kilian Kegel <kilian_kegel@outlook.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"michael.d.kinney@intel.com" <michael.d.kinney@intel.com>
Cc: "Qian, Yi" <yi.qian@intel.com>, "Sun, Zailiang" <zailiang.sun@intel.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] Vlv2TbltDevicePkg/FmpDeviceLib: Add LastAttemptStatus compatibility
Date: Tue, 10 Nov 2020 21:17:19 -0800 [thread overview]
Message-ID: <MWHPR07MB3440EFF12E7C9EE325EDE1CEE9E80@MWHPR07MB3440.namprd07.prod.outlook.com> (raw)
In-Reply-To: <AM8P190MB0945C933392BA5D8C68F213EEBE80@AM8P190MB0945.EURP190.PROD.OUTLOOK.COM>
Hi Kilian,
That was broken due to FmpDevicePkg changes unrelated to these Last
Attempt Status changes. I ran into the same isues as you when attempting
to test this.
I believe Mike made those fixes in the following two commits:
1. 2fcfe4763022c1665be8e6a11f80aa69f0a58dce
2. b0de06c7d8494d05b315fb4a2574664f151e108d
Thanks,
Michael
On 11/10/2020 8:16 PM, Kilian Kegel wrote:
> Hi all,
>
> if this patch also affects
> https://bugzilla.tianocore.org/show_bug.cgi?id=2983
> <https://bugzilla.tianocore.org/show_bug.cgi?id=2983>, please close it.
>
> Thanks,
>
> Kilian
>
> *Von: *Michael D Kinney <mailto:michael.d.kinney@intel.com>
> *Gesendet: *Wednesday, November 11, 2020 4:03 AM
> *An: *Michael Kubacki <mailto:michael.kubacki@outlook.com>;
> devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Kinney, Michael D
> <mailto:michael.d.kinney@intel.com>
> *Cc: *Qian, Yi <mailto:yi.qian@intel.com>; Sun, Zailiang
> <mailto:zailiang.sun@intel.com>
> *Betreff: *Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1]
> Vlv2TbltDevicePkg/FmpDeviceLib: Add LastAttemptStatus compatibility
>
> Hi Michael,
>
> This patch has been merged.
>
> Thanks,
>
> Mike
>
> > -----Original Message-----
> > From: Michael Kubacki <michael.kubacki@outlook.com>
> > Sent: Thursday, November 5, 2020 12:36 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> > Cc: Qian, Yi <yi.qian@intel.com>; Sun, Zailiang <zailiang.sun@intel.com>
> > Subject: Re: [edk2-platforms][PATCH v1 1/1]
> Vlv2TbltDevicePkg/FmpDeviceLib: Add LastAttemptStatus compatibility
> >
> > Hi Mike,
> >
> > I saw other Vlv2TbltDevicePkg patches were merged yesterday. Do you know
> > when this patch will be merged?
> >
> > Thanks,
> > Michael
> >
> > On 10/28/2020 6:12 PM, Kinney, Michael D wrote:
> > > Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> > >
> > > I also verified build and boot and FMP based capsule updates work
> as expected.
> > >
> > > Tested-by: Michael D Kinney <Michael.d.kinney@intel.com>
> > >
> > > Thanks,
> > >
> > > Mike
> > >
> > >
> > >> -----Original Message-----
> > >> From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
> > >> Sent: Thursday, October 1, 2020 2:59 PM
> > >> To: devel@edk2.groups.io
> > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Qian, Yi
> <yi.qian@intel.com>; Sun, Zailiang
> > <zailiang.sun@intel.com>
> > >> Subject: [edk2-platforms][PATCH v1 1/1]
> Vlv2TbltDevicePkg/FmpDeviceLib: Add LastAttemptStatus compatibility
> > >>
> > >> From: Michael Kubacki <michael.kubacki@microsoft.com>
> > >>
> > >> Makes the changes necessary for these library instances of
> > >> FmpDeviceLib to be compatible with new functions added recently
> > >> to FmpDeviceLib.
> > >>
> > >> Two new functions were introduced in FmpDeviceLib to allow a
> > >> library instance to return a Last Attempt Status code during
> > >> check image and set image operations:
> > >> 1. FmpDeviceCheckImageWithStatus ( )
> > >> 2. FmpDeviceSetImageWithStatus ( )
> > >>
> > >> FmpDxe (in FmpDevicePkg) will begin calling these new functions
> > >> instead of the previous functions. Therefore, this change:
> > >> 1. Adds these functions to Vlv2TbltDevicePkg implementations
> > >> 2. Moves the main functionality to these new functions
> > >> 3. Updates the old functions to call the new functions
> > >> (for backward compatibility)
> > >>
> > >> Note: As of this commit, the Vlv2TbltDevicePkg build is broken
> > >> due to:
> > >> 1. A required RngLib library instance not defined by the platform
> > >> 2. Other FMP libraries not being defined by the platform
> > >> (e.g. FmpDependencyLib, FmpDependencyCheckLib, etc.)
> > >>
> > >> Those changes were fixed locally to test the changes in this commit
> > >> but maintainers should make the proper changes for those issues.
> > >>
> > >> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > >> Cc: Yi Qian <yi.qian@intel.com>
> > >> Cc: Zailiang Sun <zailiang.sun@intel.com>
> > >> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> > >> ---
> > >>
> > >> Notes:
> > >> Only build was checked, I do not have access to a
> > >> VLV2 device for testing.
> > >>
> > >>
> Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLib/FmpDeviceLib.c
> | 299 +++++++++++++++-----
> > >>
> Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLibSample/FmpDeviceLib.c
> | 284 ++++++++++++++-----
> > >> 2 files changed, 446 insertions(+), 137 deletions(-)
> > >>
> > >> diff --git
> a/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLib/FmpDeviceLib.c
> > >>
> b/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLib/FmpDeviceLib.c
> > >> index d8c9036012ad..df8a36d9854c 100644
> > >> ---
> a/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLib/FmpDeviceLib.c
> > >> +++
> b/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLib/FmpDeviceLib.c
> > >> @@ -1,6 +1,6 @@
> > >> /**
> > >>
> > >> -Copyright (c) 2016, Microsoft Corporation. All rights reserved.
> > >> +Copyright (c) Microsoft Corporation.<BR>
> > >> Copyright (c) 2019, Intel Corporation. All rights reserved.
> > >>
> > >> SPDX-License-Identifier: BSD-2-Clause-Patent
> > >> @@ -8,7 +8,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > >> **/
> > >>
> > >> #include <PiDxe.h>
> > >> -
> > >> +#include <LastAttemptStatus.h>
> > >> +#include <Guid/SystemResourceTable.h>
> > >> #include <Library/FmpDeviceLib.h>
> > >>
> > >> #include <Library/DebugLib.h>
> > >> @@ -444,20 +445,17 @@ FmpDeviceGetImage (
> > >> }
> > >>
> > >> /**
> > >> - Updates the firmware image of the device.
> > >> -
> > >> - This function updates the hardware with the new firmware
> image. This function
> > >> - returns EFI_UNSUPPORTED if the firmware image is not
> updatable. If the
> > >> - firmware image is updatable, the function should perform the
> following minimal
> > >> - validations before proceeding to do the firmware image update.
> > >> - - Validate the image is a supported image for this device.
> The function
> > >> - returns EFI_ABORTED if the image is unsupported. The
> function can
> > >> - optionally provide more detailed information on why the
> image is not a
> > >> - supported image.
> > >> - - Validate the data from VendorCode if not null. Image
> validation must be
> > >> - performed before VendorCode data validation. VendorCode
> data is ignored
> > >> - or considered invalid if image validation failed. The
> function returns
> > >> - EFI_ABORTED if the data is invalid.
> > >> + Updates a firmware device with a new firmware image. This
> function returns
> > >> + EFI_UNSUPPORTED if the firmware image is not updatable. If the
> firmware image
> > >> + is updatable, the function should perform the following minimal
> validations
> > >> + before proceeding to do the firmware image update.
> > >> + - Validate that the image is a supported image for this
> firmware device.
> > >> + Return EFI_ABORTED if the image is not supported.
> Additional details
> > >> + on why the image is not a supported image may be returned
> in AbortReason.
> > >> + - Validate the data from VendorCode if is not NULL. Firmware
> image
> > >> + validation must be performed before VendorCode data validation.
> > >> + VendorCode data is ignored or considered invalid if image
> validation
> > >> + fails. Return EFI_ABORTED if the VendorCode data is invalid.
> > >>
> > >> VendorCode enables vendor to implement vendor-specific
> firmware image update
> > >> policy. Null if the caller did not specify the policy or use
> the default
> > >> @@ -470,38 +468,56 @@ FmpDeviceGetImage (
> > >> have the option to provide a more detailed description of the
> abort reason to
> > >> the caller.
> > >>
> > >> - @param[in] Image Points to the new image.
> > >> - @param[in] ImageSize Size of the new image in bytes.
> > >> + @param[in] Image Points to the new firmware image.
> > >> + @param[in] ImageSize Size, in bytes, of the new
> firmware image.
> > >> @param[in] VendorCode This enables vendor to implement
> vendor-specific
> > >> - firmware image update policy.
> Null indicates the
> > >> - caller did not specify the policy
> or use the
> > >> + firmware image update policy.
> NULL indicates
> > >> + the caller did not specify the
> policy or use the
> > >> default policy.
> > >> - @param[in] Progress A function used by the driver to
> report the
> > >> - progress of the firmware update.
> > >> - @param[in] CapsuleFwVersion FMP Payload Header version of the
> image.
> > >> - @param[out] AbortReason A pointer to a pointer to a
> null-terminated
> > >> - string providing more details for
> the aborted
> > >> - operation. The buffer is
> allocated by this
> > >> - function with AllocatePool(), and
> it is the
> > >> - caller's responsibility to free
> it with a call
> > >> - to FreePool().
> > >> + @param[in] Progress A function used to report the
> progress of
> > >> + updating the firmware device with
> the new
> > >> + firmware image.
> > >> + @param[in] CapsuleFwVersion The version of the new firmware
> image from the
> > >> + update capsule that provided the
> new firmware
> > >> + image.
> > >> + @param[out] AbortReason A pointer to a pointer to a
> Null-terminated
> > >> + Unicode string providing more
> details on an
> > >> + aborted operation. The buffer is
> allocated by
> > >> + this function with
> > >> +
> EFI_BOOT_SERVICES.AllocatePool(). It is the
> > >> + caller's responsibility to free
> this buffer with
> > >> + EFI_BOOT_SERVICES.FreePool().
> > >> + @param[out] LastAttemptStatus A pointer to a UINT32 that holds
> the last attempt
> > >> + status to report back to the ESRT
> table in case
> > >> + of error. This value will only be
> checked when this
> > >> + function returns an error.
> > >>
> > >> - @retval EFI_SUCCESS The device was successfully
> updated with the
> > >> - new image.
> > >> - @retval EFI_ABORTED The operation is aborted.
> > >> + The return status code must fall
> in the range of
> > >> +
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
> > >> +
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.
> > >> +
> > >> + If the value falls outside this
> range, it will be converted
> > >> + to
> LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL.
> > >> +
> > >> + @retval EFI_SUCCESS The firmware device was
> successfully updated
> > >> + with the new firmware image.
> > >> + @retval EFI_ABORTED The operation is aborted.
> Additional details
> > >> + are provided in AbortReason.
> > >> @retval EFI_INVALID_PARAMETER The Image was NULL.
> > >> + @retval EFI_INVALID_PARAMETER LastAttemptStatus was NULL.
> > >> @retval EFI_UNSUPPORTED The operation is not supported.
> > >>
> > >> **/
> > >> EFI_STATUS
> > >> EFIAPI
> > >> -FmpDeviceSetImage (
> > >> +FmpDeviceSetImageWithStatus (
> > >> IN CONST VOID *Image,
> > >> IN UINTN ImageSize,
> > >> - IN CONST VOID *VendorCode,
> > >> - IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS Progress,
> > >> + IN CONST VOID
> *VendorCode, OPTIONAL
> > >> + IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS
> Progress, OPTIONAL
> > >> IN UINT32
> CapsuleFwVersion,
> > >> - OUT CHAR16 **AbortReason
> > >> + OUT CHAR16 **AbortReason,
> > >> + OUT UINT32
> *LastAttemptStatus
> > >> )
> > >> {
> > >> EFI_STATUS Status;
> > >> @@ -513,25 +529,27 @@ FmpDeviceSetImage (
> > >> UINTN BytesWritten;
> > >>
> > >> Updateable = 0;
> > >> - Status = FmpDeviceCheckImage (Image, ImageSize, &Updateable);
> > >> + Status = FmpDeviceCheckImageWithStatus (Image, ImageSize,
> &Updateable, LastAttemptStatus);
> > >> if (EFI_ERROR (Status)) {
> > >> - DEBUG((DEBUG_ERROR, "FmpDeviceSetImage - Check Image failed
> with %r.\n", Status));
> > >> + DEBUG((DEBUG_ERROR, "FmpDeviceSetImageWithStatus - Check
> Image failed with %r.\n", Status));
> > >> return Status;
> > >> }
> > >>
> > >> if (Updateable != IMAGE_UPDATABLE_VALID) {
> > >> - DEBUG((DEBUG_ERROR, "FmpDeviceSetImage - Check Image returned
> that the Image was not valid for update. Updatable
> > >> value = 0x%X.\n", Updateable));
> > >> + DEBUG((DEBUG_ERROR, "FmpDeviceSetImageWithStatus - Check
> Image returned that the Image was not valid for update.
> > >> Updatable value = 0x%X.\n", Updateable));
> > >> + *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
> > >> return EFI_ABORTED;
> > >> }
> > >>
> > >> if (Progress == NULL) {
> > >> - DEBUG((DEBUG_ERROR, "FmpDeviceSetImage - Invalid progress
> callback\n"));
> > >> + DEBUG((DEBUG_ERROR, "FmpDeviceSetImageWithStatus - Invalid
> progress callback\n"));
> > >> + *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
> > >> return EFI_INVALID_PARAMETER;
> > >> }
> > >>
> > >> Status = Progress (15);
> > >> if (EFI_ERROR (Status)) {
> > >> - DEBUG((DEBUG_ERROR, "FmpDeviceSetImage - Progress Callback
> failed with Status %r.\n", Status));
> > >> + DEBUG((DEBUG_ERROR, "FmpDeviceSetImageWithStatus - Progress
> Callback failed with Status %r.\n", Status));
> > >> }
> > >>
> > >> //
> > >> @@ -539,7 +557,7 @@ FmpDeviceSetImage (
> > >> //
> > >> Progress (20);
> > >> if (EFI_ERROR (Status)) {
> > >> - DEBUG((DEBUG_ERROR, "FmpDeviceSetImage - Progress Callback
> failed with Status %r.\n", Status));
> > >> + DEBUG((DEBUG_ERROR, "FmpDeviceSetImageWithStatus - Progress
> Callback failed with Status %r.\n", Status));
> > >> }
> > >>
> > >> //
> > >> @@ -553,11 +571,11 @@ FmpDeviceSetImage (
> > >>
> > >> // Progress (Percentage);
> > >> // if (EFI_ERROR (Status)) {
> > >> -// DEBUG((DEBUG_ERROR, "FmpDeviceSetImage - Progress
> Callback failed with Status %r.\n", Status));
> > >> +// DEBUG((DEBUG_ERROR, "FmpDeviceSetImageWithStatus -
> Progress Callback failed with Status %r.\n", Status));
> > >> // }
> > >> }
> > >>
> > >> - DEBUG ((DEBUG_INFO, "FmpDeviceSetImage - %d Images ...\n",
> ARRAY_SIZE (mUpdateConfigData)));
> > >> + DEBUG ((DEBUG_INFO, "FmpDeviceSetImageWithStatus - %d Images
> ...\n", ARRAY_SIZE (mUpdateConfigData)));
> > >>
> > >> if (ARRAY_SIZE (mUpdateConfigData) == 0) {
> > >> DEBUG((DEBUG_INFO, "PlatformUpdate: BaseAddress - 0x%lx
> ImageOffset - 0x%x Length - 0x%x\n", 0, 0, ImageSize));
> > >> @@ -605,11 +623,173 @@ FmpDeviceSetImage (
> > >> BytesWritten += ConfigData->Length;
> > >> }
> > >>
> > >> - DEBUG ((DEBUG_INFO, "FmpDeviceSetImage - %r\n", Status));
> > >> + DEBUG ((DEBUG_INFO, "FmpDeviceSetImageWithStatus - %r\n", Status));
> > >> +
> > >> + if (EFI_ERROR (Status)) {
> > >> + *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
> > >> + }
> > >>
> > >> return Status;
> > >> }
> > >>
> > >> +/**
> > >> + Updates the firmware image of the device.
> > >> +
> > >> + This function updates the hardware with the new firmware
> image. This function
> > >> + returns EFI_UNSUPPORTED if the firmware image is not
> updatable. If the
> > >> + firmware image is updatable, the function should perform the
> following minimal
> > >> + validations before proceeding to do the firmware image update.
> > >> + - Validate the image is a supported image for this device.
> The function
> > >> + returns EFI_ABORTED if the image is unsupported. The
> function can
> > >> + optionally provide more detailed information on why the
> image is not a
> > >> + supported image.
> > >> + - Validate the data from VendorCode if not null. Image
> validation must be
> > >> + performed before VendorCode data validation. VendorCode
> data is ignored
> > >> + or considered invalid if image validation failed. The
> function returns
> > >> + EFI_ABORTED if the data is invalid.
> > >> +
> > >> + VendorCode enables vendor to implement vendor-specific firmware
> image update
> > >> + policy. Null if the caller did not specify the policy or use
> the default
> > >> + policy. As an example, vendor can implement a policy to allow
> an option to
> > >> + force a firmware image update when the abort reason is due to
> the new firmware
> > >> + image version is older than the current firmware image version
> or bad image
> > >> + checksum. Sensitive operations such as those wiping the entire
> firmware image
> > >> + and render the device to be non-functional should be encoded in
> the image
> > >> + itself rather than passed with the VendorCode. AbortReason
> enables vendor to
> > >> + have the option to provide a more detailed description of the
> abort reason to
> > >> + the caller.
> > >> +
> > >> + @param[in] Image Points to the new image.
> > >> + @param[in] ImageSize Size of the new image in bytes.
> > >> + @param[in] VendorCode This enables vendor to implement
> vendor-specific
> > >> + firmware image update policy.
> Null indicates the
> > >> + caller did not specify the policy
> or use the
> > >> + default policy.
> > >> + @param[in] Progress A function used by the driver to
> report the
> > >> + progress of the firmware update.
> > >> + @param[in] CapsuleFwVersion FMP Payload Header version of the
> image.
> > >> + @param[out] AbortReason A pointer to a pointer to a
> null-terminated
> > >> + string providing more details for
> the aborted
> > >> + operation. The buffer is
> allocated by this
> > >> + function with AllocatePool(), and
> it is the
> > >> + caller's responsibility to free
> it with a call
> > >> + to FreePool().
> > >> +
> > >> + @retval EFI_SUCCESS The device was successfully
> updated with the
> > >> + new image.
> > >> + @retval EFI_ABORTED The operation is aborted.
> > >> + @retval EFI_INVALID_PARAMETER The Image was NULL.
> > >> + @retval EFI_UNSUPPORTED The operation is not supported.
> > >> +
> > >> +**/
> > >> +EFI_STATUS
> > >> +EFIAPI
> > >> +FmpDeviceSetImage (
> > >> + IN CONST VOID *Image,
> > >> + IN UINTN ImageSize,
> > >> + IN CONST VOID *VendorCode,
> > >> + IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS Progress,
> > >> + IN UINT32
> CapsuleFwVersion,
> > >> + OUT CHAR16 **AbortReason
> > >> + )
> > >> +{
> > >> + UINT32 LastAttemptStatus;
> > >> +
> > >> + return FmpDeviceSetImageWithStatus (
> > >> + Image,
> > >> + ImageSize,
> > >> + VendorCode,
> > >> + Progress,
> > >> + CapsuleFwVersion,
> > >> + AbortReason,
> > >> + &LastAttemptStatus
> > >> + );
> > >> +}
> > >> +
> > >> +/**
> > >> + Checks if a new firmware image is valid for the firmware
> device. This
> > >> + function allows firmware update operation to validate the
> firmware image
> > >> + before FmpDeviceSetImage() is called.
> > >> +
> > >> + @param[in] Image Points to a new firmware image.
> > >> + @param[in] ImageSize Size, in bytes, of a new
> firmware image.
> > >> + @param[out] ImageUpdatable Indicates if a new firmware
> image is valid for
> > >> + a firmware update to the
> firmware device. The
> > >> + following values from the
> Firmware Management
> > >> + Protocol are supported:
> > >> + IMAGE_UPDATABLE_VALID
> > >> + IMAGE_UPDATABLE_INVALID
> > >> + IMAGE_UPDATABLE_INVALID_TYPE
> > >> + IMAGE_UPDATABLE_INVALID_OLD
> > >> +
> IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
> > >> + @param[out] LastAttemptStatus A pointer to a UINT32 that
> holds the last attempt
> > >> + status to report back to the
> ESRT table in case
> > >> + of error. This value will only
> be checked when this
> > >> + function returns an error.
> > >> +
> > >> + The return status code must
> fall in the range of
> > >> +
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
> > >> +
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.
> > >> +
> > >> + If the value falls outside this
> range, it will be converted
> > >> + to
> LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL.
> > >> +
> > >> + @retval EFI_SUCCESS The image was successfully
> checked. Additional
> > >> + status information is returned in
> > >> + ImageUpdatable.
> > >> + @retval EFI_INVALID_PARAMETER Image is NULL.
> > >> + @retval EFI_INVALID_PARAMETER ImageUpdatable is NULL.
> > >> + @retval EFI_INVALID_PARAMETER LastAttemptStatus is NULL.
> > >> +
> > >> +**/
> > >> +EFI_STATUS
> > >> +EFIAPI
> > >> +FmpDeviceCheckImageWithStatus (
> > >> + IN CONST VOID *Image,
> > >> + IN UINTN ImageSize,
> > >> + OUT UINT32 *ImageUpdatable,
> > >> + OUT UINT32 *LastAttemptStatus
> > >> + )
> > >> +{
> > >> + if (LastAttemptStatus == NULL) {
> > >> + DEBUG ((DEBUG_ERROR, "CheckImageWithStatus -
> LastAttemptStatus Pointer Parameter is NULL.\n"));
> > >> + return EFI_INVALID_PARAMETER;
> > >> + }
> > >> + *LastAttemptStatus = LAST_ATTEMPT_STATUS_SUCCESS;
> > >> +
> > >> + if (ImageUpdatable == NULL) {
> > >> + DEBUG((DEBUG_ERROR, "CheckImageWithStatus - ImageUpdatable
> Pointer Parameter is NULL.\n"));
> > >> + *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
> > >> + return EFI_INVALID_PARAMETER;
> > >> + }
> > >> +
> > >> + //
> > >> + //Set to valid and then if any tests fail it will update this flag.
> > >> + //
> > >> + *ImageUpdatable = IMAGE_UPDATABLE_VALID;
> > >> +
> > >> + if (Image == NULL) {
> > >> + DEBUG((DEBUG_ERROR, "CheckImageWithStatus - Image Pointer
> Parameter is NULL.\n"));
> > >> + //
> > >> + // Not sure if this is needed
> > >> + //
> > >> + *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
> > >> + *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
> > >> + return EFI_INVALID_PARAMETER;
> > >> + }
> > >> +
> > >> + //
> > >> + // Make sure the image size is correct
> > >> + //
> > >> + if (ImageSize != PcdGet32 (PcdBiosRomSize)) {
> > >> + *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
> > >> + *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
> > >> + return EFI_INVALID_PARAMETER;
> > >> + }
> > >> +
> > >> + return EFI_SUCCESS;
> > >> +}
> > >> +
> > >> /**
> > >> Checks if the firmware image is valid for the device.
> > >>
> > >> @@ -633,34 +813,9 @@ FmpDeviceCheckImage (
> > >> OUT UINT32 *ImageUpdateable
> > >> )
> > >> {
> > >> - if (ImageUpdateable == NULL) {
> > >> - DEBUG((DEBUG_ERROR, "CheckImage - ImageUpdateable Pointer
> Parameter is NULL.\n"));
> > >> - return EFI_INVALID_PARAMETER;
> > >> - }
> > >> + UINT32 LastAttemptStatus;
> > >>
> > >> - //
> > >> - //Set to valid and then if any tests fail it will update this flag.
> > >> - //
> > >> - *ImageUpdateable = IMAGE_UPDATABLE_VALID;
> > >> -
> > >> - if (Image == NULL) {
> > >> - DEBUG((DEBUG_ERROR, "CheckImage - Image Pointer Parameter is
> NULL.\n"));
> > >> - //
> > >> - // Not sure if this is needed
> > >> - //
> > >> - *ImageUpdateable = IMAGE_UPDATABLE_INVALID;
> > >> - return EFI_INVALID_PARAMETER;
> > >> - }
> > >> -
> > >> - //
> > >> - // Make sure the image size is correct
> > >> - //
> > >> - if (ImageSize != PcdGet32 (PcdBiosRomSize)) {
> > >> - *ImageUpdateable = IMAGE_UPDATABLE_INVALID;
> > >> - return EFI_INVALID_PARAMETER;
> > >> - }
> > >> -
> > >> - return EFI_SUCCESS;
> > >> + return FmpDeviceCheckImageWithStatus (Image, ImageSize,
> ImageUpdateable, &LastAttemptStatus);
> > >> }
> > >>
> > >> /**
> > >> diff --git
> a/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLibSample/FmpDeviceLib.c
> > >>
> b/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLibSample/FmpDeviceLib.c
> > >> index db0f238ea534..132b60844ad4 100644
> > >> ---
> a/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLibSample/FmpDeviceLib.c
> > >> +++
> b/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLibSample/FmpDeviceLib.c
> > >> @@ -1,6 +1,6 @@
> > >> /**
> > >>
> > >> -Copyright (c) 2016, Microsoft Corporation. All rights reserved.<BR>
> > >> +Copyright (c) Microsoft Corporation.<BR>
> > >> Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > >>
> > >> SPDX-License-Identifier: BSD-2-Clause-Patent
> > >> @@ -9,6 +9,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > >>
> > >>
> > >> #include <PiDxe.h>
> > >> +#include <LastAttemptStatus.h>
> > >> +#include <Guid/SystemResourceTable.h>
> > >> #include <Library/DebugLib.h>
> > >> #include <Protocol/FirmwareManagement.h>
> > >> #include <Library/BaseLib.h>
> > >> @@ -345,6 +347,131 @@ Return Value:
> > >> }//GetImage()
> > >>
> > >>
> > >> +/**
> > >> + Updates a firmware device with a new firmware image. This
> function returns
> > >> + EFI_UNSUPPORTED if the firmware image is not updatable. If the
> firmware image
> > >> + is updatable, the function should perform the following minimal
> validations
> > >> + before proceeding to do the firmware image update.
> > >> + - Validate that the image is a supported image for this
> firmware device.
> > >> + Return EFI_ABORTED if the image is not supported.
> Additional details
> > >> + on why the image is not a supported image may be returned
> in AbortReason.
> > >> + - Validate the data from VendorCode if is not NULL. Firmware
> image
> > >> + validation must be performed before VendorCode data validation.
> > >> + VendorCode data is ignored or considered invalid if image
> validation
> > >> + fails. Return EFI_ABORTED if the VendorCode data is invalid.
> > >> +
> > >> + VendorCode enables vendor to implement vendor-specific firmware
> image update
> > >> + policy. Null if the caller did not specify the policy or use
> the default
> > >> + policy. As an example, vendor can implement a policy to allow
> an option to
> > >> + force a firmware image update when the abort reason is due to
> the new firmware
> > >> + image version is older than the current firmware image version
> or bad image
> > >> + checksum. Sensitive operations such as those wiping the entire
> firmware image
> > >> + and render the device to be non-functional should be encoded in
> the image
> > >> + itself rather than passed with the VendorCode. AbortReason
> enables vendor to
> > >> + have the option to provide a more detailed description of the
> abort reason to
> > >> + the caller.
> > >> +
> > >> + @param[in] Image Points to the new firmware image.
> > >> + @param[in] ImageSize Size, in bytes, of the new
> firmware image.
> > >> + @param[in] VendorCode This enables vendor to implement
> vendor-specific
> > >> + firmware image update policy.
> NULL indicates
> > >> + the caller did not specify the
> policy or use the
> > >> + default policy.
> > >> + @param[in] Progress A function used to report the
> progress of
> > >> + updating the firmware device with
> the new
> > >> + firmware image.
> > >> + @param[in] CapsuleFwVersion The version of the new firmware
> image from the
> > >> + update capsule that provided the
> new firmware
> > >> + image.
> > >> + @param[out] AbortReason A pointer to a pointer to a
> Null-terminated
> > >> + Unicode string providing more
> details on an
> > >> + aborted operation. The buffer is
> allocated by
> > >> + this function with
> > >> +
> EFI_BOOT_SERVICES.AllocatePool(). It is the
> > >> + caller's responsibility to free
> this buffer with
> > >> + EFI_BOOT_SERVICES.FreePool().
> > >> + @param[out] LastAttemptStatus A pointer to a UINT32 that holds
> the last attempt
> > >> + status to report back to the ESRT
> table in case
> > >> + of error. This value will only be
> checked when this
> > >> + function returns an error.
> > >> +
> > >> + The return status code must fall
> in the range of
> > >> +
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
> > >> +
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.
> > >> +
> > >> + If the value falls outside this
> range, it will be converted
> > >> + to
> LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL.
> > >> +
> > >> + @retval EFI_SUCCESS The firmware device was
> successfully updated
> > >> + with the new firmware image.
> > >> + @retval EFI_ABORTED The operation is aborted.
> Additional details
> > >> + are provided in AbortReason.
> > >> + @retval EFI_INVALID_PARAMETER The Image was NULL.
> > >> + @retval EFI_INVALID_PARAMETER LastAttemptStatus was NULL.
> > >> + @retval EFI_UNSUPPORTED The operation is not supported.
> > >> +
> > >> +**/
> > >> +EFI_STATUS
> > >> +EFIAPI
> > >> +FmpDeviceSetImageWithStatus (
> > >> + IN CONST VOID *Image,
> > >> + IN UINTN ImageSize,
> > >> + IN CONST VOID
> *VendorCode, OPTIONAL
> > >> + IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS
> Progress, OPTIONAL
> > >> + IN UINT32
> CapsuleFwVersion,
> > >> + OUT CHAR16 **AbortReason,
> > >> + OUT UINT32
> *LastAttemptStatus
> > >> + )
> > >> +{
> > >> + EFI_STATUS Status = EFI_SUCCESS;
> > >> + UINT32 Updateable = 0;
> > >> +
> > >> + Status = FmpDeviceCheckImageWithStatus (Image, ImageSize,
> &Updateable, LastAttemptStatus);
> > >> + if (EFI_ERROR(Status))
> > >> + {
> > >> + DEBUG((DEBUG_ERROR, "SetImageWithStatus - Check Image
> failed with %r.\n", Status));
> > >> + goto cleanup;
> > >> + }
> > >> +
> > >> + if (Updateable != IMAGE_UPDATABLE_VALID)
> > >> + {
> > >> + DEBUG((DEBUG_ERROR, "SetImageWithStatus - Check Image
> returned that the Image was not valid for update.
> > >> Updatable value = 0x%X.\n", Updateable));
> > >> + Status = EFI_ABORTED;
> > >> + goto cleanup;
> > >> + }
> > >> +
> > >> + if (Progress == NULL)
> > >> + {
> > >> + DEBUG((DEBUG_ERROR, "SetImageWithStatus - Invalid
> progress callback\n"));
> > >> + Status = EFI_INVALID_PARAMETER;
> > >> + goto cleanup;
> > >> + }
> > >> +
> > >> + Status = Progress(15);
> > >> + if (EFI_ERROR(Status))
> > >> + {
> > >> + DEBUG((DEBUG_ERROR, "SetImageWithStatus - Progress
> Callback failed with Status %r.\n", Status));
> > >> + }
> > >> +
> > >> + {
> > >> + UINTN p;
> > >> +
> > >> + for (p = 20; p < 100; p++) {
> > >> + gBS->Stall (100000); //us = 0.1 seconds
> > >> + Progress (p);
> > >> + }
> > >> + }
> > >> +
> > >> + //TODO: add support for VendorCode, and AbortReason
> > >> +cleanup:
> > >> + if (EFI_ERROR (Status)) {
> > >> + *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
> > >> + }
> > >> +
> > >> + return Status;
> > >> +}// SetImageWithStatus()
> > >> +
> > >> +
> > >> /**
> > >> Updates the firmware image of the device.
> > >>
> > >> @@ -396,51 +523,98 @@ IN
> UINT32 CapsuleFwVersion,
> > >> OUT CHAR16 **AbortReason
> > >> )
> > >> {
> > >> - EFI_STATUS Status = EFI_SUCCESS;
> > >> - UINT32 Updateable = 0;
> > >> -
> > >> - Status = FmpDeviceCheckImage(Image, ImageSize, &Updateable);
> > >> - if (EFI_ERROR(Status))
> > >> - {
> > >> - DEBUG((DEBUG_ERROR, "SetImage - Check Image failed with
> %r.\n", Status));
> > >> - goto cleanup;
> > >> - }
> > >> -
> > >> - if (Updateable != IMAGE_UPDATABLE_VALID)
> > >> - {
> > >> - DEBUG((DEBUG_ERROR, "SetImage - Check Image returned that
> the Image was not valid for update. Updatable value
> > =
> > >> 0x%X.\n", Updateable));
> > >> - Status = EFI_ABORTED;
> > >> - goto cleanup;
> > >> - }
> > >> -
> > >> - if (Progress == NULL)
> > >> - {
> > >> - DEBUG((DEBUG_ERROR, "SetImage - Invalid progress
> callback\n"));
> > >> - Status = EFI_INVALID_PARAMETER;
> > >> - goto cleanup;
> > >> - }
> > >> -
> > >> - Status = Progress(15);
> > >> - if (EFI_ERROR(Status))
> > >> - {
> > >> - DEBUG((DEBUG_ERROR, "SetImage - Progress Callback failed
> with Status %r.\n", Status));
> > >> - }
> > >> -
> > >> - {
> > >> - UINTN p;
> > >> -
> > >> - for (p = 20; p < 100; p++) {
> > >> - gBS->Stall (100000); //us = 0.1 seconds
> > >> - Progress (p);
> > >> - }
> > >> - }
> > >> -
> > >> - //TODO: add support for VendorCode, and AbortReason
> > >> -cleanup:
> > >> - return Status;
> > >> + UINT32 LastAttemptStatus;
> > >> +
> > >> + return FmpDeviceSetImageWithStatus (
> > >> + Image,
> > >> + ImageSize,
> > >> + VendorCode,
> > >> + Progress,
> > >> + CapsuleFwVersion,
> > >> + AbortReason,
> > >> + &LastAttemptStatus
> > >> + );
> > >> }// SetImage()
> > >>
> > >>
> > >> +/**
> > >> + Checks if a new firmware image is valid for the firmware
> device. This
> > >> + function allows firmware update operation to validate the
> firmware image
> > >> + before FmpDeviceSetImage() is called.
> > >> +
> > >> + @param[in] Image Points to a new firmware image.
> > >> + @param[in] ImageSize Size, in bytes, of a new
> firmware image.
> > >> + @param[out] ImageUpdatable Indicates if a new firmware
> image is valid for
> > >> + a firmware update to the
> firmware device. The
> > >> + following values from the
> Firmware Management
> > >> + Protocol are supported:
> > >> + IMAGE_UPDATABLE_VALID
> > >> + IMAGE_UPDATABLE_INVALID
> > >> + IMAGE_UPDATABLE_INVALID_TYPE
> > >> + IMAGE_UPDATABLE_INVALID_OLD
> > >> +
> IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
> > >> + @param[out] LastAttemptStatus A pointer to a UINT32 that
> holds the last attempt
> > >> + status to report back to the
> ESRT table in case
> > >> + of error. This value will only
> be checked when this
> > >> + function returns an error.
> > >> +
> > >> + The return status code must
> fall in the range of
> > >> +
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
> > >> +
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.
> > >> +
> > >> + If the value falls outside this
> range, it will be converted
> > >> + to
> LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL.
> > >> +
> > >> + @retval EFI_SUCCESS The image was successfully
> checked. Additional
> > >> + status information is returned in
> > >> + ImageUpdatable.
> > >> + @retval EFI_INVALID_PARAMETER Image is NULL.
> > >> + @retval EFI_INVALID_PARAMETER ImageUpdatable is NULL.
> > >> + @retval EFI_INVALID_PARAMETER LastAttemptStatus is NULL.
> > >> +
> > >> +**/
> > >> +EFI_STATUS
> > >> +EFIAPI
> > >> +FmpDeviceCheckImageWithStatus (
> > >> + IN CONST VOID *Image,
> > >> + IN UINTN ImageSize,
> > >> + OUT UINT32 *ImageUpdatable,
> > >> + OUT UINT32 *LastAttemptStatus
> > >> + )
> > >> +{
> > >> + EFI_STATUS status = EFI_SUCCESS;
> > >> +
> > >> + if (LastAttemptStatus == NULL) {
> > >> + DEBUG ((DEBUG_ERROR, "CheckImageWithStatus -
> LastAttemptStatus Pointer Parameter is NULL.\n"));
> > >> + return EFI_INVALID_PARAMETER;
> > >> + }
> > >> + *LastAttemptStatus = LAST_ATTEMPT_STATUS_SUCCESS;
> > >> +
> > >> + if (ImageUpdatable == NULL)
> > >> + {
> > >> + DEBUG((DEBUG_ERROR, "CheckImageWithStatus -
> ImageUpdatable Pointer Parameter is NULL.\n"));
> > >> + *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
> > >> + status = EFI_INVALID_PARAMETER;
> > >> + goto cleanup;
> > >> + }
> > >> +
> > >> + //
> > >> + //Set to valid and then if any tests fail it will update this
> flag.
> > >> + //
> > >> + *ImageUpdatable = IMAGE_UPDATABLE_VALID;
> > >> +
> > >> + if (Image == NULL)
> > >> + {
> > >> + DEBUG((DEBUG_ERROR, "CheckImageWithStatus - Image Pointer
> Parameter is NULL.\n"));
> > >> + *ImageUpdatable = IMAGE_UPDATABLE_INVALID; //not sure if
> this is needed
> > >> + *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
> > >> + return EFI_INVALID_PARAMETER;
> > >> + }
> > >> +
> > >> +cleanup:
> > >> + return status;
> > >> +}// CheckImageWithStatus()
> > >> +
> > >>
> > >> /**
> > >> Checks if the firmware image is valid for the device.
> > >> @@ -465,29 +639,9 @@ IN UINTN ImageSize,
> > >> OUT UINT32 *ImageUpdateable
> > >> )
> > >> {
> > >> - EFI_STATUS status = EFI_SUCCESS;
> > >> + UINT32 LastAttemptStatus;
> > >>
> > >> - if (ImageUpdateable == NULL)
> > >> - {
> > >> - DEBUG((DEBUG_ERROR, "CheckImage - ImageUpdateable Pointer
> Parameter is NULL.\n"));
> > >> - status = EFI_INVALID_PARAMETER;
> > >> - goto cleanup;
> > >> - }
> > >> -
> > >> - //
> > >> - //Set to valid and then if any tests fail it will update this
> flag.
> > >> - //
> > >> - *ImageUpdateable = IMAGE_UPDATABLE_VALID;
> > >> -
> > >> - if (Image == NULL)
> > >> - {
> > >> - DEBUG((DEBUG_ERROR, "CheckImage - Image Pointer Parameter
> is NULL.\n"));
> > >> - *ImageUpdateable = IMAGE_UPDATABLE_INVALID; //not sure if
> this is needed
> > >> - return EFI_INVALID_PARAMETER;
> > >> - }
> > >> -
> > >> -cleanup:
> > >> - return status;
> > >> + return FmpDeviceCheckImageWithStatus (Image, ImageSize,
> ImageUpdateable, &LastAttemptStatus);
> > >> }// CheckImage()
> > >>
> > >> /**
> > >> --
> > >> 2.28.0.windows.1
> > >
>
>
>
>
next prev parent reply other threads:[~2020-11-11 5:17 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-01 21:58 [edk2-platforms][PATCH v1 1/1] Vlv2TbltDevicePkg/FmpDeviceLib: Add LastAttemptStatus compatibility Michael Kubacki
2020-10-29 1:12 ` Michael D Kinney
2020-10-29 4:45 ` Michael Kubacki
2020-11-05 20:36 ` Michael Kubacki
2020-11-11 3:03 ` Michael D Kinney
2020-11-11 4:16 ` [edk2-devel] " Kilian Kegel
2020-11-11 5:17 ` Michael Kubacki [this message]
2020-11-11 17:17 ` Michael D Kinney
[not found] <1639FD37CA1FD9A2.6392@groups.io>
2020-10-15 21:27 ` Michael Kubacki
[not found] ` <bf090dba-75d3-520e-7414-bcdb8314562b@outlook.com>
2020-10-27 0:50 ` Michael Kubacki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MWHPR07MB3440EFF12E7C9EE325EDE1CEE9E80@MWHPR07MB3440.namprd07.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox