linux-kernel - Re: [PATCH V2 RFC] 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: Tue, 18 Jun 2024 16:47:11 -0500
From: Aaron Rainbolt <arainbolt@...cus.org>
To: Mario Limonciello <mario.limonciello@....com>,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: rafael@...nel.org, lenb@...nel.org, mmikowski@...cus.org,
	Perry.Yuan@....com
Subject: Re: [PATCH V2 RFC] acpi: Allow ignoring _OSC CPPC v2 bit via kernel
 parameter

On Tue, Jun 18, 2024 at 04:24:22PM -0500, Mario Limonciello wrote:
> On 6/18/2024 15:25, Aaron Rainbolt 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.
> > 
> > To work around this, provide a new kernel parameter, "ignore_osc_cppc_bit",
> > which may be used to ignore the _OSC CPPC v2 bit and act as if the bit was
> > enabled. This allows CPPC to be properly detected even if not "enabled" by
> > _OSC, allowing users with problematic hardware to obtain decent single-core
> > performance.
> > 
> > Signed-off-by: Aaron Rainbolt <arainbolt@...cus.org>
> > 
> > ---
> > 
> > V1 -> V2: Rewrite to work in cpc_supported_by_cpu.
> > 
> > RFC: I have not yet tested this patch to ensure it functions properly,
> >   nor have I attempted to compile it against mainline. My system takes
> >   a couple of hours or so to build a kernel, and I'd like to submit this
> >   for feedback now and test once it's sent.
> 
> Thanks, this matches what I suggested, hopefully it works when you test it.
> 
> One comment below though.
> 
> > 
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index b600df82669d..af2d8973ba3a 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -2063,6 +2063,12 @@
> >   			could change it dynamically, usually by
> >   			/sys/module/printk/parameters/ignore_loglevel.
> > +	ignore_osc_cppc_bit
> > +			Assume CPPC is present and ignore the CPPC v2 bit from
> > +			the ACPI _OSC method. This is useful for working
> > +			around buggy firmware where CPPC is supported, but
> > +			_OSC incorrectly reports it as being absent.
> > +
> >   	ignore_rlimit_data
> >   			Ignore RLIMIT_DATA setting for data mappings,
> >   			print warning at first misuse.  Can be changed via
> > diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c
> > index ff8f25faca3d..7346a25e68ce 100644
> > --- a/arch/x86/kernel/acpi/cppc.c
> > +++ b/arch/x86/kernel/acpi/cppc.c
> > @@ -11,6 +11,14 @@
> >   /* Refer to drivers/acpi/cppc_acpi.c for the description of functions */
> > +static bool ignore_osc_cppc_bit;
> > +static int __init parse_ignore_osc_cppc_bit(char *arg)
> > +{
> > +	ignore_osc_cppc_bit = true;
> > +	return 0;
> > +}
> > +early_param("ignore_osc_cppc_bit", parse_ignore_osc_cppc_bit);
> > +
> >   bool cpc_supported_by_cpu(void)
> >   {
> >   	switch (boot_cpu_data.x86_vendor) {
> > @@ -24,6 +32,10 @@ bool cpc_supported_by_cpu(void)
> >   			return true;
> >   		return boot_cpu_has(X86_FEATURE_CPPC);
> >   	}
> > +
> > +	if (ignore_osc_cppc_bit) {
> > +		return true;
> > +	}
> 
> I think you should move this check before the switch statement.
> The reason is that such a workaround could then apply to any CPU
> vendors and models that are AMD or Hygon too.

Oh good catch, I thought it would apply to everyone but missed an extra
'return' in the switch statement. I'll make sure to fix that in v3.

> >   	return false;
> >   }
>