Re: [Patch v3 2/3] UefiCpuPkg/MpInitLib: Remove StartCount and volatile definition.

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

Thanks/Ray

> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Eric Dong
> Sent: Wednesday, July 25, 2018 3:50 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [edk2] [Patch v3 2/3] UefiCpuPkg/MpInitLib: Remove StartCount and
> volatile definition.
> 
> The StartCount is duplicated with RunningCount, replace it with RunningCount.
> Also the volatile for RunningCount is not needed.
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 11 +++++------
> UefiCpuPkg/Library/MpInitLib/MpLib.h |  3 +--
>  2 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index ff09a0e9e7..0e57cc86bf 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1424,7 +1424,7 @@ CheckAllAPs (
>      // value of state after setting the it to CpuStateIdle, so BSP can safely make
> use of its value.
>      //
>      if (GetApState(CpuData) == CpuStateIdle) {
> -      CpuMpData->RunningCount ++;
> +      CpuMpData->RunningCount --;
>        CpuMpData->CpuData[ProcessorNumber].Waiting = FALSE;
> 
>        //
> @@ -1449,7 +1449,7 @@ CheckAllAPs (
>    //
>    // If all APs finish, return EFI_SUCCESS.
>    //
> -  if (CpuMpData->RunningCount == CpuMpData->StartCount) {
> +  if (CpuMpData->RunningCount == 0) {
>      return EFI_SUCCESS;
>    }
> 
> @@ -1466,7 +1466,7 @@ CheckAllAPs (
>      //
>      if (CpuMpData->FailedCpuList != NULL) {
>        *CpuMpData->FailedCpuList =
> -         AllocatePool ((CpuMpData->StartCount - CpuMpData->FinishedCount + 1)
> * sizeof (UINTN));
> +         AllocatePool ((CpuMpData->RunningCount + 1) * sizeof (UINTN));
>        ASSERT (*CpuMpData->FailedCpuList != NULL);
>      }
>      ListIndex = 0;
> @@ -2212,7 +2212,7 @@ StartupAllAPsWorker (
>      return EFI_NOT_STARTED;
>    }
> 
> -  CpuMpData->StartCount = 0;
> +  CpuMpData->RunningCount = 0;
>    for (ProcessorNumber = 0; ProcessorNumber < ProcessorCount;
> ProcessorNumber++) {
>      CpuData = &CpuMpData->CpuData[ProcessorNumber];
>      CpuData->Waiting = FALSE;
> @@ -2222,7 +2222,7 @@ StartupAllAPsWorker (
>          // Mark this processor as responsible for current calling.
>          //
>          CpuData->Waiting = TRUE;
> -        CpuMpData->StartCount++;
> +        CpuMpData->RunningCount++;
>        }
>      }
>    }
> @@ -2231,7 +2231,6 @@ StartupAllAPsWorker (
>    CpuMpData->ProcArguments = ProcedureArgument;
>    CpuMpData->SingleThread  = SingleThread;
>    CpuMpData->FinishedCount = 0;
> -  CpuMpData->RunningCount  = 0;
>    CpuMpData->FailedCpuList = FailedCpuList;
>    CpuMpData->ExpectedTime  = CalculateTimeout (
>                                 TimeoutInMicroseconds, diff --git
> a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 962bce685d..5002b7e9c0 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -211,9 +211,8 @@ struct _CPU_MP_DATA {
>    UINTN                          BackupBuffer;
>    UINTN                          BackupBufferSize;
> 
> -  volatile UINT32                StartCount;
>    volatile UINT32                FinishedCount;
> -  volatile UINT32                RunningCount;
> +  UINT32                         RunningCount;
>    BOOLEAN                        SingleThread;
>    EFI_AP_PROCEDURE               Procedure;
>    VOID                           *ProcArguments;
> --
> 2.15.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel