linux-kernel - Re: [PATCH V3] acpi: Allow ignoring _OSC CPPC v2 bit via kernel parameter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 19 Jun 2024 17:19:47 -0500
From: Aaron Rainbolt <arainbolt@...cus.org>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
	Mario Limonciello <mario.limonciello@....com>,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	lenb@...nel.org, mmikowski@...cus.org, Perry.Yuan@....com,
	Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Subject: Re: [PATCH V3] acpi: Allow ignoring _OSC CPPC v2 bit via kernel
 parameter

On Wed, Jun 19, 2024 at 04:39:39PM -0500, Aaron Rainbolt wrote:
> On Wed, Jun 19, 2024 at 07:30:55PM +0200, Rafael J. Wysocki wrote:
> > On Wednesday, June 19, 2024 7:09:35 PM CEST Rafael J. Wysocki wrote:
> > > On Wed, Jun 19, 2024 at 6:33 AM Aaron Rainbolt <arainbolt@...cus.org> wrote:
> > > >
> > > > acpi: Allow ignoring _OSC CPPC v2 bit via kernel parameter
> > > >
> > > > The _OSC is supposed to contain a bit indicating whether the hardware
> > > > supports CPPC v2 or not. This bit is not always set, causing CPPC v2 to
> > > > be considered absent. This results in severe single-core performance
> > > > issues with the EEVDF scheduler on heterogenous-core Intel processors.
> > > 
> > > While some things can be affected by this, I don't immediately see a
> > > connection between CPPC v2, Intel hybrid processors and EEVDF.
> > > 
> > > In particular, why would EEVDF alone be affected?
> > > 
> > > Care to explain this?
> > 
> > And the reason why I am asking is because I think that you really need
> > something like this (untested beyond compilation):
> 
> Alright, our team has compiled and tested the patch.
> 
> Results were mixed - with my patch, both CPPC and ITMT were enabled. This
> patch appears to enable only ITMT. (To be specific, running
> "find /proc /sys | grep '(cppc|itmt)'" shows only ITMT enabled under
> /proc, and no CPPC directories under /sys.) This causes a performance hit
> in benchmarking - using my patch with 'ignore_osc_cppc_bit', we were
> getting Geekbench 5 scores of at least 1907 single-core, and 10500
> multi-core. With this patch, we only are getting approximately 1700
> single-core, and less than 9000 multi-core. (With an entirely unpatched
> kernel, we were getting less than 1000 single-core, and about 10000
> multi-core.)
> 
> Ultimately this is an upgrade over unpatched performance, but a
> downgrade from the previous patch. It seems having CPPC and ITMT
> available at the same time made things work noticeably faster.
> 
> Is there some way that can get both CPPC and ITMT to work with an approach
> like this? Our hardware does support both, it just has an incorrectly set
> bit in _OSC.

I was premature in sending this - we reverted back to the previous patch
and are now getting similar scores to the new patch, meaning the benchmark
results may have had nothing to do with the new patch. We'll test further
and report back once we have conclusive results.
 
> > ---
> >  drivers/cpufreq/intel_pstate.c |   16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > Index: linux-pm/drivers/cpufreq/intel_pstate.c
> > ===================================================================
> > --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> > +++ linux-pm/drivers/cpufreq/intel_pstate.c
> > @@ -355,16 +355,16 @@ static void intel_pstate_set_itmt_prio(i
> >  	int ret;
> >  
> >  	ret = cppc_get_perf_caps(cpu, &cppc_perf);
> > -	if (ret)
> > -		return;
> > -
> >  	/*
> > -	 * On some systems with overclocking enabled, CPPC.highest_perf is hardcoded to 0xff.
> > -	 * In this case we can't use CPPC.highest_perf to enable ITMT.
> > -	 * In this case we can look at MSR_HWP_CAPABILITIES bits [8:0] to decide.
> > +	 * If CPPC is not available, fall back to MSR_HWP_CAPABILITIES bits [8:0].
> > +	 *
> > +	 * Also, on some systems with overclocking enabled, CPPC.highest_perf is
> > +	 * hardcoded to 0xff, so CPPC.highest_perf cannot be used to enable ITMT.
> > +	 * Fall back to MSR_HWP_CAPABILITIES then too.
> >  	 */
> > -	if (cppc_perf.highest_perf == CPPC_MAX_PERF)
> > -		cppc_perf.highest_perf = HWP_HIGHEST_PERF(READ_ONCE(all_cpu_data[cpu]->hwp_cap_cached));
> > +	if (ret || cppc_perf.highest_perf == CPPC_MAX_PERF)
> > +		cppc_perf.highest_perf =
> > +			HWP_HIGHEST_PERF(READ_ONCE(all_cpu_data[cpu]->hwp_cap_cached));
> >  
> >  	/*
> >  	 * The priorities can be set regardless of whether or not
> > 
> > 
> >