public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gao, Zhichao" <zhichao.gao@intel.com>
To: devel@edk2.groups.io
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>,
	Jian J Wang <jian.j.wang@intel.com>,
	Hao A Wu <hao.a.wu@intel.com>, Ray Ni <ray.ni@intel.com>,
	Star Zeng <star.zeng@intel.com>,
	Liming Gao <liming.gao@intel.com>,
	Sean Brogan <sean.brogan@microsoft.com>,
	Michael Turner <Michael.Turner@microsoft.com>,
	Zhichao gao <zhichao.gao@intel.com>
Subject: [PATCH] MdeModulePkg/CapsulePei: Optimize the CapsulePei
Date: Wed, 29 May 2019 08:45:55 +0800	[thread overview]
Message-ID: <20190529004555.45364-1-zhichao.gao@intel.com> (raw)

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

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

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

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>
Signed-off-by: Zhichao gao <zhichao.gao@intel.com>
---
 MdeModulePkg/Universal/CapsulePei/Capsule.h   |   3 +-
 .../Universal/CapsulePei/CapsulePei.inf       |   3 +-
 .../Universal/CapsulePei/UefiCapsule.c        | 357 ++++++++++--------
 3 files changed, 194 insertions(+), 169 deletions(-)

diff --git a/MdeModulePkg/Universal/CapsulePei/Capsule.h b/MdeModulePkg/Universal/CapsulePei/Capsule.h
index baf40423af..fc20dd8b92 100644
--- a/MdeModulePkg/Universal/CapsulePei/Capsule.h
+++ b/MdeModulePkg/Universal/CapsulePei/Capsule.h
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -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..9c88b3986f 100644
--- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
+++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
@@ -6,7 +6,7 @@
 #  This external input must be validated carefully to avoid security issue like
 #  buffer overflow, integer overflow.
 #
-# 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
@@ -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..2d003369ca 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)
@@ -791,30 +793,89 @@ 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
+  @retval TRUE              The capsules are staged.
+  @retval FALSE             The capsules are not staged.
+
+**/
+BOOLEAN
+EFIAPI
+AreCapsulesStaged (
+  VOID
+  )
+{
+  EFI_STATUS                        Status;
+  UINTN                             Size;
+  EFI_PEI_READ_ONLY_VARIABLE2_PPI   *PPIVariableServices;
+  EFI_PHYSICAL_ADDRESS              CapsuleDataPtr64;
+
+  CapsuleDataPtr64 = 0;
+
+  Status = PeiServicesLocatePpi (
+            &gEfiPeiReadOnlyVariable2PpiGuid,
+            0,
+            NULL,
+            (VOID **)&PPIVariableServices
+            );
+
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed to find ReadOnlyVariable2PPI\n"));
+    return FALSE;
+  }
+
+  //
+  // Check for Update capsule
+  //
+  Size = sizeof (CapsuleDataPtr64);
+  Status = PPIVariableServices->GetVariable(
+                                  PPIVariableServices,
+                                  EFI_CAPSULE_VARIABLE_NAME,
+                                  &gEfiCapsuleVendorGuid,
+                                  NULL,
+                                  &Size,
+                                  (VOID *)&CapsuleDataPtr64
+                                  );
+
+  if (!EFI_ERROR (Status)) {
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+/**
+  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_RESOURCE     fail to allocate memory
 
-  @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
+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_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;
@@ -822,87 +883,118 @@ 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
               );
-  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: %r\n", Status));
+    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 (TRUE) {
+    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;
+    }
+
+    //
+    // The TempList is full, enlarge it
+    //
+    if ((ValidIndex + 1) >= TempListLength) {
+      EnlargedTempList = AllocateZeroPool (TempListLength * 2);
+      CopyMem (EnlargedTempList, TempList, TempListLength);
+      FreePool (TempList);
+      TempList = EnlargedTempList;
+      TempListLength *= 2;
+    }
+
+    //
+    // Cache BlockList which has been processed
+    //
+    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,17 +1029,11 @@ 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;
   EFI_PHYSICAL_ADDRESS                 *VariableArrayAddress;
   MEMORY_RESOURCE_DESCRIPTOR           *MemoryResource;
+  UINTN                                ListLength;
 #ifdef MDE_CPU_IA32
   UINT16                               CoalesceImageMachineType;
   EFI_PHYSICAL_ADDRESS                 CoalesceImageEntryPoint;
@@ -955,10 +1041,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 +1056,11 @@ CapsuleCoalesce (
   }
 
   //
-  // User may set the same ScatterGatherList with several different variables,
-  // so cache all ScatterGatherList for check later.
+  // Get ScatterGatherList
   //
-  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;
   }
 
@@ -1117,9 +1138,11 @@ CheckCapsuleUpdate (
   IN EFI_PEI_SERVICES           **PeiServices
   )
 {
-  EFI_STATUS  Status;
-  Status = GetCapsuleDescriptors (NULL);
-  return Status;
+  if (AreCapsulesStaged ()) {
+    return EFI_SUCCESS;
+  } else {
+    return EFI_NOT_FOUND;
+  }
 }
 /**
   This function will look at a capsule and determine if it's a test pattern.
-- 
2.21.0.windows.1


             reply	other threads:[~2019-05-29  0:46 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-29  0:45 Gao, Zhichao [this message]
2019-05-29  6:54 ` [PATCH] MdeModulePkg/CapsulePei: Optimize the CapsulePei Wu, Hao A
2019-05-30  3:20   ` Gao, Zhichao
2019-05-30  5:29     ` Wu, Hao A
2019-05-29 11:11 ` [edk2-devel] " Leif Lindholm
2019-05-29 15:01   ` Gao, Zhichao
2019-05-29 15:07     ` Liming Gao
2019-05-29 15:09     ` Leif Lindholm
2019-05-31  1:46       ` Gao, Zhichao
2019-05-31 16:43         ` Leif Lindholm
2019-06-03  8:18           ` Gao, Zhichao
2019-06-03 10:11             ` Leif Lindholm

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=20190529004555.45364-1-zhichao.gao@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