Fix support for CPUS with Always Running Timers (ART)

Previously, OcCpuLib assumed all CPUs had the same core crystal clock frequency for their ART (24 Mhz). Xeon Scalable and Intel Atoms with Goldmont architecture have different frequencies (25 Mhz and 19.2 Mhz respectively). If the CPU supports reporting its frequency, use that. Otherwise, fallback to a value based on its model and family.

Cleaned up the logic for how we compute our CPU and FSB frequencies. For CPUs that support ART, use that instead of the TSC frequency.

Added documentation and renamed `OC_CPU_INFO::TSCFrequency` to `CPUFrequencyFromTSC` and added `CPUFrequencyFromART` for clarity. These are intermediate values used to compute `CPUFrequency`. `ARTFrequency` is now correctly set to the core crystal clock frequency rather than `CPUFrequencyFromART`, which it was previously.

Lastly, adjusted the `CPUFrequencyFromART` calculation to include the TSC offset in `MSR_IA32_TSC_ADJUST` if present. I'm not aware of which CPUs have a non-zero offset but it's part of Intel's calculation for computing the CPU frequency based on the ART.

Include/Library/OcCpuLib.h

@@ -67,9 +67,46 @@ typedef struct {
   UINT16                  CoreCount;
   UINT16                  ThreadCount;
 
+  //
+  // Platform-dependent frequency for the Always Running Timer (ART), normally
+  // 24Mhz. Firmwares may choose to override this. Some CPUs like Xeon Scalable
+  // use a different frequency. CPUs report the frequency through CPUID.15H.ECX.
+  // If unreported, the frequency is looked up based on the model and family.
+  //
+  // Nominal Core Crystal Clock Frequency for known processor families:
+  //   Intel Xeon Scalable with CPUID signature 0x0655: 25 Mhz
+  //   6th and 7th generation Intel Core & Xeon W: 24 Mhz
+  //   Nex Generation Intel Atom with CPUID 0x065C: 19.2 Mhz
+  //
   UINT64                  ARTFrequency;
-  UINT64                  TSCFrequency;
+
+  //
+  // The CPU frequency derived from either CPUFrequencyFromTSC (legacy) or
+  // CPUFrequencyFromART (preferred for Skylake and presumably newer processors
+  // that have an Always Running Timer).
+  //
+  // CPUFrequencyFromTSC should approximate equal CPUFrequencyFromART. If not,
+  // there is likely a bug or miscalculation.
+  //
   UINT64                  CPUFrequency;
+
+  //
+  // The CPU frequency as reported by the Time Stamp Counter (TSC).
+  //
+  UINT64                  CPUFrequencyFromTSC;
+
+  //
+  // The CPU frequency derived from the Always Running Timer (ART) frequency:
+  //   TSC_Value = (ART_Value * CPUID.15H:EBX[31:0]) / CPUID.15H:EAX[31:0] + K
+  //
+  // 0 if ART is not present.
+  //
+  UINT64                  CPUFrequencyFromART;
+
+  //
+  // The Front Side Bus (FSB) frequency calculated from dividing the CPU
+  // frequency by the Max Ratio.
+  //
   UINT64                  FSBFrequency;
 } OC_CPU_INFO;
 

Library/OcCpuLib/OcCpuLib.c

@@ -457,7 +457,7 @@ DetectAppleProcessorType (
     //
     case CPU_MODEL_SKYLAKE:     // 0x4E
     case CPU_MODEL_SKYLAKE_DT:  // 0x5E
-    case CPU_MODEL_SKYLAKE_W:   // 0x55, also SKL-X
+    case CPU_MODEL_SKYLAKE_W:   // 0x55, also SKL-X and SKL-SP
       if (AppleMajorType == AppleProcessorMajorXeonW) {
         // IMP11 (Xeon W 2140B)
         return AppleProcessorTypeXeonW;       // 0x0F01
@@ -588,7 +588,9 @@ ScanIntelProcessor (
 {
   UINT32                                            CpuidEax;
   UINT32                                            CpuidEbx;
+  UINT32                                            CpuidEcx;
   UINT64                                            Msr;
+  UINT64                                            TscAdjust;
   CPUID_CACHE_PARAMS_EAX                            CpuidCacheEax;
   CPUID_CACHE_PARAMS_EBX                            CpuidCacheEbx;
   UINT8                                             AppleMajorType;
@@ -661,46 +663,70 @@ ScanIntelProcessor (
   // SkyLake and later have an Always Running Timer
   //
   if (Cpu->Model >= CPU_MODEL_SKYLAKE) {
-    AsmCpuid (CPUID_TIME_STAMP_COUNTER, &CpuidEax, &CpuidEbx, NULL, NULL);
+    AsmCpuid (CPUID_TIME_STAMP_COUNTER, &CpuidEax, &CpuidEbx, &CpuidEcx, NULL);
+    if (CpuidEcx > 0) {
+      Cpu->ARTFrequency = CpuidEcx;
+      DEBUG ((DEBUG_INFO, "OCCPU: Core Crystal Clock Frequency %u\n", CpuidEcx));
+    } else {
+      //
+      // Fall back to identifying ART frequency based on model
+      //
+      if (Cpu->Family == 0x6 && Cpu->Model == CPU_MODEL_SKYLAKE_W) {
+        //
+        // TODO: Xeon Ws share the same CPUID but I believe they have 24 Mhz ART
+        //
+        Cpu->ARTFrequency = 25000000ULL; // 25 Mhz
+      } else if (Cpu->Family == 0x6 && Cpu->Model == CPU_MODEL_GOLDMONT){
+        Cpu->ARTFrequency = 19200000ULL; // 19.2 Mhz
+      } else {
+        Cpu->ARTFrequency = 24000000ULL; // 24 Mhz
+      }
+    }
 
     if (CpuidEax > 0 && CpuidEbx > 0) {
-      Cpu->CPUFrequency = MultU64x32 (BASE_ART_CLOCK_SOURCE, (UINT32) DivU64x32 (CpuidEbx, CpuidEax));
+      TscAdjust = AsmReadMsr64 (MSR_IA32_TSC_ADJUST);
+      DEBUG ((DEBUG_INFO, "OCCPU: TSC Adjust %llu\n", TscAdjust ));
+      ASSERT (Cpu->ARTFrequency > 0ULL);
+      Cpu->CPUFrequencyFromART = MultU64x32 (Cpu->ARTFrequency, (UINT32) DivU64x32 (CpuidEbx, CpuidEax)) + TscAdjust;
 
       DEBUG ((
         DEBUG_INFO,
-        "OCCPU: %a %a %11lld %5dMHz %u * %u / %u = %ld\n",
+        "OCCPU: %a %a %11llu %5dMHz = %u * %u / %u + %u\n",
         "ART",
         "Frequency",
-        Cpu->CPUFrequency,
-        DivU64x32 (Cpu->CPUFrequency, 1000000),
-        BASE_ART_CLOCK_SOURCE,
+        Cpu->CPUFrequencyFromART,
+        DivU64x32 (Cpu->CPUFrequencyFromART, 1000000),
+        Cpu->ARTFrequency,
         CpuidEbx,
         CpuidEax,
-        Cpu->CPUFrequency
+        TscAdjust
         ));
-
-      Cpu->FSBFrequency = DivU64x32 (Cpu->CPUFrequency, Cpu->MaxBusRatio);
     }
   }
 
   //
   // Calculate the Tsc frequency
   //
-  Cpu->TSCFrequency = GetPerformanceCounterProperties (NULL, NULL);
+  Cpu->CPUFrequencyFromTSC = GetPerformanceCounterProperties (NULL, NULL);
 
-  if (Cpu->CPUFrequency == 0) {
+  //
+  // Calculate CPU frequency based on ART if present, otherwise TSC
+  //
+  Cpu->CPUFrequency = Cpu->CPUFrequencyFromART > 0 ? Cpu->CPUFrequencyFromART : Cpu->CPUFrequencyFromTSC;
+
+  //
+  // There may be some quirks with virtual CPUs (VMware is fine).
+  // Formerly we checked Cpu->MinBusRatio > 0, but we have no MinBusRatio on Penryn.
+  //
+  if (Cpu->CPUFrequency > 0 && Cpu->MaxBusRatio > Cpu->MinBusRatio) {
+    Cpu->FSBFrequency = DivU64x32 (Cpu->CPUFrequency, Cpu->MaxBusRatio);
+  } else {
     //
-    // There may be some quirks with virtual CPUs (VMware is fine).
-    // Formerly we checked Cpu->MinBusRatio > 0, but we have no MinBusRatio on Penryn.
+    // TODO: It seems to be possible that CPU frequency == 0 here...
     //
-    if (Cpu->TSCFrequency > 0 && Cpu->MaxBusRatio > Cpu->MinBusRatio) {
-      Cpu->FSBFrequency = DivU64x32 (Cpu->TSCFrequency, Cpu->MaxBusRatio);
-      Cpu->CPUFrequency = MultU64x32 (Cpu->FSBFrequency, Cpu->MaxBusRatio);
-    } else {
-      Cpu->CPUFrequency = Cpu->TSCFrequency;
-      Cpu->FSBFrequency = 100000000;
-    }
+    Cpu->FSBFrequency = 100000000; // 100 Mhz
   }
+
   //
   // Calculate number of cores
   //
@@ -764,8 +790,8 @@ ScanAmdProcessor (
   //
   // get TSC Frequency calculated in OcTimerLib
   //
-  Cpu->TSCFrequency = GetPerformanceCounterProperties (NULL, NULL);
-  Cpu->CPUFrequency = Cpu->TSCFrequency;
+  Cpu->CPUFrequencyFromTSC = GetPerformanceCounterProperties (NULL, NULL);
+  Cpu->CPUFrequency = Cpu->CPUFrequencyFromTSC;
   //
   // Get core and thread count from CPUID
   //
@@ -815,7 +841,7 @@ ScanAmdProcessor (
     Cpu->CurBusRatio = Cpu->MaxBusRatio;
     Cpu->MinBusRatio = Cpu->MaxBusRatio;
 
-    Cpu->FSBFrequency = DivU64x32 (Cpu->TSCFrequency, Cpu->MaxBusRatio);
+    Cpu->FSBFrequency = DivU64x32 (Cpu->CPUFrequency, Cpu->MaxBusRatio);
   }
 }
 
@@ -956,8 +982,8 @@ OcCpuScanProcessor (
     "OCCPU: %a %a %11lld %5dMHz\n",
     "TSC",
     "Frequency",
-    Cpu->TSCFrequency,
-    DivU64x32 (Cpu->TSCFrequency, 1000000)
+    Cpu->CPUFrequencyFromTSC,
+    DivU64x32 (Cpu->CPUFrequencyFromTSC, 1000000)
     ));
 
   DEBUG ((