Hi Leo, Some of the change here do not follow the coding standard. For function calls with many arguments, either put the function call on a single line, or break it up with each arg on its own line.
This was clarified in the latest version of the EDK II C Coding Standard. https://bugzilla.tianocore.org/show_bug.cgi?id=425 https://github.com/tianocore-docs/edk2-CCodingStandardsSpecification/commit/3f1100beda60843361a9761b43ba7b18adf0d265 Mike -----Original Message----- From: edk2-devel [mailto:[email protected]] On Behalf Of Leo Duran Sent: Thursday, July 6, 2017 7:48 AM To: [email protected] Cc: Justen, Jordan L <[email protected]>; Leo Duran <[email protected]>; Fan, Jeff <[email protected]>; Gao, Liming <[email protected]> Subject: [edk2] [PATCH v3] UefiCpuPkg: ApicLib GetProcessorLocationByApicId () - Adjust InitialApicId to properly concatenate Package on AMD processor. - Clean-ups on C Coding standards. Cc: Jordan Justen <[email protected]> Cc: Jeff Fan <[email protected]> Cc: Liming Gao <[email protected]> Cc: Brijesh Singh <[email protected]> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leo Duran <[email protected]> --- UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 42 +++++++++++++--------- .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 40 ++++++++++++--------- 2 files changed, 49 insertions(+), 33 deletions(-) diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c index 2091e5e..ce6d9d7 100644 --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c @@ -48,7 +48,7 @@ StandardSignatureIsAuthenticAMD ( UINT32 RegEcx; UINT32 RegEdx; - AsmCpuid(CPUID_SIGNATURE, NULL, &RegEbx, &RegEcx, &RegEdx); + AsmCpuid (CPUID_SIGNATURE, NULL, &RegEbx, &RegEcx, &RegEdx); return (RegEbx == CPUID_SIGNATURE_AUTHENTIC_AMD_EBX && RegEcx == CPUID_SIGNATURE_AUTHENTIC_AMD_ECX && RegEdx == CPUID_SIGNATURE_AUTHENTIC_AMD_EDX); @@ -338,7 +338,7 @@ GetInitialApicId ( AsmCpuid (CPUID_SIGNATURE, &MaxCpuIdIndex, NULL, NULL, NULL); // - // If CPUID Leaf B is supported, + // If CPUID Leaf B is supported, // And CPUID.0BH:EBX[15:0] reports a non-zero value, // Then the initial 32-bit APIC ID = CPUID.0BH:EDX // Else the initial 8-bit APIC ID = CPUID.1:EBX[31:24] @@ -1013,13 +1013,14 @@ GetProcessorLocationByApicId ( UINT32 MaxCoresPerNode; UINT32 CorePerNodeMask; UINT32 ApicIdShift; + UINT32 ApicIdMask; UINTN ThreadBits; UINTN CoreBits; // // Check if the processor is capable of supporting more than one logical processor. // - AsmCpuid(CPUID_VERSION_INFO, NULL, NULL, NULL, &VersionInfoEdx.Uint32); + AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, NULL, + &VersionInfoEdx.Uint32); if (VersionInfoEdx.Bits.HTT == 0) { if (Thread != NULL) { *Thread = 0; @@ -1042,8 +1043,8 @@ GetProcessorLocationByApicId ( // // Get max index of CPUID // - AsmCpuid(CPUID_SIGNATURE, &MaxStandardCpuIdIndex, NULL, NULL, NULL); - AsmCpuid(CPUID_EXTENDED_FUNCTION, &MaxExtendedCpuIdIndex, NULL, NULL, NULL); + AsmCpuid (CPUID_SIGNATURE, &MaxStandardCpuIdIndex, NULL, NULL, NULL); + AsmCpuid (CPUID_EXTENDED_FUNCTION, &MaxExtendedCpuIdIndex, NULL, NULL, + NULL); // // If the extended topology enumeration leaf is available, it @@ -1051,7 +1052,7 @@ GetProcessorLocationByApicId ( // TopologyLeafSupported = FALSE; if (MaxStandardCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) { - AsmCpuidEx( + AsmCpuidEx ( CPUID_EXTENDED_TOPOLOGY, 0, &ExtendedTopologyEax.Uint32, @@ -1072,7 +1073,7 @@ GetProcessorLocationByApicId ( // the SMT sub-field of x2APIC ID. // LevelType = ExtendedTopologyEcx.Bits.LevelType; - ASSERT(LevelType == CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_SMT); + ASSERT (LevelType == CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_SMT); ThreadBits = ExtendedTopologyEax.Bits.ApicIdShift; // @@ -1081,7 +1082,7 @@ GetProcessorLocationByApicId ( // SubIndex = 1; do { - AsmCpuidEx( + AsmCpuidEx ( CPUID_EXTENDED_TOPOLOGY, SubIndex, &ExtendedTopologyEax.Uint32, @@ -1103,7 +1104,7 @@ GetProcessorLocationByApicId ( // // Get logical processor count // - AsmCpuid(CPUID_VERSION_INFO, NULL, &VersionInfoEbx.Uint32, NULL, NULL); + AsmCpuid (CPUID_VERSION_INFO, NULL, &VersionInfoEbx.Uint32, NULL, + NULL); MaxLogicProcessorsPerPackage = VersionInfoEbx.Bits.MaximumAddressableIdsForLogicalProcessors; // @@ -1114,11 +1115,11 @@ GetProcessorLocationByApicId ( // // Check for topology extensions on AMD processor // - if (StandardSignatureIsAuthenticAMD()) { + if (StandardSignatureIsAuthenticAMD ()) { if (MaxExtendedCpuIdIndex >= CPUID_AMD_PROCESSOR_TOPOLOGY) { - AsmCpuid(CPUID_EXTENDED_CPU_SIG, NULL, NULL, &AmdExtendedCpuSigEcx.Uint32, NULL); + AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, + &AmdExtendedCpuSigEcx.Uint32, NULL); if (AmdExtendedCpuSigEcx.Bits.TopologyExtensions != 0) { - AsmCpuid(CPUID_AMD_PROCESSOR_TOPOLOGY, NULL, &AmdProcessorTopologyEbx.Uint32, + AsmCpuid (CPUID_AMD_PROCESSOR_TOPOLOGY, NULL, + &AmdProcessorTopologyEbx.Uint32, &AmdProcessorTopologyEcx.Uint32, NULL); // // Get cores per processor package @@ -1128,7 +1129,7 @@ GetProcessorLocationByApicId ( // // Account for actual thread count (e.g., SMT disabled) // - AsmCpuid(CPUID_VIR_PHY_ADDRESS_SIZE, NULL, NULL, &AmdVirPhyAddressSizeEcx.Uint32, NULL); + AsmCpuid (CPUID_VIR_PHY_ADDRESS_SIZE, NULL, NULL, + &AmdVirPhyAddressSizeEcx.Uint32, NULL); MaxThreadPerPackageMask = 1 << AmdVirPhyAddressSizeEcx.Bits.ApicIdCoreIdSize; ActualThreadPerPackageMask = 1; while (ActualThreadPerPackageMask < MaxLogicProcessorsPerPackage) { @@ -1136,7 +1137,7 @@ GetProcessorLocationByApicId ( } // - // Adjust APIC Id to report concatenation of Package|Core|Thread. + // Adjust APIC Id to report concatenation of Core|Thread. // if (ActualThreadPerPackageMask < MaxThreadPerPackageMask) { MaxCoresPerNode = MaxCoresPerPackage / (AmdProcessorTopologyEcx.Bits.NodesPerProcessor + 1); @@ -1148,13 +1149,20 @@ GetProcessorLocationByApicId ( CorePerNodeMask -= 1; ApicIdShift = 0; + ApicIdMask = ActualThreadPerPackageMask; do { ApicIdShift += 1; - ActualThreadPerPackageMask <<= 1; - } while (ActualThreadPerPackageMask < MaxThreadPerPackageMask); + ApicIdMask <<= 1; + } while (ApicIdMask < MaxThreadPerPackageMask); InitialApicId = ((InitialApicId & ~CorePerNodeMask) >> ApicIdShift) | (InitialApicId & CorePerNodeMask); } + // + // Adjust APIC Id to report concatenation of Package|Core|Thread. + // + if ((InitialApicId & ~(MaxThreadPerPackageMask - 1)) != 0) { + InitialApicId = (InitialApicId & (ActualThreadPerPackageMask - 1)) | ActualThreadPerPackageMask; + } } } } @@ -1163,7 +1171,7 @@ GetProcessorLocationByApicId ( // Extract core count based on CACHE information // if (MaxStandardCpuIdIndex >= CPUID_CACHE_PARAMS) { - AsmCpuidEx(CPUID_CACHE_PARAMS, 0, &CacheParamsEax.Uint32, NULL, NULL, NULL); + AsmCpuidEx (CPUID_CACHE_PARAMS, 0, &CacheParamsEax.Uint32, + NULL, NULL, NULL); if (CacheParamsEax.Uint32 != 0) { MaxCoresPerPackage = CacheParamsEax.Bits.MaximumAddressableIdsForLogicalProcessors + 1; } diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c index d5d4efa..54ea492 100644 --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c @@ -49,7 +49,7 @@ StandardSignatureIsAuthenticAMD ( UINT32 RegEcx; UINT32 RegEdx; - AsmCpuid(CPUID_SIGNATURE, NULL, &RegEbx, &RegEcx, &RegEdx); + AsmCpuid (CPUID_SIGNATURE, NULL, &RegEbx, &RegEcx, &RegEdx); return (RegEbx == CPUID_SIGNATURE_AUTHENTIC_AMD_EBX && RegEcx == CPUID_SIGNATURE_AUTHENTIC_AMD_ECX && RegEdx == CPUID_SIGNATURE_AUTHENTIC_AMD_EDX); @@ -1108,13 +1108,14 @@ GetProcessorLocationByApicId ( UINT32 MaxCoresPerNode; UINT32 CorePerNodeMask; UINT32 ApicIdShift; + UINT32 ApicIdMask; UINTN ThreadBits; UINTN CoreBits; // // Check if the processor is capable of supporting more than one logical processor. // - AsmCpuid(CPUID_VERSION_INFO, NULL, NULL, NULL, &VersionInfoEdx.Uint32); + AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, NULL, + &VersionInfoEdx.Uint32); if (VersionInfoEdx.Bits.HTT == 0) { if (Thread != NULL) { *Thread = 0; @@ -1137,8 +1138,8 @@ GetProcessorLocationByApicId ( // // Get max index of CPUID // - AsmCpuid(CPUID_SIGNATURE, &MaxStandardCpuIdIndex, NULL, NULL, NULL); - AsmCpuid(CPUID_EXTENDED_FUNCTION, &MaxExtendedCpuIdIndex, NULL, NULL, NULL); + AsmCpuid (CPUID_SIGNATURE, &MaxStandardCpuIdIndex, NULL, NULL, NULL); + AsmCpuid (CPUID_EXTENDED_FUNCTION, &MaxExtendedCpuIdIndex, NULL, NULL, + NULL); // // If the extended topology enumeration leaf is available, it @@ -1146,7 +1147,7 @@ GetProcessorLocationByApicId ( // TopologyLeafSupported = FALSE; if (MaxStandardCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) { - AsmCpuidEx( + AsmCpuidEx ( CPUID_EXTENDED_TOPOLOGY, 0, &ExtendedTopologyEax.Uint32, @@ -1167,7 +1168,7 @@ GetProcessorLocationByApicId ( // the SMT sub-field of x2APIC ID. // LevelType = ExtendedTopologyEcx.Bits.LevelType; - ASSERT(LevelType == CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_SMT); + ASSERT (LevelType == CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_SMT); ThreadBits = ExtendedTopologyEax.Bits.ApicIdShift; // @@ -1176,7 +1177,7 @@ GetProcessorLocationByApicId ( // SubIndex = 1; do { - AsmCpuidEx( + AsmCpuidEx ( CPUID_EXTENDED_TOPOLOGY, SubIndex, &ExtendedTopologyEax.Uint32, @@ -1198,7 +1199,7 @@ GetProcessorLocationByApicId ( // // Get logical processor count // - AsmCpuid(CPUID_VERSION_INFO, NULL, &VersionInfoEbx.Uint32, NULL, NULL); + AsmCpuid (CPUID_VERSION_INFO, NULL, &VersionInfoEbx.Uint32, NULL, + NULL); MaxLogicProcessorsPerPackage = VersionInfoEbx.Bits.MaximumAddressableIdsForLogicalProcessors; // @@ -1209,11 +1210,11 @@ GetProcessorLocationByApicId ( // // Check for topology extensions on AMD processor // - if (StandardSignatureIsAuthenticAMD()) { + if (StandardSignatureIsAuthenticAMD ()) { if (MaxExtendedCpuIdIndex >= CPUID_AMD_PROCESSOR_TOPOLOGY) { - AsmCpuid(CPUID_EXTENDED_CPU_SIG, NULL, NULL, &AmdExtendedCpuSigEcx.Uint32, NULL); + AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, + &AmdExtendedCpuSigEcx.Uint32, NULL); if (AmdExtendedCpuSigEcx.Bits.TopologyExtensions != 0) { - AsmCpuid(CPUID_AMD_PROCESSOR_TOPOLOGY, NULL, &AmdProcessorTopologyEbx.Uint32, + AsmCpuid (CPUID_AMD_PROCESSOR_TOPOLOGY, NULL, + &AmdProcessorTopologyEbx.Uint32, &AmdProcessorTopologyEcx.Uint32, NULL); // // Get cores per processor package @@ -1223,7 +1224,7 @@ GetProcessorLocationByApicId ( // // Account for actual thread count (e.g., SMT disabled) // - AsmCpuid(CPUID_VIR_PHY_ADDRESS_SIZE, NULL, NULL, &AmdVirPhyAddressSizeEcx.Uint32, NULL); + AsmCpuid (CPUID_VIR_PHY_ADDRESS_SIZE, NULL, NULL, + &AmdVirPhyAddressSizeEcx.Uint32, NULL); MaxThreadPerPackageMask = 1 << AmdVirPhyAddressSizeEcx.Bits.ApicIdCoreIdSize; ActualThreadPerPackageMask = 1; while (ActualThreadPerPackageMask < MaxLogicProcessorsPerPackage) { @@ -1231,7 +1232,7 @@ GetProcessorLocationByApicId ( } // - // Adjust APIC Id to report concatenation of Package|Core|Thread. + // Adjust APIC Id to report concatenation of Core|Thread. // if (ActualThreadPerPackageMask < MaxThreadPerPackageMask) { MaxCoresPerNode = MaxCoresPerPackage / (AmdProcessorTopologyEcx.Bits.NodesPerProcessor + 1); @@ -1243,13 +1244,20 @@ GetProcessorLocationByApicId ( CorePerNodeMask -= 1; ApicIdShift = 0; + ApicIdMask = ActualThreadPerPackageMask; do { ApicIdShift += 1; - ActualThreadPerPackageMask <<= 1; - } while (ActualThreadPerPackageMask < MaxThreadPerPackageMask); + ApicIdMask <<= 1; + } while (ApicIdMask < MaxThreadPerPackageMask); InitialApicId = ((InitialApicId & ~CorePerNodeMask) >> ApicIdShift) | (InitialApicId & CorePerNodeMask); } + // + // Adjust APIC Id to report concatenation of Package|Core|Thread. + // + if ((InitialApicId & ~(MaxThreadPerPackageMask - 1)) != 0) { + InitialApicId = (InitialApicId & (ActualThreadPerPackageMask - 1)) | ActualThreadPerPackageMask; + } } } } @@ -1258,7 +1266,7 @@ GetProcessorLocationByApicId ( // Extract core count based on CACHE information // if (MaxStandardCpuIdIndex >= CPUID_CACHE_PARAMS) { - AsmCpuidEx(CPUID_CACHE_PARAMS, 0, &CacheParamsEax.Uint32, NULL, NULL, NULL); + AsmCpuidEx (CPUID_CACHE_PARAMS, 0, &CacheParamsEax.Uint32, + NULL, NULL, NULL); if (CacheParamsEax.Uint32 != 0) { MaxCoresPerPackage = CacheParamsEax.Bits.MaximumAddressableIdsForLogicalProcessors + 1; } -- 2.7.4 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
