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

Now that we have a function to detect AMD processors:
1) SendInitSipiSipi ()
   Skip repeating SendIpi () on AMD processor.
    
2) SendInitSipiSipiAllExcludingSelf ()
   Skip repeating SendIpi () on AMD processor.
    
3) GetProcessorLocationByApicId ()
   Adjust InitialApicId to properly concatenate Package on AMD processor.
   Clean-ups on C Coding standards.

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

Leo Duran (1):
  UefiCpuPkg: ApicLib

 UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c     | 52 +++++++++++++---------
 .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c        | 50 +++++++++++++--------
 2 files changed, 63 insertions(+), 39 deletions(-)

-- 
2.7.4



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

* [PATCH v2] UefiCpuPkg: ApicLib
  2017-07-01 22:45 [PATCH v2] UefiCpuPkg: ApicLib Leo Duran
@ 2017-07-01 22:45 ` Leo Duran
  2017-07-06  1:27   ` Fan, Jeff
  0 siblings, 1 reply; 6+ messages in thread
From: Leo Duran @ 2017-07-01 22:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Leo Duran, Jordan Justen, Jeff Fan, Liming Gao, Brijesh Singh

1) SendInitSipiSipi ()
Skip repeating SendIpi () on AMD processor.

2) SendInitSipiSipiAllExcludingSelf ()
Skip repeating SendIpi () on AMD processor.

3) 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     | 52 +++++++++++++---------
 .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c        | 50 +++++++++++++--------
 2 files changed, 63 insertions(+), 39 deletions(-)

diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
index 2091e5e..a6e4e2e 100644
--- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
@@ -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]
@@ -554,8 +554,10 @@ SendInitSipiSipi (
   IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
   IcrLow.Bits.Level = 1;
   SendIpi (IcrLow.Uint32, ApicId);
-  MicroSecondDelay (200);
-  SendIpi (IcrLow.Uint32, ApicId);
+  if (!StandardSignatureIsAuthenticAMD ()) {
+    MicroSecondDelay (200);
+    SendIpi (IcrLow.Uint32, ApicId);
+  }
 }
 
 /**
@@ -588,8 +590,10 @@ SendInitSipiSipiAllExcludingSelf (
   IcrLow.Bits.Level = 1;
   IcrLow.Bits.DestinationShorthand = LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
   SendIpi (IcrLow.Uint32, 0);
-  MicroSecondDelay (200);
-  SendIpi (IcrLow.Uint32, 0);
+  if (!StandardSignatureIsAuthenticAMD ()) {
+    MicroSecondDelay (200);
+    SendIpi (IcrLow.Uint32, 0);
+  }
 }
 
 /**
@@ -1013,13 +1017,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 +1047,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 +1056,7 @@ GetProcessorLocationByApicId (
   //
   TopologyLeafSupported = FALSE;
   if (MaxStandardCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) {
-    AsmCpuidEx(
+    AsmCpuidEx (
       CPUID_EXTENDED_TOPOLOGY,
       0,
       &ExtendedTopologyEax.Uint32,
@@ -1072,7 +1077,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 +1086,7 @@ GetProcessorLocationByApicId (
       //
       SubIndex = 1;
       do {
-        AsmCpuidEx(
+        AsmCpuidEx (
           CPUID_EXTENDED_TOPOLOGY,
           SubIndex,
           &ExtendedTopologyEax.Uint32,
@@ -1103,7 +1108,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 +1119,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 +1133,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 +1141,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 +1153,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 +1175,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..5945b55 100644
--- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
@@ -649,8 +649,10 @@ SendInitSipiSipi (
   IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
   IcrLow.Bits.Level = 1;
   SendIpi (IcrLow.Uint32, ApicId);
-  MicroSecondDelay (200);
-  SendIpi (IcrLow.Uint32, ApicId);
+  if (!StandardSignatureIsAuthenticAMD ()) {
+    MicroSecondDelay (200);
+    SendIpi (IcrLow.Uint32, ApicId);
+  }
 }
 
 /**
@@ -683,8 +685,10 @@ SendInitSipiSipiAllExcludingSelf (
   IcrLow.Bits.Level = 1;
   IcrLow.Bits.DestinationShorthand = LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
   SendIpi (IcrLow.Uint32, 0);
-  MicroSecondDelay (200);
-  SendIpi (IcrLow.Uint32, 0);
+  if (!StandardSignatureIsAuthenticAMD ()) {
+    MicroSecondDelay (200);
+    SendIpi (IcrLow.Uint32, 0);
+  }
 }
 
 /**
@@ -1108,13 +1112,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 +1142,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 +1151,7 @@ GetProcessorLocationByApicId (
   //
   TopologyLeafSupported = FALSE;
   if (MaxStandardCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) {
-    AsmCpuidEx(
+    AsmCpuidEx (
       CPUID_EXTENDED_TOPOLOGY,
       0,
       &ExtendedTopologyEax.Uint32,
@@ -1167,7 +1172,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 +1181,7 @@ GetProcessorLocationByApicId (
       //
       SubIndex = 1;
       do {
-        AsmCpuidEx(
+        AsmCpuidEx (
           CPUID_EXTENDED_TOPOLOGY,
           SubIndex,
           &ExtendedTopologyEax.Uint32,
@@ -1198,7 +1203,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 +1214,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 +1228,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 +1236,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 +1248,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 +1270,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] 6+ messages in thread

* Re: [PATCH v2] UefiCpuPkg: ApicLib
  2017-07-01 22:45 ` Leo Duran
@ 2017-07-06  1:27   ` Fan, Jeff
  2017-07-06  1:53     ` Ni, Ruiyu
  2017-07-06 13:47     ` Duran, Leo
  0 siblings, 2 replies; 6+ messages in thread
From: Fan, Jeff @ 2017-07-06  1:27 UTC (permalink / raw)
  To: Leo Duran, edk2-devel@lists.01.org
  Cc: Justen, Jordan L, Gao, Liming, Brijesh Singh, Dong, Eric

Leo,

How AMD public spec dos define the manner of sending startup IPI to Aps? Which chapter is it defined in AMD public spec?

Does AMD public spec indicate the second startup IPI is not required?

Thanks!
Jeff

-----Original Message-----
From: Leo Duran [mailto:leo.duran@amd.com] 
Sent: Sunday, July 02, 2017 6:45 AM
To: edk2-devel@lists.01.org
Cc: Leo Duran; Justen, Jordan L; Fan, Jeff; Gao, Liming; Brijesh Singh
Subject: [PATCH v2] UefiCpuPkg: ApicLib

1) SendInitSipiSipi ()
Skip repeating SendIpi () on AMD processor.

2) SendInitSipiSipiAllExcludingSelf ()
Skip repeating SendIpi () on AMD processor.

3) 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     | 52 +++++++++++++---------
 .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c        | 50 +++++++++++++--------
 2 files changed, 63 insertions(+), 39 deletions(-)

diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
index 2091e5e..a6e4e2e 100644
--- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
@@ -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] @@ -554,8 +554,10 @@ SendInitSipiSipi (
   IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
   IcrLow.Bits.Level = 1;
   SendIpi (IcrLow.Uint32, ApicId);
-  MicroSecondDelay (200);
-  SendIpi (IcrLow.Uint32, ApicId);
+  if (!StandardSignatureIsAuthenticAMD ()) {
+    MicroSecondDelay (200);
+    SendIpi (IcrLow.Uint32, ApicId);
+  }
 }
 
 /**
@@ -588,8 +590,10 @@ SendInitSipiSipiAllExcludingSelf (
   IcrLow.Bits.Level = 1;
   IcrLow.Bits.DestinationShorthand = LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
   SendIpi (IcrLow.Uint32, 0);
-  MicroSecondDelay (200);
-  SendIpi (IcrLow.Uint32, 0);
+  if (!StandardSignatureIsAuthenticAMD ()) {
+    MicroSecondDelay (200);
+    SendIpi (IcrLow.Uint32, 0);
+  }
 }
 
 /**
@@ -1013,13 +1017,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 +1047,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 +1056,7 @@ GetProcessorLocationByApicId (
   //
   TopologyLeafSupported = FALSE;
   if (MaxStandardCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) {
-    AsmCpuidEx(
+    AsmCpuidEx (
       CPUID_EXTENDED_TOPOLOGY,
       0,
       &ExtendedTopologyEax.Uint32,
@@ -1072,7 +1077,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 +1086,7 @@ GetProcessorLocationByApicId (
       //
       SubIndex = 1;
       do {
-        AsmCpuidEx(
+        AsmCpuidEx (
           CPUID_EXTENDED_TOPOLOGY,
           SubIndex,
           &ExtendedTopologyEax.Uint32,
@@ -1103,7 +1108,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 +1119,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 +1133,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 +1141,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 +1153,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 +1175,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..5945b55 100644
--- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
@@ -649,8 +649,10 @@ SendInitSipiSipi (
   IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
   IcrLow.Bits.Level = 1;
   SendIpi (IcrLow.Uint32, ApicId);
-  MicroSecondDelay (200);
-  SendIpi (IcrLow.Uint32, ApicId);
+  if (!StandardSignatureIsAuthenticAMD ()) {
+    MicroSecondDelay (200);
+    SendIpi (IcrLow.Uint32, ApicId);
+  }
 }
 
 /**
@@ -683,8 +685,10 @@ SendInitSipiSipiAllExcludingSelf (
   IcrLow.Bits.Level = 1;
   IcrLow.Bits.DestinationShorthand = LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
   SendIpi (IcrLow.Uint32, 0);
-  MicroSecondDelay (200);
-  SendIpi (IcrLow.Uint32, 0);
+  if (!StandardSignatureIsAuthenticAMD ()) {
+    MicroSecondDelay (200);
+    SendIpi (IcrLow.Uint32, 0);
+  }
 }
 
 /**
@@ -1108,13 +1112,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 +1142,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 +1151,7 @@ GetProcessorLocationByApicId (
   //
   TopologyLeafSupported = FALSE;
   if (MaxStandardCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) {
-    AsmCpuidEx(
+    AsmCpuidEx (
       CPUID_EXTENDED_TOPOLOGY,
       0,
       &ExtendedTopologyEax.Uint32,
@@ -1167,7 +1172,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 +1181,7 @@ GetProcessorLocationByApicId (
       //
       SubIndex = 1;
       do {
-        AsmCpuidEx(
+        AsmCpuidEx (
           CPUID_EXTENDED_TOPOLOGY,
           SubIndex,
           &ExtendedTopologyEax.Uint32,
@@ -1198,7 +1203,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 +1214,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 +1228,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 +1236,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 +1248,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 +1270,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] 6+ messages in thread

* Re: [PATCH v2] UefiCpuPkg: ApicLib
  2017-07-06  1:27   ` Fan, Jeff
@ 2017-07-06  1:53     ` Ni, Ruiyu
  2017-07-06 13:48       ` Duran, Leo
  2017-07-06 13:47     ` Duran, Leo
  1 sibling, 1 reply; 6+ messages in thread
From: Ni, Ruiyu @ 2017-07-06  1:53 UTC (permalink / raw)
  To: Fan, Jeff, Leo Duran, edk2-devel@lists.01.org
  Cc: Justen, Jordan L, Dong, Eric, Gao, Liming

Leo,
Could you please separate the clean-up code into a separate patch?

Thanks/Ray

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Fan, Jeff
> Sent: Thursday, July 6, 2017 9:28 AM
> To: Leo Duran <leo.duran@amd.com>; edk2-devel@lists.01.org
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [PATCH v2] UefiCpuPkg: ApicLib
> 
> Leo,
> 
> How AMD public spec dos define the manner of sending startup IPI to Aps?
> Which chapter is it defined in AMD public spec?
> 
> Does AMD public spec indicate the second startup IPI is not required?
> 
> Thanks!
> Jeff
> 
> -----Original Message-----
> From: Leo Duran [mailto:leo.duran@amd.com]
> Sent: Sunday, July 02, 2017 6:45 AM
> To: edk2-devel@lists.01.org
> Cc: Leo Duran; Justen, Jordan L; Fan, Jeff; Gao, Liming; Brijesh Singh
> Subject: [PATCH v2] UefiCpuPkg: ApicLib
> 
> 1) SendInitSipiSipi ()
> Skip repeating SendIpi () on AMD processor.
> 
> 2) SendInitSipiSipiAllExcludingSelf ()
> Skip repeating SendIpi () on AMD processor.
> 
> 3) 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     | 52 +++++++++++++----
> -----
>  .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c        | 50 +++++++++++++-------
> -
>  2 files changed, 63 insertions(+), 39 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> index 2091e5e..a6e4e2e 100644
> --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> @@ -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] @@ -554,8 +554,10 @@
> SendInitSipiSipi (
>    IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
>    IcrLow.Bits.Level = 1;
>    SendIpi (IcrLow.Uint32, ApicId);
> -  MicroSecondDelay (200);
> -  SendIpi (IcrLow.Uint32, ApicId);
> +  if (!StandardSignatureIsAuthenticAMD ()) {
> +    MicroSecondDelay (200);
> +    SendIpi (IcrLow.Uint32, ApicId);
> +  }
>  }
> 
>  /**
> @@ -588,8 +590,10 @@ SendInitSipiSipiAllExcludingSelf (
>    IcrLow.Bits.Level = 1;
>    IcrLow.Bits.DestinationShorthand =
> LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
>    SendIpi (IcrLow.Uint32, 0);
> -  MicroSecondDelay (200);
> -  SendIpi (IcrLow.Uint32, 0);
> +  if (!StandardSignatureIsAuthenticAMD ()) {
> +    MicroSecondDelay (200);
> +    SendIpi (IcrLow.Uint32, 0);
> +  }
>  }
> 
>  /**
> @@ -1013,13 +1017,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 +1047,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
> +1056,7 @@ GetProcessorLocationByApicId (
>    //
>    TopologyLeafSupported = FALSE;
>    if (MaxStandardCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) {
> -    AsmCpuidEx(
> +    AsmCpuidEx (
>        CPUID_EXTENDED_TOPOLOGY,
>        0,
>        &ExtendedTopologyEax.Uint32,
> @@ -1072,7 +1077,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 +1086,7 @@ GetProcessorLocationByApicId (
>        //
>        SubIndex = 1;
>        do {
> -        AsmCpuidEx(
> +        AsmCpuidEx (
>            CPUID_EXTENDED_TOPOLOGY,
>            SubIndex,
>            &ExtendedTopologyEax.Uint32,
> @@ -1103,7 +1108,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 +1119,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 +1133,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 +1141,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
> +1153,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 +1175,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..5945b55 100644
> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> @@ -649,8 +649,10 @@ SendInitSipiSipi (
>    IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
>    IcrLow.Bits.Level = 1;
>    SendIpi (IcrLow.Uint32, ApicId);
> -  MicroSecondDelay (200);
> -  SendIpi (IcrLow.Uint32, ApicId);
> +  if (!StandardSignatureIsAuthenticAMD ()) {
> +    MicroSecondDelay (200);
> +    SendIpi (IcrLow.Uint32, ApicId);
> +  }
>  }
> 
>  /**
> @@ -683,8 +685,10 @@ SendInitSipiSipiAllExcludingSelf (
>    IcrLow.Bits.Level = 1;
>    IcrLow.Bits.DestinationShorthand =
> LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
>    SendIpi (IcrLow.Uint32, 0);
> -  MicroSecondDelay (200);
> -  SendIpi (IcrLow.Uint32, 0);
> +  if (!StandardSignatureIsAuthenticAMD ()) {
> +    MicroSecondDelay (200);
> +    SendIpi (IcrLow.Uint32, 0);
> +  }
>  }
> 
>  /**
> @@ -1108,13 +1112,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 +1142,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
> +1151,7 @@ GetProcessorLocationByApicId (
>    //
>    TopologyLeafSupported = FALSE;
>    if (MaxStandardCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) {
> -    AsmCpuidEx(
> +    AsmCpuidEx (
>        CPUID_EXTENDED_TOPOLOGY,
>        0,
>        &ExtendedTopologyEax.Uint32,
> @@ -1167,7 +1172,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 +1181,7 @@ GetProcessorLocationByApicId (
>        //
>        SubIndex = 1;
>        do {
> -        AsmCpuidEx(
> +        AsmCpuidEx (
>            CPUID_EXTENDED_TOPOLOGY,
>            SubIndex,
>            &ExtendedTopologyEax.Uint32,
> @@ -1198,7 +1203,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 +1214,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 +1228,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 +1236,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
> +1248,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 +1270,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] 6+ messages in thread

* Re: [PATCH v2] UefiCpuPkg: ApicLib
  2017-07-06  1:27   ` Fan, Jeff
  2017-07-06  1:53     ` Ni, Ruiyu
@ 2017-07-06 13:47     ` Duran, Leo
  1 sibling, 0 replies; 6+ messages in thread
From: Duran, Leo @ 2017-07-06 13:47 UTC (permalink / raw)
  To: 'Fan, Jeff', edk2-devel@lists.01.org
  Cc: Justen, Jordan L, Gao, Liming, Singh, Brijesh, Dong, Eric

Hi Jeff,

> -----Original Message-----
> From: Fan, Jeff [mailto:jeff.fan@intel.com]
> Sent: Wednesday, July 05, 2017 8:28 PM
> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@lists.01.org
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Singh, Brijesh <brijesh.singh@amd.com>; Dong,
> Eric <eric.dong@intel.com>
> Subject: RE: [PATCH v2] UefiCpuPkg: ApicLib
> 
> Leo,
> 
> How AMD public spec dos define the manner of sending startup IPI to Aps?
> Which chapter is it defined in AMD public spec?
> 
> Does AMD public spec indicate the second startup IPI is not required?
> 
[Duran, Leo] 
Ok, I will try to find some documentation for this.

> Thanks!
> Jeff
> 
> -----Original Message-----
> From: Leo Duran [mailto:leo.duran@amd.com]
> Sent: Sunday, July 02, 2017 6:45 AM
> To: edk2-devel@lists.01.org
> Cc: Leo Duran; Justen, Jordan L; Fan, Jeff; Gao, Liming; Brijesh Singh
> Subject: [PATCH v2] UefiCpuPkg: ApicLib
> 
> 1) SendInitSipiSipi ()
> Skip repeating SendIpi () on AMD processor.
> 
> 2) SendInitSipiSipiAllExcludingSelf ()
> Skip repeating SendIpi () on AMD processor.
> 
> 3) 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     | 52 +++++++++++++----
> -----
>  .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c        | 50 +++++++++++++-------
> -
>  2 files changed, 63 insertions(+), 39 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> index 2091e5e..a6e4e2e 100644
> --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> @@ -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] @@ -554,8 +554,10 @@
> SendInitSipiSipi (
>    IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
>    IcrLow.Bits.Level = 1;
>    SendIpi (IcrLow.Uint32, ApicId);
> -  MicroSecondDelay (200);
> -  SendIpi (IcrLow.Uint32, ApicId);
> +  if (!StandardSignatureIsAuthenticAMD ()) {
> +    MicroSecondDelay (200);
> +    SendIpi (IcrLow.Uint32, ApicId);
> +  }
>  }
> 
>  /**
> @@ -588,8 +590,10 @@ SendInitSipiSipiAllExcludingSelf (
>    IcrLow.Bits.Level = 1;
>    IcrLow.Bits.DestinationShorthand =
> LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
>    SendIpi (IcrLow.Uint32, 0);
> -  MicroSecondDelay (200);
> -  SendIpi (IcrLow.Uint32, 0);
> +  if (!StandardSignatureIsAuthenticAMD ()) {
> +    MicroSecondDelay (200);
> +    SendIpi (IcrLow.Uint32, 0);
> +  }
>  }
> 
>  /**
> @@ -1013,13 +1017,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 +1047,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
> +1056,7 @@ GetProcessorLocationByApicId (
>    //
>    TopologyLeafSupported = FALSE;
>    if (MaxStandardCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) {
> -    AsmCpuidEx(
> +    AsmCpuidEx (
>        CPUID_EXTENDED_TOPOLOGY,
>        0,
>        &ExtendedTopologyEax.Uint32,
> @@ -1072,7 +1077,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 +1086,7 @@ GetProcessorLocationByApicId (
>        //
>        SubIndex = 1;
>        do {
> -        AsmCpuidEx(
> +        AsmCpuidEx (
>            CPUID_EXTENDED_TOPOLOGY,
>            SubIndex,
>            &ExtendedTopologyEax.Uint32,
> @@ -1103,7 +1108,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 +1119,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 +1133,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 +1141,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
> +1153,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 +1175,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..5945b55 100644
> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> @@ -649,8 +649,10 @@ SendInitSipiSipi (
>    IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
>    IcrLow.Bits.Level = 1;
>    SendIpi (IcrLow.Uint32, ApicId);
> -  MicroSecondDelay (200);
> -  SendIpi (IcrLow.Uint32, ApicId);
> +  if (!StandardSignatureIsAuthenticAMD ()) {
> +    MicroSecondDelay (200);
> +    SendIpi (IcrLow.Uint32, ApicId);
> +  }
>  }
> 
>  /**
> @@ -683,8 +685,10 @@ SendInitSipiSipiAllExcludingSelf (
>    IcrLow.Bits.Level = 1;
>    IcrLow.Bits.DestinationShorthand =
> LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
>    SendIpi (IcrLow.Uint32, 0);
> -  MicroSecondDelay (200);
> -  SendIpi (IcrLow.Uint32, 0);
> +  if (!StandardSignatureIsAuthenticAMD ()) {
> +    MicroSecondDelay (200);
> +    SendIpi (IcrLow.Uint32, 0);
> +  }
>  }
> 
>  /**
> @@ -1108,13 +1112,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 +1142,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
> +1151,7 @@ GetProcessorLocationByApicId (
>    //
>    TopologyLeafSupported = FALSE;
>    if (MaxStandardCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) {
> -    AsmCpuidEx(
> +    AsmCpuidEx (
>        CPUID_EXTENDED_TOPOLOGY,
>        0,
>        &ExtendedTopologyEax.Uint32,
> @@ -1167,7 +1172,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 +1181,7 @@ GetProcessorLocationByApicId (
>        //
>        SubIndex = 1;
>        do {
> -        AsmCpuidEx(
> +        AsmCpuidEx (
>            CPUID_EXTENDED_TOPOLOGY,
>            SubIndex,
>            &ExtendedTopologyEax.Uint32,
> @@ -1198,7 +1203,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 +1214,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 +1228,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 +1236,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
> +1248,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 +1270,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	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] UefiCpuPkg: ApicLib
  2017-07-06  1:53     ` Ni, Ruiyu
@ 2017-07-06 13:48       ` Duran, Leo
  0 siblings, 0 replies; 6+ messages in thread
From: Duran, Leo @ 2017-07-06 13:48 UTC (permalink / raw)
  To: 'Ni, Ruiyu', Fan, Jeff, edk2-devel@lists.01.org
  Cc: Justen, Jordan L, Dong, Eric, Gao, Liming

Hi Ray,

> -----Original Message-----
> From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com]
> Sent: Wednesday, July 05, 2017 8:54 PM
> To: Fan, Jeff <jeff.fan@intel.com>; Duran, Leo <leo.duran@amd.com>;
> edk2-devel@lists.01.org
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [PATCH v2] UefiCpuPkg: ApicLib
> 
> Leo,
> Could you please separate the clean-up code into a separate patch?
> 
[Duran, Leo] 
Sure thing, I will submit separate patches.

> Thanks/Ray
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Fan, Jeff
> > Sent: Thursday, July 6, 2017 9:28 AM
> > To: Leo Duran <leo.duran@amd.com>; edk2-devel@lists.01.org
> > Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Dong, Eric
> > <eric.dong@intel.com>; Gao, Liming <liming.gao@intel.com>
> > Subject: Re: [edk2] [PATCH v2] UefiCpuPkg: ApicLib
> >
> > Leo,
> >
> > How AMD public spec dos define the manner of sending startup IPI to Aps?
> > Which chapter is it defined in AMD public spec?
> >
> > Does AMD public spec indicate the second startup IPI is not required?
> >
> > Thanks!
> > Jeff
> >
> > -----Original Message-----
> > From: Leo Duran [mailto:leo.duran@amd.com]
> > Sent: Sunday, July 02, 2017 6:45 AM
> > To: edk2-devel@lists.01.org
> > Cc: Leo Duran; Justen, Jordan L; Fan, Jeff; Gao, Liming; Brijesh Singh
> > Subject: [PATCH v2] UefiCpuPkg: ApicLib
> >
> > 1) SendInitSipiSipi ()
> > Skip repeating SendIpi () on AMD processor.
> >
> > 2) SendInitSipiSipiAllExcludingSelf () Skip repeating SendIpi () on
> > AMD processor.
> >
> > 3) 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     | 52 +++++++++++++-
> ---
> > -----
> >  .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c        | 50 +++++++++++++-----
> --
> > -
> >  2 files changed, 63 insertions(+), 39 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> > b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> > index 2091e5e..a6e4e2e 100644
> > --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> > +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> > @@ -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] @@ -554,8
> > +554,10 @@ SendInitSipiSipi (
> >    IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
> >    IcrLow.Bits.Level = 1;
> >    SendIpi (IcrLow.Uint32, ApicId);
> > -  MicroSecondDelay (200);
> > -  SendIpi (IcrLow.Uint32, ApicId);
> > +  if (!StandardSignatureIsAuthenticAMD ()) {
> > +    MicroSecondDelay (200);
> > +    SendIpi (IcrLow.Uint32, ApicId);
> > +  }
> >  }
> >
> >  /**
> > @@ -588,8 +590,10 @@ SendInitSipiSipiAllExcludingSelf (
> >    IcrLow.Bits.Level = 1;
> >    IcrLow.Bits.DestinationShorthand =
> > LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
> >    SendIpi (IcrLow.Uint32, 0);
> > -  MicroSecondDelay (200);
> > -  SendIpi (IcrLow.Uint32, 0);
> > +  if (!StandardSignatureIsAuthenticAMD ()) {
> > +    MicroSecondDelay (200);
> > +    SendIpi (IcrLow.Uint32, 0);
> > +  }
> >  }
> >
> >  /**
> > @@ -1013,13 +1017,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 +1047,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
> > +1056,7 @@ GetProcessorLocationByApicId (
> >    //
> >    TopologyLeafSupported = FALSE;
> >    if (MaxStandardCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) {
> > -    AsmCpuidEx(
> > +    AsmCpuidEx (
> >        CPUID_EXTENDED_TOPOLOGY,
> >        0,
> >        &ExtendedTopologyEax.Uint32,
> > @@ -1072,7 +1077,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 +1086,7 @@ GetProcessorLocationByApicId (
> >        //
> >        SubIndex = 1;
> >        do {
> > -        AsmCpuidEx(
> > +        AsmCpuidEx (
> >            CPUID_EXTENDED_TOPOLOGY,
> >            SubIndex,
> >            &ExtendedTopologyEax.Uint32, @@ -1103,7 +1108,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 +1119,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 +1133,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 +1141,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
> > +1153,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 +1175,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..5945b55 100644
> > --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> > +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> > @@ -649,8 +649,10 @@ SendInitSipiSipi (
> >    IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP;
> >    IcrLow.Bits.Level = 1;
> >    SendIpi (IcrLow.Uint32, ApicId);
> > -  MicroSecondDelay (200);
> > -  SendIpi (IcrLow.Uint32, ApicId);
> > +  if (!StandardSignatureIsAuthenticAMD ()) {
> > +    MicroSecondDelay (200);
> > +    SendIpi (IcrLow.Uint32, ApicId);
> > +  }
> >  }
> >
> >  /**
> > @@ -683,8 +685,10 @@ SendInitSipiSipiAllExcludingSelf (
> >    IcrLow.Bits.Level = 1;
> >    IcrLow.Bits.DestinationShorthand =
> > LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF;
> >    SendIpi (IcrLow.Uint32, 0);
> > -  MicroSecondDelay (200);
> > -  SendIpi (IcrLow.Uint32, 0);
> > +  if (!StandardSignatureIsAuthenticAMD ()) {
> > +    MicroSecondDelay (200);
> > +    SendIpi (IcrLow.Uint32, 0);
> > +  }
> >  }
> >
> >  /**
> > @@ -1108,13 +1112,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 +1142,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
> > +1151,7 @@ GetProcessorLocationByApicId (
> >    //
> >    TopologyLeafSupported = FALSE;
> >    if (MaxStandardCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) {
> > -    AsmCpuidEx(
> > +    AsmCpuidEx (
> >        CPUID_EXTENDED_TOPOLOGY,
> >        0,
> >        &ExtendedTopologyEax.Uint32,
> > @@ -1167,7 +1172,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 +1181,7 @@ GetProcessorLocationByApicId (
> >        //
> >        SubIndex = 1;
> >        do {
> > -        AsmCpuidEx(
> > +        AsmCpuidEx (
> >            CPUID_EXTENDED_TOPOLOGY,
> >            SubIndex,
> >            &ExtendedTopologyEax.Uint32, @@ -1198,7 +1203,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 +1214,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 +1228,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 +1236,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
> > +1248,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 +1270,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] 6+ messages in thread

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

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-01 22:45 [PATCH v2] UefiCpuPkg: ApicLib Leo Duran
2017-07-01 22:45 ` Leo Duran
2017-07-06  1:27   ` Fan, Jeff
2017-07-06  1:53     ` Ni, Ruiyu
2017-07-06 13:48       ` Duran, Leo
2017-07-06 13:47     ` Duran, Leo

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