[edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT

JackX Lin posted 1 patch 1 year, 5 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 142 insertions(+), 7 deletions(-)
[edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
Posted by JackX Lin 1 year, 5 months ago
BIOS should keep MADT ordering by big core first then small core

Signed-off-by: JackX Lin <JackX.Lin@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@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 | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 142 insertions(+), 7 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 6e57b638e0..02c1dd3a91 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -18,6 +18,7 @@ typedef struct {
   UINT32   Flags;
   UINT32   SocketNum;
   UINT32   Thread;
+  BOOLEAN  IsBigCore;
 } EFI_CPU_ID_ORDER_MAP;
 
 //
@@ -131,6 +132,104 @@ AppendCpuMapTableEntry (
 
 }
 
+/**
+  Detect if Hetero Core is supported.
+
+  @retval TRUE - Processor support HeteroCore
+  @retval FALSE - Processor doesnt support HeteroCore
+**/
+BOOLEAN
+EFIAPI
+IsHeteroCoreSupported (
+  VOID
+  )
+{
+  CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_EDX Edx;
+
+  ///
+  /// Check Hetero feature is supported
+  /// with CPUID.(EAX=7,ECX=0):EDX[15]=1
+  ///
+  AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, 0, NULL, NULL, NULL, &Edx.Uint32);
+  if (Edx.Bits.Hybrid == 1) {
+    return TRUE;
+  }
+  return FALSE;
+}
+
+/**
+  Detect the type of core, whether it is Big/Small Core.
+
+  @param[out]    CoreType      Output pointer that get CPUID_NATIVE_MODEL_ID_INFO data
+                 10h - Quark
+                 20h - Atom
+                 30H - Knights
+                 40H - Core
+**/
+VOID
+EFIAPI
+GetCoreType (
+  OUT  UINT8   *CoreType
+  )
+{
+  UINT32   Eax;
+
+  if (IsHeteroCoreSupported ()) {
+    //
+    // Check which is the running core by reading CPUID.(EAX=1AH, ECX=00H):EAX
+    //
+    AsmCpuid (CPUID_HYBRID_INFORMATION, &Eax, NULL, NULL, NULL);
+    *CoreType = (UINT8)((Eax & 0xFF000000) >> 24);
+  } else {
+    *CoreType = CPUID_CORE_TYPE_INTEL_CORE;
+  }
+}
+
+/**
+  Function will go through all processors to identify Core or Atom
+  by checking Core Type and update in IsBigCore.
+
+  @param[in] CpuApicIdOrderTable         Point to a buffer which will be filled in Core type information.
+**/
+VOID
+STATIC
+EFIAPI
+CollectCpuCoreType (
+  IN EFI_CPU_ID_ORDER_MAP        *CpuApicIdOrderTable
+  )
+{
+  CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_EDX     Edx;
+  UINT32                                          Eax;
+  UINTN                                           ApNumber;
+  EFI_STATUS                                      Status;
+  UINT8                                           CoreType;
+
+  Status = mMpService->WhoAmI (
+                         mMpService,
+                         &ApNumber
+                         );
+  ASSERT_EFI_ERROR (Status);
+
+  ///
+  /// Check Hetero feature is supported
+  /// with CPUID.(EAX=7,ECX=0):EDX[15]=1
+  ///
+  AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, 0, NULL, NULL, NULL, &Edx.Uint32);
+  if (Edx.Bits.Hybrid == 1) {
+    //
+    // Check which is the running core by reading CPUID.(EAX=1AH, ECX=00H):EAX
+    //
+    AsmCpuid (CPUID_HYBRID_INFORMATION, &Eax, NULL, NULL, NULL);
+    CoreType = (UINT8) ((Eax & 0xFF000000) >> 24);
+  } else {
+    CoreType = CPUID_CORE_TYPE_INTEL_CORE;
+  }
+
+  if (CoreType == CPUID_CORE_TYPE_INTEL_CORE) {
+    CpuApicIdOrderTable[ApNumber].IsBigCore = TRUE;
+  }
+}
+
 /**
   Collect all processors information and create a Cpu Apic Id table.
 
@@ -138,7 +237,7 @@ AppendCpuMapTableEntry (
 **/
 EFI_STATUS
 CreateCpuLocalApicInTable (
-  IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
+  IN EFI_CPU_ID_ORDER_MAP                   *CpuApicIdOrderTable
   )
 {
   EFI_STATUS                                Status;
@@ -146,9 +245,24 @@ CreateCpuLocalApicInTable (
   UINT32                                    Index;
   UINT32                                    CurrProcessor;
   EFI_CPU_ID_ORDER_MAP                      *CpuIdMapPtr;
+  EFI_CPU_ID_ORDER_MAP                      *TempCpuApicIdOrderTable;
   UINT32                                    Socket;
 
-  Status     = EFI_SUCCESS;
+  TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
+  if (TempCpuApicIdOrderTable == NULL) {
+    return EFI_UNSUPPORTED;
+  }
+
+  CollectCpuCoreType (TempCpuApicIdOrderTable);
+  mMpService->StartupAllAPs (
+                mMpService,                               // This
+                (EFI_AP_PROCEDURE) CollectCpuCoreType,    // Procedure
+                TRUE,                                     // SingleThread
+                NULL,                                     // WaitEvent
+                0,                                        // TimeoutInMicrosecsond
+                TempCpuApicIdOrderTable,                  // ProcedureArgument
+                NULL                                      // FailedCpuList
+                );
 
   for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++, Index++) {
     Status = mMpService->GetProcessorInfo (
@@ -157,9 +271,9 @@ CreateCpuLocalApicInTable (
                            &ProcessorInfoBuffer
                            );
 
-    CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &CpuApicIdOrderTable[Index];
+    CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &TempCpuApicIdOrderTable[Index];
     if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
-      CpuIdMapPtr->ApicId  = (UINT32)ProcessorInfoBuffer.ProcessorId;
+      CpuIdMapPtr->ApicId  = (UINT32) ProcessorInfoBuffer.ProcessorId;
       CpuIdMapPtr->Thread  = ProcessorInfoBuffer.Location.Thread;
       CpuIdMapPtr->Flags   = ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0);
       CpuIdMapPtr->SocketNum = ProcessorInfoBuffer.Location.Package;
@@ -184,22 +298,43 @@ CreateCpuLocalApicInTable (
   //
   DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
 
-
   //
   // Fill in AcpiProcessorUid.
   //
   for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount); Socket++) {
     for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
-      if (CpuApicIdOrderTable[CurrProcessor].Flags && (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
-        CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) + Index;
+      if (TempCpuApicIdOrderTable[CurrProcessor].Flags && (TempCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
+        TempCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (TempCpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift) + Index;
         Index++;
       }
     }
   }
 
+  //
+  // Re-ordering Cpu cores information to CpuApicIdOrderTable
+  // by big core first, then small core.
+  //
+  for (Index = 0, CurrProcessor = 0; Index < mNumberOfCpus; Index++) {
+    if (TempCpuApicIdOrderTable[Index].IsBigCore) {
+      CopyMem (&CpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
+      CurrProcessor++;
+    }
+  }
+
+  for (Index = 0; Index < mNumberOfCpus; Index++) {
+    if (!(TempCpuApicIdOrderTable[Index].IsBigCore)) {
+      CopyMem (&CpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
+      CurrProcessor++;
+    }
+  }
+
   DEBUG ((DEBUG_INFO, "::ACPI::  APIC ID Order Table Init.   mNumOfBitShift = %x\n", mNumOfBitShift));
   DebugDisplayReOrderTable (CpuApicIdOrderTable);
 
+  if (TempCpuApicIdOrderTable != NULL) {
+    FreePool (TempCpuApicIdOrderTable);
+  }
+
   return Status;
 }
 
-- 
2.32.0.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96415): https://edk2.groups.io/g/devel/message/96415
Mute This Topic: https://groups.io/mt/95059445/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
Posted by Michael D Kinney 1 year, 5 months ago
I see the CoreType is an 8-bit field, but this gets filtered down to IsBigCore BOOLEAN.

If there may be different core type values, would it be more flexible to collect
the 8-bit core type value from each CPU and have a configurable policy on how to
sort the CPUs in the MADT based on the core type values?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of JackX Lin
> Sent: Tuesday, November 15, 2022 6:53 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>; Desimone,
> Nathaniel L <nathaniel.l.desimone@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>;
> Dong, Eric <eric.dong@intel.com>; Kuo, Donald <donald.kuo@intel.com>; Kumar, Chandana C <chandana.c.kumar@intel.com>
> Subject: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
> 
> BIOS should keep MADT ordering by big core first then small core
> 
> Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@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 | 149
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++-------
>  1 file changed, 142 insertions(+), 7 deletions(-)
> 
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 6e57b638e0..02c1dd3a91 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -18,6 +18,7 @@ typedef struct {
>    UINT32   Flags;
>    UINT32   SocketNum;
>    UINT32   Thread;
> +  BOOLEAN  IsBigCore;
>  } EFI_CPU_ID_ORDER_MAP;
> 
>  //
> @@ -131,6 +132,104 @@ AppendCpuMapTableEntry (
> 
>  }
> 
> +/**
> +  Detect if Hetero Core is supported.
> +
> +  @retval TRUE - Processor support HeteroCore
> +  @retval FALSE - Processor doesnt support HeteroCore
> +**/
> +BOOLEAN
> +EFIAPI
> +IsHeteroCoreSupported (
> +  VOID
> +  )
> +{
> +  CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_EDX Edx;
> +
> +  ///
> +  /// Check Hetero feature is supported
> +  /// with CPUID.(EAX=7,ECX=0):EDX[15]=1
> +  ///
> +  AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, 0, NULL, NULL, NULL, &Edx.Uint32);
> +  if (Edx.Bits.Hybrid == 1) {
> +    return TRUE;
> +  }
> +  return FALSE;
> +}
> +
> +/**
> +  Detect the type of core, whether it is Big/Small Core.
> +
> +  @param[out]    CoreType      Output pointer that get CPUID_NATIVE_MODEL_ID_INFO data
> +                 10h - Quark
> +                 20h - Atom
> +                 30H - Knights
> +                 40H - Core
> +**/
> +VOID
> +EFIAPI
> +GetCoreType (
> +  OUT  UINT8   *CoreType
> +  )
> +{
> +  UINT32   Eax;
> +
> +  if (IsHeteroCoreSupported ()) {
> +    //
> +    // Check which is the running core by reading CPUID.(EAX=1AH, ECX=00H):EAX
> +    //
> +    AsmCpuid (CPUID_HYBRID_INFORMATION, &Eax, NULL, NULL, NULL);
> +    *CoreType = (UINT8)((Eax & 0xFF000000) >> 24);
> +  } else {
> +    *CoreType = CPUID_CORE_TYPE_INTEL_CORE;
> +  }
> +}
> +
> +/**
> +  Function will go through all processors to identify Core or Atom
> +  by checking Core Type and update in IsBigCore.
> +
> +  @param[in] CpuApicIdOrderTable         Point to a buffer which will be filled in Core type information.
> +**/
> +VOID
> +STATIC
> +EFIAPI
> +CollectCpuCoreType (
> +  IN EFI_CPU_ID_ORDER_MAP        *CpuApicIdOrderTable
> +  )
> +{
> +  CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_EDX     Edx;
> +  UINT32                                          Eax;
> +  UINTN                                           ApNumber;
> +  EFI_STATUS                                      Status;
> +  UINT8                                           CoreType;
> +
> +  Status = mMpService->WhoAmI (
> +                         mMpService,
> +                         &ApNumber
> +                         );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  ///
> +  /// Check Hetero feature is supported
> +  /// with CPUID.(EAX=7,ECX=0):EDX[15]=1
> +  ///
> +  AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, 0, NULL, NULL, NULL, &Edx.Uint32);
> +  if (Edx.Bits.Hybrid == 1) {
> +    //
> +    // Check which is the running core by reading CPUID.(EAX=1AH, ECX=00H):EAX
> +    //
> +    AsmCpuid (CPUID_HYBRID_INFORMATION, &Eax, NULL, NULL, NULL);
> +    CoreType = (UINT8) ((Eax & 0xFF000000) >> 24);
> +  } else {
> +    CoreType = CPUID_CORE_TYPE_INTEL_CORE;
> +  }
> +
> +  if (CoreType == CPUID_CORE_TYPE_INTEL_CORE) {
> +    CpuApicIdOrderTable[ApNumber].IsBigCore = TRUE;
> +  }
> +}
> +
>  /**
>    Collect all processors information and create a Cpu Apic Id table.
> 
> @@ -138,7 +237,7 @@ AppendCpuMapTableEntry (
>  **/
>  EFI_STATUS
>  CreateCpuLocalApicInTable (
> -  IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> +  IN EFI_CPU_ID_ORDER_MAP                   *CpuApicIdOrderTable
>    )
>  {
>    EFI_STATUS                                Status;
> @@ -146,9 +245,24 @@ CreateCpuLocalApicInTable (
>    UINT32                                    Index;
>    UINT32                                    CurrProcessor;
>    EFI_CPU_ID_ORDER_MAP                      *CpuIdMapPtr;
> +  EFI_CPU_ID_ORDER_MAP                      *TempCpuApicIdOrderTable;
>    UINT32                                    Socket;
> 
> -  Status     = EFI_SUCCESS;
> +  TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
> +  if (TempCpuApicIdOrderTable == NULL) {
> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  CollectCpuCoreType (TempCpuApicIdOrderTable);
> +  mMpService->StartupAllAPs (
> +                mMpService,                               // This
> +                (EFI_AP_PROCEDURE) CollectCpuCoreType,    // Procedure
> +                TRUE,                                     // SingleThread
> +                NULL,                                     // WaitEvent
> +                0,                                        // TimeoutInMicrosecsond
> +                TempCpuApicIdOrderTable,                  // ProcedureArgument
> +                NULL                                      // FailedCpuList
> +                );
> 
>    for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++, Index++) {
>      Status = mMpService->GetProcessorInfo (
> @@ -157,9 +271,9 @@ CreateCpuLocalApicInTable (
>                             &ProcessorInfoBuffer
>                             );
> 
> -    CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &CpuApicIdOrderTable[Index];
> +    CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &TempCpuApicIdOrderTable[Index];
>      if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
> -      CpuIdMapPtr->ApicId  = (UINT32)ProcessorInfoBuffer.ProcessorId;
> +      CpuIdMapPtr->ApicId  = (UINT32) ProcessorInfoBuffer.ProcessorId;
>        CpuIdMapPtr->Thread  = ProcessorInfoBuffer.Location.Thread;
>        CpuIdMapPtr->Flags   = ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0);
>        CpuIdMapPtr->SocketNum = ProcessorInfoBuffer.Location.Package;
> @@ -184,22 +298,43 @@ CreateCpuLocalApicInTable (
>    //
>    DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
> 
> -
>    //
>    // Fill in AcpiProcessorUid.
>    //
>    for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount); Socket++) {
>      for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
> -      if (CpuApicIdOrderTable[CurrProcessor].Flags && (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> -        CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift)
> + Index;
> +      if (TempCpuApicIdOrderTable[CurrProcessor].Flags && (TempCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> +        TempCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (TempCpuApicIdOrderTable[CurrProcessor].SocketNum <<
> mNumOfBitShift) + Index;
>          Index++;
>        }
>      }
>    }
> 
> +  //
> +  // Re-ordering Cpu cores information to CpuApicIdOrderTable
> +  // by big core first, then small core.
> +  //
> +  for (Index = 0, CurrProcessor = 0; Index < mNumberOfCpus; Index++) {
> +    if (TempCpuApicIdOrderTable[Index].IsBigCore) {
> +      CopyMem (&CpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> +      CurrProcessor++;
> +    }
> +  }
> +
> +  for (Index = 0; Index < mNumberOfCpus; Index++) {
> +    if (!(TempCpuApicIdOrderTable[Index].IsBigCore)) {
> +      CopyMem (&CpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> +      CurrProcessor++;
> +    }
> +  }
> +
>    DEBUG ((DEBUG_INFO, "::ACPI::  APIC ID Order Table Init.   mNumOfBitShift = %x\n", mNumOfBitShift));
>    DebugDisplayReOrderTable (CpuApicIdOrderTable);
> 
> +  if (TempCpuApicIdOrderTable != NULL) {
> +    FreePool (TempCpuApicIdOrderTable);
> +  }
> +
>    return Status;
>  }
> 
> --
> 2.32.0.windows.2
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96419): https://edk2.groups.io/g/devel/message/96419
Mute This Topic: https://groups.io/mt/95059445/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
Posted by JackX Lin 1 year, 5 months ago
Hi Michael,

I think I understand what you mentioned, 
I will provide another patch.
Thank you.

Jack

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Wednesday, November 16, 2022 11:51 AM
To: devel@edk2.groups.io; Lin, JackX <jackx.lin@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Kuo, Donald <donald.kuo@intel.com>; Kumar, Chandana C <chandana.c.kumar@intel.com>
Subject: RE: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT

I see the CoreType is an 8-bit field, but this gets filtered down to IsBigCore BOOLEAN.

If there may be different core type values, would it be more flexible to collect the 8-bit core type value from each CPU and have a configurable policy on how to sort the CPUs in the MADT based on the core type values?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of JackX 
> Lin
> Sent: Tuesday, November 15, 2022 6:53 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>; Desimone, 
> Nathaniel L <nathaniel.l.desimone@intel.com>; Oram, Isaac W 
> <isaac.w.oram@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; 
> Dong, Eric <eric.dong@intel.com>; Kuo, Donald <donald.kuo@intel.com>; 
> Kumar, Chandana C <chandana.c.kumar@intel.com>
> Subject: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present 
> cores in order of relative performance in MADT
> 
> BIOS should keep MADT ordering by big core first then small core
> 
> Signed-off-by: JackX Lin <JackX.Lin@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@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 | 149
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++-------
>  1 file changed, 142 insertions(+), 7 deletions(-)
> 
> diff --git 
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 6e57b638e0..02c1dd3a91 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -18,6 +18,7 @@ typedef struct {
>    UINT32   Flags;
>    UINT32   SocketNum;
>    UINT32   Thread;
> +  BOOLEAN  IsBigCore;
>  } EFI_CPU_ID_ORDER_MAP;
> 
>  //
> @@ -131,6 +132,104 @@ AppendCpuMapTableEntry (
> 
>  }
> 
> +/**
> +  Detect if Hetero Core is supported.
> +
> +  @retval TRUE - Processor support HeteroCore
> +  @retval FALSE - Processor doesnt support HeteroCore **/ BOOLEAN 
> +EFIAPI IsHeteroCoreSupported (
> +  VOID
> +  )
> +{
> +  CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_EDX Edx;
> +
> +  ///
> +  /// Check Hetero feature is supported
> +  /// with CPUID.(EAX=7,ECX=0):EDX[15]=1
> +  ///
> +  AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, 0, NULL, NULL, 
> +NULL, &Edx.Uint32);
> +  if (Edx.Bits.Hybrid == 1) {
> +    return TRUE;
> +  }
> +  return FALSE;
> +}
> +
> +/**
> +  Detect the type of core, whether it is Big/Small Core.
> +
> +  @param[out]    CoreType      Output pointer that get CPUID_NATIVE_MODEL_ID_INFO data
> +                 10h - Quark
> +                 20h - Atom
> +                 30H - Knights
> +                 40H - Core
> +**/
> +VOID
> +EFIAPI
> +GetCoreType (
> +  OUT  UINT8   *CoreType
> +  )
> +{
> +  UINT32   Eax;
> +
> +  if (IsHeteroCoreSupported ()) {
> +    //
> +    // Check which is the running core by reading CPUID.(EAX=1AH, ECX=00H):EAX
> +    //
> +    AsmCpuid (CPUID_HYBRID_INFORMATION, &Eax, NULL, NULL, NULL);
> +    *CoreType = (UINT8)((Eax & 0xFF000000) >> 24);
> +  } else {
> +    *CoreType = CPUID_CORE_TYPE_INTEL_CORE;
> +  }
> +}
> +
> +/**
> +  Function will go through all processors to identify Core or Atom
> +  by checking Core Type and update in IsBigCore.
> +
> +  @param[in] CpuApicIdOrderTable         Point to a buffer which will be filled in Core type information.
> +**/
> +VOID
> +STATIC
> +EFIAPI
> +CollectCpuCoreType (
> +  IN EFI_CPU_ID_ORDER_MAP        *CpuApicIdOrderTable
> +  )
> +{
> +  CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_EDX     Edx;
> +  UINT32                                          Eax;
> +  UINTN                                           ApNumber;
> +  EFI_STATUS                                      Status;
> +  UINT8                                           CoreType;
> +
> +  Status = mMpService->WhoAmI (
> +                         mMpService,
> +                         &ApNumber
> +                         );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  ///
> +  /// Check Hetero feature is supported  /// with 
> + CPUID.(EAX=7,ECX=0):EDX[15]=1  ///  AsmCpuidEx 
> + (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, 0, NULL, NULL, NULL, 
> + &Edx.Uint32);  if (Edx.Bits.Hybrid == 1) {
> +    //
> +    // Check which is the running core by reading CPUID.(EAX=1AH, ECX=00H):EAX
> +    //
> +    AsmCpuid (CPUID_HYBRID_INFORMATION, &Eax, NULL, NULL, NULL);
> +    CoreType = (UINT8) ((Eax & 0xFF000000) >> 24);  } else {
> +    CoreType = CPUID_CORE_TYPE_INTEL_CORE;  }
> +
> +  if (CoreType == CPUID_CORE_TYPE_INTEL_CORE) {
> +    CpuApicIdOrderTable[ApNumber].IsBigCore = TRUE;
> +  }
> +}
> +
>  /**
>    Collect all processors information and create a Cpu Apic Id table.
> 
> @@ -138,7 +237,7 @@ AppendCpuMapTableEntry (  **/  EFI_STATUS  
> CreateCpuLocalApicInTable (
> -  IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> +  IN EFI_CPU_ID_ORDER_MAP                   *CpuApicIdOrderTable
>    )
>  {
>    EFI_STATUS                                Status;
> @@ -146,9 +245,24 @@ CreateCpuLocalApicInTable (
>    UINT32                                    Index;
>    UINT32                                    CurrProcessor;
>    EFI_CPU_ID_ORDER_MAP                      *CpuIdMapPtr;
> +  EFI_CPU_ID_ORDER_MAP                      *TempCpuApicIdOrderTable;
>    UINT32                                    Socket;
> 
> -  Status     = EFI_SUCCESS;
> +  TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof 
> + (EFI_CPU_ID_ORDER_MAP));  if (TempCpuApicIdOrderTable == NULL) {
> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  CollectCpuCoreType (TempCpuApicIdOrderTable);  
> + mMpService->StartupAllAPs (
> +                mMpService,                               // This
> +                (EFI_AP_PROCEDURE) CollectCpuCoreType,    // Procedure
> +                TRUE,                                     // SingleThread
> +                NULL,                                     // WaitEvent
> +                0,                                        // TimeoutInMicrosecsond
> +                TempCpuApicIdOrderTable,                  // ProcedureArgument
> +                NULL                                      // FailedCpuList
> +                );
> 
>    for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++, Index++) {
>      Status = mMpService->GetProcessorInfo ( @@ -157,9 +271,9 @@ 
> CreateCpuLocalApicInTable (
>                             &ProcessorInfoBuffer
>                             );
> 
> -    CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &CpuApicIdOrderTable[Index];
> +    CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) 
> + &TempCpuApicIdOrderTable[Index];
>      if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
> -      CpuIdMapPtr->ApicId  = (UINT32)ProcessorInfoBuffer.ProcessorId;
> +      CpuIdMapPtr->ApicId  = (UINT32) 
> + ProcessorInfoBuffer.ProcessorId;
>        CpuIdMapPtr->Thread  = ProcessorInfoBuffer.Location.Thread;
>        CpuIdMapPtr->Flags   = ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0);
>        CpuIdMapPtr->SocketNum = ProcessorInfoBuffer.Location.Package;
> @@ -184,22 +298,43 @@ CreateCpuLocalApicInTable (
>    //
>    DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
> 
> -
>    //
>    // Fill in AcpiProcessorUid.
>    //
>    for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount); Socket++) {
>      for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
> -      if (CpuApicIdOrderTable[CurrProcessor].Flags && (CpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> -        CpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (CpuApicIdOrderTable[CurrProcessor].SocketNum << mNumOfBitShift)
> + Index;
> +      if (TempCpuApicIdOrderTable[CurrProcessor].Flags && (TempCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> +        TempCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = 
> + (TempCpuApicIdOrderTable[CurrProcessor].SocketNum <<
> mNumOfBitShift) + Index;
>          Index++;
>        }
>      }
>    }
> 
> +  //
> +  // Re-ordering Cpu cores information to CpuApicIdOrderTable  // by 
> + big core first, then small core.
> +  //
> +  for (Index = 0, CurrProcessor = 0; Index < mNumberOfCpus; Index++) {
> +    if (TempCpuApicIdOrderTable[Index].IsBigCore) {
> +      CopyMem (&CpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> +      CurrProcessor++;
> +    }
> +  }
> +
> +  for (Index = 0; Index < mNumberOfCpus; Index++) {
> +    if (!(TempCpuApicIdOrderTable[Index].IsBigCore)) {
> +      CopyMem (&CpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> +      CurrProcessor++;
> +    }
> +  }
> +
>    DEBUG ((DEBUG_INFO, "::ACPI::  APIC ID Order Table Init.   mNumOfBitShift = %x\n", mNumOfBitShift));
>    DebugDisplayReOrderTable (CpuApicIdOrderTable);
> 
> +  if (TempCpuApicIdOrderTable != NULL) {
> +    FreePool (TempCpuApicIdOrderTable);  }
> +
>    return Status;
>  }
> 
> --
> 2.32.0.windows.2
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96421): https://edk2.groups.io/g/devel/message/96421
Mute This Topic: https://groups.io/mt/95059445/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-