On Tue, Oct 19, 2021 at 09:07:39AM +0200, Jan Beulich wrote:
> From: Thomas Gleixner <t...@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 <k...@kernel.org>
> Signed-off-by: Thomas Gleixner <t...@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 <jbeul...@suse.com>
Acked-by: Roger Pau Monné <roger....@citrix.com>
Just one comment below.
> ---
> 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;
Should we print a message that HPET is being disabled?
Thanks, Roger.
