All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>
Subject: [PATCH 1/2] x86/hpet: Use another crystalball to evaluate HPET usability
Date: Tue, 19 Oct 2021 09:07:39 +0200	[thread overview]
Message-ID: <da80b8dd-177c-d27a-9a00-c9538a139d37@suse.com> (raw)
In-Reply-To: <9963c7a2-f880-66fc-8f12-b1ddd0619c91@suse.com>

From: Thomas Gleixner <tglx@linutronix.de>

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@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
[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.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Fully different replacement of "x86: avoid HPET use also on certain
    Coffee Lake H".

--- 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 p
             }
 
         /*
-         * 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");
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -1308,3 +1308,20 @@ int __init mwait_idle_init(struct notifi
 
 	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);
+}
--- 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__ */



  reply	other threads:[~2021-10-19  7:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-19  7:06 [PATCH 0/2] x86: HPET PC10 quirk workaround and some cleanup Jan Beulich
2021-10-19  7:07 ` Jan Beulich [this message]
2021-10-19 11:30   ` [PATCH 1/2] x86/hpet: Use another crystalball to evaluate HPET usability Roger Pau Monné
2021-10-19 12:08     ` Jan Beulich
2021-10-19 12:45       ` Roger Pau Monné
2021-10-25 22:53   ` [REGRESSION] " Andrew Cooper
2021-10-26 15:12     ` [PATCH] x86/hpet: setup HPET even when disabled due to stopping in deep C states Roger Pau Monne
2021-10-27  0:08       ` Andrew Cooper
2021-10-27  9:49       ` Ian Jackson
2021-11-02 12:26       ` Jan Beulich
2021-10-19  7:08 ` [PATCH 2/2] x86: de-duplicate MONITOR/MWAIT CPUID-related definitions Jan Beulich
2021-10-19 11:36   ` Roger Pau Monné
2021-10-27  9:49     ` Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=da80b8dd-177c-d27a-9a00-c9538a139d37@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.