public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3] UefiCpuPkg: ApicLib
@ 2017-07-06 14:47 Leo Duran
  2017-07-06 14:47 ` Leo Duran
  0 siblings, 1 reply; 4+ messages in thread
From: Leo Duran @ 2017-07-06 14:47 UTC (permalink / raw)
  To: edk2-devel; +Cc: Leo Duran

Now that we have a function to detect AMD processors:
GetProcessorLocationByApicId ()
- Adjust InitialApicId to properly concatenate Package on AMD processor.
- Clean-ups on C Coding standards.

Changes since v2:
Remove changes regarding SIPI sequence.

Changes since v1:
Just a few more C Coding standards fix-ups.

Leo Duran (1):
  UefiCpuPkg: ApicLib

 UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c     | 42 +++++++++++++---------
 .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c        | 40 ++++++++++++---------
 2 files changed, 49 insertions(+), 33 deletions(-)

-- 
2.7.4



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3] UefiCpuPkg: ApicLib
  2017-07-06 14:47 [PATCH v3] UefiCpuPkg: ApicLib Leo Duran
@ 2017-07-06 14:47 ` Leo Duran
  2017-07-06 16:35   ` Kinney, Michael D
  0 siblings, 1 reply; 4+ messages in thread
From: Leo Duran @ 2017-07-06 14:47 UTC (permalink / raw)
  To: edk2-devel; +Cc: Leo Duran, Jordan Justen, Jeff Fan, Liming Gao, Brijesh Singh

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



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] UefiCpuPkg: ApicLib
  2017-07-06 14:47 ` Leo Duran
@ 2017-07-06 16:35   ` Kinney, Michael D
  2017-07-06 18:08     ` Duran, Leo
  0 siblings, 1 reply; 4+ messages in thread
From: Kinney, Michael D @ 2017-07-06 16:35 UTC (permalink / raw)
  To: Leo Duran, edk2-devel@lists.01.org, Kinney, Michael D
  Cc: Justen, Jordan L, Fan, Jeff, Gao, Liming

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] UefiCpuPkg: ApicLib
  2017-07-06 16:35   ` Kinney, Michael D
@ 2017-07-06 18:08     ` Duran, Leo
  0 siblings, 0 replies; 4+ messages in thread
From: Duran, Leo @ 2017-07-06 18:08 UTC (permalink / raw)
  To: 'Kinney, Michael D', edk2-devel@lists.01.org
  Cc: Justen, Jordan L, Fan, Jeff, Gao, Liming

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-07-06 18:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-06 14:47 [PATCH v3] UefiCpuPkg: ApicLib Leo Duran
2017-07-06 14:47 ` Leo Duran
2017-07-06 16:35   ` Kinney, Michael D
2017-07-06 18:08     ` Duran, Leo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox