[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen master] x86/hpet: Use another crystalball to evaluate HPET usability



commit c12731493ae39640c4e44d9fe2029c3165f9f429
Author:     Thomas Gleixner <tglx@xxxxxxxxxxxxx>
AuthorDate: Wed Oct 20 12:50:15 2021 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Wed Oct 20 12:50:15 2021 +0200

    x86/hpet: Use another crystalball to evaluate HPET usability
    
    On recent Intel systems the HPET stops working when the system reaches PC10
    idle state.
    
    The approach of adding PCI ids to the early quirks to disable HPET on
    these systems is a whack a mole game which makes no sense.
    
    Check for PC10 instead and force disable HPET if supported. The check is
    overbroad as it does not take ACPI, mwait-idle enablement and command
    line parameters into account. That's fine as long as there is at least
    PMTIMER available to calibrate the TSC frequency. The decision can be
    overruled by adding "clocksource=hpet" on the Xen command line.
    
    Remove the related PCI quirks for affected Coffee Lake systems as they
    are not longer required. That should also cover all other systems, i.e.
    Ice Lake, Tiger Lake, and newer generations, which are most likely
    affected by this as well.
    
    Fixes: Yet another hardware trainwreck
    Reported-by: Jakub Kicinski <kuba@xxxxxxxxxx>
    Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
    [Linux commit: 6e3cd95234dc1eda488f4f487c281bac8fef4d9b]
    
    I have to admit that the purpose of checking CPUID5_ECX_INTERRUPT_BREAK
    is unclear to me, but I didn't want to diverge in technical aspects from
    the Linux commit.
    
    In mwait_pc10_supported(), besides some cosmetic adjustments, avoid UB
    from shifting left a signed 4-bit constant by 28 bits.
    
    Pull in Linux'es MSR_PKG_CST_CONFIG_CONTROL.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 xen/arch/x86/cpu/mwait-idle.c   | 23 ++++++++++++++++++---
 xen/arch/x86/time.c             | 44 ++++++++++++++++++++++++++++++++++-------
 xen/include/asm-x86/msr-index.h | 12 ++++++-----
 xen/include/asm-x86/mwait.h     |  3 +++
 4 files changed, 67 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c
index f0c6ff9d52..d1739f6fc3 100644
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -840,9 +840,9 @@ static void auto_demotion_disable(void *dummy)
 {
        u64 msr_bits;
 
-       rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
+       rdmsrl(MSR_PKG_CST_CONFIG_CONTROL, msr_bits);
        msr_bits &= ~(icpu->auto_demotion_disable_flags);
-       wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
+       wrmsrl(MSR_PKG_CST_CONFIG_CONTROL, msr_bits);
 }
 
 static void byt_auto_demotion_disable(void *dummy)
@@ -1105,7 +1105,7 @@ static void __init sklh_idle_state_table_update(void)
        if ((mwait_substates & (MWAIT_CSTATE_MASK << 28)) == 0)
                return;
 
-       rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr);
+       rdmsrl(MSR_PKG_CST_CONFIG_CONTROL, msr);
 
        /* PC10 is not enabled in PKG C-state limit */
        if ((msr & 0xF) != 8)
@@ -1308,3 +1308,20 @@ int __init mwait_idle_init(struct notifier_block *nfb)
 
        return err;
 }
+
+/* Helper function for HPET. */
+bool __init mwait_pc10_supported(void)
+{
+       unsigned int ecx, edx, dummy;
+
+       if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+           !cpu_has_monitor ||
+           boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF)
+               return false;
+
+       cpuid(CPUID_MWAIT_LEAF, &dummy, &dummy, &ecx, &edx);
+
+       return (ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED) &&
+              (ecx & CPUID5_ECX_INTERRUPT_BREAK) &&
+              (edx >> 28);
+}
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 10418cdc54..cbadaa036f 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -34,6 +34,7 @@
 #include <asm/fixmap.h>
 #include <asm/guest.h>
 #include <asm/mc146818rtc.h>
+#include <asm/mwait.h>
 #include <asm/div64.h>
 #include <asm/acpi.h>
 #include <asm/hpet.h>
@@ -395,14 +396,43 @@ static int64_t __init init_hpet(struct 
platform_timesource *pts)
             }
 
         /*
-         * Some Coffee Lake platforms have a skewed HPET timer once the SoCs
-         * entered PC10.
+         * Some Coffee Lake and later platforms have a skewed HPET timer once
+         * they entered PC10.
+         *
+         * Check whether the system supports PC10. If so force disable HPET as
+         * that stops counting in PC10. This check is overbroad as it does not
+         * take any of the following into account:
+         *
+         *     - ACPI tables
+         *     - Enablement of mwait-idle
+         *     - Command line arguments which limit mwait-idle C-state support
+         *
+         * That's perfectly fine. HPET is a piece of hardware designed by
+         * committee and the only reasons why it is still in use on modern
+         * systems is the fact that it is impossible to reliably query TSC and
+         * CPU frequency via CPUID or firmware.
+         *
+         * If HPET is functional it is useful for calibrating TSC, but this can
+         * be done via PMTIMER as well which seems to be the last remaining
+         * timer on X86/INTEL platforms that has not been completely wreckaged
+         * by feature creep.
+         *
+         * In theory HPET support should be removed altogether, but there are
+         * older systems out there which depend on it because TSC and APIC 
timer
+         * are dysfunctional in deeper C-states.
          */
-        if ( pci_conf_read16(PCI_SBDF(0, 0, 0, 0),
-                             PCI_VENDOR_ID) == PCI_VENDOR_ID_INTEL &&
-             pci_conf_read16(PCI_SBDF(0, 0, 0, 0),
-                             PCI_DEVICE_ID) == 0x3ec4 )
-            hpet_address = 0;
+        if ( mwait_pc10_supported() )
+        {
+            uint64_t pcfg;
+
+            rdmsrl(MSR_PKG_CST_CONFIG_CONTROL, pcfg);
+            if ( (pcfg & 0xf) < 8 )
+                /* nothing */;
+            else if ( !strcmp(opt_clocksource, pts->id) )
+                printk("HPET use requested via command line, but dysfunctional 
in PC10\n");
+            else
+                hpet_address = 0;
+        }
 
         if ( !hpet_address )
             printk("Disabling HPET for being unreliable\n");
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index 90c0589cd6..ab68ef2681 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -46,6 +46,13 @@
 #define MSR_CORE_CAPABILITIES               0x000000cf
 #define  CORE_CAPS_SPLITLOCK_DETECT         (_AC(1, ULL) <<  5)
 
+#define MSR_PKG_CST_CONFIG_CONTROL          0x000000e2
+#define  NHM_C3_AUTO_DEMOTE                 (_AC(1, ULL) << 25)
+#define  NHM_C1_AUTO_DEMOTE                 (_AC(1, ULL) << 26)
+#define  ATM_LNC_C6_AUTO_DEMOTE             (_AC(1, ULL) << 25)
+#define  SNB_C3_AUTO_UNDEMOTE               (_AC(1, ULL) << 27)
+#define  SNB_C1_AUTO_UNDEMOTE               (_AC(1, ULL) << 28)
+
 #define MSR_ARCH_CAPABILITIES               0x0000010a
 #define  ARCH_CAPS_RDCL_NO                  (_AC(1, ULL) <<  0)
 #define  ARCH_CAPS_IBRS_ALL                 (_AC(1, ULL) <<  1)
@@ -179,11 +186,6 @@
 #define MSR_IA32_A_PERFCTR0            0x000004c1
 #define MSR_FSB_FREQ                   0x000000cd
 
-#define MSR_NHM_SNB_PKG_CST_CFG_CTL    0x000000e2
-#define NHM_C3_AUTO_DEMOTE             (1UL << 25)
-#define NHM_C1_AUTO_DEMOTE             (1UL << 26)
-#define ATM_LNC_C6_AUTO_DEMOTE         (1UL << 25)
-
 #define MSR_MTRRcap                    0x000000fe
 #define MTRRcap_VCNT                   0x000000ff
 
diff --git a/xen/include/asm-x86/mwait.h b/xen/include/asm-x86/mwait.h
index ba9c0ea096..f377d9fdca 100644
--- a/xen/include/asm-x86/mwait.h
+++ b/xen/include/asm-x86/mwait.h
@@ -1,6 +1,8 @@
 #ifndef __ASM_X86_MWAIT_H__
 #define __ASM_X86_MWAIT_H__
 
+#include <xen/types.h>
+
 #define MWAIT_SUBSTATE_MASK            0xf
 #define MWAIT_CSTATE_MASK              0xf
 #define MWAIT_SUBSTATE_SIZE            4
@@ -12,5 +14,6 @@
 #define MWAIT_ECX_INTERRUPT_BREAK      0x1
 
 void mwait_idle_with_hints(unsigned int eax, unsigned int ecx);
+bool mwait_pc10_supported(void);
 
 #endif /* __ASM_X86_MWAIT_H__ */
--
generated by git-patchbot for /home/xen/git/xen.git#master



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.