From: "Michael Kubacki" <michael.kubacki@outlook.com>
To: devel@edk2.groups.io
Cc: Michael D Kinney <michael.d.kinney@intel.com>,
Yi Qian <yi.qian@intel.com>,
Zailiang Sun <zailiang.sun@intel.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] Vlv2TbltDevicePkg/FmpDeviceLib: Add LastAttemptStatus compatibility
Date: Thu, 15 Oct 2020 14:27:01 -0700 [thread overview]
Message-ID: <MWHPR07MB3440DD69616546F721E2DABBE9020@MWHPR07MB3440.namprd07.prod.outlook.com> (raw)
In-Reply-To: <1639FD37CA1FD9A2.6392@groups.io>
Hi,
It's been two weeks with no feedback.
A review would be appreciated.
Thanks,
Michael
On 10/1/2020 2:58 PM, Michael Kubacki wrote:
> 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()
>
> /**
>
next parent reply other threads:[~2020-10-15 21:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1639FD37CA1FD9A2.6392@groups.io>
2020-10-15 21:27 ` Michael Kubacki [this message]
[not found] ` <bf090dba-75d3-520e-7414-bcdb8314562b@outlook.com>
2020-10-27 0:50 ` [edk2-devel] [edk2-platforms][PATCH v1 1/1] Vlv2TbltDevicePkg/FmpDeviceLib: Add LastAttemptStatus compatibility Michael Kubacki
2020-10-01 21:58 Michael Kubacki
2020-10-29 1:12 ` Michael D Kinney
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
2020-11-11 17:17 ` Michael D Kinney
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=MWHPR07MB3440DD69616546F721E2DABBE9020@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