* [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
@ 2022-07-27 6:11 JackX Lin
0 siblings, 0 replies; 16+ messages in thread
From: JackX Lin @ 2022-07-27 6:11 UTC (permalink / raw)
To: devel
Cc: JackX Lin, JackX Lin, Chasel Chiu, Dong Eric, Jiewen Yao, Ray Ni,
Rangasai V Chaganty, Donald Kuo, Chandana C Kumar
BIOS should not reordering cpu processor_uid
Signed-off-by: JackX Lin <JackX.Lin@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Dong Eric <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Donald Kuo <Donald.Kuo@intel.com>
Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
Cc: JackX Lin <JackX.Lin@intel.com>
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index c7e87cbd7d..f4c45336c5 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -173,7 +173,6 @@ SortCpuLocalApicInTable (
EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
UINT32 CoreThreadMask;
EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
- UINT32 Socket;
Index = 0;
Status = EFI_SUCCESS;
@@ -198,6 +197,7 @@ SortCpuLocalApicInTable (
CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
CpuIdMapPtr->Flags = ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0);
CpuIdMapPtr->SocketNum = ProcessorInfoBuffer.Location.Package;
+ CpuIdMapPtr->AcpiProcessorUid = (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) + CurrProcessor;
//update processorbitMask
if (CpuIdMapPtr->Flags == 1) {
@@ -280,18 +280,6 @@ SortCpuLocalApicInTable (
}
}
- //
- // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
- //
- for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount); Socket++) {
- for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
- if (mCpuApicIdOrderTable[CurrProcessor].Flags && (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
- mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) + Index;
- Index++;
- }
- }
- }
-
//keep for debug purpose
DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
DebugDisplayReOrderTable (mCpuApicIdOrderTable);
--
2.32.0.windows.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [edk2-platforms:PATCH] Modify processor _UID ordering by CPU default fused in MADT
@ 2022-07-28 7:24 JackX Lin
2022-08-01 3:02 ` Donald Kuo
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: JackX Lin @ 2022-07-28 7:24 UTC (permalink / raw)
To: devel
Cc: JackX Lin, JackX Lin, Chasel Chiu, Dong Eric, Jiewen Yao, Ray Ni,
Rangasai V Chaganty, Donald Kuo, Chandana C Kumar, Palakshareddy,
Lavanya C
BIOS should not reordering cpu processor_uid
Signed-off-by: JackX Lin <JackX.Lin@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Dong Eric <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Donald Kuo <Donald.Kuo@intel.com>
Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Cc: JackX Lin <JackX.Lin@intel.com>
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 99 ++++-----------------------------------------------------------------------------------------------
1 file changed, 4 insertions(+), 95 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index c7e87cbd7d..d0e8891918 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -62,33 +62,6 @@ EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL;
UINTN mNumberOfCpus = 0;
UINTN mNumberOfEnabledCPUs = 0;
-
-/**
- The function is called by PerformQuickSort to compare int values.
-
- @param[in] Left The pointer to first buffer.
- @param[in] Right The pointer to second buffer.
-
- @return -1 Buffer1 is less than Buffer2.
- @return 1 Buffer1 is greater than Buffer2.
-
-**/
-INTN
-EFIAPI
-ApicIdCompareFunction (
- IN CONST VOID *Left,
- IN CONST VOID *Right
- )
-{
- UINT32 LeftApicId;
- UINT32 RightApicId;
-
- LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
- RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
-
- return (LeftApicId > RightApicId)? 1 : (-1);
-}
-
/**
Print Cpu Apic ID Table
@@ -168,21 +141,16 @@ SortCpuLocalApicInTable (
EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
UINT32 Index;
UINT32 CurrProcessor;
- UINT32 BspApicId;
- EFI_CPU_ID_ORDER_MAP TempVal;
EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
UINT32 CoreThreadMask;
- EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
UINT32 Socket;
- Index = 0;
Status = EFI_SUCCESS;
if (mCpuOrderSorted) {
return Status;
}
- TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++, Index++) {
@@ -192,7 +160,7 @@ SortCpuLocalApicInTable (
&ProcessorInfoBuffer
);
- CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &TempCpuApicIdOrderTable[Index];
+ CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &mCpuApicIdOrderTable[Index];
if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
@@ -214,74 +182,16 @@ SortCpuLocalApicInTable (
} //end if PROC ENABLE
} //end for CurrentProcessor
- //keep for debug purpose
DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift));
- DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
//
// Get Bsp Apic Id
//
- BspApicId = GetApicId ();
- DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
-
- //
- //check to see if 1st entry is BSP, if not swap it
- //
- if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Flags == 1) && (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
- CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CopyMem (&TempCpuApicIdOrderTable[Index], &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
- CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof (EFI_CPU_ID_ORDER_MAP));
- break;
- }
- }
-
- if (mNumberOfCpus <= Index) {
- DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n"));
- return EFI_INVALID_PARAMETER;
- }
- }
-
- //
- // 1. Sort TempCpuApicIdOrderTable,
- // sort it by using ApicId from minimum to maximum (Socket0 to SocketN), and the BSP must in the fist location of the table.
- // So, start sorting the table from the second element and total elements are mNumberOfCpus-1.
- //
- PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 1), sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE) ApicIdCompareFunction);
+ DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
- //
- // 2. Sort and map the primary threads to the front of the CpuApicIdOrderTable
- //
- for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
//
- // 3. Sort and map the second threads to the middle of the CpuApicIdOrderTable
- //
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- //
- // 4. Sort and map the not enabled threads to the bottom of the CpuApicIdOrderTable
- //
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- //
- // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
+ // Fill in AcpiProcessorUid.
//
for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount); Socket++) {
for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
@@ -292,8 +202,7 @@ SortCpuLocalApicInTable (
}
}
- //keep for debug purpose
- DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
+ DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift));
DebugDisplayReOrderTable (mCpuApicIdOrderTable);
mCpuOrderSorted = TRUE;
--
2.32.0.windows.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [edk2-platforms:PATCH] Modify processor _UID ordering by CPU default fused in MADT
2022-07-28 7:24 [edk2-platforms:PATCH] " JackX Lin
@ 2022-08-01 3:02 ` Donald Kuo
2022-08-02 16:37 ` Kumar, Chandana C
2022-08-05 6:42 ` Ni, Ray
2 siblings, 0 replies; 16+ messages in thread
From: Donald Kuo @ 2022-08-01 3:02 UTC (permalink / raw)
To: Lin, JackX, devel@edk2.groups.io
Cc: Chiu, Chasel, Dong, Eric, Yao, Jiewen, Ni, Ray,
Chaganty, Rangasai V, Kumar, Chandana C, Palakshareddy, Lavanya C,
Palakshareddy, Lavanya C
Thanks Jack
The changes looks good to me.
Donald
-----Original Message-----
From: Lin, JackX <jackx.lin@intel.com>
Sent: Thursday, July 28, 2022 3:25 PM
To: devel@edk2.groups.io
Cc: Lin, JackX <jackx.lin@intel.com>; Lin, JackX <jackx.lin@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Kuo, Donald <donald.kuo@intel.com>; Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Subject: [edk2-platforms:PATCH] Modify processor _UID ordering by CPU default fused in MADT
BIOS should not reordering cpu processor_uid
Signed-off-by: JackX Lin <JackX.Lin@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Dong Eric <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Donald Kuo <Donald.Kuo@intel.com>
Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Cc: JackX Lin <JackX.Lin@intel.com>
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 99 ++++-----------------------------------------------------------------------------------------------
1 file changed, 4 insertions(+), 95 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index c7e87cbd7d..d0e8891918 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -62,33 +62,6 @@ EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL;
UINTN mNumberOfCpus = 0;
UINTN mNumberOfEnabledCPUs = 0;
-
-/**
- The function is called by PerformQuickSort to compare int values.
-
- @param[in] Left The pointer to first buffer.
- @param[in] Right The pointer to second buffer.
-
- @return -1 Buffer1 is less than Buffer2.
- @return 1 Buffer1 is greater than Buffer2.
-
-**/
-INTN
-EFIAPI
-ApicIdCompareFunction (
- IN CONST VOID *Left,
- IN CONST VOID *Right
- )
-{
- UINT32 LeftApicId;
- UINT32 RightApicId;
-
- LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
- RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
-
- return (LeftApicId > RightApicId)? 1 : (-1); -}
-
/**
Print Cpu Apic ID Table
@@ -168,21 +141,16 @@ SortCpuLocalApicInTable (
EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
UINT32 Index;
UINT32 CurrProcessor;
- UINT32 BspApicId;
- EFI_CPU_ID_ORDER_MAP TempVal;
EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
UINT32 CoreThreadMask;
- EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
UINT32 Socket;
- Index = 0;
Status = EFI_SUCCESS;
if (mCpuOrderSorted) {
return Status;
}
- TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++, Index++) { @@ -192,7 +160,7 @@ SortCpuLocalApicInTable (
&ProcessorInfoBuffer
);
- CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &TempCpuApicIdOrderTable[Index];
+ CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
+ &mCpuApicIdOrderTable[Index];
if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
@@ -214,74 +182,16 @@ SortCpuLocalApicInTable (
} //end if PROC ENABLE
} //end for CurrentProcessor
- //keep for debug purpose
DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift));
- DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
//
// Get Bsp Apic Id
//
- BspApicId = GetApicId ();
- DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
-
- //
- //check to see if 1st entry is BSP, if not swap it
- //
- if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Flags == 1) && (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
- CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CopyMem (&TempCpuApicIdOrderTable[Index], &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
- CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof (EFI_CPU_ID_ORDER_MAP));
- break;
- }
- }
-
- if (mNumberOfCpus <= Index) {
- DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n"));
- return EFI_INVALID_PARAMETER;
- }
- }
-
- //
- // 1. Sort TempCpuApicIdOrderTable,
- // sort it by using ApicId from minimum to maximum (Socket0 to SocketN), and the BSP must in the fist location of the table.
- // So, start sorting the table from the second element and total elements are mNumberOfCpus-1.
- //
- PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 1), sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE) ApicIdCompareFunction);
+ DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
- //
- // 2. Sort and map the primary threads to the front of the CpuApicIdOrderTable
- //
- for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
//
- // 3. Sort and map the second threads to the middle of the CpuApicIdOrderTable
- //
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- //
- // 4. Sort and map the not enabled threads to the bottom of the CpuApicIdOrderTable
- //
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- //
- // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
+ // Fill in AcpiProcessorUid.
//
for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount); Socket++) {
for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) { @@ -292,8 +202,7 @@ SortCpuLocalApicInTable (
}
}
- //keep for debug purpose
- DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
+ DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift));
DebugDisplayReOrderTable (mCpuApicIdOrderTable);
mCpuOrderSorted = TRUE;
--
2.32.0.windows.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [edk2-platforms:PATCH] Modify processor _UID ordering by CPU default fused in MADT
2022-07-28 7:24 [edk2-platforms:PATCH] " JackX Lin
2022-08-01 3:02 ` Donald Kuo
@ 2022-08-02 16:37 ` Kumar, Chandana C
2022-08-05 6:42 ` Ni, Ray
2 siblings, 0 replies; 16+ messages in thread
From: Kumar, Chandana C @ 2022-08-02 16:37 UTC (permalink / raw)
To: Lin, JackX, devel@edk2.groups.io
Cc: Chiu, Chasel, Dong, Eric, Yao, Jiewen, Ni, Ray,
Chaganty, Rangasai V, Kuo, Donald, Palakshareddy, Lavanya C,
Palakshareddy, Lavanya C
Reviewed-by: Kumar, Chandana C <chandana.c.kumar@intel.com>
> -----Original Message-----
> From: Lin, JackX <jackx.lin@intel.com>
> Sent: Thursday, July 28, 2022 12:55 PM
> To: devel@edk2.groups.io
> Cc: Lin, JackX <jackx.lin@intel.com>; Lin, JackX <jackx.lin@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Kuo, Donald <donald.kuo@intel.com>; Kumar,
> Chandana C <chandana.c.kumar@intel.com>; Palakshareddy; Palakshareddy,
> Lavanya C <lavanya.c.palakshareddy@intel.com>
> Subject: [edk2-platforms:PATCH] Modify processor _UID ordering by CPU default
> fused in MADT
>
> BIOS should not reordering cpu processor_uid
>
> Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Dong Eric <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Donald Kuo <Donald.Kuo@intel.com>
> Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
> Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Cc: JackX Lin <JackX.Lin@intel.com>
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 99 ++++-------------
> ----------------------------------------------------------------------------------
> 1 file changed, 4 insertions(+), 95 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index c7e87cbd7d..d0e8891918 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -62,33 +62,6 @@ EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable
> = NULL;
> UINTN mNumberOfCpus = 0;
> UINTN mNumberOfEnabledCPUs = 0;
>
> -
> -/**
> - The function is called by PerformQuickSort to compare int values.
> -
> - @param[in] Left The pointer to first buffer.
> - @param[in] Right The pointer to second buffer.
> -
> - @return -1 Buffer1 is less than Buffer2.
> - @return 1 Buffer1 is greater than Buffer2.
> -
> -**/
> -INTN
> -EFIAPI
> -ApicIdCompareFunction (
> - IN CONST VOID *Left,
> - IN CONST VOID *Right
> - )
> -{
> - UINT32 LeftApicId;
> - UINT32 RightApicId;
> -
> - LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
> - RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
> -
> - return (LeftApicId > RightApicId)? 1 : (-1); -}
> -
> /**
> Print Cpu Apic ID Table
>
> @@ -168,21 +141,16 @@ SortCpuLocalApicInTable (
> EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
> UINT32 Index;
> UINT32 CurrProcessor;
> - UINT32 BspApicId;
> - EFI_CPU_ID_ORDER_MAP TempVal;
> EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
> UINT32 CoreThreadMask;
> - EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
> UINT32 Socket;
>
> - Index = 0;
> Status = EFI_SUCCESS;
>
> if (mCpuOrderSorted) {
> return Status;
> }
>
> - TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
>
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++, Index++) { @@ -192,7 +160,7 @@ SortCpuLocalApicInTable (
> &ProcessorInfoBuffer
> );
>
> - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &TempCpuApicIdOrderTable[Index];
> + CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> + &mCpuApicIdOrderTable[Index];
> if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
> CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
> CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
> @@ -214,74 +182,16 @@ SortCpuLocalApicInTable (
> } //end if PROC ENABLE
> } //end for CurrentProcessor
>
> - //keep for debug purpose
> DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. CoreThreadMask
> = %x, mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift));
> - DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
>
> //
> // Get Bsp Apic Id
> //
> - BspApicId = GetApicId ();
> - DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
> -
> - //
> - //check to see if 1st entry is BSP, if not swap it
> - //
> - if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
> (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
> - CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[Index],
> &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - break;
> - }
> - }
> -
> - if (mNumberOfCpus <= Index) {
> - DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
> Bufferflow\n"));
> - return EFI_INVALID_PARAMETER;
> - }
> - }
> -
> - //
> - // 1. Sort TempCpuApicIdOrderTable,
> - // sort it by using ApicId from minimum to maximum (Socket0 to SocketN), and
> the BSP must in the fist location of the table.
> - // So, start sorting the table from the second element and total elements are
> mNumberOfCpus-1.
> - //
> - PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 1),
> sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE)
> ApicIdCompareFunction);
> + DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
>
> - //
> - // 2. Sort and map the primary threads to the front of the CpuApicIdOrderTable
> - //
> - for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
>
> //
> - // 3. Sort and map the second threads to the middle of the CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - //
> - // 4. Sort and map the not enabled threads to the bottom of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - //
> - // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
> + // Fill in AcpiProcessorUid.
> //
> for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount); Socket++)
> {
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++) { @@ -292,8 +202,7 @@ SortCpuLocalApicInTable (
> }
> }
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
> + DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. CoreThreadMask
> = %x, mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift));
> DebugDisplayReOrderTable (mCpuApicIdOrderTable);
>
> mCpuOrderSorted = TRUE;
> --
> 2.32.0.windows.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [edk2-platforms:PATCH] Modify processor _UID ordering by CPU default fused in MADT
2022-07-28 7:24 [edk2-platforms:PATCH] " JackX Lin
2022-08-01 3:02 ` Donald Kuo
2022-08-02 16:37 ` Kumar, Chandana C
@ 2022-08-05 6:42 ` Ni, Ray
2022-08-08 8:19 ` JackX Lin
2 siblings, 1 reply; 16+ messages in thread
From: Ni, Ray @ 2022-08-05 6:42 UTC (permalink / raw)
To: Lin, JackX, devel@edk2.groups.io
Cc: Chiu, Chasel, Dong, Eric, Yao, Jiewen, Chaganty, Rangasai V,
Kuo, Donald, Kumar, Chandana C, Palakshareddy, Lavanya C,
Palakshareddy, Lavanya C
Jack,
The patch removes all the sorting logic. It simplifies the logic a lot. Thanks for that!
ACPI spec " 5.2.12.1 MADT Processor Local APIC / SAPIC Structure Entry Order"
talked about why the BSP needs to be in the first entry of MADT and why first logical
processor of each core needs to be before second logical processor.
With the reasons, I totally agree that we don't need to sort the MADT any more
after the hyper-threading and many-core support have been enabled for quite
a long time in OS.
Some minor comments:
1. Can you check if "CoreThreadMask" can be removed?
2. Can you please rename the SortCpuLocalApicInTable? Maybe just use "CreateCpuLocalApicInTable"? There is no sorting any more😊
3. Can you please check if " mCpuOrderSorted" is needed? It's needed when the function is called multiple times.
4. If it's needed, can you please rename it to "mCpuLocalApicInTableCreated"?
5. If it's not needed, can you please think about if " mCpuApicIdOrderTable" can be changed to a local variable?
Thanks,
Ray
> -----Original Message-----
> From: Lin, JackX <jackx.lin@intel.com>
> Sent: Thursday, July 28, 2022 3:25 PM
> To: devel@edk2.groups.io
> Cc: Lin, JackX <jackx.lin@intel.com>; Lin, JackX <jackx.lin@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Ni, Ray <ray.ni@intel.com>; Chaganty,
> Rangasai V <rangasai.v.chaganty@intel.com>; Kuo, Donald
> <donald.kuo@intel.com>; Kumar, Chandana C
> <chandana.c.kumar@intel.com>; Palakshareddy; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>
> Subject: [edk2-platforms:PATCH] Modify processor _UID ordering by CPU
> default fused in MADT
>
> BIOS should not reordering cpu processor_uid
>
> Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Dong Eric <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Donald Kuo <Donald.Kuo@intel.com>
> Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
> Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Cc: JackX Lin <JackX.Lin@intel.com>
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 99 ++++---
> --------------------------------------------------------------------------------------------
> 1 file changed, 4 insertions(+), 95 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index c7e87cbd7d..d0e8891918 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -62,33 +62,6 @@ EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable
> = NULL;
> UINTN mNumberOfCpus = 0;
> UINTN mNumberOfEnabledCPUs = 0;
>
> -
> -/**
> - The function is called by PerformQuickSort to compare int values.
> -
> - @param[in] Left The pointer to first buffer.
> - @param[in] Right The pointer to second buffer.
> -
> - @return -1 Buffer1 is less than Buffer2.
> - @return 1 Buffer1 is greater than Buffer2.
> -
> -**/
> -INTN
> -EFIAPI
> -ApicIdCompareFunction (
> - IN CONST VOID *Left,
> - IN CONST VOID *Right
> - )
> -{
> - UINT32 LeftApicId;
> - UINT32 RightApicId;
> -
> - LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
> - RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
> -
> - return (LeftApicId > RightApicId)? 1 : (-1);
> -}
> -
> /**
> Print Cpu Apic ID Table
>
> @@ -168,21 +141,16 @@ SortCpuLocalApicInTable (
> EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
> UINT32 Index;
> UINT32 CurrProcessor;
> - UINT32 BspApicId;
> - EFI_CPU_ID_ORDER_MAP TempVal;
> EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
> UINT32 CoreThreadMask;
> - EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
> UINT32 Socket;
>
> - Index = 0;
> Status = EFI_SUCCESS;
>
> if (mCpuOrderSorted) {
> return Status;
> }
>
> - TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
>
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++, Index++) {
> @@ -192,7 +160,7 @@ SortCpuLocalApicInTable (
> &ProcessorInfoBuffer
> );
>
> - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &TempCpuApicIdOrderTable[Index];
> + CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &mCpuApicIdOrderTable[Index];
> if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
> CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
> CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
> @@ -214,74 +182,16 @@ SortCpuLocalApicInTable (
> } //end if PROC ENABLE
> } //end for CurrentProcessor
>
> - //keep for debug purpose
> DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
> mNumOfBitShift));
> - DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
>
> //
> // Get Bsp Apic Id
> //
> - BspApicId = GetApicId ();
> - DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
> -
> - //
> - //check to see if 1st entry is BSP, if not swap it
> - //
> - if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
> (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
> - CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[Index],
> &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - break;
> - }
> - }
> -
> - if (mNumberOfCpus <= Index) {
> - DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
> Bufferflow\n"));
> - return EFI_INVALID_PARAMETER;
> - }
> - }
> -
> - //
> - // 1. Sort TempCpuApicIdOrderTable,
> - // sort it by using ApicId from minimum to maximum (Socket0 to SocketN),
> and the BSP must in the fist location of the table.
> - // So, start sorting the table from the second element and total elements
> are mNumberOfCpus-1.
> - //
> - PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 1),
> sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE)
> ApicIdCompareFunction);
> + DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
>
> - //
> - // 2. Sort and map the primary threads to the front of the
> CpuApicIdOrderTable
> - //
> - for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
>
> //
> - // 3. Sort and map the second threads to the middle of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - //
> - // 4. Sort and map the not enabled threads to the bottom of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - //
> - // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
> + // Fill in AcpiProcessorUid.
> //
> for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount);
> Socket++) {
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++) {
> @@ -292,8 +202,7 @@ SortCpuLocalApicInTable (
> }
> }
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
> + DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
> mNumOfBitShift));
> DebugDisplayReOrderTable (mCpuApicIdOrderTable);
>
> mCpuOrderSorted = TRUE;
> --
> 2.32.0.windows.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [edk2-platforms:PATCH] Modify processor _UID ordering by CPU default fused in MADT
2022-08-05 6:42 ` Ni, Ray
@ 2022-08-08 8:19 ` JackX Lin
0 siblings, 0 replies; 16+ messages in thread
From: JackX Lin @ 2022-08-08 8:19 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io
Cc: Chiu, Chasel, Dong, Eric, Yao, Jiewen, Chaganty, Rangasai V,
Kuo, Donald, Kumar, Chandana C, Palakshareddy, Lavanya C,
Palakshareddy, Lavanya C
Hi Ray,
Sure, I will complete all these below.
Thank you.
Jack
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Friday, August 5, 2022 2:43 PM
To: Lin, JackX <jackx.lin@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Kuo, Donald <donald.kuo@intel.com>; Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Subject: RE: [edk2-platforms:PATCH] Modify processor _UID ordering by CPU default fused in MADT
Jack,
The patch removes all the sorting logic. It simplifies the logic a lot. Thanks for that!
ACPI spec " 5.2.12.1 MADT Processor Local APIC / SAPIC Structure Entry Order"
talked about why the BSP needs to be in the first entry of MADT and why first logical processor of each core needs to be before second logical processor.
With the reasons, I totally agree that we don't need to sort the MADT any more after the hyper-threading and many-core support have been enabled for quite a long time in OS.
Some minor comments:
1. Can you check if "CoreThreadMask" can be removed?
2. Can you please rename the SortCpuLocalApicInTable? Maybe just use "CreateCpuLocalApicInTable"? There is no sorting any more😊
3. Can you please check if " mCpuOrderSorted" is needed? It's needed when the function is called multiple times.
4. If it's needed, can you please rename it to "mCpuLocalApicInTableCreated"?
5. If it's not needed, can you please think about if " mCpuApicIdOrderTable" can be changed to a local variable?
Thanks,
Ray
> -----Original Message-----
> From: Lin, JackX <jackx.lin@intel.com>
> Sent: Thursday, July 28, 2022 3:25 PM
> To: devel@edk2.groups.io
> Cc: Lin, JackX <jackx.lin@intel.com>; Lin, JackX
> <jackx.lin@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Dong,
> Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Kuo, Donald <donald.kuo@intel.com>;
> Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy;
> Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Subject: [edk2-platforms:PATCH] Modify processor _UID ordering by CPU
> default fused in MADT
>
> BIOS should not reordering cpu processor_uid
>
> Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Dong Eric <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Donald Kuo <Donald.Kuo@intel.com>
> Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
> Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Cc: JackX Lin <JackX.Lin@intel.com>
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 99
> ++++---
> ----------------------------------------------------------------------
> ----------------------
> 1 file changed, 4 insertions(+), 95 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index c7e87cbd7d..d0e8891918 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -62,33 +62,6 @@ EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable
> = NULL;
> UINTN mNumberOfCpus = 0;
> UINTN mNumberOfEnabledCPUs = 0;
>
> -
> -/**
> - The function is called by PerformQuickSort to compare int values.
> -
> - @param[in] Left The pointer to first buffer.
> - @param[in] Right The pointer to second buffer.
> -
> - @return -1 Buffer1 is less than Buffer2.
> - @return 1 Buffer1 is greater than Buffer2.
> -
> -**/
> -INTN
> -EFIAPI
> -ApicIdCompareFunction (
> - IN CONST VOID *Left,
> - IN CONST VOID *Right
> - )
> -{
> - UINT32 LeftApicId;
> - UINT32 RightApicId;
> -
> - LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
> - RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
> -
> - return (LeftApicId > RightApicId)? 1 : (-1); -}
> -
> /**
> Print Cpu Apic ID Table
>
> @@ -168,21 +141,16 @@ SortCpuLocalApicInTable (
> EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
> UINT32 Index;
> UINT32 CurrProcessor;
> - UINT32 BspApicId;
> - EFI_CPU_ID_ORDER_MAP TempVal;
> EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
> UINT32 CoreThreadMask;
> - EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
> UINT32 Socket;
>
> - Index = 0;
> Status = EFI_SUCCESS;
>
> if (mCpuOrderSorted) {
> return Status;
> }
>
> - TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
>
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++, Index++) {
> @@ -192,7 +160,7 @@ SortCpuLocalApicInTable (
> &ProcessorInfoBuffer
> );
>
> - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &TempCpuApicIdOrderTable[Index];
> + CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &mCpuApicIdOrderTable[Index];
> if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
> CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
> CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
> @@ -214,74 +182,16 @@ SortCpuLocalApicInTable (
> } //end if PROC ENABLE
> } //end for CurrentProcessor
>
> - //keep for debug purpose
> DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
> mNumOfBitShift));
> - DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
>
> //
> // Get Bsp Apic Id
> //
> - BspApicId = GetApicId ();
> - DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
> -
> - //
> - //check to see if 1st entry is BSP, if not swap it
> - //
> - if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
> (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
> - CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[Index],
> &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - break;
> - }
> - }
> -
> - if (mNumberOfCpus <= Index) {
> - DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
> Bufferflow\n"));
> - return EFI_INVALID_PARAMETER;
> - }
> - }
> -
> - //
> - // 1. Sort TempCpuApicIdOrderTable,
> - // sort it by using ApicId from minimum to maximum (Socket0 to SocketN),
> and the BSP must in the fist location of the table.
> - // So, start sorting the table from the second element and total elements
> are mNumberOfCpus-1.
> - //
> - PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus -
> 1), sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE)
> ApicIdCompareFunction);
> + DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
>
> - //
> - // 2. Sort and map the primary threads to the front of the
> CpuApicIdOrderTable
> - //
> - for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
>
> //
> - // 3. Sort and map the second threads to the middle of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - //
> - // 4. Sort and map the not enabled threads to the bottom of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - //
> - // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
> + // Fill in AcpiProcessorUid.
> //
> for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount);
> Socket++) {
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++) {
> @@ -292,8 +202,7 @@ SortCpuLocalApicInTable (
> }
> }
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
> + DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
> mNumOfBitShift));
> DebugDisplayReOrderTable (mCpuApicIdOrderTable);
>
> mCpuOrderSorted = TRUE;
> --
> 2.32.0.windows.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
@ 2022-08-08 8:21 JackX Lin
2022-08-09 7:12 ` Ni, Ray
2022-08-10 3:47 ` Ni, Ray
0 siblings, 2 replies; 16+ messages in thread
From: JackX Lin @ 2022-08-08 8:21 UTC (permalink / raw)
To: devel
Cc: JackX Lin, JackX Lin, Chasel Chiu, Dong Eric, Jiewen Yao, Ray Ni,
Rangasai V Chaganty, Donald Kuo, Chandana C Kumar, Palakshareddy,
Lavanya C
BIOS should not reordering cpu processor_uid
Signed-off-by: JackX Lin <JackX.Lin@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Dong Eric <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Donald Kuo <Donald.Kuo@intel.com>
Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Cc: JackX Lin <JackX.Lin@intel.com>
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 174 +++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------------------------------------------------------------
1 file changed, 39 insertions(+), 135 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index c7e87cbd7d..176e422e81 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -57,38 +57,9 @@ BOOLEAN mForceX2ApicId;
BOOLEAN mX2ApicEnabled;
EFI_MP_SERVICES_PROTOCOL *mMpService;
-BOOLEAN mCpuOrderSorted;
-EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL;
UINTN mNumberOfCpus = 0;
UINTN mNumberOfEnabledCPUs = 0;
-
-/**
- The function is called by PerformQuickSort to compare int values.
-
- @param[in] Left The pointer to first buffer.
- @param[in] Right The pointer to second buffer.
-
- @return -1 Buffer1 is less than Buffer2.
- @return 1 Buffer1 is greater than Buffer2.
-
-**/
-INTN
-EFIAPI
-ApicIdCompareFunction (
- IN CONST VOID *Left,
- IN CONST VOID *Right
- )
-{
- UINT32 LeftApicId;
- UINT32 RightApicId;
-
- LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
- RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
-
- return (LeftApicId > RightApicId)? 1 : (-1);
-}
-
/**
Print Cpu Apic ID Table
@@ -116,7 +87,8 @@ DebugDisplayReOrderTable (
EFI_STATUS
AppendCpuMapTableEntry (
IN VOID *ApicPtr,
- IN UINT32 LocalApicCounter
+ IN UINT32 LocalApicCounter,
+ IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
)
{
EFI_STATUS Status;
@@ -131,9 +103,9 @@ AppendCpuMapTableEntry (
if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
if(!mX2ApicEnabled) {
- LocalApicPtr->Flags = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
- LocalApicPtr->ApicId = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
- LocalApicPtr->AcpiProcessorUid = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
+ LocalApicPtr->Flags = (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
+ LocalApicPtr->ApicId = (UINT8)CpuApicIdOrderTable[LocalApicCounter].ApicId;
+ LocalApicPtr->AcpiProcessorUid = (UINT8)CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
} else {
LocalApicPtr->Flags = 0;
LocalApicPtr->ApicId = 0xFF;
@@ -142,9 +114,9 @@ AppendCpuMapTableEntry (
}
} else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
if(mX2ApicEnabled) {
- LocalX2ApicPtr->Flags = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
- LocalX2ApicPtr->X2ApicId = mCpuApicIdOrderTable[LocalApicCounter].ApicId;
- LocalX2ApicPtr->AcpiProcessorUid = mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
+ LocalX2ApicPtr->Flags = (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
+ LocalX2ApicPtr->X2ApicId = CpuApicIdOrderTable[LocalApicCounter].ApicId;
+ LocalX2ApicPtr->AcpiProcessorUid = CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
} else {
LocalX2ApicPtr->Flags = 0;
LocalX2ApicPtr->X2ApicId = (UINT32)-1;
@@ -159,32 +131,25 @@ AppendCpuMapTableEntry (
}
+/**
+ Collect all processors information and create a Cpu Apic Id table.
+
+ @param[in] CpuApicIdOrderTable Buffer to store information of Cpu.
+**/
EFI_STATUS
-SortCpuLocalApicInTable (
- VOID
+CreateCpuLocalApicInTable (
+ IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
)
{
EFI_STATUS Status;
EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
UINT32 Index;
UINT32 CurrProcessor;
- UINT32 BspApicId;
- EFI_CPU_ID_ORDER_MAP TempVal;
EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
- UINT32 CoreThreadMask;
- EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
UINT32 Socket;
- Index = 0;
Status = EFI_SUCCESS;
- if (mCpuOrderSorted) {
- return Status;
- }
-
- TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
- CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
-
for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++, Index++) {
Status = mMpService->GetProcessorInfo (
mMpService,
@@ -192,7 +157,7 @@ SortCpuLocalApicInTable (
&ProcessorInfoBuffer
);
- CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &TempCpuApicIdOrderTable[Index];
+ CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &CpuApicIdOrderTable[Index];
if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
@@ -214,89 +179,26 @@ SortCpuLocalApicInTable (
} //end if PROC ENABLE
} //end for CurrentProcessor
- //keep for debug purpose
- DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift));
- DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
-
//
// Get Bsp Apic Id
//
- BspApicId = GetApicId ();
- DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
-
- //
- //check to see if 1st entry is BSP, if not swap it
- //
- if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Flags == 1) && (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
- CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CopyMem (&TempCpuApicIdOrderTable[Index], &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
- CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof (EFI_CPU_ID_ORDER_MAP));
- break;
- }
- }
-
- if (mNumberOfCpus <= Index) {
- DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n"));
- return EFI_INVALID_PARAMETER;
- }
- }
-
- //
- // 1. Sort TempCpuApicIdOrderTable,
- // sort it by using ApicId from minimum to maximum (Socket0 to SocketN), and the BSP must in the fist location of the table.
- // So, start sorting the table from the second element and total elements are mNumberOfCpus-1.
- //
- PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 1), sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE) ApicIdCompareFunction);
-
- //
- // 2. Sort and map the primary threads to the front of the CpuApicIdOrderTable
- //
- for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
+ DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
- //
- // 3. Sort and map the second threads to the middle of the CpuApicIdOrderTable
- //
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
//
- // 4. Sort and map the not enabled threads to the bottom of the CpuApicIdOrderTable
- //
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- //
- // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
+ // Fill in AcpiProcessorUid.
//
for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount); Socket++) {
for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
- if (mCpuApicIdOrderTable[CurrProcessor].Flags && (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
- mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) + Index;
+ if (CpuApicIdOrderTable[CurrProcessor].Flags && (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
+ CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) + Index;
Index++;
}
}
}
- //keep for debug purpose
- DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
- DebugDisplayReOrderTable (mCpuApicIdOrderTable);
-
- mCpuOrderSorted = TRUE;
+ DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. mNumOfBitShift = %x\n", mNumOfBitShift));
+ DebugDisplayReOrderTable (CpuApicIdOrderTable);
return Status;
}
@@ -750,6 +652,7 @@ InstallMadtFromScratch (
EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE LocalApciNmiStruct;
EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE ProcLocalX2ApicStruct;
EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE LocalX2ApicNmiStruct;
+ EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable;
STRUCTURE_HEADER **MadtStructs;
UINTN MaxMadtStructCount;
UINTN MadtStructsIndex;
@@ -760,18 +663,19 @@ InstallMadtFromScratch (
MadtStructs = NULL;
NewMadtTable = NULL;
+ CpuApicIdOrderTable = NULL;
MaxMadtStructCount = 0;
- mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
- if (mCpuApicIdOrderTable == NULL) {
- DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable structure pointer array\n"));
+ CpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
+ if (CpuApicIdOrderTable == NULL) {
+ DEBUG ((DEBUG_ERROR, "Could not allocate CpuApicIdOrderTable structure pointer array\n"));
return EFI_OUT_OF_RESOURCES;
}
// Call for Local APIC ID Reorder
- Status = SortCpuLocalApicInTable ();
+ Status = CreateCpuLocalApicInTable (CpuApicIdOrderTable);
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
+ DEBUG ((DEBUG_ERROR, "CreateCpuLocalApicInTable failed: %r\n", Status));
goto Done;
}
@@ -824,10 +728,10 @@ InstallMadtFromScratch (
// APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
// use a processor local x2APIC structure.
//
- if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId < MAX_UINT8) {
- ProcLocalApicStruct.Flags = (UINT8) mCpuApicIdOrderTable[Index].Flags;
- ProcLocalApicStruct.ApicId = (UINT8) mCpuApicIdOrderTable[Index].ApicId;
- ProcLocalApicStruct.AcpiProcessorUid = (UINT8) mCpuApicIdOrderTable[Index].AcpiProcessorUid;
+ if (!mX2ApicEnabled && CpuApicIdOrderTable[Index].ApicId < MAX_UINT8) {
+ ProcLocalApicStruct.Flags = (UINT8) CpuApicIdOrderTable[Index].Flags;
+ ProcLocalApicStruct.ApicId = (UINT8) CpuApicIdOrderTable[Index].ApicId;
+ ProcLocalApicStruct.AcpiProcessorUid = (UINT8) CpuApicIdOrderTable[Index].AcpiProcessorUid;
ASSERT (MadtStructsIndex < MaxMadtStructCount);
Status = CopyStructure (
@@ -835,10 +739,10 @@ InstallMadtFromScratch (
(STRUCTURE_HEADER *) &ProcLocalApicStruct,
&MadtStructs[MadtStructsIndex++]
);
- } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
- ProcLocalX2ApicStruct.Flags = (UINT8) mCpuApicIdOrderTable[Index].Flags;
- ProcLocalX2ApicStruct.X2ApicId = mCpuApicIdOrderTable[Index].ApicId;
- ProcLocalX2ApicStruct.AcpiProcessorUid = mCpuApicIdOrderTable[Index].AcpiProcessorUid;
+ } else if (CpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
+ ProcLocalX2ApicStruct.Flags = (UINT8) CpuApicIdOrderTable[Index].Flags;
+ ProcLocalX2ApicStruct.X2ApicId = CpuApicIdOrderTable[Index].ApicId;
+ ProcLocalX2ApicStruct.AcpiProcessorUid = CpuApicIdOrderTable[Index].AcpiProcessorUid;
ASSERT (MadtStructsIndex < MaxMadtStructCount);
Status = CopyStructure (
@@ -1033,8 +937,8 @@ Done:
FreePool (NewMadtTable);
}
- if (mCpuApicIdOrderTable != NULL) {
- FreePool (mCpuApicIdOrderTable);
+ if (CpuApicIdOrderTable != NULL) {
+ FreePool (CpuApicIdOrderTable);
}
return Status;
--
2.32.0.windows.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
2022-08-08 8:21 JackX Lin
@ 2022-08-09 7:12 ` Ni, Ray
2022-08-09 7:52 ` Donald Kuo
2022-08-10 3:47 ` Ni, Ray
1 sibling, 1 reply; 16+ messages in thread
From: Ni, Ray @ 2022-08-09 7:12 UTC (permalink / raw)
To: Lin, JackX, devel@edk2.groups.io
Cc: Chiu, Chasel, Dong, Eric, Yao, Jiewen, Chaganty, Rangasai V,
Kuo, Donald, Kumar, Chandana C, Palakshareddy, Lavanya C,
Palakshareddy, Lavanya C
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Lin, JackX <jackx.lin@intel.com>
> Sent: Monday, August 8, 2022 4:21 PM
> To: devel@edk2.groups.io
> Cc: Lin, JackX <jackx.lin@intel.com>; Lin, JackX <jackx.lin@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Ni, Ray <ray.ni@intel.com>; Chaganty,
> Rangasai V <rangasai.v.chaganty@intel.com>; Kuo, Donald
> <donald.kuo@intel.com>; Kumar, Chandana C
> <chandana.c.kumar@intel.com>; Palakshareddy; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>
> Subject: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU
> default fused in MADT
>
> BIOS should not reordering cpu processor_uid
>
> Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Dong Eric <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Donald Kuo <Donald.Kuo@intel.com>
> Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
> Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Cc: JackX Lin <JackX.Lin@intel.com>
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 174
> +++++++++++++++++++++++++++++++++++++++-------------------------------
> ----------------------------------------------------------------------------------------------
> ----------
> 1 file changed, 39 insertions(+), 135 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index c7e87cbd7d..176e422e81 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -57,38 +57,9 @@ BOOLEAN mForceX2ApicId;
> BOOLEAN mX2ApicEnabled;
>
> EFI_MP_SERVICES_PROTOCOL *mMpService;
> -BOOLEAN mCpuOrderSorted;
> -EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL;
> UINTN mNumberOfCpus = 0;
> UINTN mNumberOfEnabledCPUs = 0;
>
> -
> -/**
> - The function is called by PerformQuickSort to compare int values.
> -
> - @param[in] Left The pointer to first buffer.
> - @param[in] Right The pointer to second buffer.
> -
> - @return -1 Buffer1 is less than Buffer2.
> - @return 1 Buffer1 is greater than Buffer2.
> -
> -**/
> -INTN
> -EFIAPI
> -ApicIdCompareFunction (
> - IN CONST VOID *Left,
> - IN CONST VOID *Right
> - )
> -{
> - UINT32 LeftApicId;
> - UINT32 RightApicId;
> -
> - LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
> - RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
> -
> - return (LeftApicId > RightApicId)? 1 : (-1);
> -}
> -
> /**
> Print Cpu Apic ID Table
>
> @@ -116,7 +87,8 @@ DebugDisplayReOrderTable (
> EFI_STATUS
> AppendCpuMapTableEntry (
> IN VOID *ApicPtr,
> - IN UINT32 LocalApicCounter
> + IN UINT32 LocalApicCounter,
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> @@ -131,9 +103,9 @@ AppendCpuMapTableEntry (
>
> if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
> if(!mX2ApicEnabled) {
> - LocalApicPtr->Flags =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> - LocalApicPtr->ApicId =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> - LocalApicPtr->AcpiProcessorUid =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> + LocalApicPtr->Flags =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> + LocalApicPtr->ApicId =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].ApicId;
> + LocalApicPtr->AcpiProcessorUid =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> } else {
> LocalApicPtr->Flags = 0;
> LocalApicPtr->ApicId = 0xFF;
> @@ -142,9 +114,9 @@ AppendCpuMapTableEntry (
> }
> } else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
> if(mX2ApicEnabled) {
> - LocalX2ApicPtr->Flags =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> - LocalX2ApicPtr->X2ApicId =
> mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> - LocalX2ApicPtr->AcpiProcessorUid =
> mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> + LocalX2ApicPtr->Flags =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> + LocalX2ApicPtr->X2ApicId =
> CpuApicIdOrderTable[LocalApicCounter].ApicId;
> + LocalX2ApicPtr->AcpiProcessorUid =
> CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> } else {
> LocalX2ApicPtr->Flags = 0;
> LocalX2ApicPtr->X2ApicId = (UINT32)-1;
> @@ -159,32 +131,25 @@ AppendCpuMapTableEntry (
>
> }
>
> +/**
> + Collect all processors information and create a Cpu Apic Id table.
> +
> + @param[in] CpuApicIdOrderTable Buffer to store information of Cpu.
> +**/
> EFI_STATUS
> -SortCpuLocalApicInTable (
> - VOID
> +CreateCpuLocalApicInTable (
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
> UINT32 Index;
> UINT32 CurrProcessor;
> - UINT32 BspApicId;
> - EFI_CPU_ID_ORDER_MAP TempVal;
> EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
> - UINT32 CoreThreadMask;
> - EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
> UINT32 Socket;
>
> - Index = 0;
> Status = EFI_SUCCESS;
>
> - if (mCpuOrderSorted) {
> - return Status;
> - }
> -
> - TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
> -
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++, Index++) {
> Status = mMpService->GetProcessorInfo (
> mMpService,
> @@ -192,7 +157,7 @@ SortCpuLocalApicInTable (
> &ProcessorInfoBuffer
> );
>
> - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &TempCpuApicIdOrderTable[Index];
> + CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &CpuApicIdOrderTable[Index];
> if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
> CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
> CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
> @@ -214,89 +179,26 @@ SortCpuLocalApicInTable (
> } //end if PROC ENABLE
> } //end for CurrentProcessor
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
> mNumOfBitShift));
> - DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
> -
> //
> // Get Bsp Apic Id
> //
> - BspApicId = GetApicId ();
> - DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
> -
> - //
> - //check to see if 1st entry is BSP, if not swap it
> - //
> - if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
> (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
> - CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[Index],
> &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - break;
> - }
> - }
> -
> - if (mNumberOfCpus <= Index) {
> - DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
> Bufferflow\n"));
> - return EFI_INVALID_PARAMETER;
> - }
> - }
> -
> - //
> - // 1. Sort TempCpuApicIdOrderTable,
> - // sort it by using ApicId from minimum to maximum (Socket0 to SocketN),
> and the BSP must in the fist location of the table.
> - // So, start sorting the table from the second element and total elements
> are mNumberOfCpus-1.
> - //
> - PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 1),
> sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE)
> ApicIdCompareFunction);
> -
> - //
> - // 2. Sort and map the primary threads to the front of the
> CpuApicIdOrderTable
> - //
> - for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> + DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
>
> - //
> - // 3. Sort and map the second threads to the middle of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
>
> //
> - // 4. Sort and map the not enabled threads to the bottom of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - //
> - // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
> + // Fill in AcpiProcessorUid.
> //
> for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount);
> Socket++) {
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++) {
> - if (mCpuApicIdOrderTable[CurrProcessor].Flags &&
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> - mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> Index;
> + if (CpuApicIdOrderTable[CurrProcessor].Flags &&
> (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> + CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> Index;
> Index++;
> }
> }
> }
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
> - DebugDisplayReOrderTable (mCpuApicIdOrderTable);
> -
> - mCpuOrderSorted = TRUE;
> + DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> mNumOfBitShift = %x\n", mNumOfBitShift));
> + DebugDisplayReOrderTable (CpuApicIdOrderTable);
>
> return Status;
> }
> @@ -750,6 +652,7 @@ InstallMadtFromScratch (
> EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE LocalApciNmiStruct;
> EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE
> ProcLocalX2ApicStruct;
> EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE
> LocalX2ApicNmiStruct;
> + EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable;
> STRUCTURE_HEADER **MadtStructs;
> UINTN MaxMadtStructCount;
> UINTN MadtStructsIndex;
> @@ -760,18 +663,19 @@ InstallMadtFromScratch (
>
> MadtStructs = NULL;
> NewMadtTable = NULL;
> + CpuApicIdOrderTable = NULL;
> MaxMadtStructCount = 0;
>
> - mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - if (mCpuApicIdOrderTable == NULL) {
> - DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable
> structure pointer array\n"));
> + CpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> + if (CpuApicIdOrderTable == NULL) {
> + DEBUG ((DEBUG_ERROR, "Could not allocate CpuApicIdOrderTable
> structure pointer array\n"));
> return EFI_OUT_OF_RESOURCES;
> }
>
> // Call for Local APIC ID Reorder
> - Status = SortCpuLocalApicInTable ();
> + Status = CreateCpuLocalApicInTable (CpuApicIdOrderTable);
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
> + DEBUG ((DEBUG_ERROR, "CreateCpuLocalApicInTable failed: %r\n",
> Status));
> goto Done;
> }
>
> @@ -824,10 +728,10 @@ InstallMadtFromScratch (
> // APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
> // use a processor local x2APIC structure.
> //
> - if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> - ProcLocalApicStruct.Flags = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> - ProcLocalApicStruct.ApicId = (UINT8)
> mCpuApicIdOrderTable[Index].ApicId;
> - ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> + if (!mX2ApicEnabled && CpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> + ProcLocalApicStruct.Flags = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> + ProcLocalApicStruct.ApicId = (UINT8)
> CpuApicIdOrderTable[Index].ApicId;
> + ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
>
> ASSERT (MadtStructsIndex < MaxMadtStructCount);
> Status = CopyStructure (
> @@ -835,10 +739,10 @@ InstallMadtFromScratch (
> (STRUCTURE_HEADER *) &ProcLocalApicStruct,
> &MadtStructs[MadtStructsIndex++]
> );
> - } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> - ProcLocalX2ApicStruct.Flags = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> - ProcLocalX2ApicStruct.X2ApicId =
> mCpuApicIdOrderTable[Index].ApicId;
> - ProcLocalX2ApicStruct.AcpiProcessorUid =
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> + } else if (CpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> + ProcLocalX2ApicStruct.Flags = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> + ProcLocalX2ApicStruct.X2ApicId =
> CpuApicIdOrderTable[Index].ApicId;
> + ProcLocalX2ApicStruct.AcpiProcessorUid =
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
>
> ASSERT (MadtStructsIndex < MaxMadtStructCount);
> Status = CopyStructure (
> @@ -1033,8 +937,8 @@ Done:
> FreePool (NewMadtTable);
> }
>
> - if (mCpuApicIdOrderTable != NULL) {
> - FreePool (mCpuApicIdOrderTable);
> + if (CpuApicIdOrderTable != NULL) {
> + FreePool (CpuApicIdOrderTable);
> }
>
> return Status;
> --
> 2.32.0.windows.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
2022-08-09 7:12 ` Ni, Ray
@ 2022-08-09 7:52 ` Donald Kuo
0 siblings, 0 replies; 16+ messages in thread
From: Donald Kuo @ 2022-08-09 7:52 UTC (permalink / raw)
To: Ni, Ray, Lin, JackX, devel@edk2.groups.io
Cc: Chiu, Chasel, Dong, Eric, Yao, Jiewen, Chaganty, Rangasai V,
Kumar, Chandana C, Palakshareddy, Lavanya C,
Palakshareddy, Lavanya C
Thanks Jack
Looks good. Reviewed.
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Tuesday, August 9, 2022 3:13 PM
To: Lin, JackX <jackx.lin@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Kuo, Donald <donald.kuo@intel.com>; Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Subject: RE: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Lin, JackX <jackx.lin@intel.com>
> Sent: Monday, August 8, 2022 4:21 PM
> To: devel@edk2.groups.io
> Cc: Lin, JackX <jackx.lin@intel.com>; Lin, JackX
> <jackx.lin@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Dong,
> Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Kuo, Donald <donald.kuo@intel.com>;
> Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy;
> Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Subject: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU
> default fused in MADT
>
> BIOS should not reordering cpu processor_uid
>
> Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Dong Eric <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Donald Kuo <Donald.Kuo@intel.com>
> Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
> Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Cc: JackX Lin <JackX.Lin@intel.com>
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 174
> +++++++++++++++++++++++++++++++++++++++-------------------------------
> ----------------------------------------------------------------------
> ------------------------
> ----------
> 1 file changed, 39 insertions(+), 135 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index c7e87cbd7d..176e422e81 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -57,38 +57,9 @@ BOOLEAN mForceX2ApicId;
> BOOLEAN mX2ApicEnabled;
>
> EFI_MP_SERVICES_PROTOCOL *mMpService;
> -BOOLEAN mCpuOrderSorted;
> -EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL;
> UINTN mNumberOfCpus = 0;
> UINTN mNumberOfEnabledCPUs = 0;
>
> -
> -/**
> - The function is called by PerformQuickSort to compare int values.
> -
> - @param[in] Left The pointer to first buffer.
> - @param[in] Right The pointer to second buffer.
> -
> - @return -1 Buffer1 is less than Buffer2.
> - @return 1 Buffer1 is greater than Buffer2.
> -
> -**/
> -INTN
> -EFIAPI
> -ApicIdCompareFunction (
> - IN CONST VOID *Left,
> - IN CONST VOID *Right
> - )
> -{
> - UINT32 LeftApicId;
> - UINT32 RightApicId;
> -
> - LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
> - RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
> -
> - return (LeftApicId > RightApicId)? 1 : (-1); -}
> -
> /**
> Print Cpu Apic ID Table
>
> @@ -116,7 +87,8 @@ DebugDisplayReOrderTable ( EFI_STATUS
> AppendCpuMapTableEntry (
> IN VOID *ApicPtr,
> - IN UINT32 LocalApicCounter
> + IN UINT32 LocalApicCounter,
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> @@ -131,9 +103,9 @@ AppendCpuMapTableEntry (
>
> if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
> if(!mX2ApicEnabled) {
> - LocalApicPtr->Flags =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> - LocalApicPtr->ApicId =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> - LocalApicPtr->AcpiProcessorUid =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> + LocalApicPtr->Flags =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> + LocalApicPtr->ApicId =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].ApicId;
> + LocalApicPtr->AcpiProcessorUid =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> } else {
> LocalApicPtr->Flags = 0;
> LocalApicPtr->ApicId = 0xFF;
> @@ -142,9 +114,9 @@ AppendCpuMapTableEntry (
> }
> } else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
> if(mX2ApicEnabled) {
> - LocalX2ApicPtr->Flags =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> - LocalX2ApicPtr->X2ApicId =
> mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> - LocalX2ApicPtr->AcpiProcessorUid =
> mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> + LocalX2ApicPtr->Flags =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> + LocalX2ApicPtr->X2ApicId =
> CpuApicIdOrderTable[LocalApicCounter].ApicId;
> + LocalX2ApicPtr->AcpiProcessorUid =
> CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> } else {
> LocalX2ApicPtr->Flags = 0;
> LocalX2ApicPtr->X2ApicId = (UINT32)-1;
> @@ -159,32 +131,25 @@ AppendCpuMapTableEntry (
>
> }
>
> +/**
> + Collect all processors information and create a Cpu Apic Id table.
> +
> + @param[in] CpuApicIdOrderTable Buffer to store information of Cpu.
> +**/
> EFI_STATUS
> -SortCpuLocalApicInTable (
> - VOID
> +CreateCpuLocalApicInTable (
> + IN EFI_CPU_ID_ORDER_MAP *3
> )
> {
> EFI_STATUS Status;
> EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
> UINT32 Index;
> UINT32 CurrProcessor;
> - UINT32 BspApicId;
> - EFI_CPU_ID_ORDER_MAP TempVal;
> EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
> - UINT32 CoreThreadMask;
> - EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
> UINT32 Socket;
>
> - Index = 0;
> Status = EFI_SUCCESS;
>
> - if (mCpuOrderSorted) {
> - return Status;
> - }
> -
> - TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
> -
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++, Index++) {
> Status = mMpService->GetProcessorInfo (
> mMpService, @@ -192,7 +157,7 @@
> SortCpuLocalApicInTable (
> &ProcessorInfoBuffer
> );
>
> - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &TempCpuApicIdOrderTable[Index];
> + CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &CpuApicIdOrderTable[Index];
> if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
> CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
> CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
> @@ -214,89 +179,26 @@ SortCpuLocalApicInTable (
> } //end if PROC ENABLE
> } //end for CurrentProcessor
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
> mNumOfBitShift));
> - DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
> -
> //
> // Get Bsp Apic Id
> //
> - BspApicId = GetApicId ();
> - DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
> -
> - //
> - //check to see if 1st entry is BSP, if not swap it
> - //
> - if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
> (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
> - CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[Index],
> &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - break;
> - }
> - }
> -
> - if (mNumberOfCpus <= Index) {
> - DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
> Bufferflow\n"));
> - return EFI_INVALID_PARAMETER;
> - }
> - }
> -
> - //
> - // 1. Sort TempCpuApicIdOrderTable,
> - // sort it by using ApicId from minimum to maximum (Socket0 to SocketN),
> and the BSP must in the fist location of the table.
> - // So, start sorting the table from the second element and total elements
> are mNumberOfCpus-1.
> - //
> - PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus -
> 1), sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE)
> ApicIdCompareFunction);
> -
> - //
> - // 2. Sort and map the primary threads to the front of the
> CpuApicIdOrderTable
> - //
> - for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> + DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
>
> - //
> - // 3. Sort and map the second threads to the middle of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
>
> //
> - // 4. Sort and map the not enabled threads to the bottom of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - //
> - // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
> + // Fill in AcpiProcessorUid.
> //
> for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount);
> Socket++) {
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++) {
> - if (mCpuApicIdOrderTable[CurrProcessor].Flags &&
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> - mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> Index;
> + if (CpuApicIdOrderTable[CurrProcessor].Flags &&
> (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> + CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> Index;
> Index++;
> }
> }
> }
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
> - DebugDisplayReOrderTable (mCpuApicIdOrderTable);
> -
> - mCpuOrderSorted = TRUE;
> + DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> mNumOfBitShift = %x\n", mNumOfBitShift));
> + DebugDisplayReOrderTable (CpuApicIdOrderTable);
>
> return Status;
> }
> @@ -750,6 +652,7 @@ InstallMadtFromScratch (
> EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE LocalApciNmiStruct;
> EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE
> ProcLocalX2ApicStruct;
> EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE
> LocalX2ApicNmiStruct;
> + EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable;
> STRUCTURE_HEADER **MadtStructs;
> UINTN MaxMadtStructCount;
> UINTN MadtStructsIndex;
> @@ -760,18 +663,19 @@ InstallMadtFromScratch (
>
> MadtStructs = NULL;
> NewMadtTable = NULL;
> + CpuApicIdOrderTable = NULL;
> MaxMadtStructCount = 0;
>
> - mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - if (mCpuApicIdOrderTable == NULL) {
> - DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable
> structure pointer array\n"));
> + CpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> + if (CpuApicIdOrderTable == NULL) {
> + DEBUG ((DEBUG_ERROR, "Could not allocate CpuApicIdOrderTable
> structure pointer array\n"));
> return EFI_OUT_OF_RESOURCES;
> }
>
> // Call for Local APIC ID Reorder
> - Status = SortCpuLocalApicInTable ();
> + Status = CreateCpuLocalApicInTable (CpuApicIdOrderTable);
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
> + DEBUG ((DEBUG_ERROR, "CreateCpuLocalApicInTable failed: %r\n",
> Status));
> goto Done;
> }
>
> @@ -824,10 +728,10 @@ InstallMadtFromScratch (
> // APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
> // use a processor local x2APIC structure.
> //
> - if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> - ProcLocalApicStruct.Flags = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> - ProcLocalApicStruct.ApicId = (UINT8)
> mCpuApicIdOrderTable[Index].ApicId;
> - ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> + if (!mX2ApicEnabled && CpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> + ProcLocalApicStruct.Flags = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> + ProcLocalApicStruct.ApicId = (UINT8)
> CpuApicIdOrderTable[Index].ApicId;
> + ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
>
> ASSERT (MadtStructsIndex < MaxMadtStructCount);
> Status = CopyStructure (
> @@ -835,10 +739,10 @@ InstallMadtFromScratch (
> (STRUCTURE_HEADER *) &ProcLocalApicStruct,
> &MadtStructs[MadtStructsIndex++]
> );
> - } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> - ProcLocalX2ApicStruct.Flags = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> - ProcLocalX2ApicStruct.X2ApicId =
> mCpuApicIdOrderTable[Index].ApicId;
> - ProcLocalX2ApicStruct.AcpiProcessorUid =
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> + } else if (CpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> + ProcLocalX2ApicStruct.Flags = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> + ProcLocalX2ApicStruct.X2ApicId =
> CpuApicIdOrderTable[Index].ApicId;
> + ProcLocalX2ApicStruct.AcpiProcessorUid =
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
>
> ASSERT (MadtStructsIndex < MaxMadtStructCount);
> Status = CopyStructure (
> @@ -1033,8 +937,8 @@ Done:
> FreePool (NewMadtTable);
> }
>
> - if (mCpuApicIdOrderTable != NULL) {
> - FreePool (mCpuApicIdOrderTable);
> + if (CpuApicIdOrderTable != NULL) {
> + FreePool (CpuApicIdOrderTable);
> }
>
> return Status;
> --
> 2.32.0.windows.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
2022-08-08 8:21 JackX Lin
2022-08-09 7:12 ` Ni, Ray
@ 2022-08-10 3:47 ` Ni, Ray
2022-08-10 3:51 ` JackX Lin
1 sibling, 1 reply; 16+ messages in thread
From: Ni, Ray @ 2022-08-10 3:47 UTC (permalink / raw)
To: Lin, JackX, Sinha, Ankit
Cc: Chiu, Chasel, Dong, Eric, Yao, Jiewen, Chaganty, Rangasai V,
devel@edk2.groups.io, Kuo, Donald, Kumar, Chandana C,
Palakshareddy, Lavanya C, Palakshareddy, Lavanya C
Jack,
Your patch cannot be merged to trunk because Ankit just did some change in the same C file, in below commit.
* MinPlatformPkg/AcpiTables: Add additional thread mapping in MADT
Ankit,
It seems your patch is to add support for thread #2 and #3. Jack's patch is to remove the additional sorting that put
secondary threads after first threads.
Do you see an issue that we remove the thread sorting logic?
Thanks,
Ray
> -----Original Message-----
> From: Lin, JackX <jackx.lin@intel.com>
> Sent: Monday, August 8, 2022 4:21 PM
> To: devel@edk2.groups.io
> Cc: Lin, JackX <jackx.lin@intel.com>; Lin, JackX <jackx.lin@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Ni, Ray <ray.ni@intel.com>; Chaganty,
> Rangasai V <rangasai.v.chaganty@intel.com>; Kuo, Donald
> <donald.kuo@intel.com>; Kumar, Chandana C
> <chandana.c.kumar@intel.com>; Palakshareddy; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>
> Subject: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU
> default fused in MADT
>
> BIOS should not reordering cpu processor_uid
>
> Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Dong Eric <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Donald Kuo <Donald.Kuo@intel.com>
> Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
> Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Cc: JackX Lin <JackX.Lin@intel.com>
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 174
> +++++++++++++++++++++++++++++++++++++++-------------------------------
> ----------------------------------------------------------------------------------------------
> ----------
> 1 file changed, 39 insertions(+), 135 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index c7e87cbd7d..176e422e81 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -57,38 +57,9 @@ BOOLEAN mForceX2ApicId;
> BOOLEAN mX2ApicEnabled;
>
> EFI_MP_SERVICES_PROTOCOL *mMpService;
> -BOOLEAN mCpuOrderSorted;
> -EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL;
> UINTN mNumberOfCpus = 0;
> UINTN mNumberOfEnabledCPUs = 0;
>
> -
> -/**
> - The function is called by PerformQuickSort to compare int values.
> -
> - @param[in] Left The pointer to first buffer.
> - @param[in] Right The pointer to second buffer.
> -
> - @return -1 Buffer1 is less than Buffer2.
> - @return 1 Buffer1 is greater than Buffer2.
> -
> -**/
> -INTN
> -EFIAPI
> -ApicIdCompareFunction (
> - IN CONST VOID *Left,
> - IN CONST VOID *Right
> - )
> -{
> - UINT32 LeftApicId;
> - UINT32 RightApicId;
> -
> - LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
> - RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
> -
> - return (LeftApicId > RightApicId)? 1 : (-1);
> -}
> -
> /**
> Print Cpu Apic ID Table
>
> @@ -116,7 +87,8 @@ DebugDisplayReOrderTable (
> EFI_STATUS
> AppendCpuMapTableEntry (
> IN VOID *ApicPtr,
> - IN UINT32 LocalApicCounter
> + IN UINT32 LocalApicCounter,
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> @@ -131,9 +103,9 @@ AppendCpuMapTableEntry (
>
> if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
> if(!mX2ApicEnabled) {
> - LocalApicPtr->Flags =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> - LocalApicPtr->ApicId =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> - LocalApicPtr->AcpiProcessorUid =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> + LocalApicPtr->Flags =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> + LocalApicPtr->ApicId =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].ApicId;
> + LocalApicPtr->AcpiProcessorUid =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> } else {
> LocalApicPtr->Flags = 0;
> LocalApicPtr->ApicId = 0xFF;
> @@ -142,9 +114,9 @@ AppendCpuMapTableEntry (
> }
> } else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
> if(mX2ApicEnabled) {
> - LocalX2ApicPtr->Flags =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> - LocalX2ApicPtr->X2ApicId =
> mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> - LocalX2ApicPtr->AcpiProcessorUid =
> mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> + LocalX2ApicPtr->Flags =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> + LocalX2ApicPtr->X2ApicId =
> CpuApicIdOrderTable[LocalApicCounter].ApicId;
> + LocalX2ApicPtr->AcpiProcessorUid =
> CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> } else {
> LocalX2ApicPtr->Flags = 0;
> LocalX2ApicPtr->X2ApicId = (UINT32)-1;
> @@ -159,32 +131,25 @@ AppendCpuMapTableEntry (
>
> }
>
> +/**
> + Collect all processors information and create a Cpu Apic Id table.
> +
> + @param[in] CpuApicIdOrderTable Buffer to store information of Cpu.
> +**/
> EFI_STATUS
> -SortCpuLocalApicInTable (
> - VOID
> +CreateCpuLocalApicInTable (
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
> UINT32 Index;
> UINT32 CurrProcessor;
> - UINT32 BspApicId;
> - EFI_CPU_ID_ORDER_MAP TempVal;
> EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
> - UINT32 CoreThreadMask;
> - EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
> UINT32 Socket;
>
> - Index = 0;
> Status = EFI_SUCCESS;
>
> - if (mCpuOrderSorted) {
> - return Status;
> - }
> -
> - TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
> -
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++, Index++) {
> Status = mMpService->GetProcessorInfo (
> mMpService,
> @@ -192,7 +157,7 @@ SortCpuLocalApicInTable (
> &ProcessorInfoBuffer
> );
>
> - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &TempCpuApicIdOrderTable[Index];
> + CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &CpuApicIdOrderTable[Index];
> if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
> CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
> CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
> @@ -214,89 +179,26 @@ SortCpuLocalApicInTable (
> } //end if PROC ENABLE
> } //end for CurrentProcessor
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
> mNumOfBitShift));
> - DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
> -
> //
> // Get Bsp Apic Id
> //
> - BspApicId = GetApicId ();
> - DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
> -
> - //
> - //check to see if 1st entry is BSP, if not swap it
> - //
> - if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
> (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
> - CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[Index],
> &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - break;
> - }
> - }
> -
> - if (mNumberOfCpus <= Index) {
> - DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
> Bufferflow\n"));
> - return EFI_INVALID_PARAMETER;
> - }
> - }
> -
> - //
> - // 1. Sort TempCpuApicIdOrderTable,
> - // sort it by using ApicId from minimum to maximum (Socket0 to SocketN),
> and the BSP must in the fist location of the table.
> - // So, start sorting the table from the second element and total elements
> are mNumberOfCpus-1.
> - //
> - PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 1),
> sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE)
> ApicIdCompareFunction);
> -
> - //
> - // 2. Sort and map the primary threads to the front of the
> CpuApicIdOrderTable
> - //
> - for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> + DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
>
> - //
> - // 3. Sort and map the second threads to the middle of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
>
> //
> - // 4. Sort and map the not enabled threads to the bottom of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - //
> - // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
> + // Fill in AcpiProcessorUid.
> //
> for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount);
> Socket++) {
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++) {
> - if (mCpuApicIdOrderTable[CurrProcessor].Flags &&
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> - mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> Index;
> + if (CpuApicIdOrderTable[CurrProcessor].Flags &&
> (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> + CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> Index;
> Index++;
> }
> }
> }
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
> - DebugDisplayReOrderTable (mCpuApicIdOrderTable);
> -
> - mCpuOrderSorted = TRUE;
> + DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> mNumOfBitShift = %x\n", mNumOfBitShift));
> + DebugDisplayReOrderTable (CpuApicIdOrderTable);
>
> return Status;
> }
> @@ -750,6 +652,7 @@ InstallMadtFromScratch (
> EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE LocalApciNmiStruct;
> EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE
> ProcLocalX2ApicStruct;
> EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE
> LocalX2ApicNmiStruct;
> + EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable;
> STRUCTURE_HEADER **MadtStructs;
> UINTN MaxMadtStructCount;
> UINTN MadtStructsIndex;
> @@ -760,18 +663,19 @@ InstallMadtFromScratch (
>
> MadtStructs = NULL;
> NewMadtTable = NULL;
> + CpuApicIdOrderTable = NULL;
> MaxMadtStructCount = 0;
>
> - mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - if (mCpuApicIdOrderTable == NULL) {
> - DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable
> structure pointer array\n"));
> + CpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> + if (CpuApicIdOrderTable == NULL) {
> + DEBUG ((DEBUG_ERROR, "Could not allocate CpuApicIdOrderTable
> structure pointer array\n"));
> return EFI_OUT_OF_RESOURCES;
> }
>
> // Call for Local APIC ID Reorder
> - Status = SortCpuLocalApicInTable ();
> + Status = CreateCpuLocalApicInTable (CpuApicIdOrderTable);
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
> + DEBUG ((DEBUG_ERROR, "CreateCpuLocalApicInTable failed: %r\n",
> Status));
> goto Done;
> }
>
> @@ -824,10 +728,10 @@ InstallMadtFromScratch (
> // APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
> // use a processor local x2APIC structure.
> //
> - if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> - ProcLocalApicStruct.Flags = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> - ProcLocalApicStruct.ApicId = (UINT8)
> mCpuApicIdOrderTable[Index].ApicId;
> - ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> + if (!mX2ApicEnabled && CpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> + ProcLocalApicStruct.Flags = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> + ProcLocalApicStruct.ApicId = (UINT8)
> CpuApicIdOrderTable[Index].ApicId;
> + ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
>
> ASSERT (MadtStructsIndex < MaxMadtStructCount);
> Status = CopyStructure (
> @@ -835,10 +739,10 @@ InstallMadtFromScratch (
> (STRUCTURE_HEADER *) &ProcLocalApicStruct,
> &MadtStructs[MadtStructsIndex++]
> );
> - } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> - ProcLocalX2ApicStruct.Flags = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> - ProcLocalX2ApicStruct.X2ApicId =
> mCpuApicIdOrderTable[Index].ApicId;
> - ProcLocalX2ApicStruct.AcpiProcessorUid =
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> + } else if (CpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> + ProcLocalX2ApicStruct.Flags = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> + ProcLocalX2ApicStruct.X2ApicId =
> CpuApicIdOrderTable[Index].ApicId;
> + ProcLocalX2ApicStruct.AcpiProcessorUid =
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
>
> ASSERT (MadtStructsIndex < MaxMadtStructCount);
> Status = CopyStructure (
> @@ -1033,8 +937,8 @@ Done:
> FreePool (NewMadtTable);
> }
>
> - if (mCpuApicIdOrderTable != NULL) {
> - FreePool (mCpuApicIdOrderTable);
> + if (CpuApicIdOrderTable != NULL) {
> + FreePool (CpuApicIdOrderTable);
> }
>
> return Status;
> --
> 2.32.0.windows.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
2022-08-10 3:47 ` Ni, Ray
@ 2022-08-10 3:51 ` JackX Lin
2022-08-10 3:52 ` Ni, Ray
2022-08-10 4:32 ` JackX Lin
0 siblings, 2 replies; 16+ messages in thread
From: JackX Lin @ 2022-08-10 3:51 UTC (permalink / raw)
To: Ni, Ray, Sinha, Ankit
Cc: Chiu, Chasel, Dong, Eric, Yao, Jiewen, Chaganty, Rangasai V,
devel@edk2.groups.io, Kuo, Donald, Kumar, Chandana C,
Palakshareddy, Lavanya C, Palakshareddy, Lavanya C
Hi Ray,
I know this patch, and the thread 2 and 3 are added by my request for a reason on that time.
I will re-sent the code patch.
Thank you.
Jack
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, August 10, 2022 11:48 AM
To: Lin, JackX <jackx.lin@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; devel@edk2.groups.io; Kuo, Donald <donald.kuo@intel.com>; Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Subject: RE: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
Jack,
Your patch cannot be merged to trunk because Ankit just did some change in the same C file, in below commit.
* MinPlatformPkg/AcpiTables: Add additional thread mapping in MADT
Ankit,
It seems your patch is to add support for thread #2 and #3. Jack's patch is to remove the additional sorting that put secondary threads after first threads.
Do you see an issue that we remove the thread sorting logic?
Thanks,
Ray
> -----Original Message-----
> From: Lin, JackX <jackx.lin@intel.com>
> Sent: Monday, August 8, 2022 4:21 PM
> To: devel@edk2.groups.io
> Cc: Lin, JackX <jackx.lin@intel.com>; Lin, JackX
> <jackx.lin@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Dong,
> Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Kuo, Donald <donald.kuo@intel.com>;
> Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy;
> Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Subject: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU
> default fused in MADT
>
> BIOS should not reordering cpu processor_uid
>
> Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Dong Eric <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Donald Kuo <Donald.Kuo@intel.com>
> Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
> Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Cc: JackX Lin <JackX.Lin@intel.com>
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 174
> +++++++++++++++++++++++++++++++++++++++-------------------------------
> ----------------------------------------------------------------------
> ------------------------
> ----------
> 1 file changed, 39 insertions(+), 135 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index c7e87cbd7d..176e422e81 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -57,38 +57,9 @@ BOOLEAN mForceX2ApicId;
> BOOLEAN mX2ApicEnabled;
>
> EFI_MP_SERVICES_PROTOCOL *mMpService;
> -BOOLEAN mCpuOrderSorted;
> -EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL;
> UINTN mNumberOfCpus = 0;
> UINTN mNumberOfEnabledCPUs = 0;
>
> -
> -/**
> - The function is called by PerformQuickSort to compare int values.
> -
> - @param[in] Left The pointer to first buffer.
> - @param[in] Right The pointer to second buffer.
> -
> - @return -1 Buffer1 is less than Buffer2.
> - @return 1 Buffer1 is greater than Buffer2.
> -
> -**/
> -INTN
> -EFIAPI
> -ApicIdCompareFunction (
> - IN CONST VOID *Left,
> - IN CONST VOID *Right
> - )
> -{
> - UINT32 LeftApicId;
> - UINT32 RightApicId;
> -
> - LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
> - RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
> -
> - return (LeftApicId > RightApicId)? 1 : (-1); -}
> -
> /**
> Print Cpu Apic ID Table
>
> @@ -116,7 +87,8 @@ DebugDisplayReOrderTable ( EFI_STATUS
> AppendCpuMapTableEntry (
> IN VOID *ApicPtr,
> - IN UINT32 LocalApicCounter
> + IN UINT32 LocalApicCounter,
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> @@ -131,9 +103,9 @@ AppendCpuMapTableEntry (
>
> if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
> if(!mX2ApicEnabled) {
> - LocalApicPtr->Flags =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> - LocalApicPtr->ApicId =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> - LocalApicPtr->AcpiProcessorUid =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> + LocalApicPtr->Flags =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> + LocalApicPtr->ApicId =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].ApicId;
> + LocalApicPtr->AcpiProcessorUid =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> } else {
> LocalApicPtr->Flags = 0;
> LocalApicPtr->ApicId = 0xFF;
> @@ -142,9 +114,9 @@ AppendCpuMapTableEntry (
> }
> } else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
> if(mX2ApicEnabled) {
> - LocalX2ApicPtr->Flags =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> - LocalX2ApicPtr->X2ApicId =
> mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> - LocalX2ApicPtr->AcpiProcessorUid =
> mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> + LocalX2ApicPtr->Flags =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> + LocalX2ApicPtr->X2ApicId =
> CpuApicIdOrderTable[LocalApicCounter].ApicId;
> + LocalX2ApicPtr->AcpiProcessorUid =
> CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> } else {
> LocalX2ApicPtr->Flags = 0;
> LocalX2ApicPtr->X2ApicId = (UINT32)-1;
> @@ -159,32 +131,25 @@ AppendCpuMapTableEntry (
>
> }
>
> +/**
> + Collect all processors information and create a Cpu Apic Id table.
> +
> + @param[in] CpuApicIdOrderTable Buffer to store information of Cpu.
> +**/
> EFI_STATUS
> -SortCpuLocalApicInTable (
> - VOID
> +CreateCpuLocalApicInTable (
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
> UINT32 Index;
> UINT32 CurrProcessor;
> - UINT32 BspApicId;
> - EFI_CPU_ID_ORDER_MAP TempVal;
> EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
> - UINT32 CoreThreadMask;
> - EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
> UINT32 Socket;
>
> - Index = 0;
> Status = EFI_SUCCESS;
>
> - if (mCpuOrderSorted) {
> - return Status;
> - }
> -
> - TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
> -
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++, Index++) {
> Status = mMpService->GetProcessorInfo (
> mMpService, @@ -192,7 +157,7 @@
> SortCpuLocalApicInTable (
> &ProcessorInfoBuffer
> );
>
> - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &TempCpuApicIdOrderTable[Index];
> + CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &CpuApicIdOrderTable[Index];
> if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
> CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
> CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
> @@ -214,89 +179,26 @@ SortCpuLocalApicInTable (
> } //end if PROC ENABLE
> } //end for CurrentProcessor
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
> mNumOfBitShift));
> - DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
> -
> //
> // Get Bsp Apic Id
> //
> - BspApicId = GetApicId ();
> - DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
> -
> - //
> - //check to see if 1st entry is BSP, if not swap it
> - //
> - if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
> (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
> - CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[Index],
> &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - break;
> - }
> - }
> -
> - if (mNumberOfCpus <= Index) {
> - DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
> Bufferflow\n"));
> - return EFI_INVALID_PARAMETER;
> - }
> - }
> -
> - //
> - // 1. Sort TempCpuApicIdOrderTable,
> - // sort it by using ApicId from minimum to maximum (Socket0 to SocketN),
> and the BSP must in the fist location of the table.
> - // So, start sorting the table from the second element and total elements
> are mNumberOfCpus-1.
> - //
> - PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus -
> 1), sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE)
> ApicIdCompareFunction);
> -
> - //
> - // 2. Sort and map the primary threads to the front of the
> CpuApicIdOrderTable
> - //
> - for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> + DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
>
> - //
> - // 3. Sort and map the second threads to the middle of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
>
> //
> - // 4. Sort and map the not enabled threads to the bottom of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - //
> - // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
> + // Fill in AcpiProcessorUid.
> //
> for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount);
> Socket++) {
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++) {
> - if (mCpuApicIdOrderTable[CurrProcessor].Flags &&
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> - mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> Index;
> + if (CpuApicIdOrderTable[CurrProcessor].Flags &&
> (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> + CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> Index;
> Index++;
> }
> }
> }
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
> - DebugDisplayReOrderTable (mCpuApicIdOrderTable);
> -
> - mCpuOrderSorted = TRUE;
> + DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> mNumOfBitShift = %x\n", mNumOfBitShift));
> + DebugDisplayReOrderTable (CpuApicIdOrderTable);
>
> return Status;
> }
> @@ -750,6 +652,7 @@ InstallMadtFromScratch (
> EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE LocalApciNmiStruct;
> EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE
> ProcLocalX2ApicStruct;
> EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE
> LocalX2ApicNmiStruct;
> + EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable;
> STRUCTURE_HEADER **MadtStructs;
> UINTN MaxMadtStructCount;
> UINTN MadtStructsIndex;
> @@ -760,18 +663,19 @@ InstallMadtFromScratch (
>
> MadtStructs = NULL;
> NewMadtTable = NULL;
> + CpuApicIdOrderTable = NULL;
> MaxMadtStructCount = 0;
>
> - mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - if (mCpuApicIdOrderTable == NULL) {
> - DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable
> structure pointer array\n"));
> + CpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> + if (CpuApicIdOrderTable == NULL) {
> + DEBUG ((DEBUG_ERROR, "Could not allocate CpuApicIdOrderTable
> structure pointer array\n"));
> return EFI_OUT_OF_RESOURCES;
> }
>
> // Call for Local APIC ID Reorder
> - Status = SortCpuLocalApicInTable ();
> + Status = CreateCpuLocalApicInTable (CpuApicIdOrderTable);
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
> + DEBUG ((DEBUG_ERROR, "CreateCpuLocalApicInTable failed: %r\n",
> Status));
> goto Done;
> }
>
> @@ -824,10 +728,10 @@ InstallMadtFromScratch (
> // APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
> // use a processor local x2APIC structure.
> //
> - if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> - ProcLocalApicStruct.Flags = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> - ProcLocalApicStruct.ApicId = (UINT8)
> mCpuApicIdOrderTable[Index].ApicId;
> - ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> + if (!mX2ApicEnabled && CpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> + ProcLocalApicStruct.Flags = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> + ProcLocalApicStruct.ApicId = (UINT8)
> CpuApicIdOrderTable[Index].ApicId;
> + ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
>
> ASSERT (MadtStructsIndex < MaxMadtStructCount);
> Status = CopyStructure (
> @@ -835,10 +739,10 @@ InstallMadtFromScratch (
> (STRUCTURE_HEADER *) &ProcLocalApicStruct,
> &MadtStructs[MadtStructsIndex++]
> );
> - } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> - ProcLocalX2ApicStruct.Flags = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> - ProcLocalX2ApicStruct.X2ApicId =
> mCpuApicIdOrderTable[Index].ApicId;
> - ProcLocalX2ApicStruct.AcpiProcessorUid =
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> + } else if (CpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> + ProcLocalX2ApicStruct.Flags = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> + ProcLocalX2ApicStruct.X2ApicId =
> CpuApicIdOrderTable[Index].ApicId;
> + ProcLocalX2ApicStruct.AcpiProcessorUid =
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
>
> ASSERT (MadtStructsIndex < MaxMadtStructCount);
> Status = CopyStructure (
> @@ -1033,8 +937,8 @@ Done:
> FreePool (NewMadtTable);
> }
>
> - if (mCpuApicIdOrderTable != NULL) {
> - FreePool (mCpuApicIdOrderTable);
> + if (CpuApicIdOrderTable != NULL) {
> + FreePool (CpuApicIdOrderTable);
> }
>
> return Status;
> --
> 2.32.0.windows.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
2022-08-10 3:51 ` JackX Lin
@ 2022-08-10 3:52 ` Ni, Ray
2022-08-10 4:32 ` JackX Lin
1 sibling, 0 replies; 16+ messages in thread
From: Ni, Ray @ 2022-08-10 3:52 UTC (permalink / raw)
To: Lin, JackX, Sinha, Ankit
Cc: Chiu, Chasel, Dong, Eric, Yao, Jiewen, Chaganty, Rangasai V,
devel@edk2.groups.io, Kuo, Donald, Kumar, Chandana C,
Palakshareddy, Lavanya C, Palakshareddy, Lavanya C
Good to know. Thanks😊
> -----Original Message-----
> From: Lin, JackX <jackx.lin@intel.com>
> Sent: Wednesday, August 10, 2022 11:51 AM
> To: Ni, Ray <ray.ni@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; devel@edk2.groups.io; Kuo, Donald
> <donald.kuo@intel.com>; Kumar, Chandana C
> <chandana.c.kumar@intel.com>; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>
> Subject: RE: [edk2-platforms: PATCH] Modify processor _UID ordering by
> CPU default fused in MADT
>
> Hi Ray,
>
> I know this patch, and the thread 2 and 3 are added by my request for a
> reason on that time.
> I will re-sent the code patch.
> Thank you.
>
> Jack
>
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Wednesday, August 10, 2022 11:48 AM
> To: Lin, JackX <jackx.lin@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; devel@edk2.groups.io; Kuo, Donald
> <donald.kuo@intel.com>; Kumar, Chandana C
> <chandana.c.kumar@intel.com>; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>
> Subject: RE: [edk2-platforms: PATCH] Modify processor _UID ordering by
> CPU default fused in MADT
>
> Jack,
> Your patch cannot be merged to trunk because Ankit just did some change in
> the same C file, in below commit.
> * MinPlatformPkg/AcpiTables: Add additional thread mapping in MADT
>
> Ankit,
> It seems your patch is to add support for thread #2 and #3. Jack's patch is to
> remove the additional sorting that put secondary threads after first threads.
> Do you see an issue that we remove the thread sorting logic?
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: Lin, JackX <jackx.lin@intel.com>
> > Sent: Monday, August 8, 2022 4:21 PM
> > To: devel@edk2.groups.io
> > Cc: Lin, JackX <jackx.lin@intel.com>; Lin, JackX
> > <jackx.lin@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Dong,
> > Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ni,
> > Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@intel.com>; Kuo, Donald <donald.kuo@intel.com>;
> > Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy;
> > Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> > Subject: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU
> > default fused in MADT
> >
> > BIOS should not reordering cpu processor_uid
> >
> > Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> > Cc: Chasel Chiu <chasel.chiu@intel.com>
> > Cc: Dong Eric <eric.dong@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> > Cc: Donald Kuo <Donald.Kuo@intel.com>
> > Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
> > Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> > Cc: JackX Lin <JackX.Lin@intel.com>
> > ---
> > Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 174
> > +++++++++++++++++++++++++++++++++++++++-----------------------------
> --
> > ----------------------------------------------------------------------
> > ------------------------
> > ----------
> > 1 file changed, 39 insertions(+), 135 deletions(-)
> >
> > diff --git
> > a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> > b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> > index c7e87cbd7d..176e422e81 100644
> > --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> > @@ -57,38 +57,9 @@ BOOLEAN mForceX2ApicId;
> > BOOLEAN mX2ApicEnabled;
> >
> > EFI_MP_SERVICES_PROTOCOL *mMpService;
> > -BOOLEAN mCpuOrderSorted;
> > -EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL;
> > UINTN mNumberOfCpus = 0;
> > UINTN mNumberOfEnabledCPUs = 0;
> >
> > -
> > -/**
> > - The function is called by PerformQuickSort to compare int values.
> > -
> > - @param[in] Left The pointer to first buffer.
> > - @param[in] Right The pointer to second buffer.
> > -
> > - @return -1 Buffer1 is less than Buffer2.
> > - @return 1 Buffer1 is greater than Buffer2.
> > -
> > -**/
> > -INTN
> > -EFIAPI
> > -ApicIdCompareFunction (
> > - IN CONST VOID *Left,
> > - IN CONST VOID *Right
> > - )
> > -{
> > - UINT32 LeftApicId;
> > - UINT32 RightApicId;
> > -
> > - LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
> > - RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
> > -
> > - return (LeftApicId > RightApicId)? 1 : (-1); -}
> > -
> > /**
> > Print Cpu Apic ID Table
> >
> > @@ -116,7 +87,8 @@ DebugDisplayReOrderTable ( EFI_STATUS
> > AppendCpuMapTableEntry (
> > IN VOID *ApicPtr,
> > - IN UINT32 LocalApicCounter
> > + IN UINT32 LocalApicCounter,
> > + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> > )
> > {
> > EFI_STATUS Status;
> > @@ -131,9 +103,9 @@ AppendCpuMapTableEntry (
> >
> > if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
> > if(!mX2ApicEnabled) {
> > - LocalApicPtr->Flags =
> > (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> > - LocalApicPtr->ApicId =
> > (UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> > - LocalApicPtr->AcpiProcessorUid =
> > (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> > + LocalApicPtr->Flags =
> > (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> > + LocalApicPtr->ApicId =
> > (UINT8)CpuApicIdOrderTable[LocalApicCounter].ApicId;
> > + LocalApicPtr->AcpiProcessorUid =
> > (UINT8)CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> > } else {
> > LocalApicPtr->Flags = 0;
> > LocalApicPtr->ApicId = 0xFF;
> > @@ -142,9 +114,9 @@ AppendCpuMapTableEntry (
> > }
> > } else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
> > if(mX2ApicEnabled) {
> > - LocalX2ApicPtr->Flags =
> > (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> > - LocalX2ApicPtr->X2ApicId =
> > mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> > - LocalX2ApicPtr->AcpiProcessorUid =
> > mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> > + LocalX2ApicPtr->Flags =
> > (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> > + LocalX2ApicPtr->X2ApicId =
> > CpuApicIdOrderTable[LocalApicCounter].ApicId;
> > + LocalX2ApicPtr->AcpiProcessorUid =
> > CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> > } else {
> > LocalX2ApicPtr->Flags = 0;
> > LocalX2ApicPtr->X2ApicId = (UINT32)-1;
> > @@ -159,32 +131,25 @@ AppendCpuMapTableEntry (
> >
> > }
> >
> > +/**
> > + Collect all processors information and create a Cpu Apic Id table.
> > +
> > + @param[in] CpuApicIdOrderTable Buffer to store information of Cpu.
> > +**/
> > EFI_STATUS
> > -SortCpuLocalApicInTable (
> > - VOID
> > +CreateCpuLocalApicInTable (
> > + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> > )
> > {
> > EFI_STATUS Status;
> > EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
> > UINT32 Index;
> > UINT32 CurrProcessor;
> > - UINT32 BspApicId;
> > - EFI_CPU_ID_ORDER_MAP TempVal;
> > EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
> > - UINT32 CoreThreadMask;
> > - EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
> > UINT32 Socket;
> >
> > - Index = 0;
> > Status = EFI_SUCCESS;
> >
> > - if (mCpuOrderSorted) {
> > - return Status;
> > - }
> > -
> > - TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus *
> sizeof
> > (EFI_CPU_ID_ORDER_MAP));
> > - CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
> > -
> > for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> > CurrProcessor++, Index++) {
> > Status = mMpService->GetProcessorInfo (
> > mMpService, @@ -192,7 +157,7 @@
> > SortCpuLocalApicInTable (
> > &ProcessorInfoBuffer
> > );
> >
> > - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> > &TempCpuApicIdOrderTable[Index];
> > + CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> > &CpuApicIdOrderTable[Index];
> > if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
> > CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
> > CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
> > @@ -214,89 +179,26 @@ SortCpuLocalApicInTable (
> > } //end if PROC ENABLE
> > } //end for CurrentProcessor
> >
> > - //keep for debug purpose
> > - DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> > CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
> > mNumOfBitShift));
> > - DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
> > -
> > //
> > // Get Bsp Apic Id
> > //
> > - BspApicId = GetApicId ();
> > - DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
> > -
> > - //
> > - //check to see if 1st entry is BSP, if not swap it
> > - //
> > - if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
> > - for (Index = 0; Index < mNumberOfCpus; Index++) {
> > - if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
> > (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
> > - CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof
> > (EFI_CPU_ID_ORDER_MAP));
> > - CopyMem (&TempCpuApicIdOrderTable[Index],
> > &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
> > - CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof
> > (EFI_CPU_ID_ORDER_MAP));
> > - break;
> > - }
> > - }
> > -
> > - if (mNumberOfCpus <= Index) {
> > - DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
> > Bufferflow\n"));
> > - return EFI_INVALID_PARAMETER;
> > - }
> > - }
> > -
> > - //
> > - // 1. Sort TempCpuApicIdOrderTable,
> > - // sort it by using ApicId from minimum to maximum (Socket0 to
> SocketN),
> > and the BSP must in the fist location of the table.
> > - // So, start sorting the table from the second element and total
> elements
> > are mNumberOfCpus-1.
> > - //
> > - PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus -
> > 1), sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE)
> > ApicIdCompareFunction);
> > -
> > - //
> > - // 2. Sort and map the primary threads to the front of the
> > CpuApicIdOrderTable
> > - //
> > - for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
> > - if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
> > - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> > &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> > - CurrProcessor++;
> > - }
> > - }
> > + DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
> >
> > - //
> > - // 3. Sort and map the second threads to the middle of the
> > CpuApicIdOrderTable
> > - //
> > - for (Index = 0; Index < mNumberOfCpus; Index++) {
> > - if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
> > - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> > &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> > - CurrProcessor++;
> > - }
> > - }
> >
> > //
> > - // 4. Sort and map the not enabled threads to the bottom of the
> > CpuApicIdOrderTable
> > - //
> > - for (Index = 0; Index < mNumberOfCpus; Index++) {
> > - if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
> > - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> > &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> > - CurrProcessor++;
> > - }
> > - }
> > -
> > - //
> > - // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
> > + // Fill in AcpiProcessorUid.
> > //
> > for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount);
> > Socket++) {
> > for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> > CurrProcessor++) {
> > - if (mCpuApicIdOrderTable[CurrProcessor].Flags &&
> > (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> > - mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> > (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> > Index;
> > + if (CpuApicIdOrderTable[CurrProcessor].Flags &&
> > (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> > + CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> > (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> > Index;
> > Index++;
> > }
> > }
> > }
> >
> > - //keep for debug purpose
> > - DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
> > - DebugDisplayReOrderTable (mCpuApicIdOrderTable);
> > -
> > - mCpuOrderSorted = TRUE;
> > + DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> > mNumOfBitShift = %x\n", mNumOfBitShift));
> > + DebugDisplayReOrderTable (CpuApicIdOrderTable);
> >
> > return Status;
> > }
> > @@ -750,6 +652,7 @@ InstallMadtFromScratch (
> > EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE LocalApciNmiStruct;
> > EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE
> > ProcLocalX2ApicStruct;
> > EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE
> > LocalX2ApicNmiStruct;
> > + EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable;
> > STRUCTURE_HEADER **MadtStructs;
> > UINTN MaxMadtStructCount;
> > UINTN MadtStructsIndex;
> > @@ -760,18 +663,19 @@ InstallMadtFromScratch (
> >
> > MadtStructs = NULL;
> > NewMadtTable = NULL;
> > + CpuApicIdOrderTable = NULL;
> > MaxMadtStructCount = 0;
> >
> > - mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> > (EFI_CPU_ID_ORDER_MAP));
> > - if (mCpuApicIdOrderTable == NULL) {
> > - DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable
> > structure pointer array\n"));
> > + CpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> > (EFI_CPU_ID_ORDER_MAP));
> > + if (CpuApicIdOrderTable == NULL) {
> > + DEBUG ((DEBUG_ERROR, "Could not allocate CpuApicIdOrderTable
> > structure pointer array\n"));
> > return EFI_OUT_OF_RESOURCES;
> > }
> >
> > // Call for Local APIC ID Reorder
> > - Status = SortCpuLocalApicInTable ();
> > + Status = CreateCpuLocalApicInTable (CpuApicIdOrderTable);
> > if (EFI_ERROR (Status)) {
> > - DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n",
> Status));
> > + DEBUG ((DEBUG_ERROR, "CreateCpuLocalApicInTable failed: %r\n",
> > Status));
> > goto Done;
> > }
> >
> > @@ -824,10 +728,10 @@ InstallMadtFromScratch (
> > // APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
> > // use a processor local x2APIC structure.
> > //
> > - if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId <
> > MAX_UINT8) {
> > - ProcLocalApicStruct.Flags = (UINT8)
> > mCpuApicIdOrderTable[Index].Flags;
> > - ProcLocalApicStruct.ApicId = (UINT8)
> > mCpuApicIdOrderTable[Index].ApicId;
> > - ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> > mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> > + if (!mX2ApicEnabled && CpuApicIdOrderTable[Index].ApicId <
> > MAX_UINT8) {
> > + ProcLocalApicStruct.Flags = (UINT8)
> > CpuApicIdOrderTable[Index].Flags;
> > + ProcLocalApicStruct.ApicId = (UINT8)
> > CpuApicIdOrderTable[Index].ApicId;
> > + ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> > CpuApicIdOrderTable[Index].AcpiProcessorUid;
> >
> > ASSERT (MadtStructsIndex < MaxMadtStructCount);
> > Status = CopyStructure (
> > @@ -835,10 +739,10 @@ InstallMadtFromScratch (
> > (STRUCTURE_HEADER *) &ProcLocalApicStruct,
> > &MadtStructs[MadtStructsIndex++]
> > );
> > - } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> > - ProcLocalX2ApicStruct.Flags = (UINT8)
> > mCpuApicIdOrderTable[Index].Flags;
> > - ProcLocalX2ApicStruct.X2ApicId =
> > mCpuApicIdOrderTable[Index].ApicId;
> > - ProcLocalX2ApicStruct.AcpiProcessorUid =
> > mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> > + } else if (CpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> > + ProcLocalX2ApicStruct.Flags = (UINT8)
> > CpuApicIdOrderTable[Index].Flags;
> > + ProcLocalX2ApicStruct.X2ApicId =
> > CpuApicIdOrderTable[Index].ApicId;
> > + ProcLocalX2ApicStruct.AcpiProcessorUid =
> > CpuApicIdOrderTable[Index].AcpiProcessorUid;
> >
> > ASSERT (MadtStructsIndex < MaxMadtStructCount);
> > Status = CopyStructure (
> > @@ -1033,8 +937,8 @@ Done:
> > FreePool (NewMadtTable);
> > }
> >
> > - if (mCpuApicIdOrderTable != NULL) {
> > - FreePool (mCpuApicIdOrderTable);
> > + if (CpuApicIdOrderTable != NULL) {
> > + FreePool (CpuApicIdOrderTable);
> > }
> >
> > return Status;
> > --
> > 2.32.0.windows.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
@ 2022-08-10 4:28 JackX Lin
2022-08-10 5:06 ` Ni, Ray
0 siblings, 1 reply; 16+ messages in thread
From: JackX Lin @ 2022-08-10 4:28 UTC (permalink / raw)
To: devel
Cc: JackX Lin, JackX Lin, Chasel Chiu, Dong Eric, Jiewen Yao, Ray Ni,
Rangasai V Chaganty, Donald Kuo, Chandana C Kumar, Palakshareddy,
Lavanya C
BIOS should not reordering cpu processor_uid
Signed-off-by: JackX Lin <JackX.Lin@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Dong Eric <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Donald Kuo <Donald.Kuo@intel.com>
Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Cc: JackX Lin <JackX.Lin@intel.com>
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 183 ++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------------------------------------------------------------------------
1 file changed, 40 insertions(+), 143 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 5a282e7c18..f134c8a58f 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -57,38 +57,9 @@ BOOLEAN mForceX2ApicId;
BOOLEAN mX2ApicEnabled;
EFI_MP_SERVICES_PROTOCOL *mMpService;
-BOOLEAN mCpuOrderSorted;
-EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL;
UINTN mNumberOfCpus = 0;
UINTN mNumberOfEnabledCPUs = 0;
-
-/**
- The function is called by PerformQuickSort to compare int values.
-
- @param[in] Left The pointer to first buffer.
- @param[in] Right The pointer to second buffer.
-
- @return -1 Buffer1 is less than Buffer2.
- @return 1 Buffer1 is greater than Buffer2.
-
-**/
-INTN
-EFIAPI
-ApicIdCompareFunction (
- IN CONST VOID *Left,
- IN CONST VOID *Right
- )
-{
- UINT32 LeftApicId;
- UINT32 RightApicId;
-
- LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
- RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
-
- return (LeftApicId > RightApicId)? 1 : (-1);
-}
-
/**
Print Cpu Apic ID Table
@@ -116,7 +87,8 @@ DebugDisplayReOrderTable (
EFI_STATUS
AppendCpuMapTableEntry (
IN VOID *ApicPtr,
- IN UINT32 LocalApicCounter
+ IN UINT32 LocalApicCounter,
+ IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
)
{
EFI_STATUS Status;
@@ -131,9 +103,9 @@ AppendCpuMapTableEntry (
if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
if(!mX2ApicEnabled) {
- LocalApicPtr->Flags = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
- LocalApicPtr->ApicId = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
- LocalApicPtr->AcpiProcessorUid = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
+ LocalApicPtr->Flags = (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
+ LocalApicPtr->ApicId = (UINT8)CpuApicIdOrderTable[LocalApicCounter].ApicId;
+ LocalApicPtr->AcpiProcessorUid = (UINT8)CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
} else {
LocalApicPtr->Flags = 0;
LocalApicPtr->ApicId = 0xFF;
@@ -142,9 +114,9 @@ AppendCpuMapTableEntry (
}
} else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
if(mX2ApicEnabled) {
- LocalX2ApicPtr->Flags = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
- LocalX2ApicPtr->X2ApicId = mCpuApicIdOrderTable[LocalApicCounter].ApicId;
- LocalX2ApicPtr->AcpiProcessorUid = mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
+ LocalX2ApicPtr->Flags = (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
+ LocalX2ApicPtr->X2ApicId = CpuApicIdOrderTable[LocalApicCounter].ApicId;
+ LocalX2ApicPtr->AcpiProcessorUid = CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
} else {
LocalX2ApicPtr->Flags = 0;
LocalX2ApicPtr->X2ApicId = (UINT32)-1;
@@ -159,32 +131,25 @@ AppendCpuMapTableEntry (
}
+/**
+ Collect all processors information and create a Cpu Apic Id table.
+
+ @param[in] CpuApicIdOrderTable Buffer to store information of Cpu.
+**/
EFI_STATUS
-SortCpuLocalApicInTable (
- VOID
+CreateCpuLocalApicInTable (
+ IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
)
{
EFI_STATUS Status;
EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
UINT32 Index;
UINT32 CurrProcessor;
- UINT32 BspApicId;
- EFI_CPU_ID_ORDER_MAP TempVal;
EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
- UINT32 CoreThreadMask;
- EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
UINT32 Socket;
- Index = 0;
Status = EFI_SUCCESS;
- if (mCpuOrderSorted) {
- return Status;
- }
-
- TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
- CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
-
for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++, Index++) {
Status = mMpService->GetProcessorInfo (
mMpService,
@@ -192,7 +157,7 @@ SortCpuLocalApicInTable (
&ProcessorInfoBuffer
);
- CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &TempCpuApicIdOrderTable[Index];
+ CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &CpuApicIdOrderTable[Index];
if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
@@ -214,96 +179,26 @@ SortCpuLocalApicInTable (
} //end if PROC ENABLE
} //end for CurrentProcessor
- //keep for debug purpose
- DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift));
- DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
-
//
// Get Bsp Apic Id
//
- BspApicId = GetApicId ();
- DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
+ DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
+
//
- //check to see if 1st entry is BSP, if not swap it
+ // Fill in AcpiProcessorUid.
//
- if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Flags == 1) && (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
- CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CopyMem (&TempCpuApicIdOrderTable[Index], &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
- CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof (EFI_CPU_ID_ORDER_MAP));
- break;
- }
- }
-
- if (mNumberOfCpus <= Index) {
- DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n"));
- return EFI_INVALID_PARAMETER;
- }
- }
-
- /*
- 1. Sort TempCpuApicIdOrderTable,
- Sort it by using ApicId from minimum to maximum (Socket0 to SocketN), and the BSP must be in the fist location of the table.
-
- 2. Sort and map all the enabled threads after BSP in CpuApicIdOrderTable
-
- 3. Threads that are not enabled are placed in the bottom of CpuApicIdOrderTable
-
- 4. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
- */
-
- PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 1), sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE) ApicIdCompareFunction);
-
- for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 0) {
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 1) {
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 2) {
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 3) {
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if (TempCpuApicIdOrderTable[Index].Flags == 0) {
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount); Socket++) {
for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
- if (mCpuApicIdOrderTable[CurrProcessor].Flags && (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
- mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) + Index;
+ if (CpuApicIdOrderTable[CurrProcessor].Flags && (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
+ CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) + Index;
Index++;
}
}
}
- DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
- DebugDisplayReOrderTable (mCpuApicIdOrderTable);
-
- mCpuOrderSorted = TRUE;
+ DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. mNumOfBitShift = %x\n", mNumOfBitShift));
+ DebugDisplayReOrderTable (CpuApicIdOrderTable);
return Status;
}
@@ -757,6 +652,7 @@ InstallMadtFromScratch (
EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE LocalApciNmiStruct;
EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE ProcLocalX2ApicStruct;
EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE LocalX2ApicNmiStruct;
+ EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable;
STRUCTURE_HEADER **MadtStructs;
UINTN MaxMadtStructCount;
UINTN MadtStructsIndex;
@@ -767,18 +663,19 @@ InstallMadtFromScratch (
MadtStructs = NULL;
NewMadtTable = NULL;
+ CpuApicIdOrderTable = NULL;
MaxMadtStructCount = 0;
- mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
- if (mCpuApicIdOrderTable == NULL) {
- DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable structure pointer array\n"));
+ CpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
+ if (CpuApicIdOrderTable == NULL) {
+ DEBUG ((DEBUG_ERROR, "Could not allocate CpuApicIdOrderTable structure pointer array\n"));
return EFI_OUT_OF_RESOURCES;
}
// Call for Local APIC ID Reorder
- Status = SortCpuLocalApicInTable ();
+ Status = CreateCpuLocalApicInTable (CpuApicIdOrderTable);
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
+ DEBUG ((DEBUG_ERROR, "CreateCpuLocalApicInTable failed: %r\n", Status));
goto Done;
}
@@ -831,10 +728,10 @@ InstallMadtFromScratch (
// APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
// use a processor local x2APIC structure.
//
- if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId < MAX_UINT8) {
- ProcLocalApicStruct.Flags = (UINT8) mCpuApicIdOrderTable[Index].Flags;
- ProcLocalApicStruct.ApicId = (UINT8) mCpuApicIdOrderTable[Index].ApicId;
- ProcLocalApicStruct.AcpiProcessorUid = (UINT8) mCpuApicIdOrderTable[Index].AcpiProcessorUid;
+ if (!mX2ApicEnabled && CpuApicIdOrderTable[Index].ApicId < MAX_UINT8) {
+ ProcLocalApicStruct.Flags = (UINT8) CpuApicIdOrderTable[Index].Flags;
+ ProcLocalApicStruct.ApicId = (UINT8) CpuApicIdOrderTable[Index].ApicId;
+ ProcLocalApicStruct.AcpiProcessorUid = (UINT8) CpuApicIdOrderTable[Index].AcpiProcessorUid;
ASSERT (MadtStructsIndex < MaxMadtStructCount);
Status = CopyStructure (
@@ -842,10 +739,10 @@ InstallMadtFromScratch (
(STRUCTURE_HEADER *) &ProcLocalApicStruct,
&MadtStructs[MadtStructsIndex++]
);
- } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
- ProcLocalX2ApicStruct.Flags = (UINT8) mCpuApicIdOrderTable[Index].Flags;
- ProcLocalX2ApicStruct.X2ApicId = mCpuApicIdOrderTable[Index].ApicId;
- ProcLocalX2ApicStruct.AcpiProcessorUid = mCpuApicIdOrderTable[Index].AcpiProcessorUid;
+ } else if (CpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
+ ProcLocalX2ApicStruct.Flags = (UINT8) CpuApicIdOrderTable[Index].Flags;
+ ProcLocalX2ApicStruct.X2ApicId = CpuApicIdOrderTable[Index].ApicId;
+ ProcLocalX2ApicStruct.AcpiProcessorUid = CpuApicIdOrderTable[Index].AcpiProcessorUid;
ASSERT (MadtStructsIndex < MaxMadtStructCount);
Status = CopyStructure (
@@ -1040,8 +937,8 @@ Done:
FreePool (NewMadtTable);
}
- if (mCpuApicIdOrderTable != NULL) {
- FreePool (mCpuApicIdOrderTable);
+ if (CpuApicIdOrderTable != NULL) {
+ FreePool (CpuApicIdOrderTable);
}
return Status;
--
2.32.0.windows.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
2022-08-10 3:51 ` JackX Lin
2022-08-10 3:52 ` Ni, Ray
@ 2022-08-10 4:32 ` JackX Lin
2022-08-10 5:09 ` Ni, Ray
1 sibling, 1 reply; 16+ messages in thread
From: JackX Lin @ 2022-08-10 4:32 UTC (permalink / raw)
To: Ni, Ray, Sinha, Ankit
Cc: Chiu, Chasel, Dong, Eric, Yao, Jiewen, Chaganty, Rangasai V,
devel@edk2.groups.io, Kuo, Donald, Kumar, Chandana C,
Palakshareddy, Lavanya C, Palakshareddy, Lavanya C
[-- Attachment #1: Type: text/plain, Size: 17601 bytes --]
Hi Ray,
The latest patch is sent in the code review mail, and I also attach one here, thank you.
Best regards
Jack
-----Original Message-----
From: Lin, JackX
Sent: Wednesday, August 10, 2022 11:51 AM
To: Ni, Ray <ray.ni@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; devel@edk2.groups.io; Kuo, Donald <donald.kuo@intel.com>; Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Subject: RE: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
Hi Ray,
I know this patch, and the thread 2 and 3 are added by my request for a reason on that time.
I will re-sent the code patch.
Thank you.
Jack
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, August 10, 2022 11:48 AM
To: Lin, JackX <jackx.lin@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; devel@edk2.groups.io; Kuo, Donald <donald.kuo@intel.com>; Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Subject: RE: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
Jack,
Your patch cannot be merged to trunk because Ankit just did some change in the same C file, in below commit.
* MinPlatformPkg/AcpiTables: Add additional thread mapping in MADT
Ankit,
It seems your patch is to add support for thread #2 and #3. Jack's patch is to remove the additional sorting that put secondary threads after first threads.
Do you see an issue that we remove the thread sorting logic?
Thanks,
Ray
> -----Original Message-----
> From: Lin, JackX <jackx.lin@intel.com>
> Sent: Monday, August 8, 2022 4:21 PM
> To: devel@edk2.groups.io
> Cc: Lin, JackX <jackx.lin@intel.com>; Lin, JackX
> <jackx.lin@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Dong,
> Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Kuo, Donald <donald.kuo@intel.com>;
> Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy;
> Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Subject: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU
> default fused in MADT
>
> BIOS should not reordering cpu processor_uid
>
> Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Dong Eric <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Donald Kuo <Donald.Kuo@intel.com>
> Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
> Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Cc: JackX Lin <JackX.Lin@intel.com>
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 174
> +++++++++++++++++++++++++++++++++++++++-------------------------------
> ----------------------------------------------------------------------
> ------------------------
> ----------
> 1 file changed, 39 insertions(+), 135 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index c7e87cbd7d..176e422e81 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -57,38 +57,9 @@ BOOLEAN mForceX2ApicId;
> BOOLEAN mX2ApicEnabled;
>
> EFI_MP_SERVICES_PROTOCOL *mMpService;
> -BOOLEAN mCpuOrderSorted;
> -EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL;
> UINTN mNumberOfCpus = 0;
> UINTN mNumberOfEnabledCPUs = 0;
>
> -
> -/**
> - The function is called by PerformQuickSort to compare int values.
> -
> - @param[in] Left The pointer to first buffer.
> - @param[in] Right The pointer to second buffer.
> -
> - @return -1 Buffer1 is less than Buffer2.
> - @return 1 Buffer1 is greater than Buffer2.
> -
> -**/
> -INTN
> -EFIAPI
> -ApicIdCompareFunction (
> - IN CONST VOID *Left,
> - IN CONST VOID *Right
> - )
> -{
> - UINT32 LeftApicId;
> - UINT32 RightApicId;
> -
> - LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
> - RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
> -
> - return (LeftApicId > RightApicId)? 1 : (-1); -}
> -
> /**
> Print Cpu Apic ID Table
>
> @@ -116,7 +87,8 @@ DebugDisplayReOrderTable ( EFI_STATUS
> AppendCpuMapTableEntry (
> IN VOID *ApicPtr,
> - IN UINT32 LocalApicCounter
> + IN UINT32 LocalApicCounter,
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> @@ -131,9 +103,9 @@ AppendCpuMapTableEntry (
>
> if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
> if(!mX2ApicEnabled) {
> - LocalApicPtr->Flags =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> - LocalApicPtr->ApicId =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> - LocalApicPtr->AcpiProcessorUid =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> + LocalApicPtr->Flags =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> + LocalApicPtr->ApicId =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].ApicId;
> + LocalApicPtr->AcpiProcessorUid =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> } else {
> LocalApicPtr->Flags = 0;
> LocalApicPtr->ApicId = 0xFF;
> @@ -142,9 +114,9 @@ AppendCpuMapTableEntry (
> }
> } else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
> if(mX2ApicEnabled) {
> - LocalX2ApicPtr->Flags =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> - LocalX2ApicPtr->X2ApicId =
> mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> - LocalX2ApicPtr->AcpiProcessorUid =
> mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> + LocalX2ApicPtr->Flags =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> + LocalX2ApicPtr->X2ApicId =
> CpuApicIdOrderTable[LocalApicCounter].ApicId;
> + LocalX2ApicPtr->AcpiProcessorUid =
> CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> } else {
> LocalX2ApicPtr->Flags = 0;
> LocalX2ApicPtr->X2ApicId = (UINT32)-1;
> @@ -159,32 +131,25 @@ AppendCpuMapTableEntry (
>
> }
>
> +/**
> + Collect all processors information and create a Cpu Apic Id table.
> +
> + @param[in] CpuApicIdOrderTable Buffer to store information of Cpu.
> +**/
> EFI_STATUS
> -SortCpuLocalApicInTable (
> - VOID
> +CreateCpuLocalApicInTable (
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
> UINT32 Index;
> UINT32 CurrProcessor;
> - UINT32 BspApicId;
> - EFI_CPU_ID_ORDER_MAP TempVal;
> EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
> - UINT32 CoreThreadMask;
> - EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
> UINT32 Socket;
>
> - Index = 0;
> Status = EFI_SUCCESS;
>
> - if (mCpuOrderSorted) {
> - return Status;
> - }
> -
> - TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
> -
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++, Index++) {
> Status = mMpService->GetProcessorInfo (
> mMpService, @@ -192,7 +157,7 @@
> SortCpuLocalApicInTable (
> &ProcessorInfoBuffer
> );
>
> - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &TempCpuApicIdOrderTable[Index];
> + CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &CpuApicIdOrderTable[Index];
> if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
> CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
> CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
> @@ -214,89 +179,26 @@ SortCpuLocalApicInTable (
> } //end if PROC ENABLE
> } //end for CurrentProcessor
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
> mNumOfBitShift));
> - DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
> -
> //
> // Get Bsp Apic Id
> //
> - BspApicId = GetApicId ();
> - DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
> -
> - //
> - //check to see if 1st entry is BSP, if not swap it
> - //
> - if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
> (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
> - CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[Index],
> &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - break;
> - }
> - }
> -
> - if (mNumberOfCpus <= Index) {
> - DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
> Bufferflow\n"));
> - return EFI_INVALID_PARAMETER;
> - }
> - }
> -
> - //
> - // 1. Sort TempCpuApicIdOrderTable,
> - // sort it by using ApicId from minimum to maximum (Socket0 to SocketN),
> and the BSP must in the fist location of the table.
> - // So, start sorting the table from the second element and total elements
> are mNumberOfCpus-1.
> - //
> - PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus -
> 1), sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE)
> ApicIdCompareFunction);
> -
> - //
> - // 2. Sort and map the primary threads to the front of the
> CpuApicIdOrderTable
> - //
> - for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> + DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
>
> - //
> - // 3. Sort and map the second threads to the middle of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
>
> //
> - // 4. Sort and map the not enabled threads to the bottom of the
> CpuApicIdOrderTable
> - //
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - //
> - // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
> + // Fill in AcpiProcessorUid.
> //
> for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount);
> Socket++) {
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++) {
> - if (mCpuApicIdOrderTable[CurrProcessor].Flags &&
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> - mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> Index;
> + if (CpuApicIdOrderTable[CurrProcessor].Flags &&
> (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> + CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> Index;
> Index++;
> }
> }
> }
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
> - DebugDisplayReOrderTable (mCpuApicIdOrderTable);
> -
> - mCpuOrderSorted = TRUE;
> + DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> mNumOfBitShift = %x\n", mNumOfBitShift));
> + DebugDisplayReOrderTable (CpuApicIdOrderTable);
>
> return Status;
> }
> @@ -750,6 +652,7 @@ InstallMadtFromScratch (
> EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE LocalApciNmiStruct;
> EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE
> ProcLocalX2ApicStruct;
> EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE
> LocalX2ApicNmiStruct;
> + EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable;
> STRUCTURE_HEADER **MadtStructs;
> UINTN MaxMadtStructCount;
> UINTN MadtStructsIndex;
> @@ -760,18 +663,19 @@ InstallMadtFromScratch (
>
> MadtStructs = NULL;
> NewMadtTable = NULL;
> + CpuApicIdOrderTable = NULL;
> MaxMadtStructCount = 0;
>
> - mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - if (mCpuApicIdOrderTable == NULL) {
> - DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable
> structure pointer array\n"));
> + CpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> + if (CpuApicIdOrderTable == NULL) {
> + DEBUG ((DEBUG_ERROR, "Could not allocate CpuApicIdOrderTable
> structure pointer array\n"));
> return EFI_OUT_OF_RESOURCES;
> }
>
> // Call for Local APIC ID Reorder
> - Status = SortCpuLocalApicInTable ();
> + Status = CreateCpuLocalApicInTable (CpuApicIdOrderTable);
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
> + DEBUG ((DEBUG_ERROR, "CreateCpuLocalApicInTable failed: %r\n",
> Status));
> goto Done;
> }
>
> @@ -824,10 +728,10 @@ InstallMadtFromScratch (
> // APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
> // use a processor local x2APIC structure.
> //
> - if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> - ProcLocalApicStruct.Flags = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> - ProcLocalApicStruct.ApicId = (UINT8)
> mCpuApicIdOrderTable[Index].ApicId;
> - ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> + if (!mX2ApicEnabled && CpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> + ProcLocalApicStruct.Flags = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> + ProcLocalApicStruct.ApicId = (UINT8)
> CpuApicIdOrderTable[Index].ApicId;
> + ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
>
> ASSERT (MadtStructsIndex < MaxMadtStructCount);
> Status = CopyStructure (
> @@ -835,10 +739,10 @@ InstallMadtFromScratch (
> (STRUCTURE_HEADER *) &ProcLocalApicStruct,
> &MadtStructs[MadtStructsIndex++]
> );
> - } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> - ProcLocalX2ApicStruct.Flags = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> - ProcLocalX2ApicStruct.X2ApicId =
> mCpuApicIdOrderTable[Index].ApicId;
> - ProcLocalX2ApicStruct.AcpiProcessorUid =
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> + } else if (CpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> + ProcLocalX2ApicStruct.Flags = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> + ProcLocalX2ApicStruct.X2ApicId =
> CpuApicIdOrderTable[Index].ApicId;
> + ProcLocalX2ApicStruct.AcpiProcessorUid =
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
>
> ASSERT (MadtStructsIndex < MaxMadtStructCount);
> Status = CopyStructure (
> @@ -1033,8 +937,8 @@ Done:
> FreePool (NewMadtTable);
> }
>
> - if (mCpuApicIdOrderTable != NULL) {
> - FreePool (mCpuApicIdOrderTable);
> + if (CpuApicIdOrderTable != NULL) {
> + FreePool (CpuApicIdOrderTable);
> }
>
> return Status;
> --
> 2.32.0.windows.2
[-- Attachment #2: 0001-Modify-processor-_UID-ordering-by-CPU-default-fused-.patch --]
[-- Type: application/octet-stream, Size: 14520 bytes --]
From 4de4973504c869e40ccbcd0bec746bcf6047a2dd Mon Sep 17 00:00:00 2001
From: JackX Lin <jackx.lin@intel.com>
Date: Wed, 10 Aug 2022 12:24:09 +0800
Subject: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default
fused in MADT
BIOS should not reordering cpu processor_uid
Signed-off-by: JackX Lin <JackX.Lin@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Dong Eric <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Donald Kuo <Donald.Kuo@intel.com>
Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Cc: JackX Lin <JackX.Lin@intel.com>
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 183 ++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------------------------------------------------------------------------
1 file changed, 40 insertions(+), 143 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 5a282e7c18..f134c8a58f 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -57,38 +57,9 @@ BOOLEAN mForceX2ApicId;
BOOLEAN mX2ApicEnabled;
EFI_MP_SERVICES_PROTOCOL *mMpService;
-BOOLEAN mCpuOrderSorted;
-EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL;
UINTN mNumberOfCpus = 0;
UINTN mNumberOfEnabledCPUs = 0;
-
-/**
- The function is called by PerformQuickSort to compare int values.
-
- @param[in] Left The pointer to first buffer.
- @param[in] Right The pointer to second buffer.
-
- @return -1 Buffer1 is less than Buffer2.
- @return 1 Buffer1 is greater than Buffer2.
-
-**/
-INTN
-EFIAPI
-ApicIdCompareFunction (
- IN CONST VOID *Left,
- IN CONST VOID *Right
- )
-{
- UINT32 LeftApicId;
- UINT32 RightApicId;
-
- LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
- RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
-
- return (LeftApicId > RightApicId)? 1 : (-1);
-}
-
/**
Print Cpu Apic ID Table
@@ -116,7 +87,8 @@ DebugDisplayReOrderTable (
EFI_STATUS
AppendCpuMapTableEntry (
IN VOID *ApicPtr,
- IN UINT32 LocalApicCounter
+ IN UINT32 LocalApicCounter,
+ IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
)
{
EFI_STATUS Status;
@@ -131,9 +103,9 @@ AppendCpuMapTableEntry (
if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
if(!mX2ApicEnabled) {
- LocalApicPtr->Flags = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
- LocalApicPtr->ApicId = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
- LocalApicPtr->AcpiProcessorUid = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
+ LocalApicPtr->Flags = (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
+ LocalApicPtr->ApicId = (UINT8)CpuApicIdOrderTable[LocalApicCounter].ApicId;
+ LocalApicPtr->AcpiProcessorUid = (UINT8)CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
} else {
LocalApicPtr->Flags = 0;
LocalApicPtr->ApicId = 0xFF;
@@ -142,9 +114,9 @@ AppendCpuMapTableEntry (
}
} else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
if(mX2ApicEnabled) {
- LocalX2ApicPtr->Flags = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
- LocalX2ApicPtr->X2ApicId = mCpuApicIdOrderTable[LocalApicCounter].ApicId;
- LocalX2ApicPtr->AcpiProcessorUid = mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
+ LocalX2ApicPtr->Flags = (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
+ LocalX2ApicPtr->X2ApicId = CpuApicIdOrderTable[LocalApicCounter].ApicId;
+ LocalX2ApicPtr->AcpiProcessorUid = CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
} else {
LocalX2ApicPtr->Flags = 0;
LocalX2ApicPtr->X2ApicId = (UINT32)-1;
@@ -159,32 +131,25 @@ AppendCpuMapTableEntry (
}
+/**
+ Collect all processors information and create a Cpu Apic Id table.
+
+ @param[in] CpuApicIdOrderTable Buffer to store information of Cpu.
+**/
EFI_STATUS
-SortCpuLocalApicInTable (
- VOID
+CreateCpuLocalApicInTable (
+ IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
)
{
EFI_STATUS Status;
EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
UINT32 Index;
UINT32 CurrProcessor;
- UINT32 BspApicId;
- EFI_CPU_ID_ORDER_MAP TempVal;
EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
- UINT32 CoreThreadMask;
- EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
UINT32 Socket;
- Index = 0;
Status = EFI_SUCCESS;
- if (mCpuOrderSorted) {
- return Status;
- }
-
- TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
- CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
-
for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++, Index++) {
Status = mMpService->GetProcessorInfo (
mMpService,
@@ -192,7 +157,7 @@ SortCpuLocalApicInTable (
&ProcessorInfoBuffer
);
- CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &TempCpuApicIdOrderTable[Index];
+ CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &CpuApicIdOrderTable[Index];
if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
@@ -214,96 +179,26 @@ SortCpuLocalApicInTable (
} //end if PROC ENABLE
} //end for CurrentProcessor
- //keep for debug purpose
- DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift));
- DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
-
//
// Get Bsp Apic Id
//
- BspApicId = GetApicId ();
- DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
+ DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
+
//
- //check to see if 1st entry is BSP, if not swap it
+ // Fill in AcpiProcessorUid.
//
- if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Flags == 1) && (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
- CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CopyMem (&TempCpuApicIdOrderTable[Index], &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
- CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof (EFI_CPU_ID_ORDER_MAP));
- break;
- }
- }
-
- if (mNumberOfCpus <= Index) {
- DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n"));
- return EFI_INVALID_PARAMETER;
- }
- }
-
- /*
- 1. Sort TempCpuApicIdOrderTable,
- Sort it by using ApicId from minimum to maximum (Socket0 to SocketN), and the BSP must be in the fist location of the table.
-
- 2. Sort and map all the enabled threads after BSP in CpuApicIdOrderTable
-
- 3. Threads that are not enabled are placed in the bottom of CpuApicIdOrderTable
-
- 4. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
- */
-
- PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 1), sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE) ApicIdCompareFunction);
-
- for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 0) {
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 1) {
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 2) {
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 3) {
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if (TempCpuApicIdOrderTable[Index].Flags == 0) {
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount); Socket++) {
for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
- if (mCpuApicIdOrderTable[CurrProcessor].Flags && (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
- mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) + Index;
+ if (CpuApicIdOrderTable[CurrProcessor].Flags && (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
+ CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) + Index;
Index++;
}
}
}
- DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
- DebugDisplayReOrderTable (mCpuApicIdOrderTable);
-
- mCpuOrderSorted = TRUE;
+ DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. mNumOfBitShift = %x\n", mNumOfBitShift));
+ DebugDisplayReOrderTable (CpuApicIdOrderTable);
return Status;
}
@@ -757,6 +652,7 @@ InstallMadtFromScratch (
EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE LocalApciNmiStruct;
EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE ProcLocalX2ApicStruct;
EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE LocalX2ApicNmiStruct;
+ EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable;
STRUCTURE_HEADER **MadtStructs;
UINTN MaxMadtStructCount;
UINTN MadtStructsIndex;
@@ -767,18 +663,19 @@ InstallMadtFromScratch (
MadtStructs = NULL;
NewMadtTable = NULL;
+ CpuApicIdOrderTable = NULL;
MaxMadtStructCount = 0;
- mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
- if (mCpuApicIdOrderTable == NULL) {
- DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable structure pointer array\n"));
+ CpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
+ if (CpuApicIdOrderTable == NULL) {
+ DEBUG ((DEBUG_ERROR, "Could not allocate CpuApicIdOrderTable structure pointer array\n"));
return EFI_OUT_OF_RESOURCES;
}
// Call for Local APIC ID Reorder
- Status = SortCpuLocalApicInTable ();
+ Status = CreateCpuLocalApicInTable (CpuApicIdOrderTable);
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
+ DEBUG ((DEBUG_ERROR, "CreateCpuLocalApicInTable failed: %r\n", Status));
goto Done;
}
@@ -831,10 +728,10 @@ InstallMadtFromScratch (
// APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
// use a processor local x2APIC structure.
//
- if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId < MAX_UINT8) {
- ProcLocalApicStruct.Flags = (UINT8) mCpuApicIdOrderTable[Index].Flags;
- ProcLocalApicStruct.ApicId = (UINT8) mCpuApicIdOrderTable[Index].ApicId;
- ProcLocalApicStruct.AcpiProcessorUid = (UINT8) mCpuApicIdOrderTable[Index].AcpiProcessorUid;
+ if (!mX2ApicEnabled && CpuApicIdOrderTable[Index].ApicId < MAX_UINT8) {
+ ProcLocalApicStruct.Flags = (UINT8) CpuApicIdOrderTable[Index].Flags;
+ ProcLocalApicStruct.ApicId = (UINT8) CpuApicIdOrderTable[Index].ApicId;
+ ProcLocalApicStruct.AcpiProcessorUid = (UINT8) CpuApicIdOrderTable[Index].AcpiProcessorUid;
ASSERT (MadtStructsIndex < MaxMadtStructCount);
Status = CopyStructure (
@@ -842,10 +739,10 @@ InstallMadtFromScratch (
(STRUCTURE_HEADER *) &ProcLocalApicStruct,
&MadtStructs[MadtStructsIndex++]
);
- } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
- ProcLocalX2ApicStruct.Flags = (UINT8) mCpuApicIdOrderTable[Index].Flags;
- ProcLocalX2ApicStruct.X2ApicId = mCpuApicIdOrderTable[Index].ApicId;
- ProcLocalX2ApicStruct.AcpiProcessorUid = mCpuApicIdOrderTable[Index].AcpiProcessorUid;
+ } else if (CpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
+ ProcLocalX2ApicStruct.Flags = (UINT8) CpuApicIdOrderTable[Index].Flags;
+ ProcLocalX2ApicStruct.X2ApicId = CpuApicIdOrderTable[Index].ApicId;
+ ProcLocalX2ApicStruct.AcpiProcessorUid = CpuApicIdOrderTable[Index].AcpiProcessorUid;
ASSERT (MadtStructsIndex < MaxMadtStructCount);
Status = CopyStructure (
@@ -1040,8 +937,8 @@ Done:
FreePool (NewMadtTable);
}
- if (mCpuApicIdOrderTable != NULL) {
- FreePool (mCpuApicIdOrderTable);
+ if (CpuApicIdOrderTable != NULL) {
+ FreePool (CpuApicIdOrderTable);
}
return Status;
--
2.32.0.windows.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
2022-08-10 4:28 [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT JackX Lin
@ 2022-08-10 5:06 ` Ni, Ray
0 siblings, 0 replies; 16+ messages in thread
From: Ni, Ray @ 2022-08-10 5:06 UTC (permalink / raw)
To: Lin, JackX, devel@edk2.groups.io
Cc: Chiu, Chasel, Dong, Eric, Yao, Jiewen, Chaganty, Rangasai V,
Kuo, Donald, Kumar, Chandana C, Palakshareddy, Lavanya C,
Palakshareddy, Lavanya C
Thanks!
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Lin, JackX <jackx.lin@intel.com>
> Sent: Wednesday, August 10, 2022 12:29 PM
> To: devel@edk2.groups.io
> Cc: Lin, JackX <jackx.lin@intel.com>; Lin, JackX <jackx.lin@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Ni, Ray <ray.ni@intel.com>; Chaganty,
> Rangasai V <rangasai.v.chaganty@intel.com>; Kuo, Donald
> <donald.kuo@intel.com>; Kumar, Chandana C
> <chandana.c.kumar@intel.com>; Palakshareddy; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>
> Subject: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU
> default fused in MADT
>
> BIOS should not reordering cpu processor_uid
>
> Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Dong Eric <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Donald Kuo <Donald.Kuo@intel.com>
> Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
> Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Cc: JackX Lin <JackX.Lin@intel.com>
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 183
> ++++++++++++++++++++++++++++++++++++++++------------------------------
> ----------------------------------------------------------------------------------------------
> -------------------
> 1 file changed, 40 insertions(+), 143 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 5a282e7c18..f134c8a58f 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -57,38 +57,9 @@ BOOLEAN mForceX2ApicId;
> BOOLEAN mX2ApicEnabled;
>
> EFI_MP_SERVICES_PROTOCOL *mMpService;
> -BOOLEAN mCpuOrderSorted;
> -EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL;
> UINTN mNumberOfCpus = 0;
> UINTN mNumberOfEnabledCPUs = 0;
>
> -
> -/**
> - The function is called by PerformQuickSort to compare int values.
> -
> - @param[in] Left The pointer to first buffer.
> - @param[in] Right The pointer to second buffer.
> -
> - @return -1 Buffer1 is less than Buffer2.
> - @return 1 Buffer1 is greater than Buffer2.
> -
> -**/
> -INTN
> -EFIAPI
> -ApicIdCompareFunction (
> - IN CONST VOID *Left,
> - IN CONST VOID *Right
> - )
> -{
> - UINT32 LeftApicId;
> - UINT32 RightApicId;
> -
> - LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
> - RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
> -
> - return (LeftApicId > RightApicId)? 1 : (-1);
> -}
> -
> /**
> Print Cpu Apic ID Table
>
> @@ -116,7 +87,8 @@ DebugDisplayReOrderTable (
> EFI_STATUS
> AppendCpuMapTableEntry (
> IN VOID *ApicPtr,
> - IN UINT32 LocalApicCounter
> + IN UINT32 LocalApicCounter,
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> @@ -131,9 +103,9 @@ AppendCpuMapTableEntry (
>
> if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
> if(!mX2ApicEnabled) {
> - LocalApicPtr->Flags =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> - LocalApicPtr->ApicId =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> - LocalApicPtr->AcpiProcessorUid =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> + LocalApicPtr->Flags =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> + LocalApicPtr->ApicId =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].ApicId;
> + LocalApicPtr->AcpiProcessorUid =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> } else {
> LocalApicPtr->Flags = 0;
> LocalApicPtr->ApicId = 0xFF;
> @@ -142,9 +114,9 @@ AppendCpuMapTableEntry (
> }
> } else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
> if(mX2ApicEnabled) {
> - LocalX2ApicPtr->Flags =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> - LocalX2ApicPtr->X2ApicId =
> mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> - LocalX2ApicPtr->AcpiProcessorUid =
> mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> + LocalX2ApicPtr->Flags =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> + LocalX2ApicPtr->X2ApicId =
> CpuApicIdOrderTable[LocalApicCounter].ApicId;
> + LocalX2ApicPtr->AcpiProcessorUid =
> CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> } else {
> LocalX2ApicPtr->Flags = 0;
> LocalX2ApicPtr->X2ApicId = (UINT32)-1;
> @@ -159,32 +131,25 @@ AppendCpuMapTableEntry (
>
> }
>
> +/**
> + Collect all processors information and create a Cpu Apic Id table.
> +
> + @param[in] CpuApicIdOrderTable Buffer to store information of Cpu.
> +**/
> EFI_STATUS
> -SortCpuLocalApicInTable (
> - VOID
> +CreateCpuLocalApicInTable (
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
> UINT32 Index;
> UINT32 CurrProcessor;
> - UINT32 BspApicId;
> - EFI_CPU_ID_ORDER_MAP TempVal;
> EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
> - UINT32 CoreThreadMask;
> - EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
> UINT32 Socket;
>
> - Index = 0;
> Status = EFI_SUCCESS;
>
> - if (mCpuOrderSorted) {
> - return Status;
> - }
> -
> - TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
> -
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++, Index++) {
> Status = mMpService->GetProcessorInfo (
> mMpService,
> @@ -192,7 +157,7 @@ SortCpuLocalApicInTable (
> &ProcessorInfoBuffer
> );
>
> - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &TempCpuApicIdOrderTable[Index];
> + CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &CpuApicIdOrderTable[Index];
> if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
> CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
> CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
> @@ -214,96 +179,26 @@ SortCpuLocalApicInTable (
> } //end if PROC ENABLE
> } //end for CurrentProcessor
>
> - //keep for debug purpose
> - DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
> mNumOfBitShift));
> - DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
> -
> //
> // Get Bsp Apic Id
> //
> - BspApicId = GetApicId ();
> - DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
> + DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
> +
>
> //
> - //check to see if 1st entry is BSP, if not swap it
> + // Fill in AcpiProcessorUid.
> //
> - if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
> (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
> - CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[Index],
> &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - break;
> - }
> - }
> -
> - if (mNumberOfCpus <= Index) {
> - DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
> Bufferflow\n"));
> - return EFI_INVALID_PARAMETER;
> - }
> - }
> -
> - /*
> - 1. Sort TempCpuApicIdOrderTable,
> - Sort it by using ApicId from minimum to maximum (Socket0 to SocketN),
> and the BSP must be in the fist location of the table.
> -
> - 2. Sort and map all the enabled threads after BSP in CpuApicIdOrderTable
> -
> - 3. Threads that are not enabled are placed in the bottom of
> CpuApicIdOrderTable
> -
> - 4. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
> - */
> -
> - PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 1),
> sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE)
> ApicIdCompareFunction);
> -
> - for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 0) {
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 1) {
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 2) {
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if ((TempCpuApicIdOrderTable[Index].Thread) == 3) {
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> - for (Index = 0; Index < mNumberOfCpus; Index++) {
> - if (TempCpuApicIdOrderTable[Index].Flags == 0) {
> - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> - CurrProcessor++;
> - }
> - }
> -
> for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount);
> Socket++) {
> for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++) {
> - if (mCpuApicIdOrderTable[CurrProcessor].Flags &&
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> - mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> Index;
> + if (CpuApicIdOrderTable[CurrProcessor].Flags &&
> (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> + CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> Index;
> Index++;
> }
> }
> }
>
> - DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
> - DebugDisplayReOrderTable (mCpuApicIdOrderTable);
> -
> - mCpuOrderSorted = TRUE;
> + DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> mNumOfBitShift = %x\n", mNumOfBitShift));
> + DebugDisplayReOrderTable (CpuApicIdOrderTable);
>
> return Status;
> }
> @@ -757,6 +652,7 @@ InstallMadtFromScratch (
> EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE LocalApciNmiStruct;
> EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE
> ProcLocalX2ApicStruct;
> EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE
> LocalX2ApicNmiStruct;
> + EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable;
> STRUCTURE_HEADER **MadtStructs;
> UINTN MaxMadtStructCount;
> UINTN MadtStructsIndex;
> @@ -767,18 +663,19 @@ InstallMadtFromScratch (
>
> MadtStructs = NULL;
> NewMadtTable = NULL;
> + CpuApicIdOrderTable = NULL;
> MaxMadtStructCount = 0;
>
> - mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> - if (mCpuApicIdOrderTable == NULL) {
> - DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable
> structure pointer array\n"));
> + CpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> + if (CpuApicIdOrderTable == NULL) {
> + DEBUG ((DEBUG_ERROR, "Could not allocate CpuApicIdOrderTable
> structure pointer array\n"));
> return EFI_OUT_OF_RESOURCES;
> }
>
> // Call for Local APIC ID Reorder
> - Status = SortCpuLocalApicInTable ();
> + Status = CreateCpuLocalApicInTable (CpuApicIdOrderTable);
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
> + DEBUG ((DEBUG_ERROR, "CreateCpuLocalApicInTable failed: %r\n",
> Status));
> goto Done;
> }
>
> @@ -831,10 +728,10 @@ InstallMadtFromScratch (
> // APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
> // use a processor local x2APIC structure.
> //
> - if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> - ProcLocalApicStruct.Flags = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> - ProcLocalApicStruct.ApicId = (UINT8)
> mCpuApicIdOrderTable[Index].ApicId;
> - ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> + if (!mX2ApicEnabled && CpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> + ProcLocalApicStruct.Flags = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> + ProcLocalApicStruct.ApicId = (UINT8)
> CpuApicIdOrderTable[Index].ApicId;
> + ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
>
> ASSERT (MadtStructsIndex < MaxMadtStructCount);
> Status = CopyStructure (
> @@ -842,10 +739,10 @@ InstallMadtFromScratch (
> (STRUCTURE_HEADER *) &ProcLocalApicStruct,
> &MadtStructs[MadtStructsIndex++]
> );
> - } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> - ProcLocalX2ApicStruct.Flags = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> - ProcLocalX2ApicStruct.X2ApicId =
> mCpuApicIdOrderTable[Index].ApicId;
> - ProcLocalX2ApicStruct.AcpiProcessorUid =
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> + } else if (CpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> + ProcLocalX2ApicStruct.Flags = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> + ProcLocalX2ApicStruct.X2ApicId =
> CpuApicIdOrderTable[Index].ApicId;
> + ProcLocalX2ApicStruct.AcpiProcessorUid =
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
>
> ASSERT (MadtStructsIndex < MaxMadtStructCount);
> Status = CopyStructure (
> @@ -1040,8 +937,8 @@ Done:
> FreePool (NewMadtTable);
> }
>
> - if (mCpuApicIdOrderTable != NULL) {
> - FreePool (mCpuApicIdOrderTable);
> + if (CpuApicIdOrderTable != NULL) {
> + FreePool (CpuApicIdOrderTable);
> }
>
> return Status;
> --
> 2.32.0.windows.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT
2022-08-10 4:32 ` JackX Lin
@ 2022-08-10 5:09 ` Ni, Ray
0 siblings, 0 replies; 16+ messages in thread
From: Ni, Ray @ 2022-08-10 5:09 UTC (permalink / raw)
To: Lin, JackX, Sinha, Ankit
Cc: Chiu, Chasel, Dong, Eric, Yao, Jiewen, Chaganty, Rangasai V,
devel@edk2.groups.io, Kuo, Donald, Kumar, Chandana C,
Palakshareddy, Lavanya C, Palakshareddy, Lavanya C
Jack, the patch is merged.
> -----Original Message-----
> From: Lin, JackX <jackx.lin@intel.com>
> Sent: Wednesday, August 10, 2022 12:32 PM
> To: Ni, Ray <ray.ni@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; devel@edk2.groups.io; Kuo, Donald
> <donald.kuo@intel.com>; Kumar, Chandana C
> <chandana.c.kumar@intel.com>; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>
> Subject: RE: [edk2-platforms: PATCH] Modify processor _UID ordering by
> CPU default fused in MADT
>
> Hi Ray,
>
> The latest patch is sent in the code review mail, and I also attach one here,
> thank you.
>
> Best regards
> Jack
>
> -----Original Message-----
> From: Lin, JackX
> Sent: Wednesday, August 10, 2022 11:51 AM
> To: Ni, Ray <ray.ni@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; devel@edk2.groups.io; Kuo, Donald
> <donald.kuo@intel.com>; Kumar, Chandana C
> <chandana.c.kumar@intel.com>; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>
> Subject: RE: [edk2-platforms: PATCH] Modify processor _UID ordering by
> CPU default fused in MADT
>
> Hi Ray,
>
> I know this patch, and the thread 2 and 3 are added by my request for a
> reason on that time.
> I will re-sent the code patch.
> Thank you.
>
> Jack
>
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Wednesday, August 10, 2022 11:48 AM
> To: Lin, JackX <jackx.lin@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; devel@edk2.groups.io; Kuo, Donald
> <donald.kuo@intel.com>; Kumar, Chandana C
> <chandana.c.kumar@intel.com>; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>; Palakshareddy, Lavanya C
> <lavanya.c.palakshareddy@intel.com>
> Subject: RE: [edk2-platforms: PATCH] Modify processor _UID ordering by
> CPU default fused in MADT
>
> Jack,
> Your patch cannot be merged to trunk because Ankit just did some change in
> the same C file, in below commit.
> * MinPlatformPkg/AcpiTables: Add additional thread mapping in MADT
>
> Ankit,
> It seems your patch is to add support for thread #2 and #3. Jack's patch is to
> remove the additional sorting that put secondary threads after first threads.
> Do you see an issue that we remove the thread sorting logic?
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: Lin, JackX <jackx.lin@intel.com>
> > Sent: Monday, August 8, 2022 4:21 PM
> > To: devel@edk2.groups.io
> > Cc: Lin, JackX <jackx.lin@intel.com>; Lin, JackX
> > <jackx.lin@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Dong,
> > Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ni,
> > Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@intel.com>; Kuo, Donald <donald.kuo@intel.com>;
> > Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy;
> > Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> > Subject: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU
> > default fused in MADT
> >
> > BIOS should not reordering cpu processor_uid
> >
> > Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> > Cc: Chasel Chiu <chasel.chiu@intel.com>
> > Cc: Dong Eric <eric.dong@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> > Cc: Donald Kuo <Donald.Kuo@intel.com>
> > Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
> > Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> > Cc: JackX Lin <JackX.Lin@intel.com>
> > ---
> > Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 174
> > +++++++++++++++++++++++++++++++++++++++-----------------------------
> --
> > ----------------------------------------------------------------------
> > ------------------------
> > ----------
> > 1 file changed, 39 insertions(+), 135 deletions(-)
> >
> > diff --git
> > a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> > b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> > index c7e87cbd7d..176e422e81 100644
> > --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> > @@ -57,38 +57,9 @@ BOOLEAN mForceX2ApicId;
> > BOOLEAN mX2ApicEnabled;
> >
> > EFI_MP_SERVICES_PROTOCOL *mMpService;
> > -BOOLEAN mCpuOrderSorted;
> > -EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL;
> > UINTN mNumberOfCpus = 0;
> > UINTN mNumberOfEnabledCPUs = 0;
> >
> > -
> > -/**
> > - The function is called by PerformQuickSort to compare int values.
> > -
> > - @param[in] Left The pointer to first buffer.
> > - @param[in] Right The pointer to second buffer.
> > -
> > - @return -1 Buffer1 is less than Buffer2.
> > - @return 1 Buffer1 is greater than Buffer2.
> > -
> > -**/
> > -INTN
> > -EFIAPI
> > -ApicIdCompareFunction (
> > - IN CONST VOID *Left,
> > - IN CONST VOID *Right
> > - )
> > -{
> > - UINT32 LeftApicId;
> > - UINT32 RightApicId;
> > -
> > - LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
> > - RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
> > -
> > - return (LeftApicId > RightApicId)? 1 : (-1); -}
> > -
> > /**
> > Print Cpu Apic ID Table
> >
> > @@ -116,7 +87,8 @@ DebugDisplayReOrderTable ( EFI_STATUS
> > AppendCpuMapTableEntry (
> > IN VOID *ApicPtr,
> > - IN UINT32 LocalApicCounter
> > + IN UINT32 LocalApicCounter,
> > + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> > )
> > {
> > EFI_STATUS Status;
> > @@ -131,9 +103,9 @@ AppendCpuMapTableEntry (
> >
> > if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
> > if(!mX2ApicEnabled) {
> > - LocalApicPtr->Flags =
> > (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> > - LocalApicPtr->ApicId =
> > (UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> > - LocalApicPtr->AcpiProcessorUid =
> > (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> > + LocalApicPtr->Flags =
> > (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> > + LocalApicPtr->ApicId =
> > (UINT8)CpuApicIdOrderTable[LocalApicCounter].ApicId;
> > + LocalApicPtr->AcpiProcessorUid =
> > (UINT8)CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> > } else {
> > LocalApicPtr->Flags = 0;
> > LocalApicPtr->ApicId = 0xFF;
> > @@ -142,9 +114,9 @@ AppendCpuMapTableEntry (
> > }
> > } else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
> > if(mX2ApicEnabled) {
> > - LocalX2ApicPtr->Flags =
> > (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> > - LocalX2ApicPtr->X2ApicId =
> > mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> > - LocalX2ApicPtr->AcpiProcessorUid =
> > mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> > + LocalX2ApicPtr->Flags =
> > (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> > + LocalX2ApicPtr->X2ApicId =
> > CpuApicIdOrderTable[LocalApicCounter].ApicId;
> > + LocalX2ApicPtr->AcpiProcessorUid =
> > CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> > } else {
> > LocalX2ApicPtr->Flags = 0;
> > LocalX2ApicPtr->X2ApicId = (UINT32)-1;
> > @@ -159,32 +131,25 @@ AppendCpuMapTableEntry (
> >
> > }
> >
> > +/**
> > + Collect all processors information and create a Cpu Apic Id table.
> > +
> > + @param[in] CpuApicIdOrderTable Buffer to store information of Cpu.
> > +**/
> > EFI_STATUS
> > -SortCpuLocalApicInTable (
> > - VOID
> > +CreateCpuLocalApicInTable (
> > + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> > )
> > {
> > EFI_STATUS Status;
> > EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
> > UINT32 Index;
> > UINT32 CurrProcessor;
> > - UINT32 BspApicId;
> > - EFI_CPU_ID_ORDER_MAP TempVal;
> > EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
> > - UINT32 CoreThreadMask;
> > - EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
> > UINT32 Socket;
> >
> > - Index = 0;
> > Status = EFI_SUCCESS;
> >
> > - if (mCpuOrderSorted) {
> > - return Status;
> > - }
> > -
> > - TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus *
> sizeof
> > (EFI_CPU_ID_ORDER_MAP));
> > - CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
> > -
> > for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> > CurrProcessor++, Index++) {
> > Status = mMpService->GetProcessorInfo (
> > mMpService, @@ -192,7 +157,7 @@
> > SortCpuLocalApicInTable (
> > &ProcessorInfoBuffer
> > );
> >
> > - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> > &TempCpuApicIdOrderTable[Index];
> > + CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> > &CpuApicIdOrderTable[Index];
> > if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
> > CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
> > CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
> > @@ -214,89 +179,26 @@ SortCpuLocalApicInTable (
> > } //end if PROC ENABLE
> > } //end for CurrentProcessor
> >
> > - //keep for debug purpose
> > - DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> > CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
> > mNumOfBitShift));
> > - DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
> > -
> > //
> > // Get Bsp Apic Id
> > //
> > - BspApicId = GetApicId ();
> > - DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
> > -
> > - //
> > - //check to see if 1st entry is BSP, if not swap it
> > - //
> > - if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
> > - for (Index = 0; Index < mNumberOfCpus; Index++) {
> > - if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
> > (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
> > - CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof
> > (EFI_CPU_ID_ORDER_MAP));
> > - CopyMem (&TempCpuApicIdOrderTable[Index],
> > &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
> > - CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof
> > (EFI_CPU_ID_ORDER_MAP));
> > - break;
> > - }
> > - }
> > -
> > - if (mNumberOfCpus <= Index) {
> > - DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
> > Bufferflow\n"));
> > - return EFI_INVALID_PARAMETER;
> > - }
> > - }
> > -
> > - //
> > - // 1. Sort TempCpuApicIdOrderTable,
> > - // sort it by using ApicId from minimum to maximum (Socket0 to
> SocketN),
> > and the BSP must in the fist location of the table.
> > - // So, start sorting the table from the second element and total
> elements
> > are mNumberOfCpus-1.
> > - //
> > - PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus -
> > 1), sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE)
> > ApicIdCompareFunction);
> > -
> > - //
> > - // 2. Sort and map the primary threads to the front of the
> > CpuApicIdOrderTable
> > - //
> > - for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
> > - if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
> > - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> > &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> > - CurrProcessor++;
> > - }
> > - }
> > + DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
> >
> > - //
> > - // 3. Sort and map the second threads to the middle of the
> > CpuApicIdOrderTable
> > - //
> > - for (Index = 0; Index < mNumberOfCpus; Index++) {
> > - if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
> > - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> > &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> > - CurrProcessor++;
> > - }
> > - }
> >
> > //
> > - // 4. Sort and map the not enabled threads to the bottom of the
> > CpuApicIdOrderTable
> > - //
> > - for (Index = 0; Index < mNumberOfCpus; Index++) {
> > - if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
> > - CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> > &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> > - CurrProcessor++;
> > - }
> > - }
> > -
> > - //
> > - // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
> > + // Fill in AcpiProcessorUid.
> > //
> > for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount);
> > Socket++) {
> > for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> > CurrProcessor++) {
> > - if (mCpuApicIdOrderTable[CurrProcessor].Flags &&
> > (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> > - mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> > (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> > Index;
> > + if (CpuApicIdOrderTable[CurrProcessor].Flags &&
> > (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> > + CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> > (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) +
> > Index;
> > Index++;
> > }
> > }
> > }
> >
> > - //keep for debug purpose
> > - DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
> > - DebugDisplayReOrderTable (mCpuApicIdOrderTable);
> > -
> > - mCpuOrderSorted = TRUE;
> > + DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
> > mNumOfBitShift = %x\n", mNumOfBitShift));
> > + DebugDisplayReOrderTable (CpuApicIdOrderTable);
> >
> > return Status;
> > }
> > @@ -750,6 +652,7 @@ InstallMadtFromScratch (
> > EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE LocalApciNmiStruct;
> > EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE
> > ProcLocalX2ApicStruct;
> > EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE
> > LocalX2ApicNmiStruct;
> > + EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable;
> > STRUCTURE_HEADER **MadtStructs;
> > UINTN MaxMadtStructCount;
> > UINTN MadtStructsIndex;
> > @@ -760,18 +663,19 @@ InstallMadtFromScratch (
> >
> > MadtStructs = NULL;
> > NewMadtTable = NULL;
> > + CpuApicIdOrderTable = NULL;
> > MaxMadtStructCount = 0;
> >
> > - mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> > (EFI_CPU_ID_ORDER_MAP));
> > - if (mCpuApicIdOrderTable == NULL) {
> > - DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable
> > structure pointer array\n"));
> > + CpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> > (EFI_CPU_ID_ORDER_MAP));
> > + if (CpuApicIdOrderTable == NULL) {
> > + DEBUG ((DEBUG_ERROR, "Could not allocate CpuApicIdOrderTable
> > structure pointer array\n"));
> > return EFI_OUT_OF_RESOURCES;
> > }
> >
> > // Call for Local APIC ID Reorder
> > - Status = SortCpuLocalApicInTable ();
> > + Status = CreateCpuLocalApicInTable (CpuApicIdOrderTable);
> > if (EFI_ERROR (Status)) {
> > - DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n",
> Status));
> > + DEBUG ((DEBUG_ERROR, "CreateCpuLocalApicInTable failed: %r\n",
> > Status));
> > goto Done;
> > }
> >
> > @@ -824,10 +728,10 @@ InstallMadtFromScratch (
> > // APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
> > // use a processor local x2APIC structure.
> > //
> > - if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId <
> > MAX_UINT8) {
> > - ProcLocalApicStruct.Flags = (UINT8)
> > mCpuApicIdOrderTable[Index].Flags;
> > - ProcLocalApicStruct.ApicId = (UINT8)
> > mCpuApicIdOrderTable[Index].ApicId;
> > - ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> > mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> > + if (!mX2ApicEnabled && CpuApicIdOrderTable[Index].ApicId <
> > MAX_UINT8) {
> > + ProcLocalApicStruct.Flags = (UINT8)
> > CpuApicIdOrderTable[Index].Flags;
> > + ProcLocalApicStruct.ApicId = (UINT8)
> > CpuApicIdOrderTable[Index].ApicId;
> > + ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> > CpuApicIdOrderTable[Index].AcpiProcessorUid;
> >
> > ASSERT (MadtStructsIndex < MaxMadtStructCount);
> > Status = CopyStructure (
> > @@ -835,10 +739,10 @@ InstallMadtFromScratch (
> > (STRUCTURE_HEADER *) &ProcLocalApicStruct,
> > &MadtStructs[MadtStructsIndex++]
> > );
> > - } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> > - ProcLocalX2ApicStruct.Flags = (UINT8)
> > mCpuApicIdOrderTable[Index].Flags;
> > - ProcLocalX2ApicStruct.X2ApicId =
> > mCpuApicIdOrderTable[Index].ApicId;
> > - ProcLocalX2ApicStruct.AcpiProcessorUid =
> > mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> > + } else if (CpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> > + ProcLocalX2ApicStruct.Flags = (UINT8)
> > CpuApicIdOrderTable[Index].Flags;
> > + ProcLocalX2ApicStruct.X2ApicId =
> > CpuApicIdOrderTable[Index].ApicId;
> > + ProcLocalX2ApicStruct.AcpiProcessorUid =
> > CpuApicIdOrderTable[Index].AcpiProcessorUid;
> >
> > ASSERT (MadtStructsIndex < MaxMadtStructCount);
> > Status = CopyStructure (
> > @@ -1033,8 +937,8 @@ Done:
> > FreePool (NewMadtTable);
> > }
> >
> > - if (mCpuApicIdOrderTable != NULL) {
> > - FreePool (mCpuApicIdOrderTable);
> > + if (CpuApicIdOrderTable != NULL) {
> > + FreePool (CpuApicIdOrderTable);
> > }
> >
> > return Status;
> > --
> > 2.32.0.windows.2
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2022-08-10 5:09 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-10 4:28 [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT JackX Lin
2022-08-10 5:06 ` Ni, Ray
-- strict thread matches above, loose matches on Subject: below --
2022-08-08 8:21 JackX Lin
2022-08-09 7:12 ` Ni, Ray
2022-08-09 7:52 ` Donald Kuo
2022-08-10 3:47 ` Ni, Ray
2022-08-10 3:51 ` JackX Lin
2022-08-10 3:52 ` Ni, Ray
2022-08-10 4:32 ` JackX Lin
2022-08-10 5:09 ` Ni, Ray
2022-07-28 7:24 [edk2-platforms:PATCH] " JackX Lin
2022-08-01 3:02 ` Donald Kuo
2022-08-02 16:37 ` Kumar, Chandana C
2022-08-05 6:42 ` Ni, Ray
2022-08-08 8:19 ` JackX Lin
2022-07-27 6:11 [edk2-platforms: PATCH] " JackX Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox