public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v4 0/2] MdeModulePkg/CapsulePei: Optimize the CapsulePei
@ 2019-06-05  1:15 Gao, Zhichao
  2019-06-05  1:15 ` [PATCH v4 1/2] " Gao, Zhichao
  2019-06-05  1:15 ` [PATCH v4 2/2] MdeMoudlePkg/CapsulePei: Substantial change on UefiCapsule.c Gao, Zhichao
  0 siblings, 2 replies; 5+ messages in thread
From: Gao, Zhichao @ 2019-06-05  1:15 UTC (permalink / raw)
  To: devel
  Cc: Jian J Wang, Hao A Wu, Ray Ni, Star Zeng, Liming Gao, Sean Brogan,
	Michael Turner, Bret Barkelew, Leif Lindholm

v1:
Sperate the capsule check function from GetCapsuleDescriptors
and name it to AreCapsulesStaged.
Rename GetCapsuleDescriptors to GetScatterGatherHeadEntries.
And optimize its to remove the duplicated code.

V2:
Clarify the changes of the patch and descript in the commit
message.

V3:
Add the code change repo location in Mu and add description of
the changes on it.
Do not need to remain 2 byte for the null-terminate while using
UnicodeValueToStringS.

V4:
Separate the patch into 2 part. first part is changes from MU.
Second part is the substantial change on it.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>

Bret Barkelew (1):
  MdeModulePkg/CapsulePei: Optimize the CapsulePei

Zhichao Gao (1):
  MdeMoudlePkg/CapsulePei: Substantial change on UefiCapsule.c

 MdeModulePkg/Universal/CapsulePei/Capsule.h   |   1 +
 .../Universal/CapsulePei/CapsulePei.inf       |   1 +
 .../Universal/CapsulePei/UefiCapsule.c        | 356 +++++++++---------
 3 files changed, 190 insertions(+), 168 deletions(-)

-- 
2.21.0.windows.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v4 1/2] MdeModulePkg/CapsulePei: Optimize the CapsulePei
  2019-06-05  1:15 [PATCH v4 0/2] MdeModulePkg/CapsulePei: Optimize the CapsulePei Gao, Zhichao
@ 2019-06-05  1:15 ` Gao, Zhichao
  2019-06-05  1:15 ` [PATCH v4 2/2] MdeMoudlePkg/CapsulePei: Substantial change on UefiCapsule.c Gao, Zhichao
  1 sibling, 0 replies; 5+ messages in thread
From: Gao, Zhichao @ 2019-06-05  1:15 UTC (permalink / raw)
  To: devel
  Cc: Bret Barkelew, Jian J Wang, Hao A Wu, Ray Ni, Star Zeng,
	Liming Gao, Sean Brogan, Michael Turner, Leif Lindholm,
	Zhichao gao

From: Bret Barkelew <Bret.Barkelew@microsoft.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1853

Code change form Mu project:
https://github.com/microsoft/mu_basecore/blob/release/201903/
MdeModulePkg/Universal/CapsulePei/UefiCapsule.c#L801

Separate the capsule check function from GetCapsuleDescriptors
to AreCapsulesStaged. The original one is unclear.

Avoid querying the capsule variable twice. Use a fixed array
to cache the SG list during count the number of SG list. Then
allocate memory buffer to save the SG list from array.

Using MemoryAllocationLib instead of memory function in Pei
services.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Zhichao gao <zhichao.gao@intel.com>
---
 MdeModulePkg/Universal/CapsulePei/Capsule.h   |   1 +
 .../Universal/CapsulePei/CapsulePei.inf       |   1 +
 .../Universal/CapsulePei/UefiCapsule.c        | 325 +++++++++---------
 3 files changed, 172 insertions(+), 155 deletions(-)

diff --git a/MdeModulePkg/Universal/CapsulePei/Capsule.h b/MdeModulePkg/Universal/CapsulePei/Capsule.h
index baf40423af..3d9cab02c4 100644
--- a/MdeModulePkg/Universal/CapsulePei/Capsule.h
+++ b/MdeModulePkg/Universal/CapsulePei/Capsule.h
@@ -30,6 +30,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/PcdLib.h>
 #include <Library/ReportStatusCodeLib.h>
 #include <Library/DebugAgentLib.h>
+#include <Library/MemoryAllocationLib.h>
 #include <IndustryStandard/PeImage.h>
 #include "Common/CommonHeader.h"
 
diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
index 5d43df3075..786c411633 100644
--- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
+++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
@@ -43,6 +43,7 @@
   BaseLib
   HobLib
   BaseMemoryLib
+  MemoryAllocationLib
   PeiServicesLib
   PeimEntryPoint
   DebugLib
diff --git a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
index e967599e96..7c8c7a0f45 100644
--- a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
+++ b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
@@ -791,18 +791,84 @@ BuildMemoryResourceDescriptor (
 }
 
 /**
-  Checks for the presence of capsule descriptors.
-  Get capsule descriptors from variable CapsuleUpdateData, CapsuleUpdateData1, CapsuleUpdateData2...
-  and save to DescriptorBuffer.
+  Check if the capsules are staged.
 
-  @param DescriptorBuffer        Pointer to the capsule descriptors
+  @param UpdateCapsules   A pointer to return the check result.
+
+  @retval EFI_INVALID_PARAMETER   The parameter is null.
+  @retval EFI_SUCCESS             The Capsules are staged.
 
-  @retval EFI_SUCCESS     a valid capsule is present
-  @retval EFI_NOT_FOUND   if a valid capsule is not present
 **/
 EFI_STATUS
-GetCapsuleDescriptors (
-  IN EFI_PHYSICAL_ADDRESS     *DescriptorBuffer
+EFIAPI
+AreCapsulesStaged(
+  OUT BOOLEAN     *UpdateCapsules
+  )
+{
+  EFI_STATUS                        Status;
+  UINTN                             Size;
+  EFI_PEI_READ_ONLY_VARIABLE2_PPI   *PPIVariableServices;
+  EFI_PHYSICAL_ADDRESS              CapsuleDataPtr64 = 0;
+
+  if (UpdateCapsules == NULL) {
+    DEBUG ((DEBUG_ERROR, "%a Invalid parameters.  Inputs can't be NULL\n", __FUNCTION__));
+    ASSERT (UpdateCapsules != NULL);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *UpdateCapsules = FALSE;
+
+  Status = PeiServicesLocatePpi(
+              &gEfiPeiReadOnlyVariable2PpiGuid,
+              0,
+              NULL,
+              (VOID **)&PPIVariableServices
+              );
+
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed to find ReadOnlyVariable2PPI\n"));
+    return Status;
+  }
+
+  //
+  // Check for Update capsule
+  //
+  Size = sizeof (CapsuleDataPtr64);
+  Status = PPIVariableServices->GetVariable (
+                                  PPIVariableServices,
+                                  EFI_CAPSULE_VARIABLE_NAME,
+                                  &gEfiCapsuleVendorGuid,
+                                  NULL,
+                                  &Size,
+                                  (VOID *)&CapsuleDataPtr64
+                                  );
+
+  if (!EFI_ERROR (Status)) {
+    *UpdateCapsules = TRUE;
+  }
+
+  return EFI_SUCCESS;
+}
+
+#define MAX_SG_LIST_HEADS (20)
+
+/**
+  Check all the variables for SG list heads and get the count and addresses.
+
+  @param ListLength               A pointer would return the SG list length.
+  @param HeadList                 A ponter to the capsule SG list.
+
+  @retval EFI_SUCCESS             a valid capsule is present
+  @retval EFI_NOT_FOUND           if a valid capsule is not present
+  @retval EFI_INVALID_PARAMETER   the input parameter is invalid
+  @retval EFI_OUT_OF_RESOURCES    fail to allocate memory
+
+**/
+EFI_STATUS
+EFIAPI
+GetScatterGatherHeadEntries(
+  OUT UINTN *ListLength,
+  OUT EFI_PHYSICAL_ADDRESS **HeadList
   )
 {
   EFI_STATUS                       Status;
@@ -815,6 +881,7 @@ GetCapsuleDescriptors (
   CHAR16                           *TempVarName;
   EFI_PHYSICAL_ADDRESS             CapsuleDataPtr64;
   EFI_PEI_READ_ONLY_VARIABLE2_PPI  *PPIVariableServices;
+  EFI_PHYSICAL_ADDRESS             TempList[MAX_SG_LIST_HEADS];
 
   Index             = 0;
   TempVarName       = NULL;
@@ -822,87 +889,96 @@ GetCapsuleDescriptors (
   ValidIndex        = 0;
   CapsuleDataPtr64  = 0;
 
+  if ((ListLength == NULL) || (HeadList == NULL)) {
+    DEBUG ((DEBUG_ERROR, "%a Invalid parameters.  Inputs can't be NULL\n", __FUNCTION__));
+    ASSERT (ListLength != NULL);
+    ASSERT (HeadList != NULL);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *ListLength = 0;
+  *HeadList = NULL;
+
   Status = PeiServicesLocatePpi (
               &gEfiPeiReadOnlyVariable2PpiGuid,
               0,
               NULL,
-              (VOID **) &PPIVariableServices
+              (VOID **)&PPIVariableServices
               );
-  if (Status == EFI_SUCCESS) {
-    StrCpyS (CapsuleVarName, sizeof(CapsuleVarName)/sizeof(CHAR16), EFI_CAPSULE_VARIABLE_NAME);
-    TempVarName = CapsuleVarName + StrLen (CapsuleVarName);
+
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed to find ReadOnlyVariable2PPI\n"));
+    return Status;
+  }
+
+  //
+  // setup var name buffer for update capsules
+  //
+  StrCpyS (CapsuleVarName, sizeof (CapsuleVarName) / sizeof (CHAR16), EFI_CAPSULE_VARIABLE_NAME);
+  TempVarName = CapsuleVarName + StrLen (CapsuleVarName);
+  while (ValidIndex < MAX_SG_LIST_HEADS) {
+    if (Index != 0) {
+      UnicodeValueToStringS (
+        TempVarName,
+        (sizeof (CapsuleVarName) - ((StrLen (CapsuleVarName) + 1) * sizeof (CHAR16))),
+        0,
+        Index,
+        0
+        );
+    }
     Size = sizeof (CapsuleDataPtr64);
-    while (1) {
-      if (Index == 0) {
-        //
-        // For the first Capsule Image
-        //
-        Status = PPIVariableServices->GetVariable (
-                                        PPIVariableServices,
-                                        CapsuleVarName,
-                                        &gEfiCapsuleVendorGuid,
-                                        NULL,
-                                        &Size,
-                                        (VOID *) &CapsuleDataPtr64
-                                        );
-        if (EFI_ERROR (Status)) {
-          DEBUG ((DEBUG_INFO, "Capsule -- capsule variable not set\n"));
-          return EFI_NOT_FOUND;
-        }
-        //
-        // We have a chicken/egg situation where the memory init code needs to
-        // know the boot mode prior to initializing memory. For this case, our
-        // validate function will fail. We can detect if this is the case if blocklist
-        // pointer is null. In that case, return success since we know that the
-        // variable is set.
-        //
-        if (DescriptorBuffer == NULL) {
-          return EFI_SUCCESS;
-        }
-      } else {
-        UnicodeValueToStringS (
-          TempVarName,
-          sizeof (CapsuleVarName) - ((UINTN)TempVarName - (UINTN)CapsuleVarName),
-          0,
-          Index,
-          0
-          );
-        Status = PPIVariableServices->GetVariable (
-                                        PPIVariableServices,
-                                        CapsuleVarName,
-                                        &gEfiCapsuleVendorGuid,
-                                        NULL,
-                                        &Size,
-                                        (VOID *) &CapsuleDataPtr64
-                                        );
-        if (EFI_ERROR (Status)) {
-          break;
-        }
+    Status = PPIVariableServices->GetVariable (
+                                    PPIVariableServices,
+                                    CapsuleVarName,
+                                    &gEfiCapsuleVendorGuid,
+                                    NULL,
+                                    &Size,
+                                    (VOID *)&CapsuleDataPtr64
+                                    );
 
-        //
-        // If this BlockList has been linked before, skip this variable
-        //
-        Flag = FALSE;
-        for (TempIndex = 0; TempIndex < ValidIndex; TempIndex++) {
-          if (DescriptorBuffer[TempIndex] == CapsuleDataPtr64)  {
-            Flag = TRUE;
-            break;
-          }
-        }
-        if (Flag) {
-          Index ++;
-          continue;
-        }
+    if (EFI_ERROR (Status)) {
+      if (Status != EFI_NOT_FOUND) {
+        DEBUG ((DEBUG_ERROR, "Unexpected error getting Capsule Update variable.  Status = %r\n"));
       }
+      break;
+    }
 
-      //
-      // Cache BlockList which has been processed
-      //
-      DescriptorBuffer[ValidIndex++] = CapsuleDataPtr64;
-      Index ++;
+    //
+    // If this BlockList has been linked before, skip this variable
+    //
+    Flag = FALSE;
+    for (TempIndex = 0; TempIndex < ValidIndex; TempIndex++) {
+      if (TempList[TempIndex] == CapsuleDataPtr64) {
+        Flag = TRUE;
+        break;
+      }
     }
+    if (Flag) {
+      Index++;
+      continue;
+    }
+
+    //
+    // add it to the cached list
+    //
+    TempList[ValidIndex++] = CapsuleDataPtr64;
+    Index++;
+  }
+
+  if (ValidIndex == 0) {
+    DEBUG ((DEBUG_ERROR, "%a didn't find any SG lists in variables\n", __FUNCTION__));
+    return EFI_NOT_FOUND;
   }
 
+  *HeadList = AllocateZeroPool ((ValidIndex + 1) * sizeof (EFI_PHYSICAL_ADDRESS));
+  if (*HeadList == NULL) {
+    DEBUG ((DEBUG_ERROR, "Failed to allocate memory\n"));
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  CopyMem (*HeadList, TempList, (ValidIndex) * sizeof (EFI_PHYSICAL_ADDRESS));
+  *ListLength = ValidIndex;
+
   return EFI_SUCCESS;
 }
 
@@ -937,15 +1013,9 @@ CapsuleCoalesce (
   IN OUT UINTN                       *MemorySize
   )
 {
-  UINTN                                Index;
-  UINTN                                Size;
-  UINTN                                VariableCount;
-  CHAR16                               CapsuleVarName[30];
-  CHAR16                               *TempVarName;
-  EFI_PHYSICAL_ADDRESS                 CapsuleDataPtr64;
   EFI_STATUS                           Status;
   EFI_BOOT_MODE                        BootMode;
-  EFI_PEI_READ_ONLY_VARIABLE2_PPI      *PPIVariableServices;
+  UINTN                                ListLength;
   EFI_PHYSICAL_ADDRESS                 *VariableArrayAddress;
   MEMORY_RESOURCE_DESCRIPTOR           *MemoryResource;
 #ifdef MDE_CPU_IA32
@@ -955,10 +1025,8 @@ CapsuleCoalesce (
   EFI_CAPSULE_LONG_MODE_BUFFER         LongModeBuffer;
 #endif
 
-  Index                   = 0;
-  VariableCount           = 0;
-  CapsuleVarName[0]       = 0;
-  CapsuleDataPtr64        = 0;
+  ListLength = 0;
+  VariableArrayAddress = NULL;
 
   //
   // Someone should have already ascertained the boot mode. If it's not
@@ -972,74 +1040,11 @@ CapsuleCoalesce (
   }
 
   //
-  // User may set the same ScatterGatherList with several different variables,
-  // so cache all ScatterGatherList for check later.
+  // Get SG list entries
   //
-  Status = PeiServicesLocatePpi (
-              &gEfiPeiReadOnlyVariable2PpiGuid,
-              0,
-              NULL,
-              (VOID **) &PPIVariableServices
-              );
+  Status = GetScatterGatherHeadEntries (&ListLength, &VariableArrayAddress);
   if (EFI_ERROR (Status)) {
-    goto Done;
-  }
-  Size = sizeof (CapsuleDataPtr64);
-  StrCpyS (CapsuleVarName, sizeof(CapsuleVarName)/sizeof(CHAR16), EFI_CAPSULE_VARIABLE_NAME);
-  TempVarName = CapsuleVarName + StrLen (CapsuleVarName);
-  while (TRUE) {
-    if (Index > 0) {
-      UnicodeValueToStringS (
-        TempVarName,
-        sizeof (CapsuleVarName) - ((UINTN)TempVarName - (UINTN)CapsuleVarName),
-        0,
-        Index,
-        0
-        );
-    }
-    Status = PPIVariableServices->GetVariable (
-                                    PPIVariableServices,
-                                    CapsuleVarName,
-                                    &gEfiCapsuleVendorGuid,
-                                    NULL,
-                                    &Size,
-                                    (VOID *) &CapsuleDataPtr64
-                                    );
-    if (EFI_ERROR (Status)) {
-      //
-      // There is no capsule variables, quit
-      //
-      DEBUG ((DEBUG_INFO,"Capsule variable Index = %d\n", Index));
-      break;
-    }
-    VariableCount++;
-    Index++;
-  }
-
-  DEBUG ((DEBUG_INFO,"Capsule variable count = %d\n", VariableCount));
-
-  //
-  // The last entry is the end flag.
-  //
-  Status = PeiServicesAllocatePool (
-             (VariableCount + 1) * sizeof (EFI_PHYSICAL_ADDRESS),
-             (VOID **)&VariableArrayAddress
-             );
-
-  if (Status != EFI_SUCCESS) {
-    DEBUG ((DEBUG_ERROR, "AllocatePages Failed!, Status = %x\n", Status));
-    goto Done;
-  }
-
-  ZeroMem (VariableArrayAddress, (VariableCount + 1) * sizeof (EFI_PHYSICAL_ADDRESS));
-
-  //
-  // Find out if we actually have a capsule.
-  // GetCapsuleDescriptors depends on variable PPI, so it should run in 32-bit environment.
-  //
-  Status = GetCapsuleDescriptors (VariableArrayAddress);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "Fail to find capsule variables.\n"));
+    DEBUG ((DEBUG_ERROR, "%a failed to get Scatter Gather List Head Entries.  Status = %r\n", __FUNCTION__, Status));
     goto Done;
   }
 
@@ -1118,7 +1123,17 @@ CheckCapsuleUpdate (
   )
 {
   EFI_STATUS  Status;
-  Status = GetCapsuleDescriptors (NULL);
+  BOOLEAN     Update;
+
+  Status = AreCapsulesStaged (&Update);
+
+  if (!EFI_ERROR (Status)) {
+    if (Update) {
+      Status = EFI_SUCCESS;
+    } else {
+      Status = EFI_NOT_FOUND;
+    }
+  }
   return Status;
 }
 /**
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v4 2/2] MdeMoudlePkg/CapsulePei: Substantial change on UefiCapsule.c
  2019-06-05  1:15 [PATCH v4 0/2] MdeModulePkg/CapsulePei: Optimize the CapsulePei Gao, Zhichao
  2019-06-05  1:15 ` [PATCH v4 1/2] " Gao, Zhichao
@ 2019-06-05  1:15 ` Gao, Zhichao
  2019-06-06 11:18   ` Leif Lindholm
  1 sibling, 1 reply; 5+ messages in thread
From: Gao, Zhichao @ 2019-06-05  1:15 UTC (permalink / raw)
  To: devel
  Cc: Jian J Wang, Hao A Wu, Ray Ni, Star Zeng, Liming Gao, Sean Brogan,
	Michael Turner, Bret Barkelew, Leif Lindholm, Zhichao gao

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1853

AreCapsulesStaged do not need to return the status, only boolean
result is useful. So directly return a boolean value.
Cannot initialize the variable at its definition.

GetScatterGatherHeadEntries: use allocated buffer instead of fixed
array to handle the condition which the SG list is larger then the
array size.

Remove API specifier AreCapsulesStaged and GetScatterGatherHeadEntries
because they are internal used.

Fix some coding style issues.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Zhichao gao <zhichao.gao@intel.com>
---
 .../Universal/CapsulePei/UefiCapsule.c        | 103 +++++++++---------
 1 file changed, 54 insertions(+), 49 deletions(-)

diff --git a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
index 7c8c7a0f45..fabf30926c 100644
--- a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
+++ b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
@@ -1,7 +1,7 @@
 /** @file
   Capsule update PEIM for UEFI2.0
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -10,6 +10,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include "Capsule.h"
 
+#define DEFAULT_SG_LIST_HEADS       (20)
+
 #ifdef MDE_CPU_IA32
 //
 // Global Descriptor Table (GDT)
@@ -793,30 +795,21 @@ BuildMemoryResourceDescriptor (
 /**
   Check if the capsules are staged.
 
-  @param UpdateCapsules   A pointer to return the check result.
-
-  @retval EFI_INVALID_PARAMETER   The parameter is null.
-  @retval EFI_SUCCESS             The Capsules are staged.
+  @retval TRUE              The capsules are staged.
+  @retval FALSE             The capsules are not staged.
 
 **/
-EFI_STATUS
-EFIAPI
-AreCapsulesStaged(
-  OUT BOOLEAN     *UpdateCapsules
+BOOLEAN
+AreCapsulesStaged (
+  VOID
   )
 {
   EFI_STATUS                        Status;
   UINTN                             Size;
   EFI_PEI_READ_ONLY_VARIABLE2_PPI   *PPIVariableServices;
-  EFI_PHYSICAL_ADDRESS              CapsuleDataPtr64 = 0;
-
-  if (UpdateCapsules == NULL) {
-    DEBUG ((DEBUG_ERROR, "%a Invalid parameters.  Inputs can't be NULL\n", __FUNCTION__));
-    ASSERT (UpdateCapsules != NULL);
-    return EFI_INVALID_PARAMETER;
-  }
+  EFI_PHYSICAL_ADDRESS              CapsuleDataPtr64;
 
-  *UpdateCapsules = FALSE;
+  CapsuleDataPtr64 = 0;
 
   Status = PeiServicesLocatePpi(
               &gEfiPeiReadOnlyVariable2PpiGuid,
@@ -827,7 +820,7 @@ AreCapsulesStaged(
 
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "Failed to find ReadOnlyVariable2PPI\n"));
-    return Status;
+    return FALSE;
   }
 
   //
@@ -844,14 +837,12 @@ AreCapsulesStaged(
                                   );
 
   if (!EFI_ERROR (Status)) {
-    *UpdateCapsules = TRUE;
+    return TRUE;
   }
 
-  return EFI_SUCCESS;
+  return FALSE;
 }
 
-#define MAX_SG_LIST_HEADS (20)
-
 /**
   Check all the variables for SG list heads and get the count and addresses.
 
@@ -865,23 +856,24 @@ AreCapsulesStaged(
 
 **/
 EFI_STATUS
-EFIAPI
-GetScatterGatherHeadEntries(
+GetScatterGatherHeadEntries (
   OUT UINTN *ListLength,
   OUT EFI_PHYSICAL_ADDRESS **HeadList
   )
 {
-  EFI_STATUS                       Status;
-  UINTN                            Size;
-  UINTN                            Index;
-  UINTN                            TempIndex;
-  UINTN                            ValidIndex;
-  BOOLEAN                          Flag;
-  CHAR16                           CapsuleVarName[30];
-  CHAR16                           *TempVarName;
-  EFI_PHYSICAL_ADDRESS             CapsuleDataPtr64;
-  EFI_PEI_READ_ONLY_VARIABLE2_PPI  *PPIVariableServices;
-  EFI_PHYSICAL_ADDRESS             TempList[MAX_SG_LIST_HEADS];
+  EFI_STATUS                        Status;
+  UINTN                             Size;
+  UINTN                             Index;
+  UINTN                             TempIndex;
+  UINTN                             ValidIndex;
+  BOOLEAN                           Flag;
+  CHAR16                            CapsuleVarName[30];
+  CHAR16                            *TempVarName;
+  EFI_PHYSICAL_ADDRESS              CapsuleDataPtr64;
+  EFI_PEI_READ_ONLY_VARIABLE2_PPI   *PPIVariableServices;
+  EFI_PHYSICAL_ADDRESS              *TempList;
+  EFI_PHYSICAL_ADDRESS              *EnlargedTempList;
+  UINTN                             TempListLength;
 
   Index             = 0;
   TempVarName       = NULL;
@@ -911,16 +903,26 @@ GetScatterGatherHeadEntries(
     return Status;
   }
 
+  //
+  // Allocate memory for sg list head
+  //
+  TempListLength = DEFAULT_SG_LIST_HEADS * sizeof (EFI_PHYSICAL_ADDRESS);
+  TempList = AllocateZeroPool (TempListLength);
+  if (TempList == NULL) {
+    DEBUG((DEBUG_ERROR, "Failed to allocate memory\n"));
+    return EFI_OUT_OF_RESOURCES;
+  }
+
   //
   // setup var name buffer for update capsules
   //
   StrCpyS (CapsuleVarName, sizeof (CapsuleVarName) / sizeof (CHAR16), EFI_CAPSULE_VARIABLE_NAME);
   TempVarName = CapsuleVarName + StrLen (CapsuleVarName);
-  while (ValidIndex < MAX_SG_LIST_HEADS) {
+  while (TRUE) {
     if (Index != 0) {
       UnicodeValueToStringS (
         TempVarName,
-        (sizeof (CapsuleVarName) - ((StrLen (CapsuleVarName) + 1) * sizeof (CHAR16))),
+        (sizeof(CapsuleVarName) - ((UINTN)TempVarName - (UINTN)CapsuleVarName)),
         0,
         Index,
         0
@@ -958,6 +960,17 @@ GetScatterGatherHeadEntries(
       continue;
     }
 
+    //
+    // The TempList is full, enlarge it
+    //
+    if ((ValidIndex + 1) >= TempListLength) {
+      EnlargedTempList = AllocateZeroPool (TempListLength * 2);
+      CopyMem (EnlargedTempList, TempList, TempListLength);
+      FreePool (TempList);
+      TempList = EnlargedTempList;
+      TempListLength *= 2;
+    }
+
     //
     // add it to the cached list
     //
@@ -1122,19 +1135,11 @@ CheckCapsuleUpdate (
   IN EFI_PEI_SERVICES           **PeiServices
   )
 {
-  EFI_STATUS  Status;
-  BOOLEAN     Update;
-
-  Status = AreCapsulesStaged (&Update);
-
-  if (!EFI_ERROR (Status)) {
-    if (Update) {
-      Status = EFI_SUCCESS;
-    } else {
-      Status = EFI_NOT_FOUND;
-    }
+  if (AreCapsulesStaged ()) {
+    return EFI_SUCCESS;
+  } else {
+    return EFI_NOT_FOUND;
   }
-  return Status;
 }
 /**
   This function will look at a capsule and determine if it's a test pattern.
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 2/2] MdeMoudlePkg/CapsulePei: Substantial change on UefiCapsule.c
  2019-06-05  1:15 ` [PATCH v4 2/2] MdeMoudlePkg/CapsulePei: Substantial change on UefiCapsule.c Gao, Zhichao
@ 2019-06-06 11:18   ` Leif Lindholm
  2019-06-10  2:09     ` Gao, Zhichao
  0 siblings, 1 reply; 5+ messages in thread
From: Leif Lindholm @ 2019-06-06 11:18 UTC (permalink / raw)
  To: Zhichao Gao
  Cc: devel, Jian J Wang, Hao A Wu, Ray Ni, Star Zeng, Liming Gao,
	Sean Brogan, Michael Turner, Bret Barkelew

Zhichao,

Thank you for splitting the patches up. This makes for much better history.
However,

On Wed, Jun 05, 2019 at 09:15:45AM +0800, Zhichao Gao wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1853
> 
> AreCapsulesStaged do not need to return the status, only boolean
> result is useful. So directly return a boolean value.
> Cannot initialize the variable at its definition.
> 
> GetScatterGatherHeadEntries: use allocated buffer instead of fixed
> array to handle the condition which the SG list is larger then the
> array size.
> 
> Remove API specifier AreCapsulesStaged and GetScatterGatherHeadEntries
> because they are internal used.
> 
> Fix some coding style issues.

The above are three or four (is the EFIAPI change a coding style issue
or not?) unrelated changes. Could you please break this up further?

Best Regards,

Leif

> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Signed-off-by: Zhichao gao <zhichao.gao@intel.com>
> ---
>  .../Universal/CapsulePei/UefiCapsule.c        | 103 +++++++++---------
>  1 file changed, 54 insertions(+), 49 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
> index 7c8c7a0f45..fabf30926c 100644
> --- a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
> +++ b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Capsule update PEIM for UEFI2.0
>  
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
>  
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -10,6 +10,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  #include "Capsule.h"
>  
> +#define DEFAULT_SG_LIST_HEADS       (20)
> +
>  #ifdef MDE_CPU_IA32
>  //
>  // Global Descriptor Table (GDT)
> @@ -793,30 +795,21 @@ BuildMemoryResourceDescriptor (
>  /**
>    Check if the capsules are staged.
>  
> -  @param UpdateCapsules   A pointer to return the check result.
> -
> -  @retval EFI_INVALID_PARAMETER   The parameter is null.
> -  @retval EFI_SUCCESS             The Capsules are staged.
> +  @retval TRUE              The capsules are staged.
> +  @retval FALSE             The capsules are not staged.
>  
>  **/
> -EFI_STATUS
> -EFIAPI
> -AreCapsulesStaged(
> -  OUT BOOLEAN     *UpdateCapsules
> +BOOLEAN
> +AreCapsulesStaged (
> +  VOID
>    )
>  {
>    EFI_STATUS                        Status;
>    UINTN                             Size;
>    EFI_PEI_READ_ONLY_VARIABLE2_PPI   *PPIVariableServices;
> -  EFI_PHYSICAL_ADDRESS              CapsuleDataPtr64 = 0;
> -
> -  if (UpdateCapsules == NULL) {
> -    DEBUG ((DEBUG_ERROR, "%a Invalid parameters.  Inputs can't be NULL\n", __FUNCTION__));
> -    ASSERT (UpdateCapsules != NULL);
> -    return EFI_INVALID_PARAMETER;
> -  }
> +  EFI_PHYSICAL_ADDRESS              CapsuleDataPtr64;
>  
> -  *UpdateCapsules = FALSE;
> +  CapsuleDataPtr64 = 0;
>  
>    Status = PeiServicesLocatePpi(
>                &gEfiPeiReadOnlyVariable2PpiGuid,
> @@ -827,7 +820,7 @@ AreCapsulesStaged(
>  
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "Failed to find ReadOnlyVariable2PPI\n"));
> -    return Status;
> +    return FALSE;
>    }
>  
>    //
> @@ -844,14 +837,12 @@ AreCapsulesStaged(
>                                    );
>  
>    if (!EFI_ERROR (Status)) {
> -    *UpdateCapsules = TRUE;
> +    return TRUE;
>    }
>  
> -  return EFI_SUCCESS;
> +  return FALSE;
>  }
>  
> -#define MAX_SG_LIST_HEADS (20)
> -
>  /**
>    Check all the variables for SG list heads and get the count and addresses.
>  
> @@ -865,23 +856,24 @@ AreCapsulesStaged(
>  
>  **/
>  EFI_STATUS
> -EFIAPI
> -GetScatterGatherHeadEntries(
> +GetScatterGatherHeadEntries (
>    OUT UINTN *ListLength,
>    OUT EFI_PHYSICAL_ADDRESS **HeadList
>    )
>  {
> -  EFI_STATUS                       Status;
> -  UINTN                            Size;
> -  UINTN                            Index;
> -  UINTN                            TempIndex;
> -  UINTN                            ValidIndex;
> -  BOOLEAN                          Flag;
> -  CHAR16                           CapsuleVarName[30];
> -  CHAR16                           *TempVarName;
> -  EFI_PHYSICAL_ADDRESS             CapsuleDataPtr64;
> -  EFI_PEI_READ_ONLY_VARIABLE2_PPI  *PPIVariableServices;
> -  EFI_PHYSICAL_ADDRESS             TempList[MAX_SG_LIST_HEADS];
> +  EFI_STATUS                        Status;
> +  UINTN                             Size;
> +  UINTN                             Index;
> +  UINTN                             TempIndex;
> +  UINTN                             ValidIndex;
> +  BOOLEAN                           Flag;
> +  CHAR16                            CapsuleVarName[30];
> +  CHAR16                            *TempVarName;
> +  EFI_PHYSICAL_ADDRESS              CapsuleDataPtr64;
> +  EFI_PEI_READ_ONLY_VARIABLE2_PPI   *PPIVariableServices;
> +  EFI_PHYSICAL_ADDRESS              *TempList;
> +  EFI_PHYSICAL_ADDRESS              *EnlargedTempList;
> +  UINTN                             TempListLength;
>  
>    Index             = 0;
>    TempVarName       = NULL;
> @@ -911,16 +903,26 @@ GetScatterGatherHeadEntries(
>      return Status;
>    }
>  
> +  //
> +  // Allocate memory for sg list head
> +  //
> +  TempListLength = DEFAULT_SG_LIST_HEADS * sizeof (EFI_PHYSICAL_ADDRESS);
> +  TempList = AllocateZeroPool (TempListLength);
> +  if (TempList == NULL) {
> +    DEBUG((DEBUG_ERROR, "Failed to allocate memory\n"));
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
>    //
>    // setup var name buffer for update capsules
>    //
>    StrCpyS (CapsuleVarName, sizeof (CapsuleVarName) / sizeof (CHAR16), EFI_CAPSULE_VARIABLE_NAME);
>    TempVarName = CapsuleVarName + StrLen (CapsuleVarName);
> -  while (ValidIndex < MAX_SG_LIST_HEADS) {
> +  while (TRUE) {
>      if (Index != 0) {
>        UnicodeValueToStringS (
>          TempVarName,
> -        (sizeof (CapsuleVarName) - ((StrLen (CapsuleVarName) + 1) * sizeof (CHAR16))),
> +        (sizeof(CapsuleVarName) - ((UINTN)TempVarName - (UINTN)CapsuleVarName)),
>          0,
>          Index,
>          0
> @@ -958,6 +960,17 @@ GetScatterGatherHeadEntries(
>        continue;
>      }
>  
> +    //
> +    // The TempList is full, enlarge it
> +    //
> +    if ((ValidIndex + 1) >= TempListLength) {
> +      EnlargedTempList = AllocateZeroPool (TempListLength * 2);
> +      CopyMem (EnlargedTempList, TempList, TempListLength);
> +      FreePool (TempList);
> +      TempList = EnlargedTempList;
> +      TempListLength *= 2;
> +    }
> +
>      //
>      // add it to the cached list
>      //
> @@ -1122,19 +1135,11 @@ CheckCapsuleUpdate (
>    IN EFI_PEI_SERVICES           **PeiServices
>    )
>  {
> -  EFI_STATUS  Status;
> -  BOOLEAN     Update;
> -
> -  Status = AreCapsulesStaged (&Update);
> -
> -  if (!EFI_ERROR (Status)) {
> -    if (Update) {
> -      Status = EFI_SUCCESS;
> -    } else {
> -      Status = EFI_NOT_FOUND;
> -    }
> +  if (AreCapsulesStaged ()) {
> +    return EFI_SUCCESS;
> +  } else {
> +    return EFI_NOT_FOUND;
>    }
> -  return Status;
>  }
>  /**
>    This function will look at a capsule and determine if it's a test pattern.
> -- 
> 2.21.0.windows.1
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 2/2] MdeMoudlePkg/CapsulePei: Substantial change on UefiCapsule.c
  2019-06-06 11:18   ` Leif Lindholm
@ 2019-06-10  2:09     ` Gao, Zhichao
  0 siblings, 0 replies; 5+ messages in thread
From: Gao, Zhichao @ 2019-06-10  2:09 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: devel@edk2.groups.io, Wang, Jian J, Wu, Hao A, Ni, Ray,
	Zeng, Star, Gao, Liming, Sean Brogan, Michael Turner,
	Bret Barkelew



> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Thursday, June 6, 2019 7:18 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>
> Cc: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean Brogan
> <sean.brogan@microsoft.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: Re: [PATCH v4 2/2] MdeMoudlePkg/CapsulePei: Substantial change
> on UefiCapsule.c
> 
> Zhichao,
> 
> Thank you for splitting the patches up. This makes for much better history.
> However,
> 
> On Wed, Jun 05, 2019 at 09:15:45AM +0800, Zhichao Gao wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1853
> >
> > AreCapsulesStaged do not need to return the status, only boolean
> > result is useful. So directly return a boolean value.
> > Cannot initialize the variable at its definition.
> >
> > GetScatterGatherHeadEntries: use allocated buffer instead of fixed
> > array to handle the condition which the SG list is larger then the
> > array size.
> >
> > Remove API specifier AreCapsulesStaged and
> GetScatterGatherHeadEntries
> > because they are internal used.
> >
> > Fix some coding style issues.
> 
> The above are three or four (is the EFIAPI change a coding style issue or not?)
> unrelated changes. Could you please break this up further?

EFIAPI is mentioned in CCS_2_1 spec. So I think it is a coding style issue too. The other coding style issue is lacking space between the function name and bracket.
I would separate 2/2 base on the purpose to make the history clear.

Thanks,
Zhichao

> 
> Best Regards,
> 
> Leif
> 
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > Cc: Michael Turner <Michael.Turner@microsoft.com>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Signed-off-by: Zhichao gao <zhichao.gao@intel.com>
> > ---
> >  .../Universal/CapsulePei/UefiCapsule.c        | 103 +++++++++---------
> >  1 file changed, 54 insertions(+), 49 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
> > b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
> > index 7c8c7a0f45..fabf30926c 100644
> > --- a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
> > +++ b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    Capsule update PEIM for UEFI2.0
> >
> > -Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > reserved.<BR>
> > +Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > +reserved.<BR>
> >  Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> >
> >  SPDX-License-Identifier: BSD-2-Clause-Patent @@ -10,6 +10,8 @@
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  #include "Capsule.h"
> >
> > +#define DEFAULT_SG_LIST_HEADS       (20)
> > +
> >  #ifdef MDE_CPU_IA32
> >  //
> >  // Global Descriptor Table (GDT)
> > @@ -793,30 +795,21 @@ BuildMemoryResourceDescriptor (
> >  /**
> >    Check if the capsules are staged.
> >
> > -  @param UpdateCapsules   A pointer to return the check result.
> > -
> > -  @retval EFI_INVALID_PARAMETER   The parameter is null.
> > -  @retval EFI_SUCCESS             The Capsules are staged.
> > +  @retval TRUE              The capsules are staged.
> > +  @retval FALSE             The capsules are not staged.
> >
> >  **/
> > -EFI_STATUS
> > -EFIAPI
> > -AreCapsulesStaged(
> > -  OUT BOOLEAN     *UpdateCapsules
> > +BOOLEAN
> > +AreCapsulesStaged (
> > +  VOID
> >    )
> >  {
> >    EFI_STATUS                        Status;
> >    UINTN                             Size;
> >    EFI_PEI_READ_ONLY_VARIABLE2_PPI   *PPIVariableServices;
> > -  EFI_PHYSICAL_ADDRESS              CapsuleDataPtr64 = 0;
> > -
> > -  if (UpdateCapsules == NULL) {
> > -    DEBUG ((DEBUG_ERROR, "%a Invalid parameters.  Inputs can't be
> NULL\n", __FUNCTION__));
> > -    ASSERT (UpdateCapsules != NULL);
> > -    return EFI_INVALID_PARAMETER;
> > -  }
> > +  EFI_PHYSICAL_ADDRESS              CapsuleDataPtr64;
> >
> > -  *UpdateCapsules = FALSE;
> > +  CapsuleDataPtr64 = 0;
> >
> >    Status = PeiServicesLocatePpi(
> >                &gEfiPeiReadOnlyVariable2PpiGuid, @@ -827,7 +820,7 @@
> > AreCapsulesStaged(
> >
> >    if (EFI_ERROR (Status)) {
> >      DEBUG ((DEBUG_ERROR, "Failed to find ReadOnlyVariable2PPI\n"));
> > -    return Status;
> > +    return FALSE;
> >    }
> >
> >    //
> > @@ -844,14 +837,12 @@ AreCapsulesStaged(
> >                                    );
> >
> >    if (!EFI_ERROR (Status)) {
> > -    *UpdateCapsules = TRUE;
> > +    return TRUE;
> >    }
> >
> > -  return EFI_SUCCESS;
> > +  return FALSE;
> >  }
> >
> > -#define MAX_SG_LIST_HEADS (20)
> > -
> >  /**
> >    Check all the variables for SG list heads and get the count and addresses.
> >
> > @@ -865,23 +856,24 @@ AreCapsulesStaged(
> >
> >  **/
> >  EFI_STATUS
> > -EFIAPI
> > -GetScatterGatherHeadEntries(
> > +GetScatterGatherHeadEntries (
> >    OUT UINTN *ListLength,
> >    OUT EFI_PHYSICAL_ADDRESS **HeadList
> >    )
> >  {
> > -  EFI_STATUS                       Status;
> > -  UINTN                            Size;
> > -  UINTN                            Index;
> > -  UINTN                            TempIndex;
> > -  UINTN                            ValidIndex;
> > -  BOOLEAN                          Flag;
> > -  CHAR16                           CapsuleVarName[30];
> > -  CHAR16                           *TempVarName;
> > -  EFI_PHYSICAL_ADDRESS             CapsuleDataPtr64;
> > -  EFI_PEI_READ_ONLY_VARIABLE2_PPI  *PPIVariableServices;
> > -  EFI_PHYSICAL_ADDRESS             TempList[MAX_SG_LIST_HEADS];
> > +  EFI_STATUS                        Status;
> > +  UINTN                             Size;
> > +  UINTN                             Index;
> > +  UINTN                             TempIndex;
> > +  UINTN                             ValidIndex;
> > +  BOOLEAN                           Flag;
> > +  CHAR16                            CapsuleVarName[30];
> > +  CHAR16                            *TempVarName;
> > +  EFI_PHYSICAL_ADDRESS              CapsuleDataPtr64;
> > +  EFI_PEI_READ_ONLY_VARIABLE2_PPI   *PPIVariableServices;
> > +  EFI_PHYSICAL_ADDRESS              *TempList;
> > +  EFI_PHYSICAL_ADDRESS              *EnlargedTempList;
> > +  UINTN                             TempListLength;
> >
> >    Index             = 0;
> >    TempVarName       = NULL;
> > @@ -911,16 +903,26 @@ GetScatterGatherHeadEntries(
> >      return Status;
> >    }
> >
> > +  //
> > +  // Allocate memory for sg list head  //  TempListLength =
> > + DEFAULT_SG_LIST_HEADS * sizeof (EFI_PHYSICAL_ADDRESS);  TempList =
> > + AllocateZeroPool (TempListLength);  if (TempList == NULL) {
> > +    DEBUG((DEBUG_ERROR, "Failed to allocate memory\n"));
> > +    return EFI_OUT_OF_RESOURCES;
> > +  }
> > +
> >    //
> >    // setup var name buffer for update capsules
> >    //
> >    StrCpyS (CapsuleVarName, sizeof (CapsuleVarName) / sizeof (CHAR16),
> EFI_CAPSULE_VARIABLE_NAME);
> >    TempVarName = CapsuleVarName + StrLen (CapsuleVarName);
> > -  while (ValidIndex < MAX_SG_LIST_HEADS) {
> > +  while (TRUE) {
> >      if (Index != 0) {
> >        UnicodeValueToStringS (
> >          TempVarName,
> > -        (sizeof (CapsuleVarName) - ((StrLen (CapsuleVarName) + 1) * sizeof
> (CHAR16))),
> > +        (sizeof(CapsuleVarName) - ((UINTN)TempVarName -
> > + (UINTN)CapsuleVarName)),
> >          0,
> >          Index,
> >          0
> > @@ -958,6 +960,17 @@ GetScatterGatherHeadEntries(
> >        continue;
> >      }
> >
> > +    //
> > +    // The TempList is full, enlarge it
> > +    //
> > +    if ((ValidIndex + 1) >= TempListLength) {
> > +      EnlargedTempList = AllocateZeroPool (TempListLength * 2);
> > +      CopyMem (EnlargedTempList, TempList, TempListLength);
> > +      FreePool (TempList);
> > +      TempList = EnlargedTempList;
> > +      TempListLength *= 2;
> > +    }
> > +
> >      //
> >      // add it to the cached list
> >      //
> > @@ -1122,19 +1135,11 @@ CheckCapsuleUpdate (
> >    IN EFI_PEI_SERVICES           **PeiServices
> >    )
> >  {
> > -  EFI_STATUS  Status;
> > -  BOOLEAN     Update;
> > -
> > -  Status = AreCapsulesStaged (&Update);
> > -
> > -  if (!EFI_ERROR (Status)) {
> > -    if (Update) {
> > -      Status = EFI_SUCCESS;
> > -    } else {
> > -      Status = EFI_NOT_FOUND;
> > -    }
> > +  if (AreCapsulesStaged ()) {
> > +    return EFI_SUCCESS;
> > +  } else {
> > +    return EFI_NOT_FOUND;
> >    }
> > -  return Status;
> >  }
> >  /**
> >    This function will look at a capsule and determine if it's a test pattern.
> > --
> > 2.21.0.windows.1
> >

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-06-10  2:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-05  1:15 [PATCH v4 0/2] MdeModulePkg/CapsulePei: Optimize the CapsulePei Gao, Zhichao
2019-06-05  1:15 ` [PATCH v4 1/2] " Gao, Zhichao
2019-06-05  1:15 ` [PATCH v4 2/2] MdeMoudlePkg/CapsulePei: Substantial change on UefiCapsule.c Gao, Zhichao
2019-06-06 11:18   ` Leif Lindholm
2019-06-10  2:09     ` Gao, Zhichao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox