From nobody Sun Apr 28 11:35:58 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of groups.io designates 66.175.222.12 as permitted sender) client-ip=66.175.222.12; envelope-from=bounce+27952+41702+1787277+3901457@groups.io; helo=web01.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zoho.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+41702+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=intel.com ARC-Seal: i=1; a=rsa-sha256; t=1559267291; cv=none; d=zoho.com; s=zohoarc; b=ceQz1MnRPfRJNJDEK4yixIv/K5Q+nzeJt/cGmRV9leJWt1uMMELNjAmJM9O5xqTT++9UTY4StijIjQFxeaVF6KTGIBQ2W6aj5/RIphqUch8n2BwKvy6tVENbX0Psy45wOL+XWDJy0DUzBP/ofs79nn4xeAoT7rOuxXFBmAv072g= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1559267291; h=Content-Transfer-Encoding:Cc:Date:From:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Sender:Subject:To:ARC-Authentication-Results; bh=SxfZwxpkl+d4DF1QLnGt+2lcoZDhKhkKrWzVhe5VROM=; b=SXDJtfmoppRmMn+rhuzzjqO73XMHc/1i6LPgnESICSrBSAfwgjQzMzBVkqSCnSzZ9z0Oy0jN4Qa+JED2s3ishdyW3QVMcOlQHCjz3yybRI9BIhq2LRsnT5cRr5vtcERpbtolRTq4iwieqFp8OLVQwtBiOYV6vKQRnA6AxoYol58= ARC-Authentication-Results: i=1; mx.zoho.com; dkim=pass; spf=pass (zoho.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+41702+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) header.from= Received: from web01.groups.io (web01.groups.io [66.175.222.12]) by mx.zohomail.com with SMTPS id 15592672910801017.3162905567987; Thu, 30 May 2019 18:48:11 -0700 (PDT) Return-Path: X-Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by groups.io with SMTP; Thu, 30 May 2019 18:48:10 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 May 2019 18:48:09 -0700 X-ExtLoop1: 1 X-Received: from fieedk001.ccr.corp.intel.com ([10.239.33.119]) by orsmga007.jf.intel.com with ESMTP; 30 May 2019 18:48:06 -0700 From: "Gao, Zhichao" To: devel@edk2.groups.io Cc: Bret Barkelew , Jian J Wang , Hao A Wu , Ray Ni , Star Zeng , Liming Gao , Sean Brogan , Michael Turner , Zhichao gao Subject: [edk2-devel] [PATCH V2] MdeModulePkg/CapsulePei: Optimize the CapsulePei Date: Fri, 31 May 2019 09:48:05 +0800 Message-Id: <20190531014805.9676-1-zhichao.gao@intel.com> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,zhichao.gao@intel.com Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1559267290; bh=ZCHRWqZQSsWxVOEACs9OotnsN0QquUDewUQ0Hu37JuA=; h=Cc:Date:From:Reply-To:Subject:To; b=UBXLvZRs62L61C7yujUkWlmYTkp8S6fxceb9AcO7+My8Q5b8tSrCmBJY+0LPlcOfJ4X h6pzBUF0ZWhebesvm92xcmvNLK+0DLNMiRH2OB10odjlmRDeDPT6Ljk3fNU6HcgmleYHu drXLM6zzQiwGMNB8+fiOYJ996D3oLcTBHjg= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" From: Bret Barkelew REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1853 Optimize some function in CapsulePei to make it easier to maintian. 1. Separate the capsule check function form GetCapsuleDescriptors to AreCapsulesStaged. The original logic is unclear. 2. Avoid querying the capsule variable twice. First time to count the number of SG list and allocate a buffer to save SG list data. Second time to save the SG list data to the buffer. Modified: Using a template buffer to save the SG list data. After query, we get the number of SG list, then allocate memory and copy data form template buffer to the allocated memory. 3. Using MemoryAllocationLib instead of memory function in Pei services. 4, Remain 2 byte(CHAR16) to be the null-terminate of CapsuleVarName. Cc: Jian J Wang Cc: Hao A Wu Cc: Ray Ni Cc: Star Zeng Cc: Liming Gao Cc: Sean Brogan Cc: Michael Turner Cc: Bret Barkelew Signed-off-by: Zhichao gao Reviewed-by: Hao A Wu --- Code change from https://github.com/microsoft/mu_basecore/blob/release/2019= 03/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c#L801 Note: 1. Change the AreCapsulesStaged: directly return the BOOLEAN result. 2. While the template buffer is to small, double its size through memory fu= nction. MdeModulePkg/Universal/CapsulePei/Capsule.h | 3 +- .../Universal/CapsulePei/CapsulePei.inf | 3 +- .../Universal/CapsulePei/UefiCapsule.c | 355 ++++++++++-------- 3 files changed, 192 insertions(+), 169 deletions(-) diff --git a/MdeModulePkg/Universal/CapsulePei/Capsule.h b/MdeModulePkg/Uni= versal/CapsulePei/Capsule.h index baf40423af..fc20dd8b92 100644 --- a/MdeModulePkg/Universal/CapsulePei/Capsule.h +++ b/MdeModulePkg/Universal/CapsulePei/Capsule.h @@ -1,6 +1,6 @@ /** @file =20 -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
=20 SPDX-License-Identifier: BSD-2-Clause-Patent =20 @@ -30,6 +30,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include #include #include +#include #include #include "Common/CommonHeader.h" =20 diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf b/MdeModulePk= g/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.
+# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
# Copyright (c) 2017, AMD Incorporated. All rights reserved.
# # 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..b3014478a3 100644 --- a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c +++ b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c @@ -1,7 +1,7 @@ /** @file Capsule update PEIM for UEFI2.0 =20 -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
Copyright (c) 2017, AMD Incorporated. All rights reserved.
=20 SPDX-License-Identifier: BSD-2-Clause-Patent @@ -10,6 +10,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent =20 #include "Capsule.h" =20 +#define DEFAULT_SG_LIST_HEADS (20) + #ifdef MDE_CPU_IA32 // // Global Descriptor Table (GDT) @@ -791,30 +793,87 @@ BuildMemoryResourceDescriptor ( } =20 /** - Checks for the presence of capsule descriptors. - Get capsule descriptors from variable CapsuleUpdateData, CapsuleUpdateDa= ta1, CapsuleUpdateData2... - and save to DescriptorBuffer. + Check if the capsules are staged. =20 - @param DescriptorBuffer Pointer to the capsule descriptors + @retval TRUE The capsules are staged. + @retval FALSE The capsules are not staged. + +**/ +BOOLEAN +AreCapsulesStaged ( + VOID + ) +{ + EFI_STATUS Status; + UINTN Size; + EFI_PEI_READ_ONLY_VARIABLE2_PPI *PPIVariableServices; + EFI_PHYSICAL_ADDRESS CapsuleDataPtr64; + + CapsuleDataPtr64 =3D 0; + + Status =3D 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 =3D sizeof (CapsuleDataPtr64); + Status =3D 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 addresse= s. + + @param ListLength A pointer would return the SG list lengt= h. + @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 =20 - @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 +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; =20 Index =3D 0; TempVarName =3D NULL; @@ -822,87 +881,118 @@ GetCapsuleDescriptors ( ValidIndex =3D 0; CapsuleDataPtr64 =3D 0; =20 + if ((ListLength =3D=3D NULL) || (HeadList =3D=3D NULL)) { + DEBUG ((DEBUG_ERROR, "%a Invalid parameters. Inputs can't be NULL\n",= __FUNCTION__)); + ASSERT (ListLength !=3D NULL); + ASSERT (HeadList !=3D NULL); + return EFI_INVALID_PARAMETER; + } + + *ListLength =3D 0; + *HeadList =3D NULL; + Status =3D PeiServicesLocatePpi ( &gEfiPeiReadOnlyVariable2PpiGuid, 0, NULL, (VOID **) &PPIVariableServices ); - if (Status =3D=3D EFI_SUCCESS) { - StrCpyS (CapsuleVarName, sizeof(CapsuleVarName)/sizeof(CHAR16), EFI_CA= PSULE_VARIABLE_NAME); - TempVarName =3D CapsuleVarName + StrLen (CapsuleVarName); + + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "Failed to find ReadOnlyVariable2PPI: %r\n", Stat= us)); + return Status; + } + + // + // Allocate memory for sg list head + // + TempListLength =3D DEFAULT_SG_LIST_HEADS * sizeof (EFI_PHYSICAL_ADDRESS); + TempList =3D AllocateZeroPool (TempListLength); + if (TempList =3D=3D 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 =3D CapsuleVarName + StrLen (CapsuleVarName); + while (TRUE) { + if (Index !=3D 0) { + UnicodeValueToStringS ( + TempVarName, + (sizeof(CapsuleVarName) - ((StrLen(CapsuleVarName) + 1) * sizeof(C= HAR16))), + 0, + Index, + 0 + ); + } + Size =3D sizeof (CapsuleDataPtr64); - while (1) { - if (Index =3D=3D 0) { - // - // For the first Capsule Image - // - Status =3D 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 need= s 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 tha= t the - // variable is set. - // - if (DescriptorBuffer =3D=3D NULL) { - return EFI_SUCCESS; - } - } else { - UnicodeValueToStringS ( - TempVarName, - sizeof (CapsuleVarName) - ((UINTN)TempVarName - (UINTN)CapsuleVa= rName), - 0, - Index, - 0 - ); - Status =3D PPIVariableServices->GetVariable ( - PPIVariableServices, - CapsuleVarName, - &gEfiCapsuleVendorGuid, - NULL, - &Size, - (VOID *) &CapsuleDataPtr64 - ); - if (EFI_ERROR (Status)) { - break; - } + Status =3D PPIVariableServices->GetVariable ( + PPIVariableServices, + CapsuleVarName, + &gEfiCapsuleVendorGuid, + NULL, + &Size, + (VOID *)&CapsuleDataPtr64 + ); =20 - // - // If this BlockList has been linked before, skip this variable - // - Flag =3D FALSE; - for (TempIndex =3D 0; TempIndex < ValidIndex; TempIndex++) { - if (DescriptorBuffer[TempIndex] =3D=3D CapsuleDataPtr64) { - Flag =3D TRUE; - break; - } - } - if (Flag) { - Index ++; - continue; - } + if (EFI_ERROR (Status)) { + if (Status !=3D EFI_NOT_FOUND) { + DEBUG ((DEBUG_ERROR, "Unexpected error getting Capsule Update vari= able. Status =3D %r\n")); + } + break; + } + + // + // If this BlockList has been linked before, skip this variable + // + Flag =3D FALSE; + for (TempIndex =3D 0; TempIndex < ValidIndex; TempIndex++) { + if (TempList[TempIndex] =3D=3D CapsuleDataPtr64) { + Flag =3D TRUE; + break; } + } + if (Flag) { + Index++; + continue; + } =20 - // - // Cache BlockList which has been processed - // - DescriptorBuffer[ValidIndex++] =3D CapsuleDataPtr64; - Index ++; + // + // The TempList is full, enlarge it + // + if ((ValidIndex + 1) >=3D TempListLength) { + EnlargedTempList =3D AllocateZeroPool (TempListLength * 2); + CopyMem (EnlargedTempList, TempList, TempListLength); + FreePool (TempList); + TempList =3D EnlargedTempList; + TempListLength *=3D 2; } + + // + // Cache BlockList which has been processed + // + TempList[ValidIndex++] =3D CapsuleDataPtr64; + Index++; + } + + if (ValidIndex =3D=3D 0) { + DEBUG((DEBUG_ERROR, "%a didn't find any SG lists in variables\n", __FU= NCTION__)); + return EFI_NOT_FOUND; } =20 + *HeadList =3D AllocateZeroPool ((ValidIndex + 1) * sizeof (EFI_PHYSICAL_= ADDRESS)); + if (*HeadList =3D=3D NULL) { + DEBUG((DEBUG_ERROR, "Failed to allocate memory\n")); + return EFI_OUT_OF_RESOURCES; + } + + CopyMem (*HeadList, TempList, (ValidIndex) * sizeof (EFI_PHYSICAL_ADDRES= S)); + *ListLength =3D ValidIndex;=20 + return EFI_SUCCESS; } =20 @@ -937,17 +1027,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 +1039,8 @@ CapsuleCoalesce ( EFI_CAPSULE_LONG_MODE_BUFFER LongModeBuffer; #endif =20 - Index =3D 0; - VariableCount =3D 0; - CapsuleVarName[0] =3D 0; - CapsuleDataPtr64 =3D 0; + ListLength =3D 0; + VariableArrayAddress =3D NULL; =20 // // Someone should have already ascertained the boot mode. If it's not @@ -972,74 +1054,11 @@ CapsuleCoalesce ( } =20 // - // User may set the same ScatterGatherList with several different variab= les, - // so cache all ScatterGatherList for check later. - // - Status =3D PeiServicesLocatePpi ( - &gEfiPeiReadOnlyVariable2PpiGuid, - 0, - NULL, - (VOID **) &PPIVariableServices - ); - if (EFI_ERROR (Status)) { - goto Done; - } - Size =3D sizeof (CapsuleDataPtr64); - StrCpyS (CapsuleVarName, sizeof(CapsuleVarName)/sizeof(CHAR16), EFI_CAPS= ULE_VARIABLE_NAME); - TempVarName =3D CapsuleVarName + StrLen (CapsuleVarName); - while (TRUE) { - if (Index > 0) { - UnicodeValueToStringS ( - TempVarName, - sizeof (CapsuleVarName) - ((UINTN)TempVarName - (UINTN)CapsuleVarN= ame), - 0, - Index, - 0 - ); - } - Status =3D 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 =3D %d\n", Index)); - break; - } - VariableCount++; - Index++; - } - - DEBUG ((DEBUG_INFO,"Capsule variable count =3D %d\n", VariableCount)); - - // - // The last entry is the end flag. - // - Status =3D PeiServicesAllocatePool ( - (VariableCount + 1) * sizeof (EFI_PHYSICAL_ADDRESS), - (VOID **)&VariableArrayAddress - ); - - if (Status !=3D EFI_SUCCESS) { - DEBUG ((DEBUG_ERROR, "AllocatePages Failed!, Status =3D %x\n", Status)= ); - goto Done; - } - - ZeroMem (VariableArrayAddress, (VariableCount + 1) * sizeof (EFI_PHYSICA= L_ADDRESS)); - - // - // Find out if we actually have a capsule. - // GetCapsuleDescriptors depends on variable PPI, so it should run in 32= -bit environment. + // Get ScatterGatherList // - Status =3D GetCapsuleDescriptors (VariableArrayAddress); + Status =3D GetScatterGatherHeadEntries (&ListLength, &VariableArrayAddre= ss); if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "Fail to find capsule variables.\n")); + DEBUG ((DEBUG_ERROR, "%a failed to get Scatter Gather List Head Entrie= s. Status =3D %r\n", __FUNCTION__, Status)); goto Done; } =20 @@ -1117,9 +1136,11 @@ CheckCapsuleUpdate ( IN EFI_PEI_SERVICES **PeiServices ) { - EFI_STATUS Status; - Status =3D 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 patter= n. --=20 2.21.0.windows.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#41702): https://edk2.groups.io/g/devel/message/41702 Mute This Topic: https://groups.io/mt/31875980/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-