[edk2-devel] [PATCH v3] UefiCpuPkg: Fix potential spinLock issue in SmmStartupThisAp

[Ni, Ray]

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

> -----Original Message-----
> From: Nikodem, Damian
> Sent: Wednesday, September 11, 2019 1:23 AM
> To: devel at edk2.groups.io
> Cc: Nikodem, Damian <damian.nikodem at intel.com>; Dong, Eric <eric.dong at intel.com>; Ni, Ray <ray.ni at intel.com>; You,
> Benjamin <benjamin.you at intel.com>; Laszlo Ersek <lersek at redhat.com>; Rusocki, Krzysztof <krzysztof.rusocki at 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 at intel.com>
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Benjamin You <benjamin.you at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Krzysztof Rusocki <krzysztof.rusocki at 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 ();
>    }

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47134): https://edk2.groups.io/g/devel/message/47134
Mute This Topic: https://groups.io/mt/34101181/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-