[edk2] [PATCH v3] UefiCpuPkg: ApicLib

Leo Duran posted 1 patch 6 years, 9 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c     | 42 +++++++++++++---------
.../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c        | 40 ++++++++++++---------
2 files changed, 49 insertions(+), 33 deletions(-)
[edk2] [PATCH v3] UefiCpuPkg: ApicLib
Posted by Leo Duran 6 years, 9 months ago
GetProcessorLocationByApicId ()
- Adjust InitialApicId to properly concatenate Package on AMD processor.
- Clean-ups on C Coding standards.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
 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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3] UefiCpuPkg: ApicLib
Posted by Kinney, Michael D 6 years, 9 months ago
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:edk2-devel-bounces@lists.01.org] On Behalf Of Leo Duran
Sent: Thursday, July 6, 2017 7:48 AM
To: edk2-devel@lists.01.org
Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Leo Duran <leo.duran@amd.com>; Fan, Jeff <jeff.fan@intel.com>; Gao, Liming <liming.gao@intel.com>
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 <jordan.l.justen@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
 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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3] UefiCpuPkg: ApicLib
Posted by Duran, Leo 6 years, 9 months ago
Hi Mike,


> -----Original Message-----
> From: Kinney, Michael D [mailto:michael.d.kinney@intel.com]
> Sent: Thursday, July 06, 2017 11:36 AM
> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@lists.01.org; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Fan, Jeff
> <jeff.fan@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [edk2] [PATCH v3] UefiCpuPkg: ApicLib
> 
> 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.
> 
[Duran, Leo] 
Yes, I do see a malformed AsmCpuId() function call... Somehow that was missed in the original submission that's already upstream.
Anyway, good catch!... I'll fix it.
Thanks.

> 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/3f1100beda60843361a9761b43ba7b1
> 8adf0d265
> 
> Mike
> 
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Leo Duran
> Sent: Thursday, July 6, 2017 7:48 AM
> To: edk2-devel@lists.01.org
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Leo Duran
> <leo.duran@amd.com>; Fan, Jeff <jeff.fan@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> 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 <jordan.l.justen@intel.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leo Duran <leo.duran@amd.com>
> ---
>  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
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel