Re: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired

["Fan, Jeff" <jeff.fan@intel.com>]

Laszlo,

There is no any real issue we encountered.

Some static code check tool reported AcquireSpinLockOrFai() return value was not been checked.
Then I found we may ignore some issue if AcquireSpinLockOrFai() return FALSE (even it will not be happened).

Using AcquireSpinLock() is due to the following code are using AcquireSpinLock() to check AP's BUSY state also.

Thanks!
Jeff

-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Monday, April 24, 2017 7:41 PM
To: Fan, Jeff; edk2-devel@lists.01.org
Cc: Wu, Hao A; Kinney, Michael D; Tian, Feng
Subject: Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired

Hi Jeff,

On 04/18/17 04:16, Jeff Fan wrote:
> SMM BSP's *busy* state should be acquired. We could use 
> AcquireSpinLock() instead of AcquireSpinLockOrFail().
> 
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff.fan@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index a1d16b4..e03f1e0 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -407,7 +407,7 @@ BSPHandler (
>    //
>    // The BUSY lock is initialized to Acquired state
>    //
> -  AcquireSpinLockOrFail (mSmmMpSyncData->CpuData[CpuIndex].Busy);
> +  AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
>  
>    //
>    // Perform the pre tasks
> 

what symptoms did you experience without the fix?

Thanks
Laszlo