From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "Gao, Zhichao" <zhichao.gao@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>,
"Wang, Jian J" <jian.j.wang@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>
Subject: Re: [PATCH] MdeModulePkg/CapsulePei: Optimize the CapsulePei
Date: Thu, 30 May 2019 05:29:24 +0000 [thread overview]
Message-ID: <B80AF82E9BFB8E4FBD8C89DA810C6A093C8DFC78@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <3CE959C139B4C44DBEA1810E3AA6F9000B7DFA91@SHSMSX101.ccr.corp.intel.com>
> -----Original Message-----
> From: Gao, Zhichao
> Sent: Thursday, May 30, 2019 11:21 AM
> To: Wu, Hao A; devel@edk2.groups.io
> Cc: Bret Barkelew; Wang, Jian J; Ni, Ray; Zeng, Star; Gao, Liming; Sean Brogan;
> Michael Turner
> Subject: RE: [PATCH] MdeModulePkg/CapsulePei: Optimize the CapsulePei
>
>
>
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Wednesday, May 29, 2019 2:55 PM
> > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > <jian.j.wang@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>
> > Subject: RE: [PATCH] MdeModulePkg/CapsulePei: Optimize the CapsulePei
> >
> > > -----Original Message-----
> > > From: Gao, Zhichao
> > > Sent: Wednesday, May 29, 2019 8:46 AM
> > > To: devel@edk2.groups.io
> > > Cc: Bret Barkelew; Wang, Jian J; Wu, Hao A; Ni, Ray; Zeng, Star; Gao,
> > > Liming; Sean Brogan; Michael Turner; Gao, Zhichao
> > > Subject: [PATCH] MdeModulePkg/CapsulePei: Optimize the CapsulePei
> > >
> > > From: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1853
> > >
> > > Sperate the capsule check function from GetCapsuleDescriptors
> >
> > Sperate -> Separate
> >
> > > 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(-)
> >
> > I am a bit confused for the purpose of this patch.
> >
> > My understanding is that this patch will refine this driver to remove
> > duplicated code by abstract common codes into a new function. And there
> > will be no functional impact.
> >
> > However, after the change, the line of codes of this driver increased by
> > 20+ lines.
> >
> > Did I miss something for the purpose of this patch?
>
> The commit message should be update:
> I view the code change again, here is the purpose of this patch:
> 1. separate the check function from GetCapsuleDescriptors. The original logic
> GetCapsuleDescriptors in is unclear.
Understood, thanks for the clarification.
> 2. avoid calling query capsule variable twice, first time to get the SG list
> number and allocate buffer to save it, second time to copy the SG list to the
> buffer.
> After the patch it would put the SG list data into a template buffer and count
> the number. Then allocate the memory and copy data.
Agree.
Best Regards,
Hao Wu
>
> I would update the above info to next patch. And remove the incorrect
> description.
>
> >
> > Some additional comments below.
> >
> > >
> > > 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 (
> >
> > Keyword 'EFIAPI' seems not needed here.
> > AreCapsulesStaged() is an internal function here.
> >
> > > + 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 (
> >
> > Keyword 'EFIAPI' seems not needed here.
> > GetScatterGatherHeadEntries() is an internal function here.
> >
> > > + 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))),
> >
> > Compared with the origin code:
> > '''
> > sizeof (CapsuleVarName) - ((UINTN)TempVarName -
> > (UINTN)CapsuleVarName), '''
> >
> > The size of buffer passed into function UnicodeValueToStringS() is 2 bytes
> > smaller for the modified code.
> >
> > Is there a reason for such change?
>
> Just my thought. This can remain two byte to be zero as the null-terminate of
> the string. The original one may override the last two byte and the buffer can
> not be regard as a legal string.
>
> Thanks,
> Zhichao
>
> >
> > Best Regards,
> > Hao Wu
> >
> > > + 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
next prev parent reply other threads:[~2019-05-30 5:29 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-29 0:45 [PATCH] MdeModulePkg/CapsulePei: Optimize the CapsulePei Gao, Zhichao
2019-05-29 6:54 ` Wu, Hao A
2019-05-30 3:20 ` Gao, Zhichao
2019-05-30 5:29 ` Wu, Hao A [this message]
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=B80AF82E9BFB8E4FBD8C89DA810C6A093C8DFC78@SHSMSX104.ccr.corp.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