* [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; 8+ 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] 8+ messages in thread
* [edk2-devel] [PATCH v2 0/2] SMM CPU Optimization
@ 2024-02-04 8:47 Wu, Jiaxin
2024-02-04 8:47 ` [edk2-devel] [PATCH v2 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Execute CET and XD check only on BSP Wu, Jiaxin
2024-02-04 8:47 ` [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex first before lock cmpxchg Wu, Jiaxin
0 siblings, 2 replies; 8+ messages in thread
From: Wu, Jiaxin @ 2024-02-04 8:47 UTC (permalink / raw)
To: devel
Refine the commit log.
Jiaxin Wu (2):
UefiCpuPkg/PiSmmCpuDxeSmm: Execute CET and XD check only on BSP
UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex first before lock cmpxchg
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 12 +++--
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 6 +--
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 78 +++++++++++++++++-------------
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 6 ++-
4 files changed, 59 insertions(+), 43 deletions(-)
--
2.16.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115079): https://edk2.groups.io/g/devel/message/115079
Mute This Topic: https://groups.io/mt/104153487/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 8+ messages in thread
* [edk2-devel] [PATCH v2 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Execute CET and XD check only on BSP
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 16:59 ` Brian J. Johnson
2024-02-04 8:47 ` [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex first before lock cmpxchg Wu, Jiaxin
1 sibling, 1 reply; 8+ 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
Existing CheckFeatureSupported function will check CET & XD
features on each processor.
The CPUIDs for CET & XD features are software visible domain,
which means a properly configured platform will have consistent
values for these CPUID Leafs/SubLeafs/Fields on each logical
processor. So, execute Execute CET and XD check only on BSP.This
belongs to the optimization, which can avoid unnecessary and
duplicate check on multiple processors.
Note 1: we don't split those functionality check out of the
SmmInitHandler. Because the CET & XD & BTS features might be
different in non-smm & smm environment. Keep in SMM is more safer.
As for MSR_IA32_MISC_ENABLE.BTS, it's core scope according SDM.
So, still keep it check on each processor.
Note 2: For BTS, using global variable to indicate feature
supported or not (mBtsSupported), the function only performs
variable modification from TRUE to FALSE. So even the code runs
in parallel, it's safe.
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>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@Intel.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 6 +--
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 78 +++++++++++++++++-------------
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 6 ++-
3 files changed, 52 insertions(+), 38 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index cd394826ff..15d26dd88f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -1,9 +1,9 @@
/** @file
Agent Module to load other modules to deploy SMM Entry Vector for X86 CPU.
-Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2024, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -375,13 +375,13 @@ SmmInitHandler (
&mCpuHotPlugData
);
if (!mSmmS3Flag) {
//
- // Check XD and BTS features on each processor on normal boot
+ // Check CET & XD & BTS features on each processor on normal boot
//
- CheckFeatureSupported ();
+ CheckFeatureSupported (IsBsp);
} else if (IsBsp) {
//
// BSP rebase is already done above.
// Initialize private data during S3 resume
//
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 8142d3ceac..44c352ad98 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -1,9 +1,9 @@
/** @file
Enable SMM profile.
-Copyright (c) 2012 - 2023, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2012 - 2024, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -892,62 +892,74 @@ InitSmmProfileInternal (
}
/**
Check if feature is supported by a processor.
+ @param[in] IsBsp Indicate it's called by BSP or not.
+
**/
VOID
CheckFeatureSupported (
- VOID
+ IN BOOLEAN IsBsp
)
{
UINT32 RegEax;
UINT32 RegEcx;
UINT32 RegEdx;
MSR_IA32_MISC_ENABLE_REGISTER MiscEnableMsr;
- if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) && mCetSupported) {
- AsmCpuid (CPUID_SIGNATURE, &RegEax, NULL, NULL, NULL);
- if (RegEax >= CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS) {
- AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_SUB_LEAF_INFO, NULL, NULL, &RegEcx, NULL);
- if ((RegEcx & CPUID_CET_SS) == 0) {
+ //
+ // The feature scope is software visible domain.
+ // Only need check on BSP.
+ //
+ if (IsBsp) {
+ if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) && mCetSupported) {
+ AsmCpuid (CPUID_SIGNATURE, &RegEax, NULL, NULL, NULL);
+ if (RegEax >= CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS) {
+ AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_SUB_LEAF_INFO, NULL, NULL, &RegEcx, NULL);
+ if ((RegEcx & CPUID_CET_SS) == 0) {
+ mCetSupported = FALSE;
+ PatchInstructionX86 (mPatchCetSupported, mCetSupported, 1);
+ }
+ } else {
mCetSupported = FALSE;
PatchInstructionX86 (mPatchCetSupported, mCetSupported, 1);
}
- } else {
- mCetSupported = FALSE;
- PatchInstructionX86 (mPatchCetSupported, mCetSupported, 1);
}
- }
- if (mXdSupported) {
- AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
- if (RegEax <= CPUID_EXTENDED_FUNCTION) {
- //
- // Extended CPUID functions are not supported on this processor.
- //
- mXdSupported = FALSE;
- PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
- }
+ if (mXdSupported) {
+ AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
+ if (RegEax <= CPUID_EXTENDED_FUNCTION) {
+ //
+ // Extended CPUID functions are not supported on this processor.
+ //
+ mXdSupported = FALSE;
+ PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
+ }
- AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx);
- if ((RegEdx & CPUID1_EDX_XD_SUPPORT) == 0) {
- //
- // Execute Disable Bit feature is not supported on this processor.
- //
- mXdSupported = FALSE;
- PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
- }
+ AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx);
+ if ((RegEdx & CPUID1_EDX_XD_SUPPORT) == 0) {
+ //
+ // Execute Disable Bit feature is not supported on this processor.
+ //
+ mXdSupported = FALSE;
+ PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
+ }
- if (StandardSignatureIsAuthenticAMD ()) {
- //
- // AMD processors do not support MSR_IA32_MISC_ENABLE
- //
- PatchInstructionX86 (gPatchMsrIa32MiscEnableSupported, FALSE, 1);
+ if (StandardSignatureIsAuthenticAMD ()) {
+ //
+ // AMD processors do not support MSR_IA32_MISC_ENABLE
+ //
+ PatchInstructionX86 (gPatchMsrIa32MiscEnableSupported, FALSE, 1);
+ }
}
}
+ //
+ // The feature scope is core.
+ // Need check on each processor.
+ //
if (mBtsSupported) {
AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, NULL, &RegEdx);
if ((RegEdx & CPUID1_EDX_BTS_AVAILABLE) != 0) {
//
// Per IA32 manuals:
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
index 1a82ac05ce..02554a9983 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
@@ -1,9 +1,9 @@
/** @file
SMM profile header file.
-Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2012 - 2024, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#ifndef _SMM_PROFILE_H_
@@ -81,14 +81,16 @@ PageFaultIdtHandlerSmmProfile (
);
/**
Check if feature is supported by a processor.
+ @param[in] IsBsp Indicate it's called by BSP or not.
+
**/
VOID
CheckFeatureSupported (
- VOID
+ IN BOOLEAN IsBsp
);
/**
Update page table according to protected memory ranges and the 4KB-page mapped memory ranges.
--
2.16.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115080): https://edk2.groups.io/g/devel/message/115080
Mute This Topic: https://groups.io/mt/104153488/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] 8+ 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 ` [edk2-devel] [PATCH v2 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Execute CET and XD check only on BSP Wu, Jiaxin
@ 2024-02-04 8:47 ` Wu, Jiaxin
2024-02-05 5:06 ` Ni, Ray
1 sibling, 1 reply; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Execute CET and XD check only on BSP
2024-02-04 8:47 ` [edk2-devel] [PATCH v2 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Execute CET and XD check only on BSP Wu, Jiaxin
@ 2024-02-05 16:59 ` Brian J. Johnson
2024-02-06 7:00 ` Wu, Jiaxin
0 siblings, 1 reply; 8+ messages in thread
From: Brian J. Johnson @ 2024-02-05 16:59 UTC (permalink / raw)
To: devel, jiaxin.wu
Cc: Ray Ni, Laszlo Ersek, Eric Dong, Zeng Star, Gerd Hoffmann,
Rahul Kumar
As others have pointed out, setting BOOLEAN values from TRUE to FALSE on
multiple CPUs at once "should" be safe, in that the final values of the
variables are the same no matter what order the CPUs write the data.
Now that only the BSP is writing these values, what guarantees that they
are written before the APs need to read them? The commit message didn't
mention it, and I haven't traced through the SMM initialization flow.
Brian J. Johnson
-------- Original Message --------
From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
Sent: Sunday, February 4, 2024 at 2:47 AM
To: devel@edk2.groups.io
Cc: Ray Ni <ray.ni@intel.com>, Laszlo Ersek <lersek@redhat.com>, Eric
Dong <eric.dong@intel.com>, Zeng Star <star.zeng@intel.com>, Gerd
Hoffmann <kraxel@redhat.com>, Rahul Kumar <rahul1.kumar@intel.com>
Subject: [edk2-devel] [PATCH v2 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Execute
CET and XD check only on BSP
Existing CheckFeatureSupported function will check CET & XD
features on each processor.
The CPUIDs for CET & XD features are software visible domain,
which means a properly configured platform will have consistent
values for these CPUID Leafs/SubLeafs/Fields on each logical
processor. So, execute Execute CET and XD check only on BSP.This
belongs to the optimization, which can avoid unnecessary and
duplicate check on multiple processors.
Note 1: we don't split those functionality check out of the
SmmInitHandler. Because the CET & XD & BTS features might be
different in non-smm & smm environment. Keep in SMM is more safer.
As for MSR_IA32_MISC_ENABLE.BTS, it's core scope according SDM.
So, still keep it check on each processor.
Note 2: For BTS, using global variable to indicate feature
supported or not (mBtsSupported), the function only performs
variable modification from TRUE to FALSE. So even the code runs
in parallel, it's safe.
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>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@Intel.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 6 +--
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 78
+++++++++++++++++-------------
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 6 ++-
3 files changed, 52 insertions(+), 38 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index cd394826ff..15d26dd88f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -1,9 +1,9 @@
/** @file
Agent Module to load other modules to deploy SMM Entry Vector for X86
CPU.
-Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2024, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -375,13 +375,13 @@ SmmInitHandler (
&mCpuHotPlugData
);
if (!mSmmS3Flag) {
//
- // Check XD and BTS features on each processor on normal boot
+ // Check CET & XD & BTS features on each processor on normal boot
//
- CheckFeatureSupported ();
+ CheckFeatureSupported (IsBsp);
} else if (IsBsp) {
//
// BSP rebase is already done above.
// Initialize private data during S3 resume
//
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 8142d3ceac..44c352ad98 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -1,9 +1,9 @@
/** @file
Enable SMM profile.
-Copyright (c) 2012 - 2023, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2012 - 2024, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -892,62 +892,74 @@ InitSmmProfileInternal (
}
/**
Check if feature is supported by a processor.
+ @param[in] IsBsp Indicate it's called by BSP or not.
+
**/
VOID
CheckFeatureSupported (
- VOID
+ IN BOOLEAN IsBsp
)
{
UINT32 RegEax;
UINT32 RegEcx;
UINT32 RegEdx;
MSR_IA32_MISC_ENABLE_REGISTER MiscEnableMsr;
- if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) &&
mCetSupported) {
- AsmCpuid (CPUID_SIGNATURE, &RegEax, NULL, NULL, NULL);
- if (RegEax >= CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS) {
- AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS,
CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_SUB_LEAF_INFO, NULL, NULL,
&RegEcx, NULL);
- if ((RegEcx & CPUID_CET_SS) == 0) {
+ //
+ // The feature scope is software visible domain.
+ // Only need check on BSP.
+ //
+ if (IsBsp) {
+ if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) &&
mCetSupported) {
+ AsmCpuid (CPUID_SIGNATURE, &RegEax, NULL, NULL, NULL);
+ if (RegEax >= CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS) {
+ AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS,
CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_SUB_LEAF_INFO, NULL, NULL,
&RegEcx, NULL);
+ if ((RegEcx & CPUID_CET_SS) == 0) {
+ mCetSupported = FALSE;
+ PatchInstructionX86 (mPatchCetSupported, mCetSupported, 1);
+ }
+ } else {
mCetSupported = FALSE;
PatchInstructionX86 (mPatchCetSupported, mCetSupported, 1);
}
- } else {
- mCetSupported = FALSE;
- PatchInstructionX86 (mPatchCetSupported, mCetSupported, 1);
}
- }
- if (mXdSupported) {
- AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
- if (RegEax <= CPUID_EXTENDED_FUNCTION) {
- //
- // Extended CPUID functions are not supported on this processor.
- //
- mXdSupported = FALSE;
- PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
- }
+ if (mXdSupported) {
+ AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
+ if (RegEax <= CPUID_EXTENDED_FUNCTION) {
+ //
+ // Extended CPUID functions are not supported on this processor.
+ //
+ mXdSupported = FALSE;
+ PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
+ }
- AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx);
- if ((RegEdx & CPUID1_EDX_XD_SUPPORT) == 0) {
- //
- // Execute Disable Bit feature is not supported on this processor.
- //
- mXdSupported = FALSE;
- PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
- }
+ AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx);
+ if ((RegEdx & CPUID1_EDX_XD_SUPPORT) == 0) {
+ //
+ // Execute Disable Bit feature is not supported on this processor.
+ //
+ mXdSupported = FALSE;
+ PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
+ }
- if (StandardSignatureIsAuthenticAMD ()) {
- //
- // AMD processors do not support MSR_IA32_MISC_ENABLE
- //
- PatchInstructionX86 (gPatchMsrIa32MiscEnableSupported, FALSE, 1);
+ if (StandardSignatureIsAuthenticAMD ()) {
+ //
+ // AMD processors do not support MSR_IA32_MISC_ENABLE
+ //
+ PatchInstructionX86 (gPatchMsrIa32MiscEnableSupported, FALSE, 1);
+ }
}
}
+ //
+ // The feature scope is core.
+ // Need check on each processor.
+ //
if (mBtsSupported) {
AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, NULL, &RegEdx);
if ((RegEdx & CPUID1_EDX_BTS_AVAILABLE) != 0) {
//
// Per IA32 manuals:
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
index 1a82ac05ce..02554a9983 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
@@ -1,9 +1,9 @@
/** @file
SMM profile header file.
-Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2012 - 2024, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#ifndef _SMM_PROFILE_H_
@@ -81,14 +81,16 @@ PageFaultIdtHandlerSmmProfile (
);
/**
Check if feature is supported by a processor.
+ @param[in] IsBsp Indicate it's called by BSP or not.
+
**/
VOID
CheckFeatureSupported (
- VOID
+ IN BOOLEAN IsBsp
);
/**
Update page table according to protected memory ranges and the
4KB-page mapped memory ranges.
--
Brian J. Johnson
Enterprise X86 Lab
Hewlett Packard Enterprise
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115122): https://edk2.groups.io/g/devel/message/115122
Mute This Topic: https://groups.io/mt/104153488/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] 8+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Execute CET and XD check only on BSP
2024-02-05 16:59 ` Brian J. Johnson
@ 2024-02-06 7:00 ` Wu, Jiaxin
0 siblings, 0 replies; 8+ messages in thread
From: Wu, Jiaxin @ 2024-02-06 7:00 UTC (permalink / raw)
To: Johnson, Brian, devel@edk2.groups.io
Cc: Ni, Ray, Laszlo Ersek, Dong, Eric, Zeng, Star, Gerd Hoffmann,
Kumar, Rahul R
Hi Brian,
You raised a good open here:
Can we guarantee all features value updated before use them? Actually, it's not related to my patch here, I'm thinking about those features flag usage *even before* executing CheckFeatureSupported().
mCetSupported & mPatchCetSupported,
mXdSupported & gPatchXdSupported,
mBtsSupported
After investigation, the answer is no, that means existing work flow has the potential problem using those features flag before calling CheckFeatureSupported. The easy way to fix it is to move those features flag check in early driver entry point. But before that, we need confirm the CET & XD & BTS features are same in non-smm & smm environment. After that, I will create another patch to do this movement, then we can totally resolve this problem.
So, currently, let's ignore this patch.
Thanks,
Jiaxin
> -----Original Message-----
> From: Brian J. Johnson <brian.johnson@hpe.com>
> Sent: Tuesday, February 6, 2024 1:00 AM
> To: devel@edk2.groups.io; Wu, Jiaxin <jiaxin.wu@intel.com>
> 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>
> Subject: Re: [edk2-devel] [PATCH v2 1/2] UefiCpuPkg/PiSmmCpuDxeSmm:
> Execute CET and XD check only on BSP
>
> As others have pointed out, setting BOOLEAN values from TRUE to FALSE on
> multiple CPUs at once "should" be safe, in that the final values of the
> variables are the same no matter what order the CPUs write the data.
>
> Now that only the BSP is writing these values, what guarantees that they
> are written before the APs need to read them? The commit message didn't
> mention it, and I haven't traced through the SMM initialization flow.
>
> Brian J. Johnson
>
> -------- Original Message --------
> From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> Sent: Sunday, February 4, 2024 at 2:47 AM
> To: devel@edk2.groups.io
> Cc: Ray Ni <ray.ni@intel.com>, Laszlo Ersek <lersek@redhat.com>, Eric
> Dong <eric.dong@intel.com>, Zeng Star <star.zeng@intel.com>, Gerd
> Hoffmann <kraxel@redhat.com>, Rahul Kumar <rahul1.kumar@intel.com>
> Subject: [edk2-devel] [PATCH v2 1/2] UefiCpuPkg/PiSmmCpuDxeSmm:
> Execute
> CET and XD check only on BSP
>
> Existing CheckFeatureSupported function will check CET & XD
> features on each processor.
>
> The CPUIDs for CET & XD features are software visible domain,
> which means a properly configured platform will have consistent
> values for these CPUID Leafs/SubLeafs/Fields on each logical
> processor. So, execute Execute CET and XD check only on BSP.This
> belongs to the optimization, which can avoid unnecessary and
> duplicate check on multiple processors.
>
> Note 1: we don't split those functionality check out of the
> SmmInitHandler. Because the CET & XD & BTS features might be
> different in non-smm & smm environment. Keep in SMM is more safer.
>
> As for MSR_IA32_MISC_ENABLE.BTS, it's core scope according SDM.
> So, still keep it check on each processor.
>
> Note 2: For BTS, using global variable to indicate feature
> supported or not (mBtsSupported), the function only performs
> variable modification from TRUE to FALSE. So even the code runs
> in parallel, it's safe.
>
> 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>
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> Reviewed-by: Ray Ni <ray.ni@Intel.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 6 +--
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 78
> +++++++++++++++++-------------
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 6 ++-
> 3 files changed, 52 insertions(+), 38 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index cd394826ff..15d26dd88f 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -1,9 +1,9 @@
> /** @file
> Agent Module to load other modules to deploy SMM Entry Vector for X86
> CPU.
>
> -Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2009 - 2024, Intel Corporation. All rights reserved.<BR>
> Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -375,13 +375,13 @@ SmmInitHandler (
> &mCpuHotPlugData
> );
>
> if (!mSmmS3Flag) {
> //
> - // Check XD and BTS features on each processor on normal boot
> + // Check CET & XD & BTS features on each processor on normal boot
> //
> - CheckFeatureSupported ();
> + CheckFeatureSupported (IsBsp);
> } else if (IsBsp) {
> //
> // BSP rebase is already done above.
> // Initialize private data during S3 resume
> //
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index 8142d3ceac..44c352ad98 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -1,9 +1,9 @@
> /** @file
> Enable SMM profile.
>
> -Copyright (c) 2012 - 2023, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2012 - 2024, Intel Corporation. All rights reserved.<BR>
> Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -892,62 +892,74 @@ InitSmmProfileInternal (
> }
>
> /**
> Check if feature is supported by a processor.
>
> + @param[in] IsBsp Indicate it's called by BSP or not.
> +
> **/
> VOID
> CheckFeatureSupported (
> - VOID
> + IN BOOLEAN IsBsp
> )
> {
> UINT32 RegEax;
> UINT32 RegEcx;
> UINT32 RegEdx;
> MSR_IA32_MISC_ENABLE_REGISTER MiscEnableMsr;
>
> - if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) &&
> mCetSupported) {
> - AsmCpuid (CPUID_SIGNATURE, &RegEax, NULL, NULL, NULL);
> - if (RegEax >= CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS) {
> - AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS,
> CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_SUB_LEAF_INFO, NULL,
> NULL,
> &RegEcx, NULL);
> - if ((RegEcx & CPUID_CET_SS) == 0) {
> + //
> + // The feature scope is software visible domain.
> + // Only need check on BSP.
> + //
> + if (IsBsp) {
> + if ((PcdGet32 (PcdControlFlowEnforcementPropertyMask) != 0) &&
> mCetSupported) {
> + AsmCpuid (CPUID_SIGNATURE, &RegEax, NULL, NULL, NULL);
> + if (RegEax >= CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS) {
> + AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS,
> CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_SUB_LEAF_INFO, NULL,
> NULL,
> &RegEcx, NULL);
> + if ((RegEcx & CPUID_CET_SS) == 0) {
> + mCetSupported = FALSE;
> + PatchInstructionX86 (mPatchCetSupported, mCetSupported, 1);
> + }
> + } else {
> mCetSupported = FALSE;
> PatchInstructionX86 (mPatchCetSupported, mCetSupported, 1);
> }
> - } else {
> - mCetSupported = FALSE;
> - PatchInstructionX86 (mPatchCetSupported, mCetSupported, 1);
> }
> - }
>
> - if (mXdSupported) {
> - AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
> - if (RegEax <= CPUID_EXTENDED_FUNCTION) {
> - //
> - // Extended CPUID functions are not supported on this processor.
> - //
> - mXdSupported = FALSE;
> - PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
> - }
> + if (mXdSupported) {
> + AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
> + if (RegEax <= CPUID_EXTENDED_FUNCTION) {
> + //
> + // Extended CPUID functions are not supported on this processor.
> + //
> + mXdSupported = FALSE;
> + PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
> + }
>
> - AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx);
> - if ((RegEdx & CPUID1_EDX_XD_SUPPORT) == 0) {
> - //
> - // Execute Disable Bit feature is not supported on this processor.
> - //
> - mXdSupported = FALSE;
> - PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
> - }
> + AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx);
> + if ((RegEdx & CPUID1_EDX_XD_SUPPORT) == 0) {
> + //
> + // Execute Disable Bit feature is not supported on this processor.
> + //
> + mXdSupported = FALSE;
> + PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
> + }
>
> - if (StandardSignatureIsAuthenticAMD ()) {
> - //
> - // AMD processors do not support MSR_IA32_MISC_ENABLE
> - //
> - PatchInstructionX86 (gPatchMsrIa32MiscEnableSupported, FALSE, 1);
> + if (StandardSignatureIsAuthenticAMD ()) {
> + //
> + // AMD processors do not support MSR_IA32_MISC_ENABLE
> + //
> + PatchInstructionX86 (gPatchMsrIa32MiscEnableSupported, FALSE, 1);
> + }
> }
> }
>
> + //
> + // The feature scope is core.
> + // Need check on each processor.
> + //
> if (mBtsSupported) {
> AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, NULL, &RegEdx);
> if ((RegEdx & CPUID1_EDX_BTS_AVAILABLE) != 0) {
> //
> // Per IA32 manuals:
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
> index 1a82ac05ce..02554a9983 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h
> @@ -1,9 +1,9 @@
> /** @file
> SMM profile header file.
>
> -Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2012 - 2024, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
>
> #ifndef _SMM_PROFILE_H_
> @@ -81,14 +81,16 @@ PageFaultIdtHandlerSmmProfile (
> );
>
> /**
> Check if feature is supported by a processor.
>
> + @param[in] IsBsp Indicate it's called by BSP or not.
> +
> **/
> VOID
> CheckFeatureSupported (
> - VOID
> + IN BOOLEAN IsBsp
> );
>
> /**
> Update page table according to protected memory ranges and the
> 4KB-page mapped memory ranges.
>
>
> --
> Brian J. Johnson
> Enterprise X86 Lab
> Hewlett Packard Enterprise
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115146): https://edk2.groups.io/g/devel/message/115146
Mute This Topic: https://groups.io/mt/104153488/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-02-06 7:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Execute CET and XD check only on BSP Wu, Jiaxin
2024-02-05 16:59 ` Brian J. Johnson
2024-02-06 7:00 ` 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
-- strict thread matches above, loose matches on Subject: below --
2024-02-02 7:13 Wu, Jiaxin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox