Reviewed-by: Eric Dong <eric.dong at intel.com>
-----Original Message-----
From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of Ni, Ray
Sent: Wednesday, October 30, 2019 5:53 PM
To: devel at edk2.groups.io
Cc: Dong, Eric <eric.dong at intel.com>; Laszlo Ersek <lersek at redhat.com>
Subject: [edk2-devel] [PATCH 1/2] UefiCpuPkg/MpInitLib: Set X2ApicEnable flag from BSP
Today's logic sets X2ApicEnable flag in each AP's initialization path when InitFlag == ApInitConfig.
Since all CPUs update the same global data, a spin-lock is used to avoid modifications from multiple CPUs happen at the same time.
The spin-lock causes two problems:
1. Potential performance downgrade.
2. Undefined behavior when improper timer lib is used.
For example we saw certain platforms used AcpiTimerLib from
PcAtChipsetPkg and that library depends on retrieving PeiServices
from idtr. But in fact AP's (idtr - 4) doesn't point to
PeiServices.
The patch simplifies the code to let BSP set the X2ApicEnable flag so the spin-lock acquisition from AP is not needed any more.
Signed-off-by: Ray Ni <ray.ni at intel.com>
Cc: Eric Dong <eric.dong at intel.com>
Cc: Laszlo Ersek <lersek at redhat.com>
---
UefiCpuPkg/Library/MpInitLib/MpLib.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 622b70ca3c..8f62a8d965 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -458,6 +458,7 @@ CollectProcessorCount (
)
{
UINTN Index;
+ CPU_INFO_IN_HOB *CpuInfoInHob;
//
// Send 1st broadcast IPI to APs to wakeup APs @@ -474,12 +475,27 @@ CollectProcessorCount (
CpuPause ();
}
+
+ //
+ // Enable x2APIC mode if
+ // 1. Number of CPU is greater than 255; or // 2. There are any
+ logical processors reporting an Initial APIC ID of 255 or greater.
+ //
if (CpuMpData->CpuCount > 255) {
//
// If there are more than 255 processor found, force to enable X2APIC
//
CpuMpData->X2ApicEnable = TRUE;
+ } else {
+ CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) CpuMpData->CpuInfoInHob;
+ for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+ if (CpuInfoInHob[Index].InitialApicId >= 0xFF) {
+ CpuMpData->X2ApicEnable = TRUE;
+ break;
+ }
+ }
}
+
if (CpuMpData->X2ApicEnable) {
DEBUG ((DEBUG_INFO, "Force x2APIC mode!\n"));
//
@@ -541,15 +557,6 @@ InitializeApData (
CpuMpData->CpuData[ProcessorNumber].Waiting = FALSE;
CpuMpData->CpuData[ProcessorNumber].CpuHealthy = (BistData == 0) ? TRUE : FALSE;
- if (CpuInfoInHob[ProcessorNumber].InitialApicId >= 0xFF) {
- //
- // Set x2APIC mode if there are any logical processor reporting
- // an Initial APIC ID of 255 or greater.
- //
- AcquireSpinLock(&CpuMpData->MpLock);
- CpuMpData->X2ApicEnable = TRUE;
- ReleaseSpinLock(&CpuMpData->MpLock);
- }
InitializeSpinLock(&CpuMpData->CpuData[ProcessorNumber].ApLock);
SetApState (&CpuMpData->CpuData[ProcessorNumber], CpuStateIdle);
--
2.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#49942): https://edk2.groups.io/g/devel/message/49942
Mute This Topic: https://groups.io/mt/39769103/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-