public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-platforms: PATCH V8] Platform/Intel: Correct CPU APIC IDs
@ 2021-08-16 12:15 JackX Lin
  2021-08-19 13:23 ` Ni, Ray
  0 siblings, 1 reply; 3+ messages in thread
From: JackX Lin @ 2021-08-16 12:15 UTC (permalink / raw)
  To: devel
  Cc: Chasel Chiu, Dong, Eric, Jiewen Yao, Ray Ni, Rangasai V Chaganty,
	Donald Kuo, Chandana C Kumar

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3365

BIOS cannot find correct AcpiProcId in mApicIdMap because of
there is no suitable map, that causes ACPI_BIOS_ERROR.
Remove mApicIdMap for determing AcpiProcId, uses normal
countings instead.

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 | 634 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 1 file changed, 268 insertions(+), 366 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 2b51c34ef2..dc1a8337ec 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1,14 +1,13 @@
 /** @file
   ACPI Platform Driver
 
-Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
 #include "AcpiPlatform.h"
-
-#define MAX_CPU_NUM (FixedPcdGet32(PcdMaxCpuThreadCount) * FixedPcdGet32(PcdMaxCpuCoreCount) * FixedPcdGet32(PcdMaxCpuSocketCount))
+#define MAX_SOCKET (FixedPcdGet32 (PcdMaxCpuSocketCount))
 
 #pragma pack(1)
 
@@ -16,8 +15,8 @@ typedef struct {
   UINT32   AcpiProcessorId;
   UINT32   ApicId;
   UINT32   Flags;
-  UINT32   SwProcApicId;
   UINT32   SocketNum;
+  UINT32   Thread;
 } EFI_CPU_ID_ORDER_MAP;
 
 //
@@ -50,7 +49,7 @@ VOID  *mLocalTable[] = {
   &Wsmt,
 };
 
-EFI_ACPI_TABLE_PROTOCOL       *mAcpiTable;
+EFI_ACPI_TABLE_PROTOCOL     *mAcpiTable;
 
 UINT32                      mNumOfBitShift = 6;
 BOOLEAN                     mForceX2ApicId;
@@ -58,138 +57,19 @@ BOOLEAN                     mX2ApicEnabled;
 
 EFI_MP_SERVICES_PROTOCOL    *mMpService;
 BOOLEAN                     mCpuOrderSorted;
-EFI_CPU_ID_ORDER_MAP        mCpuApicIdOrderTable[MAX_CPU_NUM];
-UINTN                       mNumberOfCPUs = 0;
+EFI_CPU_ID_ORDER_MAP        *mCpuApicIdOrderTable = NULL;
+UINTN                       mNumberOfCpus = 0;
 UINTN                       mNumberOfEnabledCPUs = 0;
 
-// following are possible APICID Map for SKX
-static const UINT32 ApicIdMapA[] = {  //for SKUs have number of core > 16
-  //it is 14 + 14 + 14 + 14 format
-  0x00000000, 0x00000001, 0x00000002, 0x00000003, 0x00000004, 0x00000005, 0x00000006, 0x00000007,
-  0x00000008, 0x00000009, 0x0000000A, 0x0000000B, 0x0000000C, 0x0000000D, 0x00000010, 0x00000011,
-  0x00000012, 0x00000013, 0x00000014, 0x00000015, 0x00000016, 0x00000017, 0x00000018, 0x00000019,
-  0x0000001A, 0x0000001B, 0x0000001C, 0x0000001D, 0x00000020, 0x00000021, 0x00000022, 0x00000023,
-  0x00000024, 0x00000025, 0x00000026, 0x00000027, 0x00000028, 0x00000029, 0x0000002A, 0x0000002B,
-  0x0000002C, 0x0000002D, 0x00000030, 0x00000031, 0x00000032, 0x00000033, 0x00000034, 0x00000035,
-  0x00000036, 0x00000037, 0x00000038, 0x00000039, 0x0000003A, 0x0000003B, 0x0000003C, 0x0000003D
-};
-
-static const UINT32 ApicIdMapB[] = { //for SKUs have number of cores <= 16 use 32 ID space
-  //it is 16+16 format
-  0x00000000, 0x00000001, 0x00000002, 0x00000003, 0x00000004, 0x00000005, 0x00000006, 0x00000007,
-  0x00000008, 0x00000009, 0x0000000A, 0x0000000B, 0x0000000C, 0x0000000D, 0x0000000E, 0x0000000F,
-  0x00000010, 0x00000011, 0x00000012, 0x00000013, 0x00000014, 0x00000015, 0x00000016, 0x00000017,
-  0x00000018, 0x00000019, 0x0000001A, 0x0000001B, 0x0000001C, 0x0000001D, 0x0000001E, 0x0000001F,
-  0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
-  0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
-  0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF
-};
-
-
-static const UINT32 ApicIdMapC[] = { //for SKUs have number of cores <= 16 use 64 ID space
-  //it is 16+0+16+0 format
-  0x00000000, 0x00000001, 0x00000002, 0x00000003, 0x00000004, 0x00000005, 0x00000006, 0x00000007,
-  0x00000008, 0x00000009, 0x0000000A, 0x0000000B, 0x0000000C, 0x0000000D, 0x0000000E, 0x0000000F,
-  0x00000020, 0x00000021, 0x00000022, 0x00000023, 0x00000024, 0x00000025, 0x00000026, 0x00000027,
-  0x00000028, 0x00000029, 0x0000002A, 0x0000002B, 0x0000002C, 0x0000002D, 0x0000002E, 0x0000002F,
-  0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
-  0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
-  0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF
-};
-
-static const UINT32 ApicIdMapD[] = { //for SKUs have number of cores <= 8 use 16 ID space
-  //it is 16 format
-  0x00000000, 0x00000001, 0x00000002, 0x00000003, 0x00000004, 0x00000005, 0x00000006, 0x00000007,
-  0x00000008, 0x00000009, 0x0000000A, 0x0000000B, 0x0000000C, 0x0000000D, 0x0000000E, 0x0000000F,
-  0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
-  0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
-  0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
-  0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF,
-  0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF
-};
-
-const UINT32 *mApicIdMap = NULL;
-
 /**
-  This function detect the APICID map and update ApicID Map pointer
+  Find BSP in mCpuApicIdOrderTable.
 
-  @param None
+  This function searches mCpuApicIdOrderTable to find the BSP ApicId, and returns a number where the BSP is.
 
-  @retval VOID
-
-**/
-VOID DetectApicIdMap(VOID)
-{
-  UINTN                  CoreCount;
-
-  CoreCount = 0;
-
-  if(mApicIdMap != NULL) {
-    return;   //aleady initialized
-  }
-
-  mApicIdMap = ApicIdMapA;  // default to > 16C SKUs
-
-  CoreCount = mNumberOfEnabledCPUs / 2;
-  DEBUG ((DEBUG_INFO, "CoreCount - %d\n", CoreCount));
-
-  //DEBUG((EFI_D_ERROR, ":: Default to use Map A @ %08X FusedCoreCount: %02d, sktlevel: %d\n",mApicIdMap, FusedCoreCount, mNumOfBitShift));
-  // Dont assert for single core, single thread system.
-  //ASSERT (CoreCount != 0);
-
-  if(CoreCount <= 16) {
-
-    if(mNumOfBitShift == 4) {
-      mApicIdMap = ApicIdMapD;
-      //DEBUG((EFI_D_ERROR, ":: Use Map B @ %08X\n",mApicIdMap));
-    }
-
-    if(mNumOfBitShift == 5) {
-      mApicIdMap = ApicIdMapB;
-      //DEBUG((EFI_D_ERROR, ":: Use Map B @ %08X\n",mApicIdMap));
-    }
-
-    if(mNumOfBitShift == 6) {
-      mApicIdMap = ApicIdMapC;
-      //DEBUG((EFI_D_ERROR, ":: Use Map C @ %08X\n",mApicIdMap));
-    }
-
-  }
-
-  return;
-}
-
-/**
-  This function return the CoreThreadId of ApicId from ACPI ApicId Map array
-
-  @param ApicId
-
-  @retval Index of ACPI ApicId Map array
+  @param[in]  ApicId                  Apic ID.
 
+  @return     Where the BSP is.
 **/
-UINT32
-GetIndexFromApicId (
-  UINT32 ApicId
-  )
-{
-  UINT32 CoreThreadId;
-  UINT32 i;
-
-  ASSERT (mApicIdMap != NULL);
-
-  CoreThreadId = ApicId & ((1 << mNumOfBitShift) - 1);
-
-  for(i = 0; i < (FixedPcdGet32(PcdMaxCpuCoreCount) * FixedPcdGet32(PcdMaxCpuThreadCount)); i++) {
-    if(mApicIdMap[i] == CoreThreadId) {
-      break;
-    }
-  }
-
-  ASSERT (i <= (FixedPcdGet32(PcdMaxCpuCoreCount) * FixedPcdGet32(PcdMaxCpuThreadCount)));
-
-  return i;
-}
-
 UINT32
 ApicId2SwProcApicId (
   UINT32 ApicId
@@ -197,7 +77,7 @@ ApicId2SwProcApicId (
 {
   UINT32 Index;
 
-  for (Index = 0; Index < MAX_CPU_NUM; Index++) {
+  for (Index = 0; Index < mNumberOfCpus; Index++) {
     if ((mCpuApicIdOrderTable[Index].Flags == 1) && (mCpuApicIdOrderTable[Index].ApicId == ApicId)) {
       return Index;
     }
@@ -207,21 +87,27 @@ ApicId2SwProcApicId (
 
 }
 
+/**
+  Print Cpu Apic ID Table
+
+  @param[in]  CpuApicIdOrderTable       Data will be dumped.
+**/
 VOID
-DebugDisplayReOrderTable(
-  VOID
+DebugDisplayReOrderTable (
+  IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
   )
 {
   UINT32 Index;
 
-  DEBUG ((EFI_D_ERROR, "Index  AcpiProcId  ApicId  Flags  SwApicId  Skt\n"));
-  for (Index=0; Index<MAX_CPU_NUM; Index++) {
-    DEBUG ((EFI_D_ERROR, " %02d       0x%02X      0x%02X      %d      0x%02X     %d\n",
-                           Index, mCpuApicIdOrderTable[Index].AcpiProcessorId,
-                           mCpuApicIdOrderTable[Index].ApicId,
-                           mCpuApicIdOrderTable[Index].Flags,
-                           mCpuApicIdOrderTable[Index].SwProcApicId,
-                           mCpuApicIdOrderTable[Index].SocketNum));
+  DEBUG ((DEBUG_INFO, "Index  AcpiProcId  ApicId   Thread  Flags   Skt\n"));
+  for (Index = 0; Index < mNumberOfCpus; Index++) {
+    DEBUG ((DEBUG_INFO, " %02d       0x%02X      0x%02X       %d      %d      %d\n",
+                           Index,
+                           CpuApicIdOrderTable[Index].AcpiProcessorId,
+                           CpuApicIdOrderTable[Index].ApicId,
+                           CpuApicIdOrderTable[Index].Thread,
+                           CpuApicIdOrderTable[Index].Flags,
+                           CpuApicIdOrderTable[Index].SocketNum));
   }
 }
 
@@ -281,131 +167,143 @@ SortCpuLocalApicInTable (
   UINT32                                    Index;
   UINT32                                    CurrProcessor;
   UINT32                                    BspApicId;
-  UINT32                                    TempVal = 0;
+  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;
 
-  CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
-
-  if(!mCpuOrderSorted) {
-
-    Index  = 0;
+  if (mCpuOrderSorted) {
+    return Status;
+  }
 
-    for (CurrProcessor = 0; CurrProcessor < mNumberOfCPUs; CurrProcessor++) {
-      Status = mMpService->GetProcessorInfo (
-                                            mMpService,
-                                            CurrProcessor,
-                                            &ProcessorInfoBuffer
-                                            );
+  TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
+  TempVal = AllocateZeroPool (sizeof (EFI_CPU_ID_ORDER_MAP));
+  CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
 
-      if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
-        if(ProcessorInfoBuffer.ProcessorId & 1) { //is 2nd thread
-          CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)&mCpuApicIdOrderTable[(Index - 1) + MAX_CPU_NUM / 2];
-        } else { //is primary thread
-          CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)&mCpuApicIdOrderTable[Index];
-          Index++;
-        }
-        CpuIdMapPtr->ApicId  = (UINT32)ProcessorInfoBuffer.ProcessorId;
-        CpuIdMapPtr->Flags   = ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0);
-        CpuIdMapPtr->SocketNum = (UINT32)ProcessorInfoBuffer.Location.Package;
-        CpuIdMapPtr->AcpiProcessorId = (CpuIdMapPtr->SocketNum * FixedPcdGet32(PcdMaxCpuCoreCount) * FixedPcdGet32(PcdMaxCpuThreadCount)) + GetIndexFromApicId(CpuIdMapPtr->ApicId); //CpuIdMapPtr->ApicId;
-        CpuIdMapPtr->SwProcApicId = ((UINT32)(ProcessorInfoBuffer.Location.Package << mNumOfBitShift) + (((UINT32)ProcessorInfoBuffer.ProcessorId) & CoreThreadMask));
-        if(mX2ApicEnabled) { //if X2Apic, re-order the socket # so it starts from base 0 and contiguous
-          //may not necessory!!!!!
+  for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++, Index++) {
+    Status = mMpService->GetProcessorInfo (
+                           mMpService,
+                           CurrProcessor,
+                           &ProcessorInfoBuffer
+                           );
+
+    CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &TempCpuApicIdOrderTable[Index];
+    if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
+      CpuIdMapPtr->ApicId  = (UINT32)ProcessorInfoBuffer.ProcessorId;
+      CpuIdMapPtr->Thread  = ProcessorInfoBuffer.Location.Thread;
+      CpuIdMapPtr->Flags   = ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0);
+      CpuIdMapPtr->SocketNum = ProcessorInfoBuffer.Location.Package;
+      CpuIdMapPtr->AcpiProcessorId = (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) + (UINT32)ProcessorInfoBuffer.ProcessorId;
+
+      //update processorbitMask
+      if (CpuIdMapPtr->Flags == 1) {
+        if (mForceX2ApicId) {
+          CpuIdMapPtr->SocketNum &= 0x7;
+          CpuIdMapPtr->AcpiProcessorId &= 0xFF; //keep lower 8bit due to use Proc obj in dsdt
         }
+      }
+    } else {  //not enabled
+      CpuIdMapPtr->ApicId  = (UINT32)-1;
+      CpuIdMapPtr->Thread = (UINT32)-1;
+      CpuIdMapPtr->Flags   = 0;
+      CpuIdMapPtr->AcpiProcessorId = (UINT32)-1;
+      CpuIdMapPtr->SocketNum = (UINT32)-1;
+    } //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);
+
+  //make sure 1st entry is BSP
+  if (mX2ApicEnabled) {
+    BspApicId = (UINT32)AsmReadMsr64 (0x802);
+  } else {
+    BspApicId = (*(volatile UINT32 *)(UINTN)0xFEE00020) >> 24;
+  }
+  DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
 
-        //update processorbitMask
-        if (CpuIdMapPtr->Flags == 1) {
+  if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
+    //check to see if 1st entry is BSP, if not swap it
+    Index = ApicId2SwProcApicId (BspApicId);
 
-          if(mForceX2ApicId) {
-            CpuIdMapPtr->SocketNum &= 0x7;
-            CpuIdMapPtr->AcpiProcessorId &= 0xFF; //keep lower 8bit due to use Proc obj in dsdt
-            CpuIdMapPtr->SwProcApicId &= 0xFF;
-          }
-        }
-      } else {  //not enabled
-        CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)&mCpuApicIdOrderTable[Index];
-        CpuIdMapPtr->ApicId  = (UINT32)-1;
-        CpuIdMapPtr->Flags   = 0;
-        CpuIdMapPtr->AcpiProcessorId = (UINT32)-1;
-        CpuIdMapPtr->SwProcApicId = (UINT32)-1;
-        CpuIdMapPtr->SocketNum = (UINT32)-1;
-      } //end if PROC ENABLE
-    } //end for CurrentProcessor
-
-    //keep for debug purpose
-    DEBUG(( EFI_D_ERROR, "::ACPI::  APIC ID Order Table Init.   CoreThreadMask = %x,  mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift));
-    DebugDisplayReOrderTable();
-
-    //make sure 1st entry is BSP
-    if(mX2ApicEnabled) {
-      BspApicId = (UINT32)AsmReadMsr64(0x802);
-    } else {
-      BspApicId = (*(volatile UINT32 *)(UINTN)0xFEE00020) >> 24;
+    if (mNumberOfCpus <= Index) {
+      DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n"));
+      return EFI_INVALID_PARAMETER;
     }
-    DEBUG ((EFI_D_INFO, "BspApicId - 0x%x\n", BspApicId));
 
-    if(mCpuApicIdOrderTable[0].ApicId != BspApicId) {
-      //check to see if 1st entry is BSP, if not swap it
-      Index = ApicId2SwProcApicId(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));
+    TempCpuApicIdOrderTable[0].Flags = 1;
+  }
 
-      if(MAX_CPU_NUM <= Index) {
-        DEBUG ((EFI_D_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n"));
-        return EFI_INVALID_PARAMETER;
+  //
+  // 1. Sort TempCpuApicIdOrderTable, 
+  //    sort it by using AcpiProcessorId 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.
+  //
+  for (CurrProcessor = 1; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
+    for (Index = CurrProcessor+1; Index < mNumberOfCpus; Index++) {
+      if (TempCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId > TempCpuApicIdOrderTable[Index].AcpiProcessorId) {
+        CopyMem (&TempVal, &TempCpuApicIdOrderTable[CurrProcessor], sizeof (EFI_CPU_ID_ORDER_MAP));
+        CopyMem (&TempCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
+        CopyMem (&TempCpuApicIdOrderTable[Index], &TempVal, sizeof (EFI_CPU_ID_ORDER_MAP));
       }
+    }
+  }
 
-      TempVal = mCpuApicIdOrderTable[Index].ApicId;
-      mCpuApicIdOrderTable[Index].ApicId = mCpuApicIdOrderTable[0].ApicId;
-      mCpuApicIdOrderTable[0].ApicId = TempVal;
-      mCpuApicIdOrderTable[Index].Flags = mCpuApicIdOrderTable[0].Flags;
-      mCpuApicIdOrderTable[0].Flags = 1;
-      TempVal = mCpuApicIdOrderTable[Index].SwProcApicId;
-      mCpuApicIdOrderTable[Index].SwProcApicId = mCpuApicIdOrderTable[0].SwProcApicId;
-      mCpuApicIdOrderTable[0].SwProcApicId = TempVal;
-      //swap AcpiProcId
-      TempVal = mCpuApicIdOrderTable[Index].AcpiProcessorId;
-      mCpuApicIdOrderTable[Index].AcpiProcessorId = mCpuApicIdOrderTable[0].AcpiProcessorId;
-      mCpuApicIdOrderTable[0].AcpiProcessorId = TempVal;
+  //
+  // 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++;
     }
+  }
 
-    //Make sure no holes between enabled threads
-    for(CurrProcessor = 0; CurrProcessor < MAX_CPU_NUM; CurrProcessor++) {
-
-      if(mCpuApicIdOrderTable[CurrProcessor].Flags == 0) {
-        //make sure disabled entry has ProcId set to FFs
-        mCpuApicIdOrderTable[CurrProcessor].ApicId = (UINT32)-1;
-        mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId = (UINT32)-1;
-        mCpuApicIdOrderTable[CurrProcessor].SwProcApicId = (UINT32)-1;
-
-        for(Index = CurrProcessor+1; Index < MAX_CPU_NUM; Index++) {
-          if(mCpuApicIdOrderTable[Index].Flags == 1) {
-            //move enabled entry up
-            mCpuApicIdOrderTable[CurrProcessor].Flags = 1;
-            mCpuApicIdOrderTable[CurrProcessor].ApicId = mCpuApicIdOrderTable[Index].ApicId;
-            mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId = mCpuApicIdOrderTable[Index].AcpiProcessorId;
-            mCpuApicIdOrderTable[CurrProcessor].SwProcApicId = mCpuApicIdOrderTable[Index].SwProcApicId;
-            mCpuApicIdOrderTable[CurrProcessor].SocketNum = mCpuApicIdOrderTable[Index].SocketNum;
-            //disable moved entry
-            mCpuApicIdOrderTable[Index].Flags = 0;
-            mCpuApicIdOrderTable[Index].ApicId = (UINT32)-1;
-            mCpuApicIdOrderTable[Index].AcpiProcessorId = (UINT32)-1;
-            mCpuApicIdOrderTable[Index].SwProcApicId = (UINT32)-1;
-            break;
-          }
-        }
+  //
+  // 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-assigen AcpiProcessorId for AcpiProcessorUId uses purpose.
+  //
+  for (Socket = 0; Socket < MAX_SOCKET; Socket++) {
+    for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
+      if (mCpuApicIdOrderTable[CurrProcessor].Flags && (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
+        mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId = (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) + Index;
+        Index++;
       }
     }
+  }
 
-    //keep for debug purpose
-    DEBUG ((EFI_D_ERROR, "APIC ID Order Table ReOrdered\n"));
-    DebugDisplayReOrderTable();
+  //keep for debug purpose
+  DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
+  DebugDisplayReOrderTable (mCpuApicIdOrderTable);
 
-    mCpuOrderSorted = TRUE;
-  }
+  mCpuOrderSorted = TRUE;
 
   return Status;
 }
@@ -602,11 +500,11 @@ InitializeMadtHeader (
   }
 
   Status = InitializeHeader (
-    &MadtHeader->Header,
-    EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
-    EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
-    0
-    );
+             &MadtHeader->Header,
+             EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
+             EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
+             0
+             );
   if (EFI_ERROR (Status)) {
     return Status;
   }
@@ -784,11 +682,11 @@ BuildAcpiTable (
   // Allocate the memory needed for the table.
   //
   Status = AllocateTable (
-    TableSpecificHdrLength,
-    Structures,
-    StructureCount,
-    &InternalTable
-    );
+             TableSpecificHdrLength,
+             Structures,
+             StructureCount,
+             &InternalTable
+             );
   if (EFI_ERROR (Status)) {
     return Status;
   }
@@ -871,18 +769,22 @@ InstallMadtFromScratch (
   NewMadtTable = NULL;
   MaxMadtStructCount = 0;
 
-  DetectApicIdMap();
+  mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
+  if (mCpuApicIdOrderTable == NULL) {
+    DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable structure pointer array\n"));
+    return EFI_OUT_OF_RESOURCES;
+  }
 
   // Call for Local APIC ID Reorder
   Status = SortCpuLocalApicInTable ();
   if (EFI_ERROR (Status)) {
-    DEBUG ((EFI_D_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
+    DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
     goto Done;
   }
 
   MaxMadtStructCount = (UINT32) (
-    MAX_CPU_NUM +    // processor local APIC structures
-    MAX_CPU_NUM +    // processor local x2APIC structures
+    mNumberOfCpus +  // processor local APIC structures
+    mNumberOfCpus +  // processor local x2APIC structures
     1 + PcdGet8(PcdPcIoApicCount) +   // I/O APIC structures
     2 +              // interrupt source override structures
     1 +              // local APIC NMI structures
@@ -906,11 +808,11 @@ InstallMadtFromScratch (
   //
   Status = InitializeMadtHeader (&MadtTableHeader);
   if (EFI_ERROR (Status)) {
-    DEBUG ((EFI_D_ERROR, "InitializeMadtHeader failed: %r\n", Status));
+    DEBUG ((DEBUG_ERROR, "InitializeMadtHeader failed: %r\n", Status));
     goto Done;
   }
 
-  DEBUG ((EFI_D_INFO, "Number of CPUs detected = %d \n", mNumberOfCPUs));
+  DEBUG ((DEBUG_INFO, "Number of CPUs detected = %d \n", mNumberOfCpus));
 
   //
   // Build Processor Local APIC Structures and Processor Local X2APIC Structures
@@ -923,7 +825,7 @@ InstallMadtFromScratch (
   ProcLocalX2ApicStruct.Reserved[0] = 0;
   ProcLocalX2ApicStruct.Reserved[1] = 0;
 
-  for (Index = 0; Index < MAX_CPU_NUM; Index++) {
+  for (Index = 0; Index < mNumberOfCpus; Index++) {
     //
     // If x2APIC mode is not enabled, and if it is possible to express the
     // APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
@@ -936,10 +838,10 @@ InstallMadtFromScratch (
 
       ASSERT (MadtStructsIndex < MaxMadtStructCount);
       Status = CopyStructure (
-        &MadtTableHeader.Header,
-        (STRUCTURE_HEADER *) &ProcLocalApicStruct,
-        &MadtStructs[MadtStructsIndex++]
-        );
+                 &MadtTableHeader.Header,
+                 (STRUCTURE_HEADER *) &ProcLocalApicStruct,
+                 &MadtStructs[MadtStructsIndex++]
+                 );
     } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
       ProcLocalX2ApicStruct.Flags            = (UINT8) mCpuApicIdOrderTable[Index].Flags;
       ProcLocalX2ApicStruct.X2ApicId         = mCpuApicIdOrderTable[Index].ApicId;
@@ -947,13 +849,13 @@ InstallMadtFromScratch (
 
       ASSERT (MadtStructsIndex < MaxMadtStructCount);
       Status = CopyStructure (
-        &MadtTableHeader.Header,
-        (STRUCTURE_HEADER *) &ProcLocalX2ApicStruct,
-        &MadtStructs[MadtStructsIndex++]
-        );
+                 &MadtTableHeader.Header,
+                 (STRUCTURE_HEADER *) &ProcLocalX2ApicStruct,
+                 &MadtStructs[MadtStructsIndex++]
+                 );
     }
     if (EFI_ERROR (Status)) {
-      DEBUG ((EFI_D_ERROR, "CopyMadtStructure (local APIC/x2APIC) failed: %r\n", Status));
+      DEBUG ((DEBUG_ERROR, "CopyMadtStructure (local APIC/x2APIC) failed: %r\n", Status));
       goto Done;
     }
   }
@@ -965,44 +867,44 @@ InstallMadtFromScratch (
   IoApicStruct.Length = sizeof (EFI_ACPI_4_0_IO_APIC_STRUCTURE);
   IoApicStruct.Reserved = 0;
 
-  PcIoApicEnable = PcdGet32(PcdPcIoApicEnable);
+  PcIoApicEnable = PcdGet32 (PcdPcIoApicEnable);
 
-  if (FixedPcdGet32(PcdMaxCpuSocketCount) <= 4) {
+  if (FixedPcdGet32 (PcdMaxCpuSocketCount) <= 4) {
     IoApicStruct.IoApicId                  = PcdGet8(PcdIoApicId);
     IoApicStruct.IoApicAddress             = PcdGet32(PcdIoApicAddress);
     IoApicStruct.GlobalSystemInterruptBase = 0;
     ASSERT (MadtStructsIndex < MaxMadtStructCount);
     Status = CopyStructure (
-      &MadtTableHeader.Header,
-      (STRUCTURE_HEADER *) &IoApicStruct,
-      &MadtStructs[MadtStructsIndex++]
-      );
+               &MadtTableHeader.Header,
+               (STRUCTURE_HEADER *) &IoApicStruct,
+               &MadtStructs[MadtStructsIndex++]
+               );
     if (EFI_ERROR (Status)) {
-      DEBUG ((EFI_D_ERROR, "CopyMadtStructure (I/O APIC) failed: %r\n", Status));
+      DEBUG ((DEBUG_ERROR, "CopyMadtStructure (I/O APIC) failed: %r\n", Status));
       goto Done;
     }
   }
 
   for (PcIoApicIndex = 0; PcIoApicIndex < PcdGet8(PcdPcIoApicCount); PcIoApicIndex++) {
-      PcIoApicMask = (1 << PcIoApicIndex);
-      if ((PcIoApicEnable & PcIoApicMask) == 0) {
-        continue;
-      }
+    PcIoApicMask = (1 << PcIoApicIndex);
+    if ((PcIoApicEnable & PcIoApicMask) == 0) {
+      continue;
+    }
 
-      IoApicStruct.IoApicId                  = (UINT8)(PcdGet8(PcdPcIoApicIdBase) + PcIoApicIndex);
-      IoApicStruct.IoApicAddress             = CurrentIoApicAddress;
-      CurrentIoApicAddress                   = (CurrentIoApicAddress & 0xFFFF8000) + 0x8000;
-      IoApicStruct.GlobalSystemInterruptBase = (UINT32)(24 + (PcIoApicIndex * 8));
-      ASSERT (MadtStructsIndex < MaxMadtStructCount);
-      Status = CopyStructure (
-        &MadtTableHeader.Header,
-        (STRUCTURE_HEADER *) &IoApicStruct,
-        &MadtStructs[MadtStructsIndex++]
-        );
-      if (EFI_ERROR (Status)) {
-        DEBUG ((EFI_D_ERROR, "CopyMadtStructure (I/O APIC) failed: %r\n", Status));
-        goto Done;
-      }
+    IoApicStruct.IoApicId                  = (UINT8)(PcdGet8(PcdPcIoApicIdBase) + PcIoApicIndex);
+    IoApicStruct.IoApicAddress             = CurrentIoApicAddress;
+    CurrentIoApicAddress                   = (CurrentIoApicAddress & 0xFFFF8000) + 0x8000;
+    IoApicStruct.GlobalSystemInterruptBase = (UINT32)(24 + (PcIoApicIndex * 8));
+    ASSERT (MadtStructsIndex < MaxMadtStructCount);
+    Status = CopyStructure (
+               &MadtTableHeader.Header,
+               (STRUCTURE_HEADER *) &IoApicStruct,
+               &MadtStructs[MadtStructsIndex++]
+               );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "CopyMadtStructure (I/O APIC) failed: %r\n", Status));
+      goto Done;
+    }
   }
 
   //
@@ -1021,12 +923,12 @@ InstallMadtFromScratch (
 
   ASSERT (MadtStructsIndex < MaxMadtStructCount);
   Status = CopyStructure (
-    &MadtTableHeader.Header,
-    (STRUCTURE_HEADER *) &IntSrcOverrideStruct,
-    &MadtStructs[MadtStructsIndex++]
-    );
+             &MadtTableHeader.Header,
+             (STRUCTURE_HEADER *) &IntSrcOverrideStruct,
+             &MadtStructs[MadtStructsIndex++]
+             );
   if (EFI_ERROR (Status)) {
-    DEBUG ((EFI_D_ERROR, "CopyMadtStructure (IRQ2 source override) failed: %r\n", Status));
+    DEBUG ((DEBUG_ERROR, "CopyMadtStructure (IRQ2 source override) failed: %r\n", Status));
     goto Done;
   }
 
@@ -1040,12 +942,12 @@ InstallMadtFromScratch (
 
   ASSERT (MadtStructsIndex < MaxMadtStructCount);
   Status = CopyStructure (
-    &MadtTableHeader.Header,
-    (STRUCTURE_HEADER *) &IntSrcOverrideStruct,
-    &MadtStructs[MadtStructsIndex++]
-    );
+             &MadtTableHeader.Header,
+             (STRUCTURE_HEADER *) &IntSrcOverrideStruct,
+             &MadtStructs[MadtStructsIndex++]
+             );
   if (EFI_ERROR (Status)) {
-    DEBUG ((EFI_D_ERROR, "CopyMadtStructure (IRQ9 source override) failed: %r\n", Status));
+    DEBUG ((DEBUG_ERROR, "CopyMadtStructure (IRQ9 source override) failed: %r\n", Status));
     goto Done;
   }
 
@@ -1060,12 +962,12 @@ InstallMadtFromScratch (
 
   ASSERT (MadtStructsIndex < MaxMadtStructCount);
   Status = CopyStructure (
-    &MadtTableHeader.Header,
-    (STRUCTURE_HEADER *) &LocalApciNmiStruct,
-    &MadtStructs[MadtStructsIndex++]
-    );
+             &MadtTableHeader.Header,
+             (STRUCTURE_HEADER *) &LocalApciNmiStruct,
+             &MadtStructs[MadtStructsIndex++]
+             );
   if (EFI_ERROR (Status)) {
-    DEBUG ((EFI_D_ERROR, "CopyMadtStructure (APIC NMI) failed: %r\n", Status));
+    DEBUG ((DEBUG_ERROR, "CopyMadtStructure (APIC NMI) failed: %r\n", Status));
     goto Done;
   }
 
@@ -1084,10 +986,10 @@ InstallMadtFromScratch (
 
     ASSERT (MadtStructsIndex < MaxMadtStructCount);
     Status = CopyStructure (
-      &MadtTableHeader.Header,
-      (STRUCTURE_HEADER *) &LocalX2ApicNmiStruct,
-      &MadtStructs[MadtStructsIndex++]
-      );
+               &MadtTableHeader.Header,
+               (STRUCTURE_HEADER *) &LocalX2ApicNmiStruct,
+               &MadtStructs[MadtStructsIndex++]
+               );
     if (EFI_ERROR (Status)) {
       DEBUG ((DEBUG_ERROR, "CopyMadtStructure (x2APIC NMI) failed: %r\n", Status));
       goto Done;
@@ -1098,14 +1000,14 @@ InstallMadtFromScratch (
   // Build Madt Structure from the Madt Header and collection of pointers in MadtStructs[]
   //
   Status = BuildAcpiTable (
-    (EFI_ACPI_DESCRIPTION_HEADER *) &MadtTableHeader,
-    sizeof (EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER),
-    MadtStructs,
-    MadtStructsIndex,
-    (UINT8 **)&NewMadtTable
-    );
+             (EFI_ACPI_DESCRIPTION_HEADER *) &MadtTableHeader,
+             sizeof (EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER),
+             MadtStructs,
+             MadtStructsIndex,
+             (UINT8 **) &NewMadtTable
+             );
   if (EFI_ERROR (Status)) {
-    DEBUG ((EFI_D_ERROR, "BuildAcpiTable failed: %r\n", Status));
+    DEBUG ((DEBUG_ERROR, "BuildAcpiTable failed: %r\n", Status));
     goto Done;
   }
 
@@ -1113,11 +1015,11 @@ InstallMadtFromScratch (
   // Publish Madt Structure to ACPI
   //
   Status = mAcpiTable->InstallAcpiTable (
-    mAcpiTable,
-    NewMadtTable,
-    NewMadtTable->Header.Length,
-    &TableHandle
-    );
+                         mAcpiTable,
+                         NewMadtTable,
+                         NewMadtTable->Header.Length,
+                         &TableHandle
+                         );
 
 Done:
   //
@@ -1136,6 +1038,10 @@ Done:
     FreePool (NewMadtTable);
   }
 
+  if (mCpuApicIdOrderTable != NULL) {
+    FreePool (mCpuApicIdOrderTable);
+  }
+
   return Status;
 }
 
@@ -1155,8 +1061,8 @@ InstallMcfgFromScratch (
   PciSegmentInfo = GetPciSegmentInfo (&SegmentCount);
 
   McfgTable = AllocateZeroPool (
-                sizeof(EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER) +
-                sizeof(EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE) * SegmentCount
+                sizeof (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER) +
+                sizeof (EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE) * SegmentCount
                 );
   if (McfgTable == NULL) {
     DEBUG ((DEBUG_ERROR, "Could not allocate MCFG structure\n"));
@@ -1164,11 +1070,11 @@ InstallMcfgFromScratch (
   }
 
   Status = InitializeHeader (
-    &McfgTable->Header,
-    EFI_ACPI_3_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
-    EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_TABLE_REVISION,
-    0
-    );
+             &McfgTable->Header,
+             EFI_ACPI_3_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
+             EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_TABLE_REVISION,
+             0
+             );
   if (EFI_ERROR (Status)) {
     return Status;
   }
@@ -1192,11 +1098,11 @@ InstallMcfgFromScratch (
   // Publish Madt Structure to ACPI
   //
   Status = mAcpiTable->InstallAcpiTable (
-    mAcpiTable,
-    McfgTable,
-    McfgTable->Header.Length,
-    &TableHandle
-    );
+                         mAcpiTable,
+                         McfgTable,
+                         McfgTable->Header.Length,
+                         &TableHandle
+                         );
 
   return Status;
 }
@@ -1280,7 +1186,7 @@ PlatformUpdateTables (
   switch (Table->Signature) {
 
   case EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE:
-    ASSERT(FALSE);
+    ASSERT (FALSE);
     break;
 
   case EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE:
@@ -1324,9 +1230,9 @@ PlatformUpdateTables (
       FadtHeader->XGpe1Blk.AccessSize = 0;
     }
 
-    DEBUG(( EFI_D_ERROR, "ACPI FADT table @ address 0x%x\n", Table ));
-    DEBUG(( EFI_D_ERROR, "  IaPcBootArch 0x%x\n", FadtHeader->IaPcBootArch ));
-    DEBUG(( EFI_D_ERROR, "  Flags 0x%x\n", FadtHeader->Flags ));
+    DEBUG ((DEBUG_INFO, "ACPI FADT table @ address 0x%x\n", Table));
+    DEBUG ((DEBUG_INFO, "  IaPcBootArch 0x%x\n", FadtHeader->IaPcBootArch));
+    DEBUG ((DEBUG_INFO, "  Flags 0x%x\n", FadtHeader->Flags));
     break;
 
   case EFI_ACPI_3_0_HIGH_PRECISION_EVENT_TIMER_TABLE_SIGNATURE:
@@ -1346,12 +1252,12 @@ PlatformUpdateTables (
     HpetBlockId.Bits.VendorId       = HpetCapabilities.Bits.VendorId;
     HpetTable->EventTimerBlockId    = HpetBlockId.Uint32;
     HpetTable->MainCounterMinimumClockTickInPeriodicMode = (UINT16)HpetCapabilities.Bits.CounterClockPeriod;
-    DEBUG(( EFI_D_ERROR, "ACPI HPET table @ address 0x%x\n", Table ));
-    DEBUG(( EFI_D_ERROR, "  HPET base 0x%x\n", PcdGet32 (PcdHpetBaseAddress) ));
+    DEBUG ((DEBUG_INFO, "ACPI HPET table @ address 0x%x\n", Table));
+    DEBUG ((DEBUG_INFO, "  HPET base 0x%x\n", PcdGet32 (PcdHpetBaseAddress)));
     break;
 
   case EFI_ACPI_3_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE:
-    ASSERT(FALSE);
+    ASSERT (FALSE);
     break;
 
   default:
@@ -1403,8 +1309,8 @@ IsHardwareChange (
   // pFADT->XDsdt
   //
   HWChangeSize = HandleCount + 1;
-  HWChange = AllocateZeroPool( sizeof(UINT32) * HWChangeSize );
-  ASSERT( HWChange != NULL );
+  HWChange = AllocateZeroPool (sizeof(UINT32) * HWChangeSize);
+  ASSERT(HWChange != NULL);
 
   if (HWChange == NULL) return;
 
@@ -1445,14 +1351,14 @@ IsHardwareChange (
   // Calculate CRC value with HWChange data.
   //
   Status = gBS->CalculateCrc32(HWChange, HWChangeSize, &CRC);
-  DEBUG((DEBUG_INFO, "CRC = %x and Status = %r\n", CRC, Status));
+  DEBUG ((DEBUG_INFO, "CRC = %x and Status = %r\n", CRC, Status));
 
   //
   // Set HardwareSignature value based on CRC value.
   //
   FacsPtr = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)pFADT->FirmwareCtrl;
   FacsPtr->HardwareSignature = CRC;
-  FreePool( HWChange );
+  FreePool (HWChange);
 }
 
 VOID
@@ -1475,17 +1381,16 @@ UpdateLocalTable (
 
     if (Version != EFI_ACPI_TABLE_VERSION_NONE) {
       Status = mAcpiTable->InstallAcpiTable (
-                              mAcpiTable,
-                              CurrentTable,
-                              CurrentTable->Length,
-                              &TableHandle
-                              );
+                             mAcpiTable,
+                             CurrentTable,
+                             CurrentTable->Length,
+                             &TableHandle
+                             );
       ASSERT_EFI_ERROR (Status);
     }
   }
 }
 
-
 VOID
 EFIAPI
 AcpiEndOfDxeEvent (
@@ -1493,16 +1398,14 @@ AcpiEndOfDxeEvent (
   VOID                *ParentImageHandle
   )
 {
-
   if (Event != NULL) {
-    gBS->CloseEvent(Event);
+    gBS->CloseEvent (Event);
   }
 
-
   //
   // Calculate Hardware Signature value based on current platform configurations
   //
-  IsHardwareChange();
+  IsHardwareChange ();
 }
 
 /**
@@ -1526,7 +1429,6 @@ InstallAcpiPlatform (
   EFI_STATUS                    Status;
   EFI_EVENT                     EndOfDxeEvent;
 
-
   Status = gBS->LocateProtocol (&gEfiMpServiceProtocolGuid, NULL, (VOID **)&mMpService);
   ASSERT_EFI_ERROR (Status);
 
@@ -1550,19 +1452,19 @@ InstallAcpiPlatform (
   // Determine the number of processors
   //
   mMpService->GetNumberOfProcessors (
-              mMpService,
-              &mNumberOfCPUs,
-              &mNumberOfEnabledCPUs
-              );
-  ASSERT (mNumberOfCPUs <= MAX_CPU_NUM && mNumberOfEnabledCPUs >= 1);
-  DEBUG ((DEBUG_INFO, "mNumberOfCPUs - %d\n", mNumberOfCPUs));
+                mMpService,
+                &mNumberOfCpus,
+                &mNumberOfEnabledCPUs
+                );
+
+  DEBUG ((DEBUG_INFO, "mNumberOfCpus - %d\n", mNumberOfCpus));
   DEBUG ((DEBUG_INFO, "mNumberOfEnabledCPUs - %d\n", mNumberOfEnabledCPUs));
 
   DEBUG ((DEBUG_INFO, "mX2ApicEnabled - 0x%x\n", mX2ApicEnabled));
   DEBUG ((DEBUG_INFO, "mForceX2ApicId - 0x%x\n", mForceX2ApicId));
 
   // support up to 64 threads/socket
-  AsmCpuidEx(CPUID_EXTENDED_TOPOLOGY, 1, &mNumOfBitShift, NULL, NULL, NULL);
+  AsmCpuidEx (CPUID_EXTENDED_TOPOLOGY, 1, &mNumOfBitShift, NULL, NULL, NULL);
   mNumOfBitShift &= 0x1F;
   DEBUG ((DEBUG_INFO, "mNumOfBitShift - 0x%x\n", mNumOfBitShift));
 
-- 
2.32.0.windows.2


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

* Re: [edk2-platforms: PATCH V8] Platform/Intel: Correct CPU APIC IDs
  2021-08-16 12:15 [edk2-platforms: PATCH V8] Platform/Intel: Correct CPU APIC IDs JackX Lin
@ 2021-08-19 13:23 ` Ni, Ray
  2021-08-20 10:01   ` JackX Lin
  0 siblings, 1 reply; 3+ messages in thread
From: Ni, Ray @ 2021-08-19 13:23 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

> +      CpuIdMapPtr->AcpiProcessorId = (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) +
> (UINT32)ProcessorInfoBuffer.ProcessorId;

1. I don't understand this assignment. Since you will re-assign this later, why not remove this assignment?


> +
> +  //make sure 1st entry is BSP
> +  if (mX2ApicEnabled) {
> +    BspApicId = (UINT32)AsmReadMsr64 (0x802);
> +  } else {
> +    BspApicId = (*(volatile UINT32 *)(UINTN)0xFEE00020) >> 24;
> +  }
> +  DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));

2. you can use GetApicId() API from LocalApicLib library in UefiCpuPkg.


> -    if(mCpuApicIdOrderTable[0].ApicId != BspApicId) {
> -      //check to see if 1st entry is BSP, if not swap it
> -      Index = ApicId2SwProcApicId(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));
> +    TempCpuApicIdOrderTable[0].Flags = 1;

3. Why is " TempCpuApicIdOrderTable[0].Flags = 1;" needed? 

> +  for (CurrProcessor = 1; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
> +    for (Index = CurrProcessor+1; Index < mNumberOfCpus; Index++) {
> +      if (TempCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId > TempCpuApicIdOrderTable[Index].AcpiProcessorId) {
> +        CopyMem (&TempVal, &TempCpuApicIdOrderTable[CurrProcessor], sizeof (EFI_CPU_ID_ORDER_MAP));
> +        CopyMem (&TempCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof
> (EFI_CPU_ID_ORDER_MAP));
> +        CopyMem (&TempCpuApicIdOrderTable[Index], &TempVal, sizeof (EFI_CPU_ID_ORDER_MAP));
>        }
> +    }
> +  }

4. Can you use PerformQuickSort() API from SortLib in MdeModulePkg?



> +
> +  //
> +  // 5. Re-assigen AcpiProcessorId for AcpiProcessorUId uses purpose.
> +  //
> +  for (Socket = 0; Socket < MAX_SOCKET; Socket++) {
> +    for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
> +      if (mCpuApicIdOrderTable[CurrProcessor].Flags && (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> +        mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId = (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) +
> Index;
> +        Index++;

5. I think you need a AcpiProcessorId[MAX_SOCKET] array to remember the "AcpiProcessorId" for each socket/package.
For example:
AcpiProcessorId[MAX_SOCKET];
ZeroMem (AcpiProcessorId);
for (Socket = 0; ...) {
  for (CurrProcessor = 0; ...) {
    if (...Flags == 1 && ...SocketNum == Socket) {
      mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId = (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) + AcpiProcessorId[Socket];
      AcpiProcessorId[Socket]++;



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

* Re: [edk2-platforms: PATCH V8] Platform/Intel: Correct CPU APIC IDs
  2021-08-19 13:23 ` Ni, Ray
@ 2021-08-20 10:01   ` JackX Lin
  0 siblings, 0 replies; 3+ messages in thread
From: JackX Lin @ 2021-08-20 10:01 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

Hi Ray,

Thanks for recommendations.

For item5, after referencing the log from server platform.
I think it is fine to keep original coding.

For item1 to item4, all of them are optimized.

I will re-sent the code patch.

Thank you.
Jack

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Thursday, August 19, 2021 9:23 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>
Subject: RE: [edk2-platforms: PATCH V8] Platform/Intel: Correct CPU APIC IDs

> +      CpuIdMapPtr->AcpiProcessorId = 
> + (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) +
> (UINT32)ProcessorInfoBuffer.ProcessorId;

1. I don't understand this assignment. Since you will re-assign this later, why not remove this assignment?


> +
> +  //make sure 1st entry is BSP
> +  if (mX2ApicEnabled) {
> +    BspApicId = (UINT32)AsmReadMsr64 (0x802);  } else {
> +    BspApicId = (*(volatile UINT32 *)(UINTN)0xFEE00020) >> 24;  }  
> + DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));

2. you can use GetApicId() API from LocalApicLib library in UefiCpuPkg.


> -    if(mCpuApicIdOrderTable[0].ApicId != BspApicId) {
> -      //check to see if 1st entry is BSP, if not swap it
> -      Index = ApicId2SwProcApicId(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));
> +    TempCpuApicIdOrderTable[0].Flags = 1;

3. Why is " TempCpuApicIdOrderTable[0].Flags = 1;" needed? 

> +  for (CurrProcessor = 1; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
> +    for (Index = CurrProcessor+1; Index < mNumberOfCpus; Index++) {
> +      if (TempCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId > TempCpuApicIdOrderTable[Index].AcpiProcessorId) {
> +        CopyMem (&TempVal, &TempCpuApicIdOrderTable[CurrProcessor], sizeof (EFI_CPU_ID_ORDER_MAP));
> +        CopyMem (&TempCpuApicIdOrderTable[CurrProcessor], 
> + &TempCpuApicIdOrderTable[Index], sizeof
> (EFI_CPU_ID_ORDER_MAP));
> +        CopyMem (&TempCpuApicIdOrderTable[Index], &TempVal, sizeof 
> + (EFI_CPU_ID_ORDER_MAP));
>        }
> +    }
> +  }

4. Can you use PerformQuickSort() API from SortLib in MdeModulePkg?



> +
> +  //
> +  // 5. Re-assigen AcpiProcessorId for AcpiProcessorUId uses purpose.
> +  //
> +  for (Socket = 0; Socket < MAX_SOCKET; Socket++) {
> +    for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
> +      if (mCpuApicIdOrderTable[CurrProcessor].Flags && (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> +        mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId = 
> + (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) +
> Index;
> +        Index++;

5. I think you need a AcpiProcessorId[MAX_SOCKET] array to remember the "AcpiProcessorId" for each socket/package.
For example:
AcpiProcessorId[MAX_SOCKET];
ZeroMem (AcpiProcessorId);
for (Socket = 0; ...) {
  for (CurrProcessor = 0; ...) {
    if (...Flags == 1 && ...SocketNum == Socket) {
      mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId = (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) + AcpiProcessorId[Socket];
      AcpiProcessorId[Socket]++;



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

end of thread, other threads:[~2021-08-20 10:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-16 12:15 [edk2-platforms: PATCH V8] Platform/Intel: Correct CPU APIC IDs JackX Lin
2021-08-19 13:23 ` Ni, Ray
2021-08-20 10:01   ` JackX Lin

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