[PATCH v1 3/7] FmpDevicePkg/FmpDxe: Add check image path Last Attempt Status capability

["Michael Kubacki" <michael.kubacki@outlook.com>]

From: Michael Kubacki <michael.kubacki@microsoft.com>

CheckTheImage() is used to provide the CheckImage() implementation
for the EFI_FIRMWARE_MANAGEMENT_PROTOCOL instance produced by
FmpDxe in addition to being called internally in the SetImage()
path.

Since CheckTheImage() plays a major role in determining the
validity of a given firmware image, an internal version of the
function is introduced - CheckTheImageInternal() that is capable
of returning a Last Attempt Status code to internal callers such
as SetTheImage().

The CheckImage() API as defined in the UEFI Specification for
EFI_FIRMWARE_MANAGEMENT_PROTOCOL is not impacted by this change.

CheckTheImageInternal() contains unique Last Attempt Status codes
during error paths in the function so it is easier to identify
the issue with a particular image through the Last Attempt Status
code value.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Wei6 Xu <wei6.xu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 FmpDevicePkg/FmpDxe/FmpDxe.c             | 102 +++++++++++++++++---
 FmpDevicePkg/FmpDxe/FmpDxe.h             |   1 +
 FmpDevicePkg/Include/LastAttemptStatus.h |  26 +++++
 3 files changed, 118 insertions(+), 11 deletions(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index 854feec0a162..dc714f7e9cd4 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -721,6 +721,24 @@ GetAllHeaderSize (
   @param[in]  ImageSize          Size of the new image in bytes.
   @param[out] ImageUpdatable     Indicates if the new image is valid for update. It also provides,
                                  if available, additional information if the image is invalid.
+  @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 function will return error codes that occur within this function
+                                 implementation within a driver range of last attempt error codes from
+                                 LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MIN
+                                 to LAST_ATTEMPT_STATUS_DRIVER_MAX_ERROR_CODE_VALUE.
+
+                                 This function will return error codes that occur within FMP dependency
+                                 related functionality used by this function within a dependency range of
+                                 last attempt error codes from LAST_ATTEMPT_STATUS_DRIVER_MAX_ERROR_CODE_VALUE
+                                 to LAST_ATTEMPT_STATUS_DEPENDENCY_MAX_ERROR_CODE_VALUE.
+
+                                 This function will additionally return error codes that are returned
+                                 from a device-specific CheckImage () implementation in the range of
+                                 LAST_ATTEMPT_STATUS_DEPENDENCY_MAX_ERROR_CODE_VALUE to
+                                 LAST_ATTEMPT_STATUS_LIBRARY_MAX_ERROR_CODE_VALUE.
 
   @retval EFI_SUCCESS            The image was successfully checked.
   @retval EFI_ABORTED            The operation is aborted.
@@ -731,15 +749,17 @@ GetAllHeaderSize (
 **/
 EFI_STATUS
 EFIAPI
-CheckTheImage (
+CheckTheImageInternal (
   IN  EFI_FIRMWARE_MANAGEMENT_PROTOCOL  *This,
   IN  UINT8                             ImageIndex,
   IN  CONST VOID                        *Image,
   IN  UINTN                             ImageSize,
-  OUT UINT32                            *ImageUpdatable
+  OUT UINT32                            *ImageUpdatable,
+  OUT UINT32                            *LastAttemptStatus
   )
 {
   EFI_STATUS                        Status;
+  UINT32                            LocalLastAttemptStatus;
   FIRMWARE_MANAGEMENT_PRIVATE_DATA  *Private;
   UINTN                             RawSize;
   VOID                              *FmpPayloadHeader;
@@ -755,23 +775,31 @@ CheckTheImage (
   EFI_FIRMWARE_IMAGE_DEP            *Dependencies;
   UINT32                            DependenciesSize;
 
-  Status           = EFI_SUCCESS;
-  RawSize          = 0;
-  FmpPayloadHeader = NULL;
-  FmpPayloadSize   = 0;
-  Version          = 0;
-  FmpHeaderSize    = 0;
-  AllHeaderSize    = 0;
-  Dependencies     = NULL;
-  DependenciesSize = 0;
+  Status                  = EFI_SUCCESS;
+  LocalLastAttemptStatus  = LAST_ATTEMPT_STATUS_SUCCESS;
+  RawSize                 = 0;
+  FmpPayloadHeader        = NULL;
+  FmpPayloadSize          = 0;
+  Version                 = 0;
+  FmpHeaderSize           = 0;
+  AllHeaderSize           = 0;
+  Dependencies            = NULL;
+  DependenciesSize        = 0;
 
   if (!FeaturePcdGet (PcdFmpDeviceStorageAccessEnable)) {
     return EFI_UNSUPPORTED;
   }
 
+  if (LastAttemptStatus == NULL) {
+    DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImageInternal() - LastAttemptStatus is NULL.\n", mImageIdName));
+    Status = EFI_INVALID_PARAMETER;
+    goto cleanup;
+  }
+
   if (This == NULL) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckImage() - This is NULL.\n", mImageIdName));
     Status = EFI_INVALID_PARAMETER;
+    *LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_PROTOCOL_ARG_MISSING;
     goto cleanup;
   }
 
@@ -789,6 +817,7 @@ CheckTheImage (
   if (ImageUpdatable == NULL) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckImage() - ImageUpdatable Pointer Parameter is NULL.\n", mImageIdName));
     Status = EFI_INVALID_PARAMETER;
+    *LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_IMAGE_NOT_UPDATABLE;
     goto cleanup;
   }
 
@@ -808,6 +837,7 @@ CheckTheImage (
     // not sure if this is needed
     //
     *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
+    *LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_IMAGE_NOT_PROVIDED;
     return EFI_INVALID_PARAMETER;
   }
 
@@ -817,6 +847,7 @@ CheckTheImage (
   if (PublicKeyDataXdr == NULL || (PublicKeyDataXdr == PublicKeyDataXdrEnd)) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): Invalid certificate, skipping it.\n", mImageIdName));
     Status = EFI_ABORTED;
+    *LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_INVALID_CERTIFICATE;
   } else {
     //
     // Try each key from PcdFmpDevicePkcs7CertBufferXdr
@@ -839,6 +870,7 @@ CheckTheImage (
         //
         DEBUG ((DEBUG_ERROR, "FmpDxe(%s): Certificate size extends beyond end of PCD, skipping it.\n", mImageIdName));
         Status = EFI_ABORTED;
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_INVALID_KEY_LENGTH_VALUE;
         break;
       }
       //
@@ -855,6 +887,7 @@ CheckTheImage (
         //
         DEBUG ((DEBUG_ERROR, "FmpDxe(%s): Certificate extends beyond end of PCD, skipping it.\n", mImageIdName));
         Status = EFI_ABORTED;
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_INVALID_KEY_LENGTH;
         break;
       }
       PublicKeyData = PublicKeyDataXdr;
@@ -874,6 +907,11 @@ CheckTheImage (
 
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - Authentication Failed %r.\n", mImageIdName, Status));
+    if (LocalLastAttemptStatus != LAST_ATTEMPT_STATUS_SUCCESS) {
+      *LastAttemptStatus = LocalLastAttemptStatus;
+    } else {
+      *LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_IMAGE_AUTH_FAILURE;
+    }
     goto cleanup;
   }
 
@@ -884,6 +922,7 @@ CheckTheImage (
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckImage() - Image Index Invalid.\n", mImageIdName));
     *ImageUpdatable = IMAGE_UPDATABLE_INVALID_TYPE;
     Status = EFI_INVALID_PARAMETER;
+    *LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_INVALID_IMAGE_INDEX;
     goto cleanup;
   }
 
@@ -899,6 +938,7 @@ CheckTheImage (
   if (FmpPayloadHeader == NULL) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - GetFmpHeader failed.\n", mImageIdName));
     Status = EFI_ABORTED;
+    *LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_FMP_HEADER;
     goto cleanup;
   }
   Status = GetFmpPayloadHeaderVersion (FmpPayloadHeader, FmpPayloadSize, &Version);
@@ -906,6 +946,7 @@ CheckTheImage (
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - GetFmpPayloadHeaderVersion failed %r.\n", mImageIdName, Status));
     *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
     Status = EFI_SUCCESS;
+    *LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_FMP_HEADER_VERSION;
     goto cleanup;
   }
 
@@ -920,6 +961,7 @@ CheckTheImage (
       );
     *ImageUpdatable = IMAGE_UPDATABLE_INVALID_OLD;
     Status = EFI_SUCCESS;
+    *LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_VERSION_TOO_LOW;
     goto cleanup;
   }
 
@@ -942,6 +984,7 @@ CheckTheImage (
     DEBUG ((DEBUG_ERROR, "FmpDxe: CheckTheImage() - GetFmpPayloadHeaderSize failed %r.\n", Status));
     *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
     Status = EFI_SUCCESS;
+    *LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_FMP_HEADER_SIZE;
     goto cleanup;
   }
 
@@ -953,6 +996,7 @@ CheckTheImage (
   if (AllHeaderSize == 0) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - GetAllHeaderSize failed.\n", mImageIdName));
     Status = EFI_ABORTED;
+    *LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_ALL_HEADER_SIZE;
     goto cleanup;
   }
   RawSize = ImageSize - AllHeaderSize;
@@ -969,6 +1013,42 @@ CheckTheImage (
   return Status;
 }
 
+/**
+  Checks if the firmware image is valid for the device.
+
+  This function allows firmware update application to validate the firmware image without
+  invoking the SetImage() first.
+
+  @param[in]  This               A pointer to the EFI_FIRMWARE_MANAGEMENT_PROTOCOL instance.
+  @param[in]  ImageIndex         A unique number identifying the firmware image(s) within the device.
+                                 The number is between 1 and DescriptorCount.
+  @param[in]  Image              Points to the new image.
+  @param[in]  ImageSize          Size of the new image in bytes.
+  @param[out] ImageUpdatable     Indicates if the new image is valid for update. It also provides,
+                                 if available, additional information if the image is invalid.
+
+  @retval EFI_SUCCESS            The image was successfully checked.
+  @retval EFI_ABORTED            The operation is aborted.
+  @retval EFI_INVALID_PARAMETER  The Image was NULL.
+  @retval EFI_UNSUPPORTED        The operation is not supported.
+  @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure.
+
+**/
+EFI_STATUS
+EFIAPI
+CheckTheImage (
+  IN  EFI_FIRMWARE_MANAGEMENT_PROTOCOL  *This,
+  IN  UINT8                             ImageIndex,
+  IN  CONST VOID                        *Image,
+  IN  UINTN                             ImageSize,
+  OUT UINT32                            *ImageUpdatable
+  )
+{
+  UINT32    LastAttemptStatus;
+
+  return CheckTheImageInternal (This, ImageIndex, Image, ImageSize, ImageUpdatable, &LastAttemptStatus);
+}
+
 /**
   Updates the firmware image of the device.
 
diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.h b/FmpDevicePkg/FmpDxe/FmpDxe.h
index 30754dea495e..b79b4faecee4 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.h
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.h
@@ -36,6 +36,7 @@
 #include <Protocol/VariableLock.h>
 #include <Guid/SystemResourceTable.h>
 #include <Guid/EventGroup.h>
+#include <LastAttemptStatus.h>
 
 #define VERSION_STRING_NOT_SUPPORTED  L"VERSION STRING NOT SUPPORTED"
 #define VERSION_STRING_NOT_AVAILABLE  L"VERSION STRING NOT AVAILABLE"
diff --git a/FmpDevicePkg/Include/LastAttemptStatus.h b/FmpDevicePkg/Include/LastAttemptStatus.h
index d39a06090a65..e177b06c4b58 100644
--- a/FmpDevicePkg/Include/LastAttemptStatus.h
+++ b/FmpDevicePkg/Include/LastAttemptStatus.h
@@ -28,4 +28,30 @@
 
 #define LAST_ATTEMPT_STATUS_LIBRARY_MAX_ERROR_CODE_VALUE    LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX - 1
 
+//
+// Last attempt status codes defined for additional granularity in the FMP stack.
+//
+// These codes are defined within the higher-level UEFI specification defined UNSUCCESSFUL_VENDOR_RANGE.
+//
+// The following last attempt status code ranges are defined for the following corresponding component:
+//   * LAST_ATTEMPT_STATUS_DRIVER - FMP driver
+//
+enum LAST_ATTEMPT_STATUS_EXPANDED_ERROR_LIST
+{
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_FMP_HEADER           = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MIN,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_FMP_HEADER_SIZE      ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_ALL_HEADER_SIZE      ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_FMP_HEADER_VERSION   ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_IMAGE_NOT_PROVIDED       ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_IMAGE_NOT_UPDATABLE      ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_INVALID_CERTIFICATE      ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_INVALID_IMAGE_INDEX      ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_INVALID_KEY_LENGTH       ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_INVALID_KEY_LENGTH_VALUE ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_VERSION_TOO_LOW          ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_IMAGE_AUTH_FAILURE       ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_PROTOCOL_ARG_MISSING     ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_MAX_ERROR_CODE           = LAST_ATTEMPT_STATUS_DRIVER_MAX_ERROR_CODE_VALUE
+};
+
 #endif
-- 
2.28.0.windows.1