public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex first before lock cmpxchg
@ 2024-02-02  7:13 Wu, Jiaxin
  0 siblings, 0 replies; 4+ messages in thread
From: Wu, Jiaxin @ 2024-02-02  7:13 UTC (permalink / raw)
  To: devel
  Cc: Ray Ni, Laszlo Ersek, Eric Dong, Zeng Star, Gerd Hoffmann,
	Rahul Kumar, Kinney Michael D

This patch is to check BspIndex first before lock cmpxchg operation.
It's the optimization to lower the resource contention caused by the
atomic compare exchange operation, so as to improve the SMI
performance for BSP election.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Kinney Michael D <michael.d.kinney@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index e988ce0542..085aa91286 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1652,15 +1652,17 @@ SmiRendezvous (
             }
           } else {
             //
             // Platform hook fails to determine, use default BSP election method
             //
-            InterlockedCompareExchange32 (
-              (UINT32 *)&mSmmMpSyncData->BspIndex,
-              (UINT32)-1,
-              (UINT32)CpuIndex
-              );
+            if (mSmmMpSyncData->BspIndex == MAX_UINT32) {
+              InterlockedCompareExchange32 (
+                (UINT32 *)&mSmmMpSyncData->BspIndex,
+                MAX_UINT32,
+                (UINT32)CpuIndex
+                );
+            }
           }
         }
       }
 
       //
-- 
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115026): https://edk2.groups.io/g/devel/message/115026
Mute This Topic: https://groups.io/mt/104115335/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex first before lock cmpxchg
  2024-02-04  8:47 [edk2-devel] [PATCH v2 0/2] SMM CPU Optimization Wu, Jiaxin
@ 2024-02-04  8:47 ` Wu, Jiaxin
  2024-02-05  5:06   ` Ni, Ray
  0 siblings, 1 reply; 4+ messages in thread
From: Wu, Jiaxin @ 2024-02-04  8:47 UTC (permalink / raw)
  To: devel
  Cc: Ray Ni, Laszlo Ersek, Eric Dong, Zeng Star, Gerd Hoffmann,
	Rahul Kumar, Kinney Michael D

This patch is to check BspIndex first before lock cmpxchg operation.
If BspIndex has not been set, then do the lock cmpxchg, otherwise,
the APs don't need to lock cmpxchg the BspIndex value since the BSP
election has been done. It's the optimization to lower the resource
contention caused by the atomic compare exchange operation, so as to
improve the SMI performance for BSP election.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Kinney Michael D <michael.d.kinney@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index e988ce0542..085aa91286 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1652,15 +1652,17 @@ SmiRendezvous (
             }
           } else {
             //
             // Platform hook fails to determine, use default BSP election method
             //
-            InterlockedCompareExchange32 (
-              (UINT32 *)&mSmmMpSyncData->BspIndex,
-              (UINT32)-1,
-              (UINT32)CpuIndex
-              );
+            if (mSmmMpSyncData->BspIndex == MAX_UINT32) {
+              InterlockedCompareExchange32 (
+                (UINT32 *)&mSmmMpSyncData->BspIndex,
+                MAX_UINT32,
+                (UINT32)CpuIndex
+                );
+            }
           }
         }
       }
 
       //
-- 
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115081): https://edk2.groups.io/g/devel/message/115081
Mute This Topic: https://groups.io/mt/104153489/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex first before lock cmpxchg
  2024-02-04  8:47 ` [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex first before lock cmpxchg Wu, Jiaxin
@ 2024-02-05  5:06   ` Ni, Ray
  2024-02-05  7:43     ` Laszlo Ersek
  0 siblings, 1 reply; 4+ messages in thread
From: Ni, Ray @ 2024-02-05  5:06 UTC (permalink / raw)
  To: Wu, Jiaxin, devel@edk2.groups.io
  Cc: Laszlo Ersek, Dong, Eric, Zeng, Star, Gerd Hoffmann,
	Kumar, Rahul R, Kinney, Michael D

Jiaxin,
Can you separate the changes in 2 patches?
1 is to change (UINT32)-1 to MAX_UINT32, the other is to add the test before test-and-set.

Thanks,
Ray
> -----Original Message-----
> From: Wu, Jiaxin <jiaxin.wu@intel.com>
> Sent: Sunday, February 4, 2024 4:48 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Dong,
> Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Gerd
> Hoffmann <kraxel@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex
> first before lock cmpxchg
> 
> This patch is to check BspIndex first before lock cmpxchg operation.
> If BspIndex has not been set, then do the lock cmpxchg, otherwise,
> the APs don't need to lock cmpxchg the BspIndex value since the BSP
> election has been done. It's the optimization to lower the resource
> contention caused by the atomic compare exchange operation, so as to
> improve the SMI performance for BSP election.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Zeng Star <star.zeng@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Kinney Michael D <michael.d.kinney@intel.com>
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index e988ce0542..085aa91286 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -1652,15 +1652,17 @@ SmiRendezvous (
>              }
>            } else {
>              //
>              // Platform hook fails to determine, use default BSP election
> method
>              //
> -            InterlockedCompareExchange32 (
> -              (UINT32 *)&mSmmMpSyncData->BspIndex,
> -              (UINT32)-1,
> -              (UINT32)CpuIndex
> -              );
> +            if (mSmmMpSyncData->BspIndex == MAX_UINT32) {
> +              InterlockedCompareExchange32 (
> +                (UINT32 *)&mSmmMpSyncData->BspIndex,
> +                MAX_UINT32,
> +                (UINT32)CpuIndex
> +                );
> +            }
>            }
>          }
>        }
> 
>        //
> --
> 2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115094): https://edk2.groups.io/g/devel/message/115094
Mute This Topic: https://groups.io/mt/104153489/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex first before lock cmpxchg
  2024-02-05  5:06   ` Ni, Ray
@ 2024-02-05  7:43     ` Laszlo Ersek
  0 siblings, 0 replies; 4+ messages in thread
From: Laszlo Ersek @ 2024-02-05  7:43 UTC (permalink / raw)
  To: Ni, Ray, Wu, Jiaxin, devel@edk2.groups.io
  Cc: Dong, Eric, Zeng, Star, Gerd Hoffmann, Kumar, Rahul R,
	Kinney, Michael D

On 2/5/24 06:06, Ni, Ray wrote:
> Jiaxin,
> Can you separate the changes in 2 patches?
> 1 is to change (UINT32)-1 to MAX_UINT32, the other is to add the test before test-and-set.

Thanks; I'll defer to Ray on this series.

Laszlo

> 
> Thanks,
> Ray
>> -----Original Message-----
>> From: Wu, Jiaxin <jiaxin.wu@intel.com>
>> Sent: Sunday, February 4, 2024 4:48 PM
>> To: devel@edk2.groups.io
>> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Dong,
>> Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Gerd
>> Hoffmann <kraxel@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>;
>> Kinney, Michael D <michael.d.kinney@intel.com>
>> Subject: [PATCH v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex
>> first before lock cmpxchg
>>
>> This patch is to check BspIndex first before lock cmpxchg operation.
>> If BspIndex has not been set, then do the lock cmpxchg, otherwise,
>> the APs don't need to lock cmpxchg the BspIndex value since the BSP
>> election has been done. It's the optimization to lower the resource
>> contention caused by the atomic compare exchange operation, so as to
>> improve the SMI performance for BSP election.
>>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Zeng Star <star.zeng@intel.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>> Cc: Kinney Michael D <michael.d.kinney@intel.com>
>> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
>> ---
>>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 12 +++++++-----
>>  1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
>> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
>> index e988ce0542..085aa91286 100644
>> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
>> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
>> @@ -1652,15 +1652,17 @@ SmiRendezvous (
>>              }
>>            } else {
>>              //
>>              // Platform hook fails to determine, use default BSP election
>> method
>>              //
>> -            InterlockedCompareExchange32 (
>> -              (UINT32 *)&mSmmMpSyncData->BspIndex,
>> -              (UINT32)-1,
>> -              (UINT32)CpuIndex
>> -              );
>> +            if (mSmmMpSyncData->BspIndex == MAX_UINT32) {
>> +              InterlockedCompareExchange32 (
>> +                (UINT32 *)&mSmmMpSyncData->BspIndex,
>> +                MAX_UINT32,
>> +                (UINT32)CpuIndex
>> +                );
>> +            }
>>            }
>>          }
>>        }
>>
>>        //
>> --
>> 2.16.2.windows.1
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115099): https://edk2.groups.io/g/devel/message/115099
Mute This Topic: https://groups.io/mt/104153489/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-02-05  7:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-02  7:13 [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex first before lock cmpxchg Wu, Jiaxin
  -- strict thread matches above, loose matches on Subject: below --
2024-02-04  8:47 [edk2-devel] [PATCH v2 0/2] SMM CPU Optimization Wu, Jiaxin
2024-02-04  8:47 ` [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex first before lock cmpxchg Wu, Jiaxin
2024-02-05  5:06   ` Ni, Ray
2024-02-05  7:43     ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox