Hi all,
if this patch also affects https://bugzilla.tianocore.org/show_bug.cgi?id=2983, please close it.
Thanks,
Kilian
Von: Michael D Kinney
Gesendet: Wednesday, November 11, 2020 4:03 AM
An: Michael Kubacki;
devel@edk2.groups.io; Kinney, Michael D
Cc: Qian, Yi;
Sun, Zailiang
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
> >