public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael Kubacki" <michael.kubacki@outlook.com>
To: devel@edk2.groups.io
Cc: Liming Gao <liming.gao@intel.com>,
	Michael D Kinney <michael.d.kinney@intel.com>,
	Guomin Jiang <guomin.jiang@intel.com>,
	Wei6 Xu <wei6.xu@intel.com>
Subject: [PATCH v2 5/6] FmpDevicePkg: Add Last Attempt Status support to dependency libs
Date: Mon, 10 Aug 2020 13:27:52 -0700	[thread overview]
Message-ID: <MWHPR07MB34408D4E74D104E29F144B74E9440@MWHPR07MB3440.namprd07.prod.outlook.com> (raw)
In-Reply-To: <20200810202753.1318-1-michael.kubacki@outlook.com>

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

The FMP dependency libraries are leveraged during firmware update
to check for dependencies required to update the image.

This change adds granular Last Attempt Status code support to these
services so failures can be more easily observed during the firmware
update process via Last Attempt Status codes.

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                                                     | 25 +++++-
 FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c               | 38 +++++---
 FmpDevicePkg/Library/FmpDependencyCheckLibNull/FmpDependencyCheckLibNull.c       |  9 +-
 FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c                         | 95 +++++++++++++++++---
 FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependencyUnitTest.c |  7 +-
 FmpDevicePkg/Include/Library/FmpDependencyCheckLib.h                             |  8 +-
 FmpDevicePkg/Include/Library/FmpDependencyLib.h                                  | 44 +++++----
 7 files changed, 179 insertions(+), 47 deletions(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index aeb888c86bc3..84a8e15cfc17 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -929,7 +929,17 @@ CheckTheImageInternal (
   //
   // Get the dependency from Image.
   //
-  Dependencies = GetImageDependency ((EFI_FIRMWARE_IMAGE_AUTHENTICATION *)Image, ImageSize, &DependenciesSize);
+  Dependencies =  GetImageDependency (
+                    (EFI_FIRMWARE_IMAGE_AUTHENTICATION *) Image,
+                    ImageSize,
+                    &DependenciesSize,
+                    &LocalLastAttemptStatus
+                    );
+  if (LocalLastAttemptStatus != LAST_ATTEMPT_STATUS_SUCCESS) {
+    Status = EFI_ABORTED;
+    *LastAttemptStatus = LocalLastAttemptStatus;
+    goto cleanup;
+  }
 
   //
   // Check the FmpPayloadHeader
@@ -968,11 +978,20 @@ CheckTheImageInternal (
   //
   // Evaluate dependency expression
   //
-  Private->DependenciesSatisfied = CheckFmpDependency (Private->Descriptor.ImageTypeId, Version, Dependencies, DependenciesSize);
+  Private->DependenciesSatisfied =  CheckFmpDependency (
+                                      Private->Descriptor.ImageTypeId,
+                                      Version,
+                                      Dependencies,
+                                      DependenciesSize,
+                                      &LocalLastAttemptStatus
+                                      );
   if (!Private->DependenciesSatisfied) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - Dependency check failed.\n", mImageIdName));
     *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
     Status = EFI_SUCCESS;
+    if (LocalLastAttemptStatus != LAST_ATTEMPT_STATUS_SUCCESS) {
+      *LastAttemptStatus = LocalLastAttemptStatus;
+    }
     goto cleanup;
   }
 
@@ -1184,7 +1203,7 @@ SetTheImage (
   //
   // Get the dependency from Image.
   //
-  Dependencies = GetImageDependency ((EFI_FIRMWARE_IMAGE_AUTHENTICATION *)Image, ImageSize, &DependenciesSize);
+  Dependencies = GetImageDependency ((EFI_FIRMWARE_IMAGE_AUTHENTICATION *)Image, ImageSize, &DependenciesSize, &LastAttemptStatus);
 
   //
   // No functional error in CheckTheImage.  Attempt to get the Version to
diff --git a/FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c b/FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c
index 02ed600e0e95..d1e3b6b93bc2 100644
--- a/FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c
+++ b/FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c
@@ -17,6 +17,8 @@
 #include <Library/MemoryAllocationLib.h>
 #include <Library/UefiLib.h>
 #include <Library/UefiBootServicesTableLib.h>
+#include <Guid/SystemResourceTable.h>
+#include <LastAttemptStatus.h>
 
 /**
   Check dependency for firmware update.
@@ -25,6 +27,10 @@
   @param[in]  Version            New version.
   @param[in]  Dependencies       Fmp dependency.
   @param[in]  DependenciesSize   Size, in bytes, of the Fmp dependency.
+  @param[out] LastAttemptStatus  An optional pointer to a UINT32 that holds the
+                                 last attempt status to report back to the caller.
+                                 A function error code may not always be accompanied
+                                 by a last attempt status code.
 
   @retval  TRUE    Dependencies are satisfied.
   @retval  FALSE   Dependencies are unsatisfied or dependency check fails.
@@ -36,7 +42,8 @@ CheckFmpDependency (
   IN  EFI_GUID                ImageTypeId,
   IN  UINT32                  Version,
   IN  EFI_FIRMWARE_IMAGE_DEP  *Dependencies,    OPTIONAL
-  IN  UINT32                  DependenciesSize
+  IN  UINT32                  DependenciesSize,
+  OUT UINT32                  *LastAttemptStatus OPTIONAL
   )
 {
   EFI_STATUS                        Status;
@@ -44,6 +51,7 @@ CheckFmpDependency (
   UINTN                             Index;
   EFI_FIRMWARE_MANAGEMENT_PROTOCOL  *Fmp;
   UINTN                             ImageInfoSize;
+  UINT32                            LocalLastAttemptStatus;
   UINT32                            *DescriptorVer;
   UINT8                             FmpImageInfoCount;
   UINTN                             *DescriptorSize;
@@ -55,14 +63,15 @@ CheckFmpDependency (
   UINTN                             FmpVersionsCount;
   BOOLEAN                           IsSatisfied;
 
-  FmpImageInfoBuf     = NULL;
-  DescriptorVer       = NULL;
-  DescriptorSize      = NULL;
-  NumberOfFmpInstance = 0;
-  FmpVersions         = NULL;
-  FmpVersionsCount    = 0;
-  IsSatisfied         = TRUE;
-  PackageVersionName  = NULL;
+  LocalLastAttemptStatus  = LAST_ATTEMPT_STATUS_SUCCESS;
+  FmpImageInfoBuf         = NULL;
+  DescriptorVer           = NULL;
+  DescriptorSize          = NULL;
+  NumberOfFmpInstance     = 0;
+  FmpVersions             = NULL;
+  FmpVersionsCount        = 0;
+  IsSatisfied             = TRUE;
+  PackageVersionName      = NULL;
 
   //
   // Get ImageDescriptors of all FMP instances, and archive them for dependency evaluation.
@@ -77,30 +86,35 @@ CheckFmpDependency (
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "CheckFmpDependency: Get Firmware Management Protocol failed. (%r)", Status));
     IsSatisfied = FALSE;
+    LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_MEMORY_ALLOCATION_FAILED;
     goto cleanup;
   }
 
   FmpImageInfoBuf = AllocateZeroPool (sizeof(EFI_FIRMWARE_IMAGE_DESCRIPTOR *) * NumberOfFmpInstance);
   if (FmpImageInfoBuf == NULL) {
     IsSatisfied = FALSE;
+    LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_MEMORY_ALLOCATION_FAILED;
     goto cleanup;
   }
 
   DescriptorVer = AllocateZeroPool (sizeof(UINT32) * NumberOfFmpInstance);
   if (DescriptorVer == NULL ) {
     IsSatisfied = FALSE;
+    LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_MEMORY_ALLOCATION_FAILED;
     goto cleanup;
   }
 
   DescriptorSize = AllocateZeroPool (sizeof(UINTN) * NumberOfFmpInstance);
   if (DescriptorSize == NULL ) {
     IsSatisfied = FALSE;
+    LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_MEMORY_ALLOCATION_FAILED;
     goto cleanup;
   }
 
   FmpVersions = AllocateZeroPool (sizeof(FMP_DEPEX_CHECK_VERSION_DATA) * NumberOfFmpInstance);
   if (FmpVersions == NULL) {
     IsSatisfied = FALSE;
+    LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_MEMORY_ALLOCATION_FAILED;
     goto cleanup;
   }
 
@@ -164,7 +178,7 @@ CheckFmpDependency (
   // Evaluate firmware image's depex, against the version of other Fmp instances.
   //
   if (Dependencies != NULL) {
-    IsSatisfied = EvaluateDependency (Dependencies, DependenciesSize, FmpVersions, FmpVersionsCount);
+    IsSatisfied = EvaluateDependency (Dependencies, DependenciesSize, FmpVersions, FmpVersionsCount, &LocalLastAttemptStatus);
   }
 
   if (!IsSatisfied) {
@@ -194,5 +208,9 @@ CheckFmpDependency (
     FreePool (FmpVersions);
   }
 
+  if (LastAttemptStatus != NULL && LocalLastAttemptStatus != LAST_ATTEMPT_STATUS_SUCCESS) {
+    *LastAttemptStatus = LocalLastAttemptStatus;
+  }
+
   return IsSatisfied;
 }
diff --git a/FmpDevicePkg/Library/FmpDependencyCheckLibNull/FmpDependencyCheckLibNull.c b/FmpDevicePkg/Library/FmpDependencyCheckLibNull/FmpDependencyCheckLibNull.c
index 55e9af22909d..8937fb56d919 100644
--- a/FmpDevicePkg/Library/FmpDependencyCheckLibNull/FmpDependencyCheckLibNull.c
+++ b/FmpDevicePkg/Library/FmpDependencyCheckLibNull/FmpDependencyCheckLibNull.c
@@ -1,6 +1,7 @@
 /** @file
   Null instance of FmpDependencyCheckLib.
 
+  Copyright (c) Microsoft Corporation.<BR>
   Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -16,7 +17,10 @@
   @param[in]  Version            New version.
   @param[in]  Dependencies       Fmp dependency.
   @param[in]  DependenciesSize   Size, in bytes, of the Fmp dependency.
-
+  @param[out] LastAttemptStatus  An optional pointer to a UINT32 that holds the
+                                 last attempt status to report back to the caller.
+                                 A function error code may not always be accompanied
+                                 by a last attempt status code.
   @retval  TRUE    Dependencies are satisfied.
   @retval  FALSE   Dependencies are unsatisfied or dependency check fails.
 
@@ -27,7 +31,8 @@ CheckFmpDependency (
   IN  EFI_GUID                ImageTypeId,
   IN  UINT32                  Version,
   IN  EFI_FIRMWARE_IMAGE_DEP  *Dependencies,    OPTIONAL
-  IN  UINT32                  DependenciesSize
+  IN  UINT32                  DependenciesSize,
+  OUT UINT32                  *LastAttemptStatus OPTIONAL
   )
 {
   return TRUE;
diff --git a/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c b/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c
index 5ef25d2415cf..2045570b0628 100644
--- a/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c
+++ b/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c
@@ -13,6 +13,8 @@
 #include <Library/DebugLib.h>
 #include <Library/FmpDependencyLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Guid/SystemResourceTable.h>
+#include <LastAttemptStatus.h>
 
 //
 // Define the initial size of the dependency expression evaluation stack
@@ -203,6 +205,10 @@ Pop (
                                   parameter is optional and can be set to NULL.
   @param[in]   FmpVersionsCount   Element count of the array. When FmpVersions
                                   is NULL, FmpVersionsCount must be 0.
+  @param[out]  LastAttemptStatus  An optional pointer to a UINT32 that holds the
+                                  last attempt status to report back to the caller.
+                                  A function error code may not always be accompanied
+                                  by a last attempt status code.
 
   @retval TRUE    Dependency expressions evaluate to TRUE.
   @retval FALSE   Dependency expressions evaluate to FALSE.
@@ -211,10 +217,11 @@ Pop (
 BOOLEAN
 EFIAPI
 EvaluateDependency (
-  IN EFI_FIRMWARE_IMAGE_DEP        *Dependencies,
-  IN UINTN                         DependenciesSize,
-  IN FMP_DEPEX_CHECK_VERSION_DATA  *FmpVersions      OPTIONAL,
-  IN UINTN                         FmpVersionsCount
+  IN  EFI_FIRMWARE_IMAGE_DEP        *Dependencies,
+  IN  UINTN                         DependenciesSize,
+  IN  FMP_DEPEX_CHECK_VERSION_DATA  *FmpVersions,      OPTIONAL
+  IN  UINTN                         FmpVersionsCount,
+  OUT UINT32                        *LastAttemptStatus OPTIONAL
   )
 {
   EFI_STATUS                        Status;
@@ -224,6 +231,9 @@ EvaluateDependency (
   DEPEX_ELEMENT                     Element2;
   GUID                              ImageTypeId;
   UINT32                            Version;
+  UINT32                            LocalLastAttemptStatus;
+
+  LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_SUCCESS;
 
   //
   // Check if parameter is valid.
@@ -249,6 +259,7 @@ EvaluateDependency (
     case EFI_FMP_DEP_PUSH_GUID:
       if (Iterator + sizeof (EFI_GUID) >= (UINT8 *) Dependencies->Dependencies + DependenciesSize) {
         DEBUG ((DEBUG_ERROR, "EvaluateDependency: GUID extends beyond end of dependency expression!\n"));
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_GUID_BEYOND_DEPEX;
         goto Error;
       }
 
@@ -259,6 +270,7 @@ EvaluateDependency (
         if(CompareGuid (&FmpVersions[Index].ImageTypeId, &ImageTypeId)){
           Status = Push (FmpVersions[Index].Version, VersionType);
           if (EFI_ERROR (Status)) {
+            LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_PUSH_FAILURE;
             goto Error;
           }
           break;
@@ -266,18 +278,21 @@ EvaluateDependency (
       }
       if (Index == FmpVersionsCount) {
         DEBUG ((DEBUG_ERROR, "EvaluateDependency: %g is not found!\n", &ImageTypeId));
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_FMP_NOT_FOUND;
         goto Error;
       }
       break;
     case EFI_FMP_DEP_PUSH_VERSION:
       if (Iterator + sizeof (UINT32) >= (UINT8 *) Dependencies->Dependencies + DependenciesSize ) {
         DEBUG ((DEBUG_ERROR, "EvaluateDependency: VERSION extends beyond end of dependency expression!\n"));
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_VERSION_BEYOND_DEPEX;
         goto Error;
       }
 
       Version = *(UINT32 *) (Iterator + 1);
       Status = Push (Version, VersionType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_PUSH_FAILURE;
         goto Error;
       }
       Iterator = Iterator + sizeof (UINT32);
@@ -286,154 +301,191 @@ EvaluateDependency (
       Iterator += AsciiStrnLenS ((CHAR8 *) Iterator, DependenciesSize - (Iterator - Dependencies->Dependencies));
       if (Iterator == (UINT8 *) Dependencies->Dependencies + DependenciesSize) {
         DEBUG ((DEBUG_ERROR, "EvaluateDependency: STRING extends beyond end of dependency expression!\n"));
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_VERSION_STR_BEYOND_DEPEX;
         goto Error;
       }
       break;
     case EFI_FMP_DEP_AND:
       Status = Pop (&Element1, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = Pop (&Element2, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = Push (Element1.Value.Boolean & Element2.Value.Boolean, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_PUSH_FAILURE;
         goto Error;
       }
       break;
     case EFI_FMP_DEP_OR:
       Status = Pop (&Element1, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = Pop(&Element2, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = Push (Element1.Value.Boolean | Element2.Value.Boolean, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_PUSH_FAILURE;
         goto Error;
       }
       break;
     case EFI_FMP_DEP_NOT:
       Status = Pop (&Element1, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = Push (!(Element1.Value.Boolean), BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_PUSH_FAILURE;
         goto Error;
       }
       break;
     case EFI_FMP_DEP_TRUE:
       Status = Push (TRUE, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_PUSH_FAILURE;
         goto Error;
       }
       break;
     case EFI_FMP_DEP_FALSE:
       Status = Push (FALSE, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_PUSH_FAILURE;
         goto Error;
       }
       break;
     case EFI_FMP_DEP_EQ:
       Status = Pop (&Element1, VersionType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = Pop (&Element2, VersionType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = (Element1.Value.Version == Element2.Value.Version) ? Push (TRUE, BooleanType) : Push (FALSE, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_PUSH_FAILURE;
         goto Error;
       }
       break;
     case EFI_FMP_DEP_GT:
       Status = Pop (&Element1, VersionType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = Pop (&Element2, VersionType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = (Element1.Value.Version >  Element2.Value.Version) ? Push (TRUE, BooleanType) : Push (FALSE, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_PUSH_FAILURE;
         goto Error;
       }
       break;
     case EFI_FMP_DEP_GTE:
       Status = Pop (&Element1, VersionType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = Pop (&Element2, VersionType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = (Element1.Value.Version >= Element2.Value.Version) ? Push (TRUE, BooleanType) : Push (FALSE, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_PUSH_FAILURE;
         goto Error;
       }
       break;
     case EFI_FMP_DEP_LT:
       Status = Pop (&Element1, VersionType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = Pop (&Element2, VersionType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus= LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = (Element1.Value.Version <  Element2.Value.Version) ? Push (TRUE, BooleanType) : Push (FALSE, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_PUSH_FAILURE;
         goto Error;
       }
       break;
     case EFI_FMP_DEP_LTE:
       Status = Pop (&Element1, VersionType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = Pop (&Element2, VersionType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       Status = (Element1.Value.Version <= Element2.Value.Version) ? Push (TRUE, BooleanType) : Push (FALSE, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_PUSH_FAILURE;
         goto Error;
       }
       break;
     case EFI_FMP_DEP_END:
       Status = Pop (&Element1, BooleanType);
       if (EFI_ERROR (Status)) {
+        LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_POP_FAILURE;
         goto Error;
       }
       return Element1.Value.Boolean;
     default:
       DEBUG ((DEBUG_ERROR, "EvaluateDependency: Unknown Opcode - %02x!\n", *Iterator));
+      LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_UNKNOWN_OPCODE;
       goto Error;
     }
     Iterator++;
   }
 
   DEBUG ((DEBUG_ERROR, "EvaluateDependency: No EFI_FMP_DEP_END Opcode in expression!\n"));
+  LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_NO_END_OPCODE;
 
 Error:
+  if (LastAttemptStatus != NULL && LocalLastAttemptStatus != LAST_ATTEMPT_STATUS_SUCCESS) {
+    *LastAttemptStatus = LocalLastAttemptStatus;
+  }
+
   return FALSE;
 }
 
 /**
   Validate the dependency expression and output its size.
 
-  @param[in]   Dependencies   Pointer to the EFI_FIRMWARE_IMAGE_DEP.
-  @param[in]   MaxDepexSize   Max size of the dependency.
-  @param[out]  DepexSize      Size of dependency.
+  @param[in]   Dependencies       Pointer to the EFI_FIRMWARE_IMAGE_DEP.
+  @param[in]   MaxDepexSize       Max size of the dependency.
+  @param[out]  DepexSize          Size of dependency.
+  @param[out]  LastAttemptStatus  An optional pointer to a UINT32 that holds the
+                                  last attempt status to report back to the caller.
+                                  A function error code may not always be accompanied
+                                  by a last attempt status code.
 
   @retval TRUE    The dependency expression is valid.
   @retval FALSE   The dependency expression is invalid.
@@ -444,7 +496,8 @@ EFIAPI
 ValidateDependency (
   IN  EFI_FIRMWARE_IMAGE_DEP  *Dependencies,
   IN  UINTN                   MaxDepexSize,
-  OUT UINT32                  *DepexSize
+  OUT UINT32                  *DepexSize,
+  OUT UINT32                  *LastAttemptStatus OPTIONAL
   )
 {
   UINT8  *Depex;
@@ -489,20 +542,30 @@ ValidateDependency (
       }
       return TRUE;
     default:
+      if (LastAttemptStatus != NULL) {
+        *LastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_UNKNOWN_OPCODE;
+      }
       return FALSE;
     }
   }
 
+   if (LastAttemptStatus != NULL) {
+     *LastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_NO_END_OPCODE;
+   }
+
   return FALSE;
 }
 
 /**
   Get dependency from firmware image.
 
-  @param[in]  Image       Points to the firmware image.
-  @param[in]  ImageSize   Size, in bytes, of the firmware image.
-  @param[out] DepexSize   Size, in bytes, of the dependency.
-
+  @param[in]  Image               Points to the firmware image.
+  @param[in]  ImageSize           Size, in bytes, of the firmware image.
+  @param[out] DepexSize           Size, in bytes, of the dependency.
+  @param[out] LastAttemptStatus   An optional pointer to a UINT32 that holds the
+                                  last attempt status to report back to the caller.
+                                  A function error code may not always be accompanied
+                                  by a last attempt status code.
   @retval  The pointer to dependency.
   @retval  Null
 
@@ -512,7 +575,8 @@ EFIAPI
 GetImageDependency (
   IN  EFI_FIRMWARE_IMAGE_AUTHENTICATION *Image,
   IN  UINTN                             ImageSize,
-  OUT UINT32                            *DepexSize
+  OUT UINT32                            *DepexSize,
+  OUT UINT32                            *LastAttemptStatus  OPTIONAL
   )
 {
   EFI_FIRMWARE_IMAGE_DEP *Depex;
@@ -530,6 +594,9 @@ GetImageDependency (
     //
     // Pointer overflow. Invalid image.
     //
+    if (LastAttemptStatus != NULL) {
+      *LastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_ERROR_GET_DEPEX_FAILURE;
+    }
     return NULL;
   }
 
@@ -539,7 +606,7 @@ GetImageDependency (
   //
   // Validate the dependency and get the size of dependency
   //
-  if (ValidateDependency (Depex, MaxDepexSize, DepexSize)) {
+  if (ValidateDependency (Depex, MaxDepexSize, DepexSize, LastAttemptStatus)) {
     return Depex;
   }
 
diff --git a/FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependencyUnitTest.c b/FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependencyUnitTest.c
index f8ccdd906f29..92a1ac815d51 100644
--- a/FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependencyUnitTest.c
+++ b/FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependencyUnitTest.c
@@ -1,6 +1,7 @@
 /** @file
   Unit tests of EvaluateDependency API in FmpDependencyLib.
 
+  Copyright (c) Microsoft Corporation.<BR>
   Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -162,6 +163,7 @@ EvaluateDependencyTest (
 {
   BASIC_TEST_CONTEXT  *TestContext;
   BOOLEAN             EvaluationResult;
+  UINT32              LastAttemptStatus;
 
   TestContext = (BASIC_TEST_CONTEXT *)Context;
 
@@ -169,8 +171,9 @@ EvaluateDependencyTest (
                        (EFI_FIRMWARE_IMAGE_DEP *)TestContext->Dependencies,
                        TestContext->DependenciesSize,
                        mFmpVersions,
-                       sizeof(mFmpVersions)/sizeof(FMP_DEPEX_CHECK_VERSION_DATA)
-                     );
+                       sizeof(mFmpVersions)/sizeof(FMP_DEPEX_CHECK_VERSION_DATA),
+                       &LastAttemptStatus
+                       );
 
   UT_ASSERT_EQUAL (EvaluationResult, TestContext->ExpectedResult);
 
diff --git a/FmpDevicePkg/Include/Library/FmpDependencyCheckLib.h b/FmpDevicePkg/Include/Library/FmpDependencyCheckLib.h
index ec380c4947bd..708cbb06ba74 100644
--- a/FmpDevicePkg/Include/Library/FmpDependencyCheckLib.h
+++ b/FmpDevicePkg/Include/Library/FmpDependencyCheckLib.h
@@ -2,6 +2,7 @@
   Fmp Capsule Dependency check functions for Firmware Management Protocol based
   firmware updates.
 
+  Copyright (c) Microsoft Corporation.<BR>
   Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -21,6 +22,10 @@
   @param[in]  Version            New version.
   @param[in]  Dependencies       Fmp dependency.
   @param[in]  DependenciesSize   Size, in bytes, of the Fmp dependency.
+  @param[out] LastAttemptStatus  An optional pointer to a UINT32 that holds the
+                                 last attempt status to report back to the caller.
+                                 A function error code may not always be accompanied
+                                 by a last attempt status code.
 
   @retval  TRUE    Dependencies are satisfied.
   @retval  FALSE   Dependencies are unsatisfied or dependency check fails.
@@ -32,7 +37,8 @@ CheckFmpDependency (
   IN  EFI_GUID                ImageTypeId,
   IN  UINT32                  Version,
   IN  EFI_FIRMWARE_IMAGE_DEP  *Dependencies,    OPTIONAL
-  IN  UINT32                  DependenciesSize
+  IN  UINT32                  DependenciesSize,
+  OUT UINT32                  *LastAttemptStatus OPTIONAL
   );
 
 #endif
diff --git a/FmpDevicePkg/Include/Library/FmpDependencyLib.h b/FmpDevicePkg/Include/Library/FmpDependencyLib.h
index c732903425b4..b8e9c4341165 100644
--- a/FmpDevicePkg/Include/Library/FmpDependencyLib.h
+++ b/FmpDevicePkg/Include/Library/FmpDependencyLib.h
@@ -26,9 +26,13 @@ typedef struct {
 /**
   Validate the dependency expression and output its size.
 
-  @param[in]   Dependencies   Pointer to the EFI_FIRMWARE_IMAGE_DEP.
-  @param[in]   MaxDepexSize   Max size of the dependency.
-  @param[out]  DepexSize      Size of dependency.
+  @param[in]   Dependencies       Pointer to the EFI_FIRMWARE_IMAGE_DEP.
+  @param[in]   MaxDepexSize       Max size of the dependency.
+  @param[out]  DepexSize          Size of dependency.
+  @param[out]  LastAttemptStatus  An optional pointer to a UINT32 that holds the
+                                  last attempt status to report back to the caller.
+                                  A function error code may not always be accompanied
+                                  by a last attempt status code.
 
   @retval TRUE    The dependency expression is valid.
   @retval FALSE   The dependency expression is invalid.
@@ -39,16 +43,20 @@ EFIAPI
 ValidateDependency (
   IN  EFI_FIRMWARE_IMAGE_DEP  *Dependencies,
   IN  UINTN                   MaxDepexSize,
-  OUT UINT32                  *DepexSize
+  OUT UINT32                  *DepexSize,
+  OUT UINT32                  *LastAttemptStatus OPTIONAL
   );
 
 /**
   Get dependency from firmware image.
 
-  @param[in]  Image       Points to the firmware image.
-  @param[in]  ImageSize   Size, in bytes, of the firmware image.
-  @param[out] DepexSize   Size, in bytes, of the dependency.
-
+  @param[in]  Image               Points to the firmware image.
+  @param[in]  ImageSize           Size, in bytes, of the firmware image.
+  @param[out] DepexSize           Size, in bytes, of the dependency.
+  @param[out] LastAttemptStatus   An optional pointer to a UINT32 that holds the
+                                  last attempt status to report back to the caller.
+                                  A function error code may not always be accompanied
+                                  by a last attempt status code.
   @retval  The pointer to dependency.
   @retval  Null
 
@@ -56,9 +64,10 @@ ValidateDependency (
 EFI_FIRMWARE_IMAGE_DEP*
 EFIAPI
 GetImageDependency (
-  IN  EFI_FIRMWARE_IMAGE_AUTHENTICATION  *Image,
-  IN  UINTN                              ImageSize,
-  OUT UINT32                             *DepexSize
+  IN  EFI_FIRMWARE_IMAGE_AUTHENTICATION *Image,
+  IN  UINTN                             ImageSize,
+  OUT UINT32                            *DepexSize,
+  OUT UINT32                            *LastAttemptStatus  OPTIONAL
   );
 
 /**
@@ -73,6 +82,10 @@ GetImageDependency (
                                   parameter is optional and can be set to NULL.
   @param[in]   FmpVersionsCount   Element count of the array. When FmpVersions
                                   is NULL, FmpVersionsCount must be 0.
+  @param[out]  LastAttemptStatus  An optional pointer to a UINT32 that holds the
+                                  last attempt status to report back to the caller.
+                                  A function error code may not always be accompanied
+                                  by a last attempt status code.
 
   @retval TRUE    Dependency expressions evaluate to TRUE.
   @retval FALSE   Dependency expressions evaluate to FALSE.
@@ -81,10 +94,11 @@ GetImageDependency (
 BOOLEAN
 EFIAPI
 EvaluateDependency (
-  IN EFI_FIRMWARE_IMAGE_DEP        *Dependencies,
-  IN UINTN                         DependenciesSize,
-  IN FMP_DEPEX_CHECK_VERSION_DATA  *FmpVersions      OPTIONAL,
-  IN UINTN                         FmpVersionsCount
+  IN  EFI_FIRMWARE_IMAGE_DEP        *Dependencies,
+  IN  UINTN                         DependenciesSize,
+  IN  FMP_DEPEX_CHECK_VERSION_DATA  *FmpVersions,      OPTIONAL
+  IN  UINTN                         FmpVersionsCount,
+  OUT UINT32                        *LastAttemptStatus OPTIONAL
   );
 
 #endif
-- 
2.28.0.windows.1


  parent reply	other threads:[~2020-08-10 20:28 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200810202753.1318-1-michael.kubacki@outlook.com>
2020-08-10 20:27 ` [PATCH v2 1/6] MdePkg/SystemResourceTable.h: Add vendor range values Michael Kubacki
2020-08-10 20:27 ` [PATCH v2 2/6] FmpDevicePkg: Add LastAttemptStatus.h Michael Kubacki
2020-08-11  0:31   ` [edk2-devel] " Nate DeSimone
2020-08-11 17:46     ` Michael Kubacki
2020-08-11 18:57       ` Michael Kubacki
2020-08-21  5:33         ` Nate DeSimone
2020-08-21 21:23           ` Michael Kubacki
2020-08-24 17:22             ` Nate DeSimone
2020-08-25  0:30               ` Michael Kubacki
     [not found]               ` <957bfe53-d369-2f06-441f-c5c1bb74aeb2@outlook.com>
2020-08-28 17:46                 ` Michael Kubacki
2020-08-10 20:27 ` [PATCH v2 3/6] FmpDevicePkg/FmpDxe: Add check image path Last Attempt Status capability Michael Kubacki
2020-08-10 20:27 ` [PATCH v2 4/6] FmpDevicePkg/FmpDxe: Improve set image path Last Attempt Status granularity Michael Kubacki
2020-08-10 20:27 ` Michael Kubacki [this message]
2020-08-10 20:27 ` [PATCH v2 6/6] FmpDevicePkg/FmpDeviceLib: Add Last Attempt Status to Check/Set API Michael Kubacki

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=MWHPR07MB34408D4E74D104E29F144B74E9440@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