public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-platforms: PATCH V9] Platform/Intel: Correct CPU APIC IDs
@ 2021-08-20 10:53 JackX Lin
  2021-08-26 17:51 ` Ni, Ray
  0 siblings, 1 reply; 3+ messages in thread
From: JackX Lin @ 2021-08-20 10:53 UTC (permalink / raw)
  To: devel
  Cc: JackX Lin, 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   | 645 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.h   |   4 +++-
 Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf |   4 +++-
 3 files changed, 276 insertions(+), 377 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 2b51c34ef2..c03d899163 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,170 +57,58 @@ 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
+  The function is called by PerformQuickSort to compare int values.
 
-  @param None
+  @param[in] Left            The pointer to first buffer.
+  @param[in] Right           The pointer to second buffer.
 
-  @retval VOID
+  @return -1                 Buffer1 is less than Buffer2.
+  @return  1                 Buffer1 is greater than Buffer2.
 
 **/
-VOID DetectApicIdMap(VOID)
+INTN
+EFIAPI
+ApicIdCompareFunction (
+  IN CONST VOID                         *Left,
+  IN CONST VOID                         *Right
+  )
 {
-  UINTN                  CoreCount;
-
-  CoreCount = 0;
-
-  if(mApicIdMap != NULL) {
-    return;   //aleady initialized
-  }
-
-  mApicIdMap = ApicIdMapA;  // default to > 16C SKUs
+  UINT32  LeftApicId;
+  UINT32  RightApicId;
 
-  CoreCount = mNumberOfEnabledCPUs / 2;
-  DEBUG ((DEBUG_INFO, "CoreCount - %d\n", CoreCount));
+  LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
+  RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
 
-  //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;
+  return (LeftApicId > RightApicId)? 1 : (-1);
 }
 
 /**
-  This function return the CoreThreadId of ApicId from ACPI ApicId Map array
-
-  @param ApicId
-
-  @retval Index of ACPI ApicId Map array
+  Print Cpu Apic ID Table
 
+  @param[in]  CpuApicIdOrderTable       Data will be dumped.
 **/
-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
-  )
-{
-  UINT32 Index;
-
-  for (Index = 0; Index < MAX_CPU_NUM; Index++) {
-    if ((mCpuApicIdOrderTable[Index].Flags == 1) && (mCpuApicIdOrderTable[Index].ApicId == ApicId)) {
-      return Index;
-    }
-  }
-
-  return (UINT32) -1;
-
-}
-
 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 +168,135 @@ 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++;
+  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;
+
+      //update processorbitMask
+      if (CpuIdMapPtr->Flags == 1) {
+        if (mForceX2ApicId) {
+          CpuIdMapPtr->SocketNum &= 0x7;
+          CpuIdMapPtr->AcpiProcessorId &= 0xFF; //keep lower 8bit due to use Proc obj in dsdt
         }
-        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!!!!!
-        }
-
-        //update processorbitMask
-        if (CpuIdMapPtr->Flags == 1) {
+      }
+    } else {  //not enabled
+      CpuIdMapPtr->ApicId  = (UINT32)-1;
+      CpuIdMapPtr->Thread = (UINT32)-1;
+      CpuIdMapPtr->Flags   = 0;
+      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);
+
+  //
+  // 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(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);
+  //
+  // 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);
 
-      if(MAX_CPU_NUM <= Index) {
-        DEBUG ((EFI_D_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n"));
-        return EFI_INVALID_PARAMETER;
-      }
+  //
+  // 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++;
+    }
+  }
 
-      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;
+  //
+  // 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++;
     }
+  }
 
-    //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;
-          }
-        }
+  //
+  // 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 +493,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 +675,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 +762,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 +801,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 +818,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 +831,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 +842,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 +860,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 +916,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 +935,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 +955,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 +979,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 +993,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 +1008,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 +1031,10 @@ Done:
     FreePool (NewMadtTable);
   }
 
+  if (mCpuApicIdOrderTable != NULL) {
+    FreePool (mCpuApicIdOrderTable);
+  }
+
   return Status;
 }
 
@@ -1155,8 +1054,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 +1063,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 +1091,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 +1179,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 +1223,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 +1245,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 +1302,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 +1344,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 +1374,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 +1391,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 +1422,6 @@ InstallAcpiPlatform (
   EFI_STATUS                    Status;
   EFI_EVENT                     EndOfDxeEvent;
 
-
   Status = gBS->LocateProtocol (&gEfiMpServiceProtocolGuid, NULL, (VOID **)&mMpService);
   ASSERT_EFI_ERROR (Status);
 
@@ -1550,19 +1445,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));
 
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.h b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.h
index bd11f9e988..61f7470f80 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.h
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.h
@@ -1,7 +1,7 @@
 /** @file
   This is an implementation of the ACPI platform driver.
 
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -35,6 +35,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/MemoryAllocationLib.h>
 #include <Library/AslUpdateLib.h>
 #include <Library/PciSegmentInfoLib.h>
+#include <Library/SortLib.h>
+#include <Library/LocalApicLib.h>
 
 #include <Protocol/AcpiTable.h>
 #include <Protocol/MpService.h>
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
index 5d9c8cab50..95f6656af0 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
@@ -1,7 +1,7 @@
 ### @file
 #  Component information file for AcpiPlatform module
 #
-# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -43,6 +43,8 @@
   PciSegmentInfoLib
   AslUpdateLib
   BoardAcpiTableLib
+  SortLib
+  LocalApicLib
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
-- 
2.32.0.windows.2


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

* Re: [edk2-platforms: PATCH V9] Platform/Intel: Correct CPU APIC IDs
  2021-08-20 10:53 [edk2-platforms: PATCH V9] Platform/Intel: Correct CPU APIC IDs JackX Lin
@ 2021-08-26 17:51 ` Ni, Ray
  2021-08-27  2:44   ` Ni, Ray
  0 siblings, 1 reply; 3+ messages in thread
From: Ni, Ray @ 2021-08-26 17:51 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

> +  //
> +  // 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++;
>        }
>      }
> +  }

1. I think you need to change above code to as below?
UINTN IndexInSocket[MAX_SOCKET];

ZeroMem (IndexInSocket, sizeof (IndexInSocket));

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) + IndexInSocket[Socket];
        IndexInSocket[Socket]++;
     }
}

2. Can you separate the code refinement change (looks like most of the changes below) in a separate patch?
(No more comments)

> 
> -    //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 +493,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 +675,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 +762,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 +801,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 +818,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 +831,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 +842,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 +860,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 +916,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 +935,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 +955,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 +979,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 +993,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 +1008,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 +1031,10 @@ Done:
>      FreePool (NewMadtTable);
>    }
> 
> +  if (mCpuApicIdOrderTable != NULL) {
> +    FreePool (mCpuApicIdOrderTable);
> +  }
> +
>    return Status;
>  }
> 
> @@ -1155,8 +1054,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 +1063,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 +1091,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 +1179,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 +1223,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 +1245,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 +1302,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 +1344,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 +1374,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 +1391,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 +1422,6 @@ InstallAcpiPlatform (
>    EFI_STATUS                    Status;
>    EFI_EVENT                     EndOfDxeEvent;
> 
> -
>    Status = gBS->LocateProtocol (&gEfiMpServiceProtocolGuid, NULL, (VOID **)&mMpService);
>    ASSERT_EFI_ERROR (Status);
> 
> @@ -1550,19 +1445,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));
> 
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.h
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.h
> index bd11f9e988..61f7470f80 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.h
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.h
> @@ -1,7 +1,7 @@
>  /** @file
>    This is an implementation of the ACPI platform driver.
> 
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -35,6 +35,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/AslUpdateLib.h>
>  #include <Library/PciSegmentInfoLib.h>
> +#include <Library/SortLib.h>
> +#include <Library/LocalApicLib.h>
> 
>  #include <Protocol/AcpiTable.h>
>  #include <Protocol/MpService.h>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
> index 5d9c8cab50..95f6656af0 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
> @@ -1,7 +1,7 @@
>  ### @file
>  #  Component information file for AcpiPlatform module
>  #
> -# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
>  #
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -43,6 +43,8 @@
>    PciSegmentInfoLib
>    AslUpdateLib
>    BoardAcpiTableLib
> +  SortLib
> +  LocalApicLib
> 
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
> --
> 2.32.0.windows.2


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

* Re: [edk2-platforms: PATCH V9] Platform/Intel: Correct CPU APIC IDs
  2021-08-26 17:51 ` Ni, Ray
@ 2021-08-27  2:44   ` Ni, Ray
  0 siblings, 0 replies; 3+ messages in thread
From: Ni, Ray @ 2021-08-27  2:44 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

> > +  //
> > +  // 5. Re-assigen AcpiProcessorId for AcpiProcessorUId uses purpose.
> > +  //
> > +  for (Socket = 0; Socket < MAX_SOCKET; Socket++) {

1. Or if you add a "Index = 0" to reset Index for each Socket, that also work.

> > +    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++;
> >        }
> >      }
> > +  }
> 
> 1. I think you need to change above code to as below?
> UINTN IndexInSocket[MAX_SOCKET];
> 
> ZeroMem (IndexInSocket, sizeof (IndexInSocket));
> 
> for (Socket = 0; Socket < MAX_SOCKET; Socket++) {

2. My code is wrong. The for-loop above is not needed. I think you can simply make sure resetting "Index" to 0 for each socket in your original logic.

>   for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
>      if (mCpuApicIdOrderTable[CurrProcessor].Flags && (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
>         mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId = (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) +
> IndexInSocket[Socket];
>         IndexInSocket[Socket]++;
>      }
> }
> 
> 2. Can you separate the code refinement change (looks like most of the changes below) in a separate patch?
> (No more comments)

Sending multiple patches together basically consists of several steps:
a. make changes and commit first patch that fix the bug
b. make changes and commit second patch that refines the existing code.
    (The 2nd change should not impact any behavior.)
c. "git format-patch -3" to generate three patch files.
d. use text editor to edit the number #0 patch file (cover letter) to describe briefly of the two patches
e. "git send-email *.patch"


> 
> >
> > -    //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 +493,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 +675,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 +762,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 +801,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 +818,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 +831,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 +842,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 +860,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 +916,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 +935,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 +955,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 +979,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 +993,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 +1008,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 +1031,10 @@ Done:
> >      FreePool (NewMadtTable);
> >    }
> >
> > +  if (mCpuApicIdOrderTable != NULL) {
> > +    FreePool (mCpuApicIdOrderTable);
> > +  }
> > +
> >    return Status;
> >  }
> >
> > @@ -1155,8 +1054,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 +1063,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 +1091,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 +1179,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 +1223,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 +1245,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 +1302,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 +1344,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 +1374,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 +1391,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 +1422,6 @@ InstallAcpiPlatform (
> >    EFI_STATUS                    Status;
> >    EFI_EVENT                     EndOfDxeEvent;
> >
> > -
> >    Status = gBS->LocateProtocol (&gEfiMpServiceProtocolGuid, NULL, (VOID **)&mMpService);
> >    ASSERT_EFI_ERROR (Status);
> >
> > @@ -1550,19 +1445,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));
> >
> > diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.h
> > b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.h
> > index bd11f9e988..61f7470f80 100644
> > --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.h
> > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.h
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    This is an implementation of the ACPI platform driver.
> >
> > -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> > +Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -35,6 +35,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #include <Library/MemoryAllocationLib.h>
> >  #include <Library/AslUpdateLib.h>
> >  #include <Library/PciSegmentInfoLib.h>
> > +#include <Library/SortLib.h>
> > +#include <Library/LocalApicLib.h>
> >
> >  #include <Protocol/AcpiTable.h>
> >  #include <Protocol/MpService.h>
> > diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
> > b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
> > index 5d9c8cab50..95f6656af0 100644
> > --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
> > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
> > @@ -1,7 +1,7 @@
> >  ### @file
> >  #  Component information file for AcpiPlatform module
> >  #
> > -# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
> >  #
> >  # SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #
> > @@ -43,6 +43,8 @@
> >    PciSegmentInfoLib
> >    AslUpdateLib
> >    BoardAcpiTableLib
> > +  SortLib
> > +  LocalApicLib
> >
> >  [Pcd]
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
> > --
> > 2.32.0.windows.2


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

end of thread, other threads:[~2021-08-27  2:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-20 10:53 [edk2-platforms: PATCH V9] Platform/Intel: Correct CPU APIC IDs JackX Lin
2021-08-26 17:51 ` Ni, Ray
2021-08-27  2:44   ` Ni, Ray

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