Re: [PATCH v3] UefiCpuPkg: Fix potential spinLock issue in SmmStartupThisAp

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

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Nikodem, Damian
> Sent: Wednesday, September 11, 2019 1:23 AM
> To: devel@edk2.groups.io
> Cc: Nikodem, Damian <damian.nikodem@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; You,
> Benjamin <benjamin.you@intel.com>; Laszlo Ersek <lersek@redhat.com>; Rusocki, Krzysztof <krzysztof.rusocki@intel.com>
> Subject: [PATCH v3] UefiCpuPkg: Fix potential spinLock issue in SmmStartupThisAp
> 
> Due to needs a tackling the deficiency of the AP API, is necessary to ensure that in non-blocking mode
> previous AP executed command is finished before start new one.
> 
> To remedy above:
>   1) execute AcquireSpinLock instead AcquireSpinLockOrFail - this will ensure time "window" to eliminate potential
>      race condition beetwen BSP and AP spinLock release in non-blocking mode.
>      This also will eliminate possibility to start executing new AP function before last is finished.
>   2) remove returns EFI_STATUS - EFI_NOT_READY - in new scenario returned status is not necessary to caller.
> 
> Signed-off-by: Damian Nikodem <damian.nikodem@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Benjamin You <benjamin.you@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Krzysztof Rusocki <krzysztof.rusocki@intel.com>
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index d8d2b6f444..0685637c2b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -1235,14 +1235,9 @@ InternalSmmStartupThisAp (
>      return EFI_INVALID_PARAMETER;
>    }
> 
> -  if (Token == NULL) {
> -    AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
> -  } else {
> -    if (!AcquireSpinLockOrFail (mSmmMpSyncData->CpuData[CpuIndex].Busy)) {
> -      DEBUG((DEBUG_ERROR, "Can't acquire mSmmMpSyncData->CpuData[%d].Busy\n", CpuIndex));
> -      return EFI_NOT_READY;
> -    }
> +  AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
> 
> +  if (Token != NULL) {
>      *Token = (MM_COMPLETION) CreateToken ();
>    }