From: "Michael Kubacki" <michael.kubacki@outlook.com>
To: "Kinney, Michael D" <michael.d.kinney@intel.com>,
"devel@edk2.groups.io" <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
Date: Wed, 28 Oct 2020 21:45:33 -0700 [thread overview]
Message-ID: <MWHPR07MB3440BA6DD386CBA8E40BE459E9140@MWHPR07MB3440.namprd07.prod.outlook.com> (raw)
In-Reply-To: <MN2PR11MB4461D254A74169B397BBA025D2140@MN2PR11MB4461.namprd11.prod.outlook.com>
Thank you for the test confirmation.
Regards,
Michael
On 10/28/2020 6:12 PM, Kinney, Michael D wrote:
> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
>
> I also verified build and boot and FMP based capsule updates work as expected.
>
> Tested-by: Michael D Kinney <Michael.d.kinney@intel.com>
>
> Thanks,
>
> Mike
>
>
>> -----Original Message-----
>> From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
>> Sent: Thursday, October 1, 2020 2:59 PM
>> To: devel@edk2.groups.io
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Qian, Yi <yi.qian@intel.com>; Sun, Zailiang <zailiang.sun@intel.com>
>> Subject: [edk2-platforms][PATCH v1 1/1] Vlv2TbltDevicePkg/FmpDeviceLib: Add LastAttemptStatus compatibility
>>
>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> Makes the changes necessary for these library instances of
>> FmpDeviceLib to be compatible with new functions added recently
>> to FmpDeviceLib.
>>
>> Two new functions were introduced in FmpDeviceLib to allow a
>> library instance to return a Last Attempt Status code during
>> check image and set image operations:
>> 1. FmpDeviceCheckImageWithStatus ( )
>> 2. FmpDeviceSetImageWithStatus ( )
>>
>> FmpDxe (in FmpDevicePkg) will begin calling these new functions
>> instead of the previous functions. Therefore, this change:
>> 1. Adds these functions to Vlv2TbltDevicePkg implementations
>> 2. Moves the main functionality to these new functions
>> 3. Updates the old functions to call the new functions
>> (for backward compatibility)
>>
>> Note: As of this commit, the Vlv2TbltDevicePkg build is broken
>> due to:
>> 1. A required RngLib library instance not defined by the platform
>> 2. Other FMP libraries not being defined by the platform
>> (e.g. FmpDependencyLib, FmpDependencyCheckLib, etc.)
>>
>> Those changes were fixed locally to test the changes in this commit
>> but maintainers should make the proper changes for those issues.
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Yi Qian <yi.qian@intel.com>
>> Cc: Zailiang Sun <zailiang.sun@intel.com>
>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>> ---
>>
>> Notes:
>> Only build was checked, I do not have access to a
>> VLV2 device for testing.
>>
>> Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLib/FmpDeviceLib.c | 299 +++++++++++++++-----
>> Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLibSample/FmpDeviceLib.c | 284 ++++++++++++++-----
>> 2 files changed, 446 insertions(+), 137 deletions(-)
>>
>> diff --git a/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLib/FmpDeviceLib.c
>> b/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLib/FmpDeviceLib.c
>> index d8c9036012ad..df8a36d9854c 100644
>> --- a/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLib/FmpDeviceLib.c
>> +++ b/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLib/FmpDeviceLib.c
>> @@ -1,6 +1,6 @@
>> /**
>>
>> -Copyright (c) 2016, Microsoft Corporation. All rights reserved.
>> +Copyright (c) Microsoft Corporation.<BR>
>> Copyright (c) 2019, Intel Corporation. All rights reserved.
>>
>> SPDX-License-Identifier: BSD-2-Clause-Patent
>> @@ -8,7 +8,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>> **/
>>
>> #include <PiDxe.h>
>> -
>> +#include <LastAttemptStatus.h>
>> +#include <Guid/SystemResourceTable.h>
>> #include <Library/FmpDeviceLib.h>
>>
>> #include <Library/DebugLib.h>
>> @@ -444,20 +445,17 @@ FmpDeviceGetImage (
>> }
>>
>> /**
>> - Updates the firmware image of the device.
>> -
>> - This function updates the hardware with the new firmware image. This function
>> - returns EFI_UNSUPPORTED if the firmware image is not updatable. If the
>> - firmware image is updatable, the function should perform the following minimal
>> - validations before proceeding to do the firmware image update.
>> - - Validate the image is a supported image for this device. The function
>> - returns EFI_ABORTED if the image is unsupported. The function can
>> - optionally provide more detailed information on why the image is not a
>> - supported image.
>> - - Validate the data from VendorCode if not null. Image validation must be
>> - performed before VendorCode data validation. VendorCode data is ignored
>> - or considered invalid if image validation failed. The function returns
>> - EFI_ABORTED if the data is invalid.
>> + Updates a firmware device with a new firmware image. This function returns
>> + EFI_UNSUPPORTED if the firmware image is not updatable. If the firmware image
>> + is updatable, the function should perform the following minimal validations
>> + before proceeding to do the firmware image update.
>> + - Validate that the image is a supported image for this firmware device.
>> + Return EFI_ABORTED if the image is not supported. Additional details
>> + on why the image is not a supported image may be returned in AbortReason.
>> + - Validate the data from VendorCode if is not NULL. Firmware image
>> + validation must be performed before VendorCode data validation.
>> + VendorCode data is ignored or considered invalid if image validation
>> + fails. Return EFI_ABORTED if the VendorCode data is invalid.
>>
>> VendorCode enables vendor to implement vendor-specific firmware image update
>> policy. Null if the caller did not specify the policy or use the default
>> @@ -470,38 +468,56 @@ FmpDeviceGetImage (
>> have the option to provide a more detailed description of the abort reason to
>> the caller.
>>
>> - @param[in] Image Points to the new image.
>> - @param[in] ImageSize Size of the new image in bytes.
>> + @param[in] Image Points to the new firmware image.
>> + @param[in] ImageSize Size, in bytes, of the new firmware image.
>> @param[in] VendorCode This enables vendor to implement vendor-specific
>> - firmware image update policy. Null indicates the
>> - caller did not specify the policy or use the
>> + firmware image update policy. NULL indicates
>> + the caller did not specify the policy or use the
>> default policy.
>> - @param[in] Progress A function used by the driver to report the
>> - progress of the firmware update.
>> - @param[in] CapsuleFwVersion FMP Payload Header version of the image.
>> - @param[out] AbortReason A pointer to a pointer to a null-terminated
>> - string providing more details for the aborted
>> - operation. The buffer is allocated by this
>> - function with AllocatePool(), and it is the
>> - caller's responsibility to free it with a call
>> - to FreePool().
>> + @param[in] Progress A function used to report the progress of
>> + updating the firmware device with the new
>> + firmware image.
>> + @param[in] CapsuleFwVersion The version of the new firmware image from the
>> + update capsule that provided the new firmware
>> + image.
>> + @param[out] AbortReason A pointer to a pointer to a Null-terminated
>> + Unicode string providing more details on an
>> + aborted operation. The buffer is allocated by
>> + this function with
>> + EFI_BOOT_SERVICES.AllocatePool(). It is the
>> + caller's responsibility to free this buffer with
>> + EFI_BOOT_SERVICES.FreePool().
>> + @param[out] LastAttemptStatus A pointer to a UINT32 that holds the last attempt
>> + status to report back to the ESRT table in case
>> + of error. This value will only be checked when this
>> + function returns an error.
>>
>> - @retval EFI_SUCCESS The device was successfully updated with the
>> - new image.
>> - @retval EFI_ABORTED The operation is aborted.
>> + The return status code must fall in the range of
>> + LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
>> + LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.
>> +
>> + If the value falls outside this range, it will be converted
>> + to LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL.
>> +
>> + @retval EFI_SUCCESS The firmware device was successfully updated
>> + with the new firmware image.
>> + @retval EFI_ABORTED The operation is aborted. Additional details
>> + are provided in AbortReason.
>> @retval EFI_INVALID_PARAMETER The Image was NULL.
>> + @retval EFI_INVALID_PARAMETER LastAttemptStatus was NULL.
>> @retval EFI_UNSUPPORTED The operation is not supported.
>>
>> **/
>> EFI_STATUS
>> EFIAPI
>> -FmpDeviceSetImage (
>> +FmpDeviceSetImageWithStatus (
>> IN CONST VOID *Image,
>> IN UINTN ImageSize,
>> - IN CONST VOID *VendorCode,
>> - IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS Progress,
>> + IN CONST VOID *VendorCode, OPTIONAL
>> + IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS Progress, OPTIONAL
>> IN UINT32 CapsuleFwVersion,
>> - OUT CHAR16 **AbortReason
>> + OUT CHAR16 **AbortReason,
>> + OUT UINT32 *LastAttemptStatus
>> )
>> {
>> EFI_STATUS Status;
>> @@ -513,25 +529,27 @@ FmpDeviceSetImage (
>> UINTN BytesWritten;
>>
>> Updateable = 0;
>> - Status = FmpDeviceCheckImage (Image, ImageSize, &Updateable);
>> + Status = FmpDeviceCheckImageWithStatus (Image, ImageSize, &Updateable, LastAttemptStatus);
>> if (EFI_ERROR (Status)) {
>> - DEBUG((DEBUG_ERROR, "FmpDeviceSetImage - Check Image failed with %r.\n", Status));
>> + DEBUG((DEBUG_ERROR, "FmpDeviceSetImageWithStatus - Check Image failed with %r.\n", Status));
>> return Status;
>> }
>>
>> if (Updateable != IMAGE_UPDATABLE_VALID) {
>> - DEBUG((DEBUG_ERROR, "FmpDeviceSetImage - Check Image returned that the Image was not valid for update. Updatable
>> value = 0x%X.\n", Updateable));
>> + DEBUG((DEBUG_ERROR, "FmpDeviceSetImageWithStatus - Check Image returned that the Image was not valid for update.
>> Updatable value = 0x%X.\n", Updateable));
>> + *LastAttemptStatus = LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
>> return EFI_ABORTED;
>> }
>>
>> if (Progress == NULL) {
>> - DEBUG((DEBUG_ERROR, "FmpDeviceSetImage - Invalid progress callback\n"));
>> + DEBUG((DEBUG_ERROR, "FmpDeviceSetImageWithStatus - Invalid progress callback\n"));
>> + *LastAttemptStatus = LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
>> return EFI_INVALID_PARAMETER;
>> }
>>
>> Status = Progress (15);
>> if (EFI_ERROR (Status)) {
>> - DEBUG((DEBUG_ERROR, "FmpDeviceSetImage - Progress Callback failed with Status %r.\n", Status));
>> + DEBUG((DEBUG_ERROR, "FmpDeviceSetImageWithStatus - Progress Callback failed with Status %r.\n", Status));
>> }
>>
>> //
>> @@ -539,7 +557,7 @@ FmpDeviceSetImage (
>> //
>> Progress (20);
>> if (EFI_ERROR (Status)) {
>> - DEBUG((DEBUG_ERROR, "FmpDeviceSetImage - Progress Callback failed with Status %r.\n", Status));
>> + DEBUG((DEBUG_ERROR, "FmpDeviceSetImageWithStatus - Progress Callback failed with Status %r.\n", Status));
>> }
>>
>> //
>> @@ -553,11 +571,11 @@ FmpDeviceSetImage (
>>
>> // Progress (Percentage);
>> // if (EFI_ERROR (Status)) {
>> -// DEBUG((DEBUG_ERROR, "FmpDeviceSetImage - Progress Callback failed with Status %r.\n", Status));
>> +// DEBUG((DEBUG_ERROR, "FmpDeviceSetImageWithStatus - Progress Callback failed with Status %r.\n", Status));
>> // }
>> }
>>
>> - DEBUG ((DEBUG_INFO, "FmpDeviceSetImage - %d Images ...\n", ARRAY_SIZE (mUpdateConfigData)));
>> + DEBUG ((DEBUG_INFO, "FmpDeviceSetImageWithStatus - %d Images ...\n", ARRAY_SIZE (mUpdateConfigData)));
>>
>> if (ARRAY_SIZE (mUpdateConfigData) == 0) {
>> DEBUG((DEBUG_INFO, "PlatformUpdate: BaseAddress - 0x%lx ImageOffset - 0x%x Length - 0x%x\n", 0, 0, ImageSize));
>> @@ -605,11 +623,173 @@ FmpDeviceSetImage (
>> BytesWritten += ConfigData->Length;
>> }
>>
>> - DEBUG ((DEBUG_INFO, "FmpDeviceSetImage - %r\n", Status));
>> + DEBUG ((DEBUG_INFO, "FmpDeviceSetImageWithStatus - %r\n", Status));
>> +
>> + if (EFI_ERROR (Status)) {
>> + *LastAttemptStatus = LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
>> + }
>>
>> return Status;
>> }
>>
>> +/**
>> + Updates the firmware image of the device.
>> +
>> + This function updates the hardware with the new firmware image. This function
>> + returns EFI_UNSUPPORTED if the firmware image is not updatable. If the
>> + firmware image is updatable, the function should perform the following minimal
>> + validations before proceeding to do the firmware image update.
>> + - Validate the image is a supported image for this device. The function
>> + returns EFI_ABORTED if the image is unsupported. The function can
>> + optionally provide more detailed information on why the image is not a
>> + supported image.
>> + - Validate the data from VendorCode if not null. Image validation must be
>> + performed before VendorCode data validation. VendorCode data is ignored
>> + or considered invalid if image validation failed. The function returns
>> + EFI_ABORTED if the data is invalid.
>> +
>> + VendorCode enables vendor to implement vendor-specific firmware image update
>> + policy. Null if the caller did not specify the policy or use the default
>> + policy. As an example, vendor can implement a policy to allow an option to
>> + force a firmware image update when the abort reason is due to the new firmware
>> + image version is older than the current firmware image version or bad image
>> + checksum. Sensitive operations such as those wiping the entire firmware image
>> + and render the device to be non-functional should be encoded in the image
>> + itself rather than passed with the VendorCode. AbortReason enables vendor to
>> + have the option to provide a more detailed description of the abort reason to
>> + the caller.
>> +
>> + @param[in] Image Points to the new image.
>> + @param[in] ImageSize Size of the new image in bytes.
>> + @param[in] VendorCode This enables vendor to implement vendor-specific
>> + firmware image update policy. Null indicates the
>> + caller did not specify the policy or use the
>> + default policy.
>> + @param[in] Progress A function used by the driver to report the
>> + progress of the firmware update.
>> + @param[in] CapsuleFwVersion FMP Payload Header version of the image.
>> + @param[out] AbortReason A pointer to a pointer to a null-terminated
>> + string providing more details for the aborted
>> + operation. The buffer is allocated by this
>> + function with AllocatePool(), and it is the
>> + caller's responsibility to free it with a call
>> + to FreePool().
>> +
>> + @retval EFI_SUCCESS The device was successfully updated with the
>> + new image.
>> + @retval EFI_ABORTED The operation is aborted.
>> + @retval EFI_INVALID_PARAMETER The Image was NULL.
>> + @retval EFI_UNSUPPORTED The operation is not supported.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +FmpDeviceSetImage (
>> + IN CONST VOID *Image,
>> + IN UINTN ImageSize,
>> + IN CONST VOID *VendorCode,
>> + IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS Progress,
>> + IN UINT32 CapsuleFwVersion,
>> + OUT CHAR16 **AbortReason
>> + )
>> +{
>> + UINT32 LastAttemptStatus;
>> +
>> + return FmpDeviceSetImageWithStatus (
>> + Image,
>> + ImageSize,
>> + VendorCode,
>> + Progress,
>> + CapsuleFwVersion,
>> + AbortReason,
>> + &LastAttemptStatus
>> + );
>> +}
>> +
>> +/**
>> + Checks if a new firmware image is valid for the firmware device. This
>> + function allows firmware update operation to validate the firmware image
>> + before FmpDeviceSetImage() is called.
>> +
>> + @param[in] Image Points to a new firmware image.
>> + @param[in] ImageSize Size, in bytes, of a new firmware image.
>> + @param[out] ImageUpdatable Indicates if a new firmware image is valid for
>> + a firmware update to the firmware device. The
>> + following values from the Firmware Management
>> + Protocol are supported:
>> + IMAGE_UPDATABLE_VALID
>> + IMAGE_UPDATABLE_INVALID
>> + IMAGE_UPDATABLE_INVALID_TYPE
>> + IMAGE_UPDATABLE_INVALID_OLD
>> + IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
>> + @param[out] LastAttemptStatus A pointer to a UINT32 that holds the last attempt
>> + status to report back to the ESRT table in case
>> + of error. This value will only be checked when this
>> + function returns an error.
>> +
>> + The return status code must fall in the range of
>> + LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
>> + LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.
>> +
>> + If the value falls outside this range, it will be converted
>> + to LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL.
>> +
>> + @retval EFI_SUCCESS The image was successfully checked. Additional
>> + status information is returned in
>> + ImageUpdatable.
>> + @retval EFI_INVALID_PARAMETER Image is NULL.
>> + @retval EFI_INVALID_PARAMETER ImageUpdatable is NULL.
>> + @retval EFI_INVALID_PARAMETER LastAttemptStatus is NULL.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +FmpDeviceCheckImageWithStatus (
>> + IN CONST VOID *Image,
>> + IN UINTN ImageSize,
>> + OUT UINT32 *ImageUpdatable,
>> + OUT UINT32 *LastAttemptStatus
>> + )
>> +{
>> + if (LastAttemptStatus == NULL) {
>> + DEBUG ((DEBUG_ERROR, "CheckImageWithStatus - LastAttemptStatus Pointer Parameter is NULL.\n"));
>> + return EFI_INVALID_PARAMETER;
>> + }
>> + *LastAttemptStatus = LAST_ATTEMPT_STATUS_SUCCESS;
>> +
>> + if (ImageUpdatable == NULL) {
>> + DEBUG((DEBUG_ERROR, "CheckImageWithStatus - ImageUpdatable Pointer Parameter is NULL.\n"));
>> + *LastAttemptStatus = LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> + //
>> + //Set to valid and then if any tests fail it will update this flag.
>> + //
>> + *ImageUpdatable = IMAGE_UPDATABLE_VALID;
>> +
>> + if (Image == NULL) {
>> + DEBUG((DEBUG_ERROR, "CheckImageWithStatus - Image Pointer Parameter is NULL.\n"));
>> + //
>> + // Not sure if this is needed
>> + //
>> + *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
>> + *LastAttemptStatus = LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> + //
>> + // Make sure the image size is correct
>> + //
>> + if (ImageSize != PcdGet32 (PcdBiosRomSize)) {
>> + *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
>> + *LastAttemptStatus = LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> /**
>> Checks if the firmware image is valid for the device.
>>
>> @@ -633,34 +813,9 @@ FmpDeviceCheckImage (
>> OUT UINT32 *ImageUpdateable
>> )
>> {
>> - if (ImageUpdateable == NULL) {
>> - DEBUG((DEBUG_ERROR, "CheckImage - ImageUpdateable Pointer Parameter is NULL.\n"));
>> - return EFI_INVALID_PARAMETER;
>> - }
>> + UINT32 LastAttemptStatus;
>>
>> - //
>> - //Set to valid and then if any tests fail it will update this flag.
>> - //
>> - *ImageUpdateable = IMAGE_UPDATABLE_VALID;
>> -
>> - if (Image == NULL) {
>> - DEBUG((DEBUG_ERROR, "CheckImage - Image Pointer Parameter is NULL.\n"));
>> - //
>> - // Not sure if this is needed
>> - //
>> - *ImageUpdateable = IMAGE_UPDATABLE_INVALID;
>> - return EFI_INVALID_PARAMETER;
>> - }
>> -
>> - //
>> - // Make sure the image size is correct
>> - //
>> - if (ImageSize != PcdGet32 (PcdBiosRomSize)) {
>> - *ImageUpdateable = IMAGE_UPDATABLE_INVALID;
>> - return EFI_INVALID_PARAMETER;
>> - }
>> -
>> - return EFI_SUCCESS;
>> + return FmpDeviceCheckImageWithStatus (Image, ImageSize, ImageUpdateable, &LastAttemptStatus);
>> }
>>
>> /**
>> diff --git a/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLibSample/FmpDeviceLib.c
>> b/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLibSample/FmpDeviceLib.c
>> index db0f238ea534..132b60844ad4 100644
>> --- a/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLibSample/FmpDeviceLib.c
>> +++ b/Platform/Intel/Vlv2TbltDevicePkg/Feature/Capsule/Library/FmpDeviceLibSample/FmpDeviceLib.c
>> @@ -1,6 +1,6 @@
>> /**
>>
>> -Copyright (c) 2016, Microsoft Corporation. All rights reserved.<BR>
>> +Copyright (c) Microsoft Corporation.<BR>
>> Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
>>
>> SPDX-License-Identifier: BSD-2-Clause-Patent
>> @@ -9,6 +9,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>>
>>
>> #include <PiDxe.h>
>> +#include <LastAttemptStatus.h>
>> +#include <Guid/SystemResourceTable.h>
>> #include <Library/DebugLib.h>
>> #include <Protocol/FirmwareManagement.h>
>> #include <Library/BaseLib.h>
>> @@ -345,6 +347,131 @@ Return Value:
>> }//GetImage()
>>
>>
>> +/**
>> + Updates a firmware device with a new firmware image. This function returns
>> + EFI_UNSUPPORTED if the firmware image is not updatable. If the firmware image
>> + is updatable, the function should perform the following minimal validations
>> + before proceeding to do the firmware image update.
>> + - Validate that the image is a supported image for this firmware device.
>> + Return EFI_ABORTED if the image is not supported. Additional details
>> + on why the image is not a supported image may be returned in AbortReason.
>> + - Validate the data from VendorCode if is not NULL. Firmware image
>> + validation must be performed before VendorCode data validation.
>> + VendorCode data is ignored or considered invalid if image validation
>> + fails. Return EFI_ABORTED if the VendorCode data is invalid.
>> +
>> + VendorCode enables vendor to implement vendor-specific firmware image update
>> + policy. Null if the caller did not specify the policy or use the default
>> + policy. As an example, vendor can implement a policy to allow an option to
>> + force a firmware image update when the abort reason is due to the new firmware
>> + image version is older than the current firmware image version or bad image
>> + checksum. Sensitive operations such as those wiping the entire firmware image
>> + and render the device to be non-functional should be encoded in the image
>> + itself rather than passed with the VendorCode. AbortReason enables vendor to
>> + have the option to provide a more detailed description of the abort reason to
>> + the caller.
>> +
>> + @param[in] Image Points to the new firmware image.
>> + @param[in] ImageSize Size, in bytes, of the new firmware image.
>> + @param[in] VendorCode This enables vendor to implement vendor-specific
>> + firmware image update policy. NULL indicates
>> + the caller did not specify the policy or use the
>> + default policy.
>> + @param[in] Progress A function used to report the progress of
>> + updating the firmware device with the new
>> + firmware image.
>> + @param[in] CapsuleFwVersion The version of the new firmware image from the
>> + update capsule that provided the new firmware
>> + image.
>> + @param[out] AbortReason A pointer to a pointer to a Null-terminated
>> + Unicode string providing more details on an
>> + aborted operation. The buffer is allocated by
>> + this function with
>> + EFI_BOOT_SERVICES.AllocatePool(). It is the
>> + caller's responsibility to free this buffer with
>> + EFI_BOOT_SERVICES.FreePool().
>> + @param[out] LastAttemptStatus A pointer to a UINT32 that holds the last attempt
>> + status to report back to the ESRT table in case
>> + of error. This value will only be checked when this
>> + function returns an error.
>> +
>> + The return status code must fall in the range of
>> + LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
>> + LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.
>> +
>> + If the value falls outside this range, it will be converted
>> + to LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL.
>> +
>> + @retval EFI_SUCCESS The firmware device was successfully updated
>> + with the new firmware image.
>> + @retval EFI_ABORTED The operation is aborted. Additional details
>> + are provided in AbortReason.
>> + @retval EFI_INVALID_PARAMETER The Image was NULL.
>> + @retval EFI_INVALID_PARAMETER LastAttemptStatus was NULL.
>> + @retval EFI_UNSUPPORTED The operation is not supported.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +FmpDeviceSetImageWithStatus (
>> + IN CONST VOID *Image,
>> + IN UINTN ImageSize,
>> + IN CONST VOID *VendorCode, OPTIONAL
>> + IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS Progress, OPTIONAL
>> + IN UINT32 CapsuleFwVersion,
>> + OUT CHAR16 **AbortReason,
>> + OUT UINT32 *LastAttemptStatus
>> + )
>> +{
>> + EFI_STATUS Status = EFI_SUCCESS;
>> + UINT32 Updateable = 0;
>> +
>> + Status = FmpDeviceCheckImageWithStatus (Image, ImageSize, &Updateable, LastAttemptStatus);
>> + if (EFI_ERROR(Status))
>> + {
>> + DEBUG((DEBUG_ERROR, "SetImageWithStatus - Check Image failed with %r.\n", Status));
>> + goto cleanup;
>> + }
>> +
>> + if (Updateable != IMAGE_UPDATABLE_VALID)
>> + {
>> + DEBUG((DEBUG_ERROR, "SetImageWithStatus - Check Image returned that the Image was not valid for update.
>> Updatable value = 0x%X.\n", Updateable));
>> + Status = EFI_ABORTED;
>> + goto cleanup;
>> + }
>> +
>> + if (Progress == NULL)
>> + {
>> + DEBUG((DEBUG_ERROR, "SetImageWithStatus - Invalid progress callback\n"));
>> + Status = EFI_INVALID_PARAMETER;
>> + goto cleanup;
>> + }
>> +
>> + Status = Progress(15);
>> + if (EFI_ERROR(Status))
>> + {
>> + DEBUG((DEBUG_ERROR, "SetImageWithStatus - Progress Callback failed with Status %r.\n", Status));
>> + }
>> +
>> + {
>> + UINTN p;
>> +
>> + for (p = 20; p < 100; p++) {
>> + gBS->Stall (100000); //us = 0.1 seconds
>> + Progress (p);
>> + }
>> + }
>> +
>> + //TODO: add support for VendorCode, and AbortReason
>> +cleanup:
>> + if (EFI_ERROR (Status)) {
>> + *LastAttemptStatus = LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
>> + }
>> +
>> + return Status;
>> +}// SetImageWithStatus()
>> +
>> +
>> /**
>> Updates the firmware image of the device.
>>
>> @@ -396,51 +523,98 @@ IN UINT32 CapsuleFwVersion,
>> OUT CHAR16 **AbortReason
>> )
>> {
>> - EFI_STATUS Status = EFI_SUCCESS;
>> - UINT32 Updateable = 0;
>> -
>> - Status = FmpDeviceCheckImage(Image, ImageSize, &Updateable);
>> - if (EFI_ERROR(Status))
>> - {
>> - DEBUG((DEBUG_ERROR, "SetImage - Check Image failed with %r.\n", Status));
>> - goto cleanup;
>> - }
>> -
>> - if (Updateable != IMAGE_UPDATABLE_VALID)
>> - {
>> - DEBUG((DEBUG_ERROR, "SetImage - Check Image returned that the Image was not valid for update. Updatable value =
>> 0x%X.\n", Updateable));
>> - Status = EFI_ABORTED;
>> - goto cleanup;
>> - }
>> -
>> - if (Progress == NULL)
>> - {
>> - DEBUG((DEBUG_ERROR, "SetImage - Invalid progress callback\n"));
>> - Status = EFI_INVALID_PARAMETER;
>> - goto cleanup;
>> - }
>> -
>> - Status = Progress(15);
>> - if (EFI_ERROR(Status))
>> - {
>> - DEBUG((DEBUG_ERROR, "SetImage - Progress Callback failed with Status %r.\n", Status));
>> - }
>> -
>> - {
>> - UINTN p;
>> -
>> - for (p = 20; p < 100; p++) {
>> - gBS->Stall (100000); //us = 0.1 seconds
>> - Progress (p);
>> - }
>> - }
>> -
>> - //TODO: add support for VendorCode, and AbortReason
>> -cleanup:
>> - return Status;
>> + UINT32 LastAttemptStatus;
>> +
>> + return FmpDeviceSetImageWithStatus (
>> + Image,
>> + ImageSize,
>> + VendorCode,
>> + Progress,
>> + CapsuleFwVersion,
>> + AbortReason,
>> + &LastAttemptStatus
>> + );
>> }// SetImage()
>>
>>
>> +/**
>> + Checks if a new firmware image is valid for the firmware device. This
>> + function allows firmware update operation to validate the firmware image
>> + before FmpDeviceSetImage() is called.
>> +
>> + @param[in] Image Points to a new firmware image.
>> + @param[in] ImageSize Size, in bytes, of a new firmware image.
>> + @param[out] ImageUpdatable Indicates if a new firmware image is valid for
>> + a firmware update to the firmware device. The
>> + following values from the Firmware Management
>> + Protocol are supported:
>> + IMAGE_UPDATABLE_VALID
>> + IMAGE_UPDATABLE_INVALID
>> + IMAGE_UPDATABLE_INVALID_TYPE
>> + IMAGE_UPDATABLE_INVALID_OLD
>> + IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
>> + @param[out] LastAttemptStatus A pointer to a UINT32 that holds the last attempt
>> + status to report back to the ESRT table in case
>> + of error. This value will only be checked when this
>> + function returns an error.
>> +
>> + The return status code must fall in the range of
>> + LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
>> + LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE.
>> +
>> + If the value falls outside this range, it will be converted
>> + to LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL.
>> +
>> + @retval EFI_SUCCESS The image was successfully checked. Additional
>> + status information is returned in
>> + ImageUpdatable.
>> + @retval EFI_INVALID_PARAMETER Image is NULL.
>> + @retval EFI_INVALID_PARAMETER ImageUpdatable is NULL.
>> + @retval EFI_INVALID_PARAMETER LastAttemptStatus is NULL.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +FmpDeviceCheckImageWithStatus (
>> + IN CONST VOID *Image,
>> + IN UINTN ImageSize,
>> + OUT UINT32 *ImageUpdatable,
>> + OUT UINT32 *LastAttemptStatus
>> + )
>> +{
>> + EFI_STATUS status = EFI_SUCCESS;
>> +
>> + if (LastAttemptStatus == NULL) {
>> + DEBUG ((DEBUG_ERROR, "CheckImageWithStatus - LastAttemptStatus Pointer Parameter is NULL.\n"));
>> + return EFI_INVALID_PARAMETER;
>> + }
>> + *LastAttemptStatus = LAST_ATTEMPT_STATUS_SUCCESS;
>> +
>> + if (ImageUpdatable == NULL)
>> + {
>> + DEBUG((DEBUG_ERROR, "CheckImageWithStatus - ImageUpdatable Pointer Parameter is NULL.\n"));
>> + *LastAttemptStatus = LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
>> + status = EFI_INVALID_PARAMETER;
>> + goto cleanup;
>> + }
>> +
>> + //
>> + //Set to valid and then if any tests fail it will update this flag.
>> + //
>> + *ImageUpdatable = IMAGE_UPDATABLE_VALID;
>> +
>> + if (Image == NULL)
>> + {
>> + DEBUG((DEBUG_ERROR, "CheckImageWithStatus - Image Pointer Parameter is NULL.\n"));
>> + *ImageUpdatable = IMAGE_UPDATABLE_INVALID; //not sure if this is needed
>> + *LastAttemptStatus = LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE;
>> + return EFI_INVALID_PARAMETER;
>> + }
>> +
>> +cleanup:
>> + return status;
>> +}// CheckImageWithStatus()
>> +
>>
>> /**
>> Checks if the firmware image is valid for the device.
>> @@ -465,29 +639,9 @@ IN UINTN ImageSize,
>> OUT UINT32 *ImageUpdateable
>> )
>> {
>> - EFI_STATUS status = EFI_SUCCESS;
>> + UINT32 LastAttemptStatus;
>>
>> - if (ImageUpdateable == NULL)
>> - {
>> - DEBUG((DEBUG_ERROR, "CheckImage - ImageUpdateable Pointer Parameter is NULL.\n"));
>> - status = EFI_INVALID_PARAMETER;
>> - goto cleanup;
>> - }
>> -
>> - //
>> - //Set to valid and then if any tests fail it will update this flag.
>> - //
>> - *ImageUpdateable = IMAGE_UPDATABLE_VALID;
>> -
>> - if (Image == NULL)
>> - {
>> - DEBUG((DEBUG_ERROR, "CheckImage - Image Pointer Parameter is NULL.\n"));
>> - *ImageUpdateable = IMAGE_UPDATABLE_INVALID; //not sure if this is needed
>> - return EFI_INVALID_PARAMETER;
>> - }
>> -
>> -cleanup:
>> - return status;
>> + return FmpDeviceCheckImageWithStatus (Image, ImageSize, ImageUpdateable, &LastAttemptStatus);
>> }// CheckImage()
>>
>> /**
>> --
>> 2.28.0.windows.1
>
next prev parent reply other threads:[~2020-10-29 4:45 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-01 21:58 [edk2-platforms][PATCH v1 1/1] Vlv2TbltDevicePkg/FmpDeviceLib: Add LastAttemptStatus compatibility Michael Kubacki
2020-10-29 1:12 ` Michael D Kinney
2020-10-29 4:45 ` Michael Kubacki [this message]
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=MWHPR07MB3440BA6DD386CBA8E40BE459E9140@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