public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Star Zeng <star.zeng@intel.com>
To: edk2-devel@lists.01.org
Cc: Star Zeng <star.zeng@intel.com>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Michael D Kinney <michael.d.kinney@intel.com>
Subject: [PATCH] MdeModulePkg DxeCapsuleLib: Use Attr to know whether reset is required
Date: Mon, 30 Jul 2018 16:54:24 +0800	[thread overview]
Message-ID: <1532940864-29952-1-git-send-email-star.zeng@intel.com> (raw)

Current DxeCapsuleLibFmp always do reset for FMP capsule.
Actually, the code should use Attributes from FMP descriptor to know
whether reset is required or not.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 .../Library/DxeCapsuleLibFmp/DxeCapsuleLib.c       | 134 ++++++++++++++++-----
 .../DxeCapsuleLibFmp/DxeCapsuleProcessLib.c        |  40 ++++--
 2 files changed, 134 insertions(+), 40 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
index e88dab8c7642..d00d7d88ff7d 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
@@ -620,11 +620,14 @@ DumpAllFmpInfo (
 
   @param[in]     UpdateImageTypeId       Used to identify device firmware targeted by this update.
   @param[in]     UpdateHardwareInstance  The HardwareInstance to target with this update.
-  @param[in,out] NoHandles               The number of handles returned in Buffer.
-  @param[out]    Buffer[out]             A pointer to the buffer to return the requested array of handles.
-
-  @retval EFI_SUCCESS            The array of handles was returned in Buffer, and the number of
-                                 handles in Buffer was returned in NoHandles.
+  @param[out]    NoHandles               The number of handles returned in HandleBuf.
+  @param[out]    HandleBuf               A pointer to the buffer to return the requested array of handles.
+  @param[out]    ResetRequiredBuf        A pointer to the buffer to return reset required flag for
+                                         the requested array of handles.
+
+  @retval EFI_SUCCESS            The array of handles and their reset required flag were returned in
+                                 HandleBuf and ResetRequiredBuf, and the number of handles in HandleBuf
+                                 was returned in NoHandles.
   @retval EFI_NOT_FOUND          No handles match the search.
   @retval EFI_OUT_OF_RESOURCES   There is not enough pool memory to store the matching results.
 **/
@@ -632,14 +635,16 @@ EFI_STATUS
 GetFmpHandleBufferByType (
   IN     EFI_GUID                     *UpdateImageTypeId,
   IN     UINT64                       UpdateHardwareInstance,
-  IN OUT UINTN                        *NoHandles,
-  OUT    EFI_HANDLE                   **Buffer
+  OUT    UINTN                        *NoHandles, OPTIONAL
+  OUT    EFI_HANDLE                   **HandleBuf, OPTIONAL
+  OUT    BOOLEAN                      **ResetRequiredBuf OPTIONAL
   )
 {
   EFI_STATUS                                    Status;
   EFI_HANDLE                                    *HandleBuffer;
   UINTN                                         NumberOfHandles;
   EFI_HANDLE                                    *MatchedHandleBuffer;
+  BOOLEAN                                       *MatchedResetRequiredBuffer;
   UINTN                                         MatchedNumberOfHandles;
   EFI_FIRMWARE_MANAGEMENT_PROTOCOL              *Fmp;
   UINTN                                         Index;
@@ -653,8 +658,15 @@ GetFmpHandleBufferByType (
   UINTN                                         Index2;
   EFI_FIRMWARE_IMAGE_DESCRIPTOR                 *TempFmpImageInfo;
 
-  *NoHandles = 0;
-  *Buffer = NULL;
+  if (NoHandles != NULL) {
+    *NoHandles = 0;
+  }
+  if (HandleBuf != NULL) {
+    *HandleBuf = NULL;
+  }
+  if (ResetRequiredBuf != NULL) {
+    *ResetRequiredBuf = NULL;
+  }
 
   Status = gBS->LocateHandleBuffer (
                   ByProtocol,
@@ -668,10 +680,26 @@ GetFmpHandleBufferByType (
   }
 
   MatchedNumberOfHandles = 0;
-  MatchedHandleBuffer = AllocateZeroPool (sizeof(EFI_HANDLE) * NumberOfHandles);
-  if (MatchedHandleBuffer == NULL) {
-    FreePool (HandleBuffer);
-    return EFI_OUT_OF_RESOURCES;
+
+  MatchedHandleBuffer = NULL;
+  if (HandleBuf != NULL) {
+    MatchedHandleBuffer = AllocateZeroPool (sizeof(EFI_HANDLE) * NumberOfHandles);
+    if (MatchedHandleBuffer == NULL) {
+      FreePool (HandleBuffer);
+      return EFI_OUT_OF_RESOURCES;
+    }
+  }
+
+  MatchedResetRequiredBuffer = NULL;
+  if (ResetRequiredBuf != NULL) {
+    MatchedResetRequiredBuffer = AllocateZeroPool (sizeof(BOOLEAN) * NumberOfHandles);
+    if (MatchedResetRequiredBuffer == NULL) {
+      if (MatchedHandleBuffer != NULL) {
+        FreePool (MatchedHandleBuffer);
+      }
+      FreePool (HandleBuffer);
+      return EFI_OUT_OF_RESOURCES;
+    }
   }
 
   for (Index = 0; Index < NumberOfHandles; Index++) {
@@ -733,7 +761,15 @@ GetFmpHandleBufferByType (
         if ((UpdateHardwareInstance == 0) ||
             ((FmpImageInfoDescriptorVer >= EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION) &&
              (UpdateHardwareInstance == TempFmpImageInfo->HardwareInstance))) {
-          MatchedHandleBuffer[MatchedNumberOfHandles] = HandleBuffer[Index];
+          if (MatchedHandleBuffer != NULL) {
+            MatchedHandleBuffer[MatchedNumberOfHandles] = HandleBuffer[Index];
+          }
+          if (MatchedResetRequiredBuffer != NULL) {
+            MatchedResetRequiredBuffer[MatchedNumberOfHandles] = (((TempFmpImageInfo->AttributesSupported & 
+                                                                 IMAGE_ATTRIBUTE_RESET_REQUIRED) != 0) &&
+                                                                 ((TempFmpImageInfo->AttributesSetting &
+                                                                 IMAGE_ATTRIBUTE_RESET_REQUIRED) != 0));
+          }
           MatchedNumberOfHandles++;
           break;
         }
@@ -749,8 +785,15 @@ GetFmpHandleBufferByType (
     return EFI_NOT_FOUND;
   }
 
-  *NoHandles = MatchedNumberOfHandles;
-  *Buffer = MatchedHandleBuffer;
+  if (NoHandles != NULL) {
+    *NoHandles = MatchedNumberOfHandles;
+  }
+  if (HandleBuf != NULL) {
+    *HandleBuf = MatchedHandleBuffer;
+  }
+  if (ResetRequiredBuf != NULL) {
+    *ResetRequiredBuf = MatchedResetRequiredBuffer;
+  }
 
   return EFI_SUCCESS;
 }
@@ -1078,7 +1121,8 @@ RecordFmpCapsuleStatus (
 
   This function need support nested FMP capsule.
 
-  @param[in]   CapsuleHeader         Points to a capsule header.
+  @param[in]  CapsuleHeader         Points to a capsule header.
+  @param[out] ResetRequired         Indicates whether reset is required or not.
 
   @retval EFI_SUCESS            Process Capsule Image successfully.
   @retval EFI_UNSUPPORTED       Capsule image is not supported by the firmware.
@@ -1088,7 +1132,8 @@ RecordFmpCapsuleStatus (
 **/
 EFI_STATUS
 ProcessFmpCapsuleImage (
-  IN EFI_CAPSULE_HEADER  *CapsuleHeader
+  IN EFI_CAPSULE_HEADER  *CapsuleHeader,
+  OUT BOOLEAN            *ResetRequired OPTIONAL
   )
 {
   EFI_STATUS                                    Status;
@@ -1098,6 +1143,7 @@ ProcessFmpCapsuleImage (
   UINT32                                        ItemNum;
   UINTN                                         Index;
   EFI_HANDLE                                    *HandleBuffer;
+  BOOLEAN                                       *ResetRequiredBuffer;
   UINTN                                         NumberOfHandles;
   UINTN                                         DriverLen;
   UINT64                                        UpdateHardwareInstance;
@@ -1106,7 +1152,7 @@ ProcessFmpCapsuleImage (
   BOOLEAN                                       Abort;
 
   if (!IsFmpCapsuleGuid(&CapsuleHeader->CapsuleGuid)) {
-    return ProcessFmpCapsuleImage ((EFI_CAPSULE_HEADER *)((UINTN)CapsuleHeader + CapsuleHeader->HeaderSize));
+    return ProcessFmpCapsuleImage ((EFI_CAPSULE_HEADER *)((UINTN)CapsuleHeader + CapsuleHeader->HeaderSize), ResetRequired);
   }
 
   NotReady = FALSE;
@@ -1176,7 +1222,8 @@ ProcessFmpCapsuleImage (
                &ImageHeader->UpdateImageTypeId,
                UpdateHardwareInstance,
                &NumberOfHandles,
-               &HandleBuffer
+               &HandleBuffer,
+               &ResetRequiredBuffer
                );
     if (EFI_ERROR(Status)) {
       NotReady = TRUE;
@@ -1209,6 +1256,10 @@ ProcessFmpCapsuleImage (
                  );
       if (Status != EFI_SUCCESS) {
         Abort = TRUE;
+      } else {
+        if (ResetRequired != NULL) {
+          *ResetRequired |= ResetRequiredBuffer[Index2];
+        }
       }
 
       RecordFmpCapsuleStatus (
@@ -1222,6 +1273,9 @@ ProcessFmpCapsuleImage (
     if (HandleBuffer != NULL) {
       FreePool(HandleBuffer);
     }
+    if (ResetRequiredBuffer != NULL) {
+      FreePool(ResetRequiredBuffer);
+    }
   }
 
   if (NotReady) {
@@ -1256,8 +1310,6 @@ IsNestedFmpCapsule (
   UINTN                      NestedCapsuleSize;
   ESRT_MANAGEMENT_PROTOCOL   *EsrtProtocol;
   EFI_SYSTEM_RESOURCE_ENTRY  Entry;
-  EFI_HANDLE                 *HandleBuffer;
-  UINTN                      NumberOfHandles;
 
   EsrtGuidFound = FALSE;
   if (mIsVirtualAddrConverted) {
@@ -1286,19 +1338,16 @@ IsNestedFmpCapsule (
     // Check Firmware Management Protocols
     //
     if (!EsrtGuidFound) {
-      HandleBuffer = NULL;
       Status = GetFmpHandleBufferByType (
                  &CapsuleHeader->CapsuleGuid,
                  0,
-                 &NumberOfHandles,
-                 &HandleBuffer
+                 NULL,
+                 NULL,
+                 NULL
                  );
       if (!EFI_ERROR(Status)) {
         EsrtGuidFound = TRUE;
       }
-      if (HandleBuffer != NULL) {
-        FreePool (HandleBuffer);
-      }
     }
   }
   if (!EsrtGuidFound) {
@@ -1386,6 +1435,7 @@ SupportCapsuleImage (
   Caution: This function may receive untrusted input.
 
   @param[in]  CapsuleHeader         Points to a capsule header.
+  @param[out] ResetRequired         Indicates whether reset is required or not.
 
   @retval EFI_SUCESS            Process Capsule Image successfully.
   @retval EFI_UNSUPPORTED       Capsule image is not supported by the firmware.
@@ -1394,8 +1444,9 @@ SupportCapsuleImage (
 **/
 EFI_STATUS
 EFIAPI
-ProcessCapsuleImage (
-  IN EFI_CAPSULE_HEADER  *CapsuleHeader
+ProcessThisCapsuleImage (
+  IN EFI_CAPSULE_HEADER  *CapsuleHeader,
+  OUT BOOLEAN            *ResetRequired OPTIONAL
   )
 {
   EFI_STATUS                   Status;
@@ -1432,7 +1483,7 @@ ProcessCapsuleImage (
     // Process EFI FMP Capsule
     //
     DEBUG((DEBUG_INFO, "ProcessFmpCapsuleImage ...\n"));
-    Status = ProcessFmpCapsuleImage(CapsuleHeader);
+    Status = ProcessFmpCapsuleImage(CapsuleHeader, ResetRequired);
     DEBUG((DEBUG_INFO, "ProcessFmpCapsuleImage - %r\n", Status));
 
     return Status;
@@ -1442,6 +1493,27 @@ ProcessCapsuleImage (
 }
 
 /**
+  The firmware implements to process the capsule image.
+
+  Caution: This function may receive untrusted input.
+
+  @param[in]  CapsuleHeader         Points to a capsule header.
+
+  @retval EFI_SUCESS            Process Capsule Image successfully.
+  @retval EFI_UNSUPPORTED       Capsule image is not supported by the firmware.
+  @retval EFI_VOLUME_CORRUPTED  FV volume in the capsule is corrupted.
+  @retval EFI_OUT_OF_RESOURCES  Not enough memory.
+**/
+EFI_STATUS
+EFIAPI
+ProcessCapsuleImage (
+  IN EFI_CAPSULE_HEADER  *CapsuleHeader
+  )
+{
+  return ProcessThisCapsuleImage (CapsuleHeader, NULL);
+}
+
+/**
   Callback function executed when the EndOfDxe event group is signaled.
 
   @param[in] Event      Event whose notification function is being invoked.
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
index 26ca4e295f20..176dea196026 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
@@ -99,13 +99,33 @@ IsValidCapsuleHeader (
   );
 
 extern BOOLEAN                   mDxeCapsuleLibEndOfDxe;
-BOOLEAN                          mNeedReset;
+BOOLEAN                          mNeedReset = FALSE;
 
 VOID                        **mCapsulePtr;
 EFI_STATUS                  *mCapsuleStatusArray;
 UINT32                      mCapsuleTotalNumber;
 
 /**
+  The firmware implements to process the capsule image.
+
+  Caution: This function may receive untrusted input.
+
+  @param[in]  CapsuleHeader         Points to a capsule header.
+  @param[out] ResetRequired         Indicates whether reset is required or not.
+
+  @retval EFI_SUCESS            Process Capsule Image successfully.
+  @retval EFI_UNSUPPORTED       Capsule image is not supported by the firmware.
+  @retval EFI_VOLUME_CORRUPTED  FV volume in the capsule is corrupted.
+  @retval EFI_OUT_OF_RESOURCES  Not enough memory.
+**/
+EFI_STATUS
+EFIAPI
+ProcessThisCapsuleImage (
+  IN EFI_CAPSULE_HEADER  *CapsuleHeader,
+  OUT BOOLEAN            *ResetRequired OPTIONAL
+  );
+
+/**
   Function indicate the current completion progress of the firmware
   update. Platform may override with own specific progress function.
 
@@ -381,6 +401,7 @@ ProcessTheseCapsules (
   UINT32                      Index;
   ESRT_MANAGEMENT_PROTOCOL    *EsrtManagement;
   UINT16                      EmbeddedDriverCount;
+  BOOLEAN                     ResetRequired;
 
   REPORT_STATUS_CODE(EFI_PROGRESS_CODE, (EFI_SOFTWARE | PcdGet32(PcdStatusCodeSubClassCapsule) | PcdGet32(PcdCapsuleStatusCodeProcessCapsulesBegin)));
 
@@ -416,11 +437,11 @@ ProcessTheseCapsules (
   for (Index = 0; Index < mCapsuleTotalNumber; Index++) {
     CapsuleHeader = (EFI_CAPSULE_HEADER*) mCapsulePtr [Index];
     if (CompareGuid (&CapsuleHeader->CapsuleGuid, &gWindowsUxCapsuleGuid)) {
-      DEBUG ((DEBUG_INFO, "ProcessCapsuleImage (Ux) - 0x%x\n", CapsuleHeader));
+      DEBUG ((DEBUG_INFO, "ProcessThisCapsuleImage (Ux) - 0x%x\n", CapsuleHeader));
       DEBUG ((DEBUG_INFO, "Display logo capsule is found.\n"));
-      Status = ProcessCapsuleImage (CapsuleHeader);
+      Status = ProcessThisCapsuleImage (CapsuleHeader, NULL);
       mCapsuleStatusArray [Index] = EFI_SUCCESS;
-      DEBUG((DEBUG_INFO, "ProcessCapsuleImage (Ux) - %r\n", Status));
+      DEBUG((DEBUG_INFO, "ProcessThisCapsuleImage (Ux) - %r\n", Status));
       break;
     }
   }
@@ -454,10 +475,11 @@ ProcessTheseCapsules (
       }
 
       if ((!FirstRound) || (EmbeddedDriverCount == 0)) {
-        DEBUG((DEBUG_INFO, "ProcessCapsuleImage - 0x%x\n", CapsuleHeader));
-        Status = ProcessCapsuleImage (CapsuleHeader);
+        DEBUG((DEBUG_INFO, "ProcessThisCapsuleImage - 0x%x\n", CapsuleHeader));
+        ResetRequired = FALSE;
+        Status = ProcessThisCapsuleImage (CapsuleHeader, &ResetRequired);
         mCapsuleStatusArray [Index] = Status;
-        DEBUG((DEBUG_INFO, "ProcessCapsuleImage - %r\n", Status));
+        DEBUG((DEBUG_INFO, "ProcessThisCapsuleImage - %r\n", Status));
 
         if (Status != EFI_NOT_READY) {
           if (EFI_ERROR(Status)) {
@@ -467,8 +489,8 @@ ProcessTheseCapsules (
             REPORT_STATUS_CODE(EFI_PROGRESS_CODE, (EFI_SOFTWARE | PcdGet32(PcdStatusCodeSubClassCapsule) | PcdGet32(PcdCapsuleStatusCodeUpdateFirmwareSuccess)));
           }
 
-          if ((CapsuleHeader->Flags & PcdGet16(PcdSystemRebootAfterCapsuleProcessFlag)) != 0 ||
-              IsFmpCapsule(CapsuleHeader)) {
+          mNeedReset |= ResetRequired;
+          if ((CapsuleHeader->Flags & PcdGet16(PcdSystemRebootAfterCapsuleProcessFlag)) != 0) {
             mNeedReset = TRUE;
           }
         }
-- 
2.7.0.windows.1



             reply	other threads:[~2018-07-30  8:54 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-30  8:54 Star Zeng [this message]
2018-07-31  0:45 ` [PATCH] MdeModulePkg DxeCapsuleLib: Use Attr to know whether reset is required Yao, Jiewen

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=1532940864-29952-1-git-send-email-star.zeng@intel.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