* Re: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
2022-11-16 2:52 JackX Lin
@ 2022-11-16 3:50 ` Michael D Kinney
2022-11-16 3:59 ` JackX Lin
0 siblings, 1 reply; 10+ messages in thread
From: Michael D Kinney @ 2022-11-16 3:50 UTC (permalink / raw)
To: devel@edk2.groups.io, Lin, JackX, Kinney, Michael D
Cc: Chiu, Chasel, Desimone, Nathaniel L, Oram, Isaac W, Gao, Liming,
Dong, Eric, Kuo, Donald, Kumar, Chandana C
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
>
>
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
2022-11-16 3:50 ` [edk2-devel] " Michael D Kinney
@ 2022-11-16 3:59 ` JackX Lin
0 siblings, 0 replies; 10+ messages in thread
From: JackX Lin @ 2022-11-16 3:59 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io
Cc: Chiu, Chasel, Desimone, Nathaniel L, Oram, Isaac W, Gao, Liming,
Dong, Eric, Kuo, Donald, Kumar, Chandana C
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
>
>
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
2022-11-16 6:29 JackX Lin
@ 2022-11-17 3:45 ` Michael D Kinney
2022-11-17 3:54 ` JackX Lin
0 siblings, 1 reply; 10+ messages in thread
From: Michael D Kinney @ 2022-11-17 3:45 UTC (permalink / raw)
To: devel@edk2.groups.io, Lin, JackX, Kinney, Michael D
Cc: Chiu, Chasel, Desimone, Nathaniel L, Oram, Isaac W, Gao, Liming,
Dong, Eric, Kuo, Donald, Kumar, Chandana C
Hi Jack,
I like these updates to capture the 8-bit core type values.
From the SDM and the MdePkg/Include/Registers/Intel/Cpuid.h file
I see the following defines:
///
/// @{ Define value for CPUID_NATIVE_MODEL_ID_AND_CORE_TYPE_EAX.CoreType
///
#define CPUID_CORE_TYPE_INTEL_ATOM 0x20
#define CPUID_CORE_TYPE_INTEL_CORE 0x40
///
/// @}
///
It looks like the default sorting policy to add CPUs with a
CPU Type of CPUID_CORE_TYPE_INTEL_CORE first, and then add
all the other in any order? I see that only CORE and ATOM
are defined at this time. Does this imply that this logic
will need to be updated if additional core types are added?
Best regards,
Mike
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of JackX Lin
> Sent: Tuesday, November 15, 2022 10:30 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 | 94
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 87 insertions(+), 7 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 6e57b638e0..bafe359668 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;
> + UINT8 CpuCoreType;
> } EFI_CPU_ID_ORDER_MAP;
>
> //
> @@ -131,6 +132,49 @@ AppendCpuMapTableEntry (
>
> }
>
> +/**
> + 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;
> + }
> +
> + CpuApicIdOrderTable[ApNumber].CpuCoreType = CoreType;
> +}
> +
> /**
> Collect all processors information and create a Cpu Apic Id table.
>
> @@ -138,7 +182,7 @@ AppendCpuMapTableEntry (
> **/
> EFI_STATUS
> CreateCpuLocalApicInTable (
> - IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> @@ -146,9 +190,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 +216,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 +243,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].CpuCoreType == CPUID_CORE_TYPE_INTEL_CORE) {
> + CopyMem (&CpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> + CurrProcessor++;
> + }
> + }
> +
> + for (Index = 0; Index < mNumberOfCpus; Index++) {
> + if (TempCpuApicIdOrderTable[Index].CpuCoreType != CPUID_CORE_TYPE_INTEL_CORE) {
> + 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
>
>
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
2022-11-17 3:45 ` [edk2-devel] " Michael D Kinney
@ 2022-11-17 3:54 ` JackX Lin
0 siblings, 0 replies; 10+ messages in thread
From: JackX Lin @ 2022-11-17 3:54 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io
Cc: Chiu, Chasel, Desimone, Nathaniel L, Oram, Isaac W, Gao, Liming,
Dong, Eric, Kuo, Donald, Kumar, Chandana C
Hi Kenny,
Yes, So far we just have 2 CPU core types, which are big core (0x40) and small core (0x20)
But I am not sure if there will be another type in the future.
In order to avoid missed CPU cores, I add all the other cores in the other order.
Do you prefer separate them by using CORE and ATOM than if/else ?
Best regards
Jack
-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Thursday, November 17, 2022 11:45 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
Hi Jack,
I like these updates to capture the 8-bit core type values.
From the SDM and the MdePkg/Include/Registers/Intel/Cpuid.h file I see the following defines:
///
/// @{ Define value for CPUID_NATIVE_MODEL_ID_AND_CORE_TYPE_EAX.CoreType
///
#define CPUID_CORE_TYPE_INTEL_ATOM 0x20
#define CPUID_CORE_TYPE_INTEL_CORE 0x40
///
/// @}
///
It looks like the default sorting policy to add CPUs with a CPU Type of CPUID_CORE_TYPE_INTEL_CORE first, and then add all the other in any order? I see that only CORE and ATOM are defined at this time. Does this imply that this logic will need to be updated if additional core types are added?
Best regards,
Mike
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of JackX
> Lin
> Sent: Tuesday, November 15, 2022 10:30 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 | 94
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 87 insertions(+), 7 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 6e57b638e0..bafe359668 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;
> + UINT8 CpuCoreType;
> } EFI_CPU_ID_ORDER_MAP;
>
> //
> @@ -131,6 +132,49 @@ AppendCpuMapTableEntry (
>
> }
>
> +/**
> + 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; }
> +
> + CpuApicIdOrderTable[ApNumber].CpuCoreType = CoreType; }
> +
> /**
> Collect all processors information and create a Cpu Apic Id table.
>
> @@ -138,7 +182,7 @@ AppendCpuMapTableEntry ( **/ EFI_STATUS
> CreateCpuLocalApicInTable (
> - IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> @@ -146,9 +190,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 +216,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 +243,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].CpuCoreType == CPUID_CORE_TYPE_INTEL_CORE) {
> + CopyMem (&CpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> + CurrProcessor++;
> + }
> + }
> +
> + for (Index = 0; Index < mNumberOfCpus; Index++) {
> + if (TempCpuApicIdOrderTable[Index].CpuCoreType != CPUID_CORE_TYPE_INTEL_CORE) {
> + 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
>
>
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
@ 2022-11-17 6:01 JackX Lin
2022-11-17 6:06 ` [edk2-devel] " Michael D Kinney
2022-11-17 14:09 ` Pedro Falcato
0 siblings, 2 replies; 10+ messages in thread
From: JackX Lin @ 2022-11-17 6:01 UTC (permalink / raw)
To: devel
Cc: JackX Lin, JackX Lin, Chasel Chiu, Nate DeSimone, Isaac Oram,
Liming Gao, Eric Dong, Donald Kuo, Chandana C Kumar
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 | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 104 insertions(+), 7 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 6e57b638e0..894790f246 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;
+ UINT8 CpuCoreType;
} EFI_CPU_ID_ORDER_MAP;
//
@@ -131,6 +132,49 @@ AppendCpuMapTableEntry (
}
+/**
+ 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;
+ }
+
+ CpuApicIdOrderTable[ApNumber].CpuCoreType = CoreType;
+}
+
/**
Collect all processors information and create a Cpu Apic Id table.
@@ -138,7 +182,7 @@ AppendCpuMapTableEntry (
**/
EFI_STATUS
CreateCpuLocalApicInTable (
- IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
+ IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
)
{
EFI_STATUS Status;
@@ -146,9 +190,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 +216,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 +243,60 @@ 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].CpuCoreType == CPUID_CORE_TYPE_INTEL_CORE) {
+ CopyMem (&CpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
+ CurrProcessor++;
+ }
+ }
+
+ for (Index = 0; Index < mNumberOfCpus; Index++) {
+ if (TempCpuApicIdOrderTable[Index].CpuCoreType == CPUID_CORE_TYPE_INTEL_ATOM) {
+ CopyMem (&CpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
+ CurrProcessor++;
+ }
+ }
+
+ //
+ // Add unknown cpu core types to the bottom of the table
+ //
+ for (Index = 0; Index < mNumberOfCpus; Index++) {
+ if ((TempCpuApicIdOrderTable[Index].CpuCoreType != CPUID_CORE_TYPE_INTEL_CORE) && (TempCpuApicIdOrderTable[Index].CpuCoreType != CPUID_CORE_TYPE_INTEL_ATOM)) {
+ DEBUG ((
+ DEBUG_INFO,
+ "Unknown Cpu Core type found: Apic = 0x%x, CoreType = 0x%x\n",
+ TempCpuApicIdOrderTable[Index].ApicId,
+ TempCpuApicIdOrderTable[Index].CpuCoreType
+ ));
+
+ 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
2022-11-17 6:01 [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT JackX Lin
@ 2022-11-17 6:06 ` Michael D Kinney
2022-11-17 14:09 ` Pedro Falcato
1 sibling, 0 replies; 10+ messages in thread
From: Michael D Kinney @ 2022-11-17 6:06 UTC (permalink / raw)
To: devel@edk2.groups.io, Lin, JackX, Kinney, Michael D
Cc: Chiu, Chasel, Desimone, Nathaniel L, Oram, Isaac W, Gao, Liming,
Dong, Eric, Kuo, Donald, Kumar, Chandana C
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Mike
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of JackX Lin
> Sent: Wednesday, November 16, 2022 10:01 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 | 111
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 104 insertions(+), 7 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 6e57b638e0..894790f246 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;
> + UINT8 CpuCoreType;
> } EFI_CPU_ID_ORDER_MAP;
>
> //
> @@ -131,6 +132,49 @@ AppendCpuMapTableEntry (
>
> }
>
> +/**
> + 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;
> + }
> +
> + CpuApicIdOrderTable[ApNumber].CpuCoreType = CoreType;
> +}
> +
> /**
> Collect all processors information and create a Cpu Apic Id table.
>
> @@ -138,7 +182,7 @@ AppendCpuMapTableEntry (
> **/
> EFI_STATUS
> CreateCpuLocalApicInTable (
> - IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable
> )
> {
> EFI_STATUS Status;
> @@ -146,9 +190,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 +216,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 +243,60 @@ 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].CpuCoreType == CPUID_CORE_TYPE_INTEL_CORE) {
> + CopyMem (&CpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> + CurrProcessor++;
> + }
> + }
> +
> + for (Index = 0; Index < mNumberOfCpus; Index++) {
> + if (TempCpuApicIdOrderTable[Index].CpuCoreType == CPUID_CORE_TYPE_INTEL_ATOM) {
> + CopyMem (&CpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> + CurrProcessor++;
> + }
> + }
> +
> + //
> + // Add unknown cpu core types to the bottom of the table
> + //
> + for (Index = 0; Index < mNumberOfCpus; Index++) {
> + if ((TempCpuApicIdOrderTable[Index].CpuCoreType != CPUID_CORE_TYPE_INTEL_CORE) &&
> (TempCpuApicIdOrderTable[Index].CpuCoreType != CPUID_CORE_TYPE_INTEL_ATOM)) {
> + DEBUG ((
> + DEBUG_INFO,
> + "Unknown Cpu Core type found: Apic = 0x%x, CoreType = 0x%x\n",
> + TempCpuApicIdOrderTable[Index].ApicId,
> + TempCpuApicIdOrderTable[Index].CpuCoreType
> + ));
> +
> + 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
>
>
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
2022-11-17 6:01 [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT JackX Lin
2022-11-17 6:06 ` [edk2-devel] " Michael D Kinney
@ 2022-11-17 14:09 ` Pedro Falcato
2022-11-18 8:34 ` JackX Lin
1 sibling, 1 reply; 10+ messages in thread
From: Pedro Falcato @ 2022-11-17 14:09 UTC (permalink / raw)
To: devel, JackX.Lin
Cc: Chasel Chiu, Nate DeSimone, Isaac Oram, Liming Gao, Eric Dong,
Donald Kuo, Chandana C Kumar
[-- Attachment #1: Type: text/plain, Size: 443 bytes --]
On Thu, Nov 17, 2022 at 6:01 AM JackX Lin <JackX.Lin@intel.com> wrote:
> BIOS should keep MADT ordering by big core first then small core
>
Hi Jack,
Can you please elaborate why this is required? AFAIK nowhere in the ACPI
spec does it say you need to order by big-little cores.
Is this some sort of regression for a specific OS? A useful commit message
and/or some comments would be really useful here for posterity.
Thanks,
Pedro Falcato
[-- Attachment #2: Type: text/html, Size: 866 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
2022-11-17 14:09 ` Pedro Falcato
@ 2022-11-18 8:34 ` JackX Lin
2022-11-18 19:11 ` Pedro Falcato
0 siblings, 1 reply; 10+ messages in thread
From: JackX Lin @ 2022-11-18 8:34 UTC (permalink / raw)
To: Pedro Falcato, devel@edk2.groups.io
Cc: Chiu, Chasel, Desimone, Nathaniel L, Oram, Isaac W, Gao, Liming,
Dong, Eric, Kuo, Donald, Kumar, Chandana C
[-- Attachment #1: Type: text/plain, Size: 1523 bytes --]
Hi Pedro,
Yes, it is for some specific test tools.
There are different performance between CPU cores, and the big cores are better than small cores.
Some legacy tools are executing with the highest performance CPU cores, In early CPU design, it is usually the first one.
We have to put them at the front of all CPU cores, otherwise some tests cannot pass.
Due to avoid this happened, we need to ensure CPU cores are ordered by big core first.
Best regards
Jack
From: Pedro Falcato <pedro.falcato@gmail.com>
Sent: Thursday, November 17, 2022 10:09 PM
To: devel@edk2.groups.io; Lin, JackX <jackx.lin@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
On Thu, Nov 17, 2022 at 6:01 AM JackX Lin <JackX.Lin@intel.com<mailto:JackX.Lin@intel.com>> wrote:
BIOS should keep MADT ordering by big core first then small core
Hi Jack,
Can you please elaborate why this is required? AFAIK nowhere in the ACPI spec does it say you need to order by big-little cores.
Is this some sort of regression for a specific OS? A useful commit message and/or some comments would be really useful here for posterity.
Thanks,
Pedro Falcato
[-- Attachment #2: Type: text/html, Size: 4640 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
2022-11-18 8:34 ` JackX Lin
@ 2022-11-18 19:11 ` Pedro Falcato
2022-11-22 3:35 ` Ni, Ray
0 siblings, 1 reply; 10+ messages in thread
From: Pedro Falcato @ 2022-11-18 19:11 UTC (permalink / raw)
To: Lin, JackX
Cc: devel@edk2.groups.io, Chiu, Chasel, Desimone, Nathaniel L,
Oram, Isaac W, Gao, Liming, Dong, Eric, Kuo, Donald,
Kumar, Chandana C
[-- Attachment #1: Type: text/plain, Size: 906 bytes --]
On Fri, Nov 18, 2022 at 8:35 AM Lin, JackX <jackx.lin@intel.com> wrote:
> Hi Pedro,
>
>
>
> Yes, it is for some specific test tools.
>
>
>
> There are different performance between CPU cores, and the big cores are
> better than small cores.
>
> Some legacy tools are executing with the highest performance CPU cores, In
> early CPU design, it is usually the first one.
>
>
>
> We have to put them at the front of all CPU cores, otherwise some tests
> cannot pass.
>
> Due to avoid this happened, we need to ensure CPU cores are ordered by big
> core first.
>
Thank you for the thorough description. Could you please (or whoever is
pushing this) add this to the commit message?
I've been following the evolution of this specific file over the last few
months with mild interest and getting things properly documented should be
pretty important!
Might help avoid any future breakage too :D
Thanks,
Pedro
[-- Attachment #2: Type: text/html, Size: 1807 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT
2022-11-18 19:11 ` Pedro Falcato
@ 2022-11-22 3:35 ` Ni, Ray
0 siblings, 0 replies; 10+ messages in thread
From: Ni, Ray @ 2022-11-22 3:35 UTC (permalink / raw)
To: devel@edk2.groups.io, pedro.falcato@gmail.com, Lin, JackX
Cc: Chiu, Chasel, Desimone, Nathaniel L, Oram, Isaac W, Gao, Liming,
Dong, Eric, Kuo, Donald, Kumar, Chandana C
[-- Attachment #1: Type: text/plain, Size: 1863 bytes --]
Pedro.
Thanks for the comments.
I agree that detailed commit message helps to explain that this MADT order change is to help some
legacy APPs that assume the high performance cores are before low performance ones in MADT
and use OS APIs to explicitly request running on certain target cpu cores to get higher APP performance.
Thanks,
Ray
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pedro Falcato
Sent: Saturday, November 19, 2022 3:11 AM
To: Lin, JackX <jackx.lin@intel.com>
Cc: devel@edk2.groups.io; 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
On Fri, Nov 18, 2022 at 8:35 AM Lin, JackX <jackx.lin@intel.com<mailto:jackx.lin@intel.com>> wrote:
Hi Pedro,
Yes, it is for some specific test tools.
There are different performance between CPU cores, and the big cores are better than small cores.
Some legacy tools are executing with the highest performance CPU cores, In early CPU design, it is usually the first one.
We have to put them at the front of all CPU cores, otherwise some tests cannot pass.
Due to avoid this happened, we need to ensure CPU cores are ordered by big core first.
Thank you for the thorough description. Could you please (or whoever is pushing this) add this to the commit message?
I've been following the evolution of this specific file over the last few months with mild interest and getting things properly documented should be pretty important!
Might help avoid any future breakage too :D
Thanks,
Pedro
[-- Attachment #2: Type: text/html, Size: 6036 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-11-22 3:36 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-17 6:01 [edk2-platforms: PATCH] BIOS needs to present cores in order of relative performance in MADT JackX Lin
2022-11-17 6:06 ` [edk2-devel] " Michael D Kinney
2022-11-17 14:09 ` Pedro Falcato
2022-11-18 8:34 ` JackX Lin
2022-11-18 19:11 ` Pedro Falcato
2022-11-22 3:35 ` Ni, Ray
-- strict thread matches above, loose matches on Subject: below --
2022-11-16 6:29 JackX Lin
2022-11-17 3:45 ` [edk2-devel] " Michael D Kinney
2022-11-17 3:54 ` JackX Lin
2022-11-16 2:52 JackX Lin
2022-11-16 3:50 ` [edk2-devel] " Michael D Kinney
2022-11-16 3:59 ` JackX Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox