Re: [PATCH] x86/intel: Disable HPET on another Intel Coffee Lake platform

From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Bjorn Helgaas <helgaas@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	jose.souza@intel.com, "H. Peter Anvin" <hpa@zytor.com>,
	Borislav Petkov <bp@alien8.de>, Ingo Molnar <mingo@redhat.com>,
	Kai-Heng Feng <kai.heng.feng@canonical.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Linux PCI <linux-pci@vger.kernel.org>,
	rudolph@fb.com, xapienz@fb.com, bmilton@fb.com,
	Stable <stable@vger.kernel.org>,
	Arjan van de Ven <arjan@linux.intel.com>,
	Tom Lendacky <thomas.lendacky@amd.com>
Subject: Re: [PATCH] x86/intel: Disable HPET on another Intel Coffee Lake platform
Date: Thu, 23 Sep 2021 12:46:36 +0200	[thread overview]
Message-ID: <CAJZ5v0g2nNHSpckhx=KHpk1OAX0sHKpjh3hHmKbK9cPDQLH1vw@mail.gmail.com> (raw)
In-Reply-To: <87sfxwgsjy.ffs@tglx>

On Thu, Sep 23, 2021 at 12:21 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Wed, Sep 22 2021 at 22:27, Rafael J. Wysocki wrote:
> > On Tue, Sep 21, 2021 at 10:18 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> >>
> >> On Tue, Sep 21 2021 at 20:05, Rafael J. Wysocki wrote:
> >> > On 9/19/2021 2:14 AM, Thomas Gleixner wrote:
> >> >> What's the proper way to figure out whether PC10 is supported?
> >> >
> >> > I can't say without research.  I think it'd be sufficient to check if
> >> > C10 is supported, because asking for it is the only way to get PC10.
> >>
> >> Do we have a common function for that or do I need to implement the
> >> gazillionst CPUID query for that?
> >
> > intel_idle has intel_idle_verify_cstate() that works on MWAIT
> > substates from CPUID.  It looks like this could be reused.
>
> Not to me. That's some cpuidle/intel_idle specific check which depends
> on cpuidle_state_table

No, it doesn't.  The only thing this depends on is mwait_substates
which directly comes from the CPUID evaluation in intel_idle_init().
The argument is an MWAIT hint, but it doesn't have to be one, it could
be a state number.

Anyway, this is just part of what is needed.

So the way to check the PC10 support is to get the mask of MWAIT
substates from CPUID (like in intel_idle_init()) and check if there
are any substates for C10 in that mask and check if PC10 is enabled in
MSR_PKG_CST_CONFIG_CONTROL (like in sklh_idle_state_table_update()).

The MWAIT substates checking part could then be used by intel_idle
(and maybe by ACPI idle too).

> being set up which is not available during early boot.

> The question I was asking whether we have a central place where we can
> retrieve such information w/o invoking CPUID over and over again and
> applying voodoo checks on it.
>
> Obviously we don't, which sucks.

Well, nobody except for intel_idle wanted it, so there was not much
point doing it centrally for just one user.