1. Patchew
  2. EDK2
  3. View series

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

Jeff Fan posted 1 patch 7 years, 8 months ago
Failed in applying to current master (apply log)
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired
Posted by Jeff Fan 7 years, 8 months ago 
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
-- 
2.9.3.windows.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired
Posted by Wu, Hao A 7 years, 8 months ago 
Reviewed-by: Hao Wu <hao.a.wu@intel.com>


Best Regards,
Hao Wu


> -----Original Message-----
> From: Fan, Jeff
> Sent: Tuesday, April 18, 2017 10:16 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A; Tian, Feng; Kinney, Michael D
> Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired
> 
> 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
> --
> 2.9.3.windows.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired
Posted by Laszlo Ersek 7 years, 8 months ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired
Posted by Fan, Jeff 7 years, 8 months ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Lock should be acquired
Posted by Laszlo Ersek 7 years, 8 months ago 
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
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.