[edk2-devel] [PATCH v1 4/7] FmpDevicePkg/FmpDxe: Improve set image path Last Attempt Status granularity

[Michael Kubacki]

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

Increases the level of granularity for Last Attempt Status codes
returned from SetTheImage() in FmpDxe. This allows better
identification of the error that occurred in the set image
operation using Last Attempt Status codes.

Cc: Liming Gao <liming.gao at intel.com>
Cc: Michael D Kinney <michael.d.kinney at intel.com>
Cc: Guomin Jiang <guomin.jiang at intel.com>
Cc: Wei6 Xu <wei6.xu at intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki at microsoft.com>
---
 FmpDevicePkg/FmpDxe/FmpDxe.c             | 17 +++++++++++++----
 FmpDevicePkg/Include/LastAttemptStatus.h |  7 +++++++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index dc714f7e9cd4..aeb888c86bc3 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -1141,6 +1141,7 @@ SetTheImage (
   if (This == NULL) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - This is NULL.\n", mImageIdName));
     Status = EFI_INVALID_PARAMETER;
+    LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_PROTOCOL_ARG_MISSING;
     goto cleanup;
   }
 
@@ -1166,6 +1167,7 @@ SetTheImage (
   //
   if (Private->FmpDeviceLocked) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - Device is already locked.  Can't update.\n", mImageIdName));
+    LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_DEVICE_LOCKED;
     Status = EFI_UNSUPPORTED;
     goto cleanup;
   }
@@ -1173,12 +1175,9 @@ SetTheImage (
   //
   // Call check image to verify the image
   //
-  Status = CheckTheImage (This, ImageIndex, Image, ImageSize, &Updateable);
+  Status = CheckTheImageInternal (This, ImageIndex, Image, ImageSize, &Updateable, &LastAttemptStatus);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - Check The Image failed with %r.\n", mImageIdName, Status));
-    if (Status == EFI_SECURITY_VIOLATION) {
-      LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_AUTH_ERROR;
-    }
     goto cleanup;
   }
 
@@ -1194,6 +1193,7 @@ SetTheImage (
   FmpHeader = GetFmpHeader ( (EFI_FIRMWARE_IMAGE_AUTHENTICATION *)Image, ImageSize, DependenciesSize, &FmpPayloadSize );
   if (FmpHeader == NULL) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - GetFmpHeader failed.\n", mImageIdName));
+    LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_FMP_HEADER;
     Status = EFI_ABORTED;
     goto cleanup;
   }
@@ -1221,6 +1221,7 @@ SetTheImage (
 
   if (Progress == NULL) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - Invalid progress callback\n", mImageIdName));
+    LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_PROGRESS_CALLBACK_ERROR;
     Status = EFI_INVALID_PARAMETER;
     goto cleanup;
   }
@@ -1241,6 +1242,7 @@ SetTheImage (
   Status = CheckSystemPower (&BooleanValue);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - CheckSystemPower - API call failed %r.\n", mImageIdName, Status));
+    LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_CHECK_POWER_API;
     goto cleanup;
   }
   if (!BooleanValue) {
@@ -1261,10 +1263,12 @@ SetTheImage (
   Status = CheckSystemThermal (&BooleanValue);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - CheckSystemThermal - API call failed %r.\n", mImageIdName, Status));
+    LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_CHECK_SYS_THERMAL_API;
     goto cleanup;
   }
   if (!BooleanValue) {
     Status = EFI_ABORTED;
+    LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_THERMAL;
     DEBUG (
       (DEBUG_ERROR,
       "FmpDxe(%s): SetTheImage() - CheckSystemThermal - returned False.  Update not allowed due to System Thermal.\n", mImageIdName)
@@ -1280,10 +1284,12 @@ SetTheImage (
   Status = CheckSystemEnvironment (&BooleanValue);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - CheckSystemEnvironment - API call failed %r.\n", mImageIdName, Status));
+    LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_CHECK_SYS_ENV_API;
     goto cleanup;
   }
   if (!BooleanValue) {
     Status = EFI_ABORTED;
+    LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_SYSTEM_ENV;
     DEBUG (
       (DEBUG_ERROR,
       "FmpDxe(%s): SetTheImage() - CheckSystemEnvironment - returned False.  Update not allowed due to System Environment.\n", mImageIdName)
@@ -1305,12 +1311,14 @@ SetTheImage (
   Status = GetFmpPayloadHeaderSize (FmpHeader, FmpPayloadSize, &FmpHeaderSize);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - GetFmpPayloadHeaderSize failed %r.\n", mImageIdName, Status));
+    LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_FMP_HEADER_SIZE;
     goto cleanup;
   }
 
   AllHeaderSize = GetAllHeaderSize ((EFI_FIRMWARE_IMAGE_AUTHENTICATION *)Image, FmpHeaderSize + DependenciesSize);
   if (AllHeaderSize == 0) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - GetAllHeaderSize failed.\n", mImageIdName));
+    LastAttemptStatus = LAST_ATTEMPT_STATUS_DRIVER_ERROR_GET_ALL_HEADER_SIZE;
     Status = EFI_ABORTED;
     goto cleanup;
   }
@@ -1373,6 +1381,7 @@ SetTheImage (
 
 cleanup:
   mProgressFunc = NULL;
+  DEBUG ((DEBUG_INFO, "FmpDxe(%s): SetTheImage() LastAttemptStatus: %u.\n", mImageIdName, LastAttemptStatus));
   SetLastAttemptStatusInVariable (Private, LastAttemptStatus);
 
   if (Progress != NULL) {
diff --git a/FmpDevicePkg/Include/LastAttemptStatus.h b/FmpDevicePkg/Include/LastAttemptStatus.h
index e177b06c4b58..03af9027cf48 100644
--- a/FmpDevicePkg/Include/LastAttemptStatus.h
+++ b/FmpDevicePkg/Include/LastAttemptStatus.h
@@ -39,6 +39,12 @@
 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_PROGRESS_CALLBACK_ERROR  ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_CHECK_POWER_API          ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_CHECK_SYS_THERMAL_API    ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_THERMAL                  ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_CHECK_SYS_ENV_API        ,
+  LAST_ATTEMPT_STATUS_DRIVER_ERROR_SYSTEM_ENV               ,
   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   ,
@@ -49,6 +55,7 @@ enum LAST_ATTEMPT_STATUS_EXPANDED_ERROR_LIST
   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_DEVICE_LOCKED            ,
   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
-- 
2.28.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#63815): https://edk2.groups.io/g/devel/message/63815
Mute This Topic: https://groups.io/mt/76044639/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-