Re: [edk2-devel] [PATCH v2 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Remove dependence between APs

["Ni, Ray" <ray.ni@intel.com>]

> 
> +  WaitForSemaphore (&Token->RunningApCount); 
> + 
> +  if (Token->RunningApCount == 0) {
> +    ReleaseSpinLock (Token->SpinLock); 
>    }

1. if (InterlockedDecrement (&Token->RunningApCount) == 0) {
      ReleaseSpinLock (Token->SpinLock);
    }

We should avoid checking RunningApCount directly because it's possible
that AP#1 decrease the Count to 1 and before AP#1 checks the value against 0
the Count is decreased by AP#2 to 0. So that causes AP#1 and AP#2 call
ReleaseSpinLock() on the same SpinLock.

> 
> +      // Decrease the count to mark this AP as finished.

2. BSP is also handled here. So this comment is mis-leading.

> 
> +      //
> 
> +      if (Token != NULL) { 
> +        WaitForSemaphore (&ProcToken->RunningApCount);

3. The code is written correctly but improperly IMO.
Token is checked but ProcToken is deferenced.
I suggest you check ProcToken directly.