[edk2-devel] [PATCH v4 2/2] MdeMoudlePkg/CapsulePei: Substantial change on UefiCapsule.c
Leif Lindholm
leif.lindholm at linaro.org
Thu Jun 6 11:18:12 UTC 2019
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 at intel.com>
> Cc: Hao A Wu <hao.a.wu at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Star Zeng <star.zeng at intel.com>
> Cc: Liming Gao <liming.gao at intel.com>
> Cc: Sean Brogan <sean.brogan at microsoft.com>
> Cc: Michael Turner <Michael.Turner at microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew at microsoft.com>
> Cc: Leif Lindholm <leif.lindholm at linaro.org>
> Signed-off-by: Zhichao gao <zhichao.gao at 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
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#41999): https://edk2.groups.io/g/devel/message/41999
Mute This Topic: https://groups.io/mt/31932897/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list