Re: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT

["JackX Lin" <JackX.Lin@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 17601 bytes --]

Hi Ray,

The latest patch is sent in the code review mail, and I also attach one here, thank you.

Best regards
Jack

-----Original Message-----
From: Lin, JackX 
Sent: Wednesday, August 10, 2022 11:51 AM
To: Ni, Ray <ray.ni@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; devel@edk2.groups.io; Kuo, Donald <donald.kuo@intel.com>; Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Subject: RE: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT

Hi Ray,

I know this patch, and the thread 2 and 3 are added by my request for a reason on that time.
I will re-sent the code patch.
Thank you.

Jack

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, August 10, 2022 11:48 AM
To: Lin, JackX <jackx.lin@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; devel@edk2.groups.io; Kuo, Donald <donald.kuo@intel.com>; Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>; Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Subject: RE: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default fused in MADT

Jack,
Your patch cannot be merged to trunk because Ankit just did some change in the same C file, in below commit.
* MinPlatformPkg/AcpiTables: Add additional thread mapping in MADT

Ankit,
It seems your patch is to add support for thread #2 and #3. Jack's patch is to remove the additional sorting that put secondary threads after first threads.
Do you see an issue that we remove the thread sorting logic?

Thanks,
Ray

> -----Original Message-----
> From: Lin, JackX <jackx.lin@intel.com>
> Sent: Monday, August 8, 2022 4:21 PM
> To: devel@edk2.groups.io
> Cc: Lin, JackX <jackx.lin@intel.com>; Lin, JackX 
> <jackx.lin@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Dong, 
> Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ni, 
> Ray <ray.ni@intel.com>; Chaganty, Rangasai V 
> <rangasai.v.chaganty@intel.com>; Kuo, Donald <donald.kuo@intel.com>; 
> Kumar, Chandana C <chandana.c.kumar@intel.com>; Palakshareddy; 
> Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Subject: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU 
> default fused in MADT
> 
> BIOS should not reordering cpu processor_uid
> 
> Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Dong Eric <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Donald Kuo <Donald.Kuo@intel.com>
> Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
> Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
> Cc: JackX Lin <JackX.Lin@intel.com>
> ---
>  Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 174
> +++++++++++++++++++++++++++++++++++++++-------------------------------
> ----------------------------------------------------------------------
> ------------------------
> ----------
>  1 file changed, 39 insertions(+), 135 deletions(-)
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index c7e87cbd7d..176e422e81 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -57,38 +57,9 @@ BOOLEAN                     mForceX2ApicId;
>  BOOLEAN                     mX2ApicEnabled;
> 
>  EFI_MP_SERVICES_PROTOCOL    *mMpService;
> -BOOLEAN                     mCpuOrderSorted;
> -EFI_CPU_ID_ORDER_MAP        *mCpuApicIdOrderTable = NULL;
>  UINTN                       mNumberOfCpus = 0;
>  UINTN                       mNumberOfEnabledCPUs = 0;
> 
> -
> -/**
> -  The function is called by PerformQuickSort to compare int values.
> -
> -  @param[in] Left            The pointer to first buffer.
> -  @param[in] Right           The pointer to second buffer.
> -
> -  @return -1                 Buffer1 is less than Buffer2.
> -  @return  1                 Buffer1 is greater than Buffer2.
> -
> -**/
> -INTN
> -EFIAPI
> -ApicIdCompareFunction (
> -  IN CONST VOID                         *Left,
> -  IN CONST VOID                         *Right
> -  )
> -{
> -  UINT32  LeftApicId;
> -  UINT32  RightApicId;
> -
> -  LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
> -  RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
> -
> -  return (LeftApicId > RightApicId)? 1 : (-1); -}
> -
>  /**
>    Print Cpu Apic ID Table
> 
> @@ -116,7 +87,8 @@ DebugDisplayReOrderTable (  EFI_STATUS 
> AppendCpuMapTableEntry (
>      IN VOID   *ApicPtr,
> -    IN UINT32 LocalApicCounter
> +    IN UINT32 LocalApicCounter,
> +    IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
>    )
>  {
>    EFI_STATUS    Status;
> @@ -131,9 +103,9 @@ AppendCpuMapTableEntry (
> 
>    if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
>      if(!mX2ApicEnabled) {
> -      LocalApicPtr->Flags            =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> -      LocalApicPtr->ApicId           =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> -      LocalApicPtr->AcpiProcessorUid =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> +      LocalApicPtr->Flags            =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> +      LocalApicPtr->ApicId           =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].ApicId;
> +      LocalApicPtr->AcpiProcessorUid =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
>      } else {
>        LocalApicPtr->Flags            = 0;
>        LocalApicPtr->ApicId           = 0xFF;
> @@ -142,9 +114,9 @@ AppendCpuMapTableEntry (
>      }
>    } else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
>      if(mX2ApicEnabled) {
> -      LocalX2ApicPtr->Flags            =
> (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
> -      LocalX2ApicPtr->X2ApicId         =
> mCpuApicIdOrderTable[LocalApicCounter].ApicId;
> -      LocalX2ApicPtr->AcpiProcessorUid =
> mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
> +      LocalX2ApicPtr->Flags            =
> (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
> +      LocalX2ApicPtr->X2ApicId         =
> CpuApicIdOrderTable[LocalApicCounter].ApicId;
> +      LocalX2ApicPtr->AcpiProcessorUid =
> CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
>      } else {
>        LocalX2ApicPtr->Flags            = 0;
>        LocalX2ApicPtr->X2ApicId         = (UINT32)-1;
> @@ -159,32 +131,25 @@ AppendCpuMapTableEntry (
> 
>  }
> 
> +/**
> +  Collect all processors information and create a Cpu Apic Id table.
> +
> +  @param[in]  CpuApicIdOrderTable       Buffer to store information of Cpu.
> +**/
>  EFI_STATUS
> -SortCpuLocalApicInTable (
> -  VOID
> +CreateCpuLocalApicInTable (
> +  IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
>    )
>  {
>    EFI_STATUS                                Status;
>    EFI_PROCESSOR_INFORMATION                 ProcessorInfoBuffer;
>    UINT32                                    Index;
>    UINT32                                    CurrProcessor;
> -  UINT32                                    BspApicId;
> -  EFI_CPU_ID_ORDER_MAP                      TempVal;
>    EFI_CPU_ID_ORDER_MAP                      *CpuIdMapPtr;
> -  UINT32                                    CoreThreadMask;
> -  EFI_CPU_ID_ORDER_MAP                      *TempCpuApicIdOrderTable;
>    UINT32                                    Socket;
> 
> -  Index      = 0;
>    Status     = EFI_SUCCESS;
> 
> -  if (mCpuOrderSorted) {
> -    return Status;
> -  }
> -
> -  TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof 
> (EFI_CPU_ID_ORDER_MAP));
> -  CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
> -
>    for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++, Index++) {
>      Status = mMpService->GetProcessorInfo (
>                             mMpService, @@ -192,7 +157,7 @@ 
> SortCpuLocalApicInTable (
>                             &ProcessorInfoBuffer
>                             );
> 
> -    CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &TempCpuApicIdOrderTable[Index];
> +    CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
> &CpuApicIdOrderTable[Index];
>      if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
>        CpuIdMapPtr->ApicId  = (UINT32)ProcessorInfoBuffer.ProcessorId;
>        CpuIdMapPtr->Thread  = ProcessorInfoBuffer.Location.Thread;
> @@ -214,89 +179,26 @@ SortCpuLocalApicInTable (
>      } //end if PROC ENABLE
>    } //end for CurrentProcessor
> 
> -  //keep for debug purpose
> -  DEBUG ((DEBUG_INFO, "::ACPI::  APIC ID Order Table Init.
> CoreThreadMask = %x,  mNumOfBitShift = %x\n", CoreThreadMask, 
> mNumOfBitShift));
> -  DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
> -
>    //
>    // Get Bsp Apic Id
>    //
> -  BspApicId = GetApicId ();
> -  DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
> -
> -  //
> -  //check to see if 1st entry is BSP, if not swap it
> -  //
> -  if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
> -    for (Index = 0; Index < mNumberOfCpus; Index++) {
> -      if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
> (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
> -        CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof
> (EFI_CPU_ID_ORDER_MAP));
> -        CopyMem (&TempCpuApicIdOrderTable[Index],
> &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
> -        CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof
> (EFI_CPU_ID_ORDER_MAP));
> -        break;
> -      }
> -    }
> -
> -    if (mNumberOfCpus <= Index) {
> -      DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
> Bufferflow\n"));
> -      return EFI_INVALID_PARAMETER;
> -    }
> -  }
> -
> -  //
> -  // 1. Sort TempCpuApicIdOrderTable,
> -  //    sort it by using ApicId from minimum to maximum (Socket0 to SocketN),
> and the BSP must in the fist location of the table.
> -  //    So, start sorting the table from the second element and total elements
> are mNumberOfCpus-1.
> -  //
> -  PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 
> 1), sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE) 
> ApicIdCompareFunction);
> -
> -  //
> -  // 2. Sort and map the primary threads to the front of the 
> CpuApicIdOrderTable
> -  //
> -  for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
> -    if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
> -      CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> -      CurrProcessor++;
> -    }
> -  }
> +  DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
> 
> -  //
> -  // 3. Sort and map the second threads to the middle of the 
> CpuApicIdOrderTable
> -  //
> -  for (Index = 0; Index < mNumberOfCpus; Index++) {
> -    if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
> -      CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> -      CurrProcessor++;
> -    }
> -  }
> 
>    //
> -  // 4. Sort and map the not enabled threads to the bottom of the 
> CpuApicIdOrderTable
> -  //
> -  for (Index = 0; Index < mNumberOfCpus; Index++) {
> -    if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
> -      CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
> &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> -      CurrProcessor++;
> -    }
> -  }
> -
> -  //
> -  // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
> +  // Fill in AcpiProcessorUid.
>    //
>    for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount);
> Socket++) {
>      for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
> CurrProcessor++) {
> -      if (mCpuApicIdOrderTable[CurrProcessor].Flags &&
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> -        mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) + 
> Index;
> +      if (CpuApicIdOrderTable[CurrProcessor].Flags &&
> (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> +        CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
> (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) + 
> Index;
>          Index++;
>        }
>      }
>    }
> 
> -  //keep for debug purpose
> -  DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
> -  DebugDisplayReOrderTable (mCpuApicIdOrderTable);
> -
> -  mCpuOrderSorted = TRUE;
> +  DEBUG ((DEBUG_INFO, "::ACPI::  APIC ID Order Table Init.
> mNumOfBitShift = %x\n", mNumOfBitShift));
> +  DebugDisplayReOrderTable (CpuApicIdOrderTable);
> 
>    return Status;
>  }
> @@ -750,6 +652,7 @@ InstallMadtFromScratch (
>    EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE               LocalApciNmiStruct;
>    EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE
> ProcLocalX2ApicStruct;
>    EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE
> LocalX2ApicNmiStruct;
> +  EFI_CPU_ID_ORDER_MAP                                *CpuApicIdOrderTable;
>    STRUCTURE_HEADER                                    **MadtStructs;
>    UINTN                                               MaxMadtStructCount;
>    UINTN                                               MadtStructsIndex;
> @@ -760,18 +663,19 @@ InstallMadtFromScratch (
> 
>    MadtStructs = NULL;
>    NewMadtTable = NULL;
> +  CpuApicIdOrderTable = NULL;
>    MaxMadtStructCount = 0;
> 
> -  mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof 
> (EFI_CPU_ID_ORDER_MAP));
> -  if (mCpuApicIdOrderTable == NULL) {
> -    DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable
> structure pointer array\n"));
> +  CpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
> (EFI_CPU_ID_ORDER_MAP));
> +  if (CpuApicIdOrderTable == NULL) {
> +    DEBUG ((DEBUG_ERROR, "Could not allocate CpuApicIdOrderTable
> structure pointer array\n"));
>      return EFI_OUT_OF_RESOURCES;
>    }
> 
>    // Call for Local APIC ID Reorder
> -  Status = SortCpuLocalApicInTable ();
> +  Status = CreateCpuLocalApicInTable (CpuApicIdOrderTable);
>    if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
> +    DEBUG ((DEBUG_ERROR, "CreateCpuLocalApicInTable failed: %r\n",
> Status));
>      goto Done;
>    }
> 
> @@ -824,10 +728,10 @@ InstallMadtFromScratch (
>      // APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
>      // use a processor local x2APIC structure.
>      //
> -    if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> -      ProcLocalApicStruct.Flags            = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> -      ProcLocalApicStruct.ApicId           = (UINT8)
> mCpuApicIdOrderTable[Index].ApicId;
> -      ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> +    if (!mX2ApicEnabled && CpuApicIdOrderTable[Index].ApicId <
> MAX_UINT8) {
> +      ProcLocalApicStruct.Flags            = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> +      ProcLocalApicStruct.ApicId           = (UINT8)
> CpuApicIdOrderTable[Index].ApicId;
> +      ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
> 
>        ASSERT (MadtStructsIndex < MaxMadtStructCount);
>        Status = CopyStructure (
> @@ -835,10 +739,10 @@ InstallMadtFromScratch (
>                   (STRUCTURE_HEADER *) &ProcLocalApicStruct,
>                   &MadtStructs[MadtStructsIndex++]
>                   );
> -    } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> -      ProcLocalX2ApicStruct.Flags            = (UINT8)
> mCpuApicIdOrderTable[Index].Flags;
> -      ProcLocalX2ApicStruct.X2ApicId         =
> mCpuApicIdOrderTable[Index].ApicId;
> -      ProcLocalX2ApicStruct.AcpiProcessorUid =
> mCpuApicIdOrderTable[Index].AcpiProcessorUid;
> +    } else if (CpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
> +      ProcLocalX2ApicStruct.Flags            = (UINT8)
> CpuApicIdOrderTable[Index].Flags;
> +      ProcLocalX2ApicStruct.X2ApicId         =
> CpuApicIdOrderTable[Index].ApicId;
> +      ProcLocalX2ApicStruct.AcpiProcessorUid =
> CpuApicIdOrderTable[Index].AcpiProcessorUid;
> 
>        ASSERT (MadtStructsIndex < MaxMadtStructCount);
>        Status = CopyStructure (
> @@ -1033,8 +937,8 @@ Done:
>      FreePool (NewMadtTable);
>    }
> 
> -  if (mCpuApicIdOrderTable != NULL) {
> -    FreePool (mCpuApicIdOrderTable);
> +  if (CpuApicIdOrderTable != NULL) {
> +    FreePool (CpuApicIdOrderTable);
>    }
> 
>    return Status;
> --
> 2.32.0.windows.2


[-- Attachment #2: 0001-Modify-processor-_UID-ordering-by-CPU-default-fused-.patch --]
[-- Type: application/octet-stream, Size: 14520 bytes --]

From 4de4973504c869e40ccbcd0bec746bcf6047a2dd Mon Sep 17 00:00:00 2001
From: JackX Lin <jackx.lin@intel.com>
Date: Wed, 10 Aug 2022 12:24:09 +0800
Subject: [edk2-platforms: PATCH] Modify processor _UID ordering by CPU default
 fused in MADT

BIOS should not reordering cpu processor_uid

Signed-off-by: JackX Lin <JackX.Lin@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Dong Eric <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Donald Kuo <Donald.Kuo@intel.com>
Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@intel.com>
Cc: JackX Lin <JackX.Lin@intel.com>
---
 Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 183 ++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------------------------------------------------------------------------
 1 file changed, 40 insertions(+), 143 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 5a282e7c18..f134c8a58f 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -57,38 +57,9 @@ BOOLEAN                     mForceX2ApicId;
 BOOLEAN                     mX2ApicEnabled;
 
 EFI_MP_SERVICES_PROTOCOL    *mMpService;
-BOOLEAN                     mCpuOrderSorted;
-EFI_CPU_ID_ORDER_MAP        *mCpuApicIdOrderTable = NULL;
 UINTN                       mNumberOfCpus = 0;
 UINTN                       mNumberOfEnabledCPUs = 0;
 
-
-/**
-  The function is called by PerformQuickSort to compare int values.
-
-  @param[in] Left            The pointer to first buffer.
-  @param[in] Right           The pointer to second buffer.
-
-  @return -1                 Buffer1 is less than Buffer2.
-  @return  1                 Buffer1 is greater than Buffer2.
-
-**/
-INTN
-EFIAPI
-ApicIdCompareFunction (
-  IN CONST VOID                         *Left,
-  IN CONST VOID                         *Right
-  )
-{
-  UINT32  LeftApicId;
-  UINT32  RightApicId;
-
-  LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
-  RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
-
-  return (LeftApicId > RightApicId)? 1 : (-1);
-}
-
 /**
   Print Cpu Apic ID Table
 
@@ -116,7 +87,8 @@ DebugDisplayReOrderTable (
 EFI_STATUS
 AppendCpuMapTableEntry (
     IN VOID   *ApicPtr,
-    IN UINT32 LocalApicCounter
+    IN UINT32 LocalApicCounter,
+    IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable 
   )
 {
   EFI_STATUS    Status;
@@ -131,9 +103,9 @@ AppendCpuMapTableEntry (
 
   if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
     if(!mX2ApicEnabled) {
-      LocalApicPtr->Flags            = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
-      LocalApicPtr->ApicId           = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
-      LocalApicPtr->AcpiProcessorUid = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
+      LocalApicPtr->Flags            = (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
+      LocalApicPtr->ApicId           = (UINT8)CpuApicIdOrderTable[LocalApicCounter].ApicId;
+      LocalApicPtr->AcpiProcessorUid = (UINT8)CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
     } else {
       LocalApicPtr->Flags            = 0;
       LocalApicPtr->ApicId           = 0xFF;
@@ -142,9 +114,9 @@ AppendCpuMapTableEntry (
     }
   } else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
     if(mX2ApicEnabled) {
-      LocalX2ApicPtr->Flags            = (UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
-      LocalX2ApicPtr->X2ApicId         = mCpuApicIdOrderTable[LocalApicCounter].ApicId;
-      LocalX2ApicPtr->AcpiProcessorUid = mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
+      LocalX2ApicPtr->Flags            = (UINT8)CpuApicIdOrderTable[LocalApicCounter].Flags;
+      LocalX2ApicPtr->X2ApicId         = CpuApicIdOrderTable[LocalApicCounter].ApicId;
+      LocalX2ApicPtr->AcpiProcessorUid = CpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
     } else {
       LocalX2ApicPtr->Flags            = 0;
       LocalX2ApicPtr->X2ApicId         = (UINT32)-1;
@@ -159,32 +131,25 @@ AppendCpuMapTableEntry (
 
 }
 
+/**
+  Collect all processors information and create a Cpu Apic Id table.
+
+  @param[in]  CpuApicIdOrderTable       Buffer to store information of Cpu.
+**/
 EFI_STATUS
-SortCpuLocalApicInTable (
-  VOID
+CreateCpuLocalApicInTable (
+  IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
   )
 {
   EFI_STATUS                                Status;
   EFI_PROCESSOR_INFORMATION                 ProcessorInfoBuffer;
   UINT32                                    Index;
   UINT32                                    CurrProcessor;
-  UINT32                                    BspApicId;
-  EFI_CPU_ID_ORDER_MAP                      TempVal;
   EFI_CPU_ID_ORDER_MAP                      *CpuIdMapPtr;
-  UINT32                                    CoreThreadMask;
-  EFI_CPU_ID_ORDER_MAP                      *TempCpuApicIdOrderTable;
   UINT32                                    Socket;
 
-  Index      = 0;
   Status     = EFI_SUCCESS;
 
-  if (mCpuOrderSorted) {
-    return Status;
-  }
-
-  TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
-  CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
-
   for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++, Index++) {
     Status = mMpService->GetProcessorInfo (
                            mMpService,
@@ -192,7 +157,7 @@ SortCpuLocalApicInTable (
                            &ProcessorInfoBuffer
                            );
 
-    CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &TempCpuApicIdOrderTable[Index];
+    CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &CpuApicIdOrderTable[Index];
     if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
       CpuIdMapPtr->ApicId  = (UINT32)ProcessorInfoBuffer.ProcessorId;
       CpuIdMapPtr->Thread  = ProcessorInfoBuffer.Location.Thread;
@@ -214,96 +179,26 @@ SortCpuLocalApicInTable (
     } //end if PROC ENABLE
   } //end for CurrentProcessor
 
-  //keep for debug purpose
-  DEBUG ((DEBUG_INFO, "::ACPI::  APIC ID Order Table Init.   CoreThreadMask = %x,  mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift));
-  DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
-
   //
   // Get Bsp Apic Id
   //
-  BspApicId = GetApicId ();
-  DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
+  DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
+
 
   //
-  //check to see if 1st entry is BSP, if not swap it
+  // Fill in AcpiProcessorUid.
   //
-  if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
-    for (Index = 0; Index < mNumberOfCpus; Index++) {
-      if ((TempCpuApicIdOrderTable[Index].Flags == 1) && (TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
-        CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
-        CopyMem (&TempCpuApicIdOrderTable[Index], &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
-        CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof (EFI_CPU_ID_ORDER_MAP));
-        break;
-      }
-    }
-
-    if (mNumberOfCpus <= Index) {
-      DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n"));
-      return EFI_INVALID_PARAMETER;
-    }
-  }
-
-  /*
-      1. Sort TempCpuApicIdOrderTable,
-        Sort it by using ApicId from minimum to maximum (Socket0 to SocketN), and the BSP must be in the fist location of the table.
-
-      2. Sort and map all the enabled threads after BSP in CpuApicIdOrderTable
-
-      3. Threads that are not enabled are placed in the bottom of CpuApicIdOrderTable
-
-      4. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
-  */
-
-  PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 1), sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE) ApicIdCompareFunction);
-
-  for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
-    if ((TempCpuApicIdOrderTable[Index].Thread) == 0) {
-      CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
-      CurrProcessor++;
-    }
-  }
-
-  for (Index = 0; Index < mNumberOfCpus; Index++) {
-    if ((TempCpuApicIdOrderTable[Index].Thread) == 1) {
-      CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
-      CurrProcessor++;
-    }
-  }
-
-  for (Index = 0; Index < mNumberOfCpus; Index++) {
-    if ((TempCpuApicIdOrderTable[Index].Thread) == 2) {
-      CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
-      CurrProcessor++;
-    }
-  }
-
-  for (Index = 0; Index < mNumberOfCpus; Index++) {
-    if ((TempCpuApicIdOrderTable[Index].Thread) == 3) {
-      CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
-      CurrProcessor++;
-    }
-  }
-
-  for (Index = 0; Index < mNumberOfCpus; Index++) {
-    if (TempCpuApicIdOrderTable[Index].Flags == 0) {
-      CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
-      CurrProcessor++;
-    }
-  }
-
   for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount); Socket++) {
     for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
-      if (mCpuApicIdOrderTable[CurrProcessor].Flags && (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
-        mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (mCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) + Index;
+      if (CpuApicIdOrderTable[CurrProcessor].Flags && (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
+        CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) + Index;
         Index++;
       }
     }
   }
 
-  DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
-  DebugDisplayReOrderTable (mCpuApicIdOrderTable);
-
-  mCpuOrderSorted = TRUE;
+  DEBUG ((DEBUG_INFO, "::ACPI::  APIC ID Order Table Init.   mNumOfBitShift = %x\n", mNumOfBitShift));
+  DebugDisplayReOrderTable (CpuApicIdOrderTable);
 
   return Status;
 }
@@ -757,6 +652,7 @@ InstallMadtFromScratch (
   EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE               LocalApciNmiStruct;
   EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE       ProcLocalX2ApicStruct;
   EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE             LocalX2ApicNmiStruct;
+  EFI_CPU_ID_ORDER_MAP                                *CpuApicIdOrderTable;
   STRUCTURE_HEADER                                    **MadtStructs;
   UINTN                                               MaxMadtStructCount;
   UINTN                                               MadtStructsIndex;
@@ -767,18 +663,19 @@ InstallMadtFromScratch (
 
   MadtStructs = NULL;
   NewMadtTable = NULL;
+  CpuApicIdOrderTable = NULL;
   MaxMadtStructCount = 0;
 
-  mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
-  if (mCpuApicIdOrderTable == NULL) {
-    DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable structure pointer array\n"));
+  CpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
+  if (CpuApicIdOrderTable == NULL) {
+    DEBUG ((DEBUG_ERROR, "Could not allocate CpuApicIdOrderTable structure pointer array\n"));
     return EFI_OUT_OF_RESOURCES;
   }
 
   // Call for Local APIC ID Reorder
-  Status = SortCpuLocalApicInTable ();
+  Status = CreateCpuLocalApicInTable (CpuApicIdOrderTable);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
+    DEBUG ((DEBUG_ERROR, "CreateCpuLocalApicInTable failed: %r\n", Status));
     goto Done;
   }
 
@@ -831,10 +728,10 @@ InstallMadtFromScratch (
     // APIC ID as a UINT8, use a processor local APIC structure. Otherwise,
     // use a processor local x2APIC structure.
     //
-    if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId < MAX_UINT8) {
-      ProcLocalApicStruct.Flags            = (UINT8) mCpuApicIdOrderTable[Index].Flags;
-      ProcLocalApicStruct.ApicId           = (UINT8) mCpuApicIdOrderTable[Index].ApicId;
-      ProcLocalApicStruct.AcpiProcessorUid = (UINT8) mCpuApicIdOrderTable[Index].AcpiProcessorUid;
+    if (!mX2ApicEnabled && CpuApicIdOrderTable[Index].ApicId < MAX_UINT8) {
+      ProcLocalApicStruct.Flags            = (UINT8) CpuApicIdOrderTable[Index].Flags;
+      ProcLocalApicStruct.ApicId           = (UINT8) CpuApicIdOrderTable[Index].ApicId;
+      ProcLocalApicStruct.AcpiProcessorUid = (UINT8) CpuApicIdOrderTable[Index].AcpiProcessorUid;
 
       ASSERT (MadtStructsIndex < MaxMadtStructCount);
       Status = CopyStructure (
@@ -842,10 +739,10 @@ InstallMadtFromScratch (
                  (STRUCTURE_HEADER *) &ProcLocalApicStruct,
                  &MadtStructs[MadtStructsIndex++]
                  );
-    } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
-      ProcLocalX2ApicStruct.Flags            = (UINT8) mCpuApicIdOrderTable[Index].Flags;
-      ProcLocalX2ApicStruct.X2ApicId         = mCpuApicIdOrderTable[Index].ApicId;
-      ProcLocalX2ApicStruct.AcpiProcessorUid = mCpuApicIdOrderTable[Index].AcpiProcessorUid;
+    } else if (CpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
+      ProcLocalX2ApicStruct.Flags            = (UINT8) CpuApicIdOrderTable[Index].Flags;
+      ProcLocalX2ApicStruct.X2ApicId         = CpuApicIdOrderTable[Index].ApicId;
+      ProcLocalX2ApicStruct.AcpiProcessorUid = CpuApicIdOrderTable[Index].AcpiProcessorUid;
 
       ASSERT (MadtStructsIndex < MaxMadtStructCount);
       Status = CopyStructure (
@@ -1040,8 +937,8 @@ Done:
     FreePool (NewMadtTable);
   }
 
-  if (mCpuApicIdOrderTable != NULL) {
-    FreePool (mCpuApicIdOrderTable);
+  if (CpuApicIdOrderTable != NULL) {
+    FreePool (CpuApicIdOrderTable);
   }
 
   return Status;
-- 
2.32.0.windows.2