On 19.10.2021 13:30, Roger Pau Monné wrote:
> 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>
Thanks.
>> @@ -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?
There is one, and it was even visible in patch context that you
did strip from your reply:
if ( !hpet_address )
printk("Disabling HPET for being unreliable\n");
Jan
