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
> >