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

[Laszlo Ersek <lersek@redhat.com>]

On 04/25/17 02:54, Fan, Jeff wrote:
> 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 for the explanation!
Laszlo

> 
> -----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
>