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

Mario Limonciello
2024-06-18T17:09:25Z

On 6/17/2024 21:54, 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.
>
> To work around this, provide a new kernel parameter,
> "processor.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.
>
> Tested-by: Michael Mikowski <mmikowski@kfocus.org>
> Signed-off-by: Aaron Rainbolt <arainbolt@kfocus.org>

This sounds like a platform bug and if we do accept a patch like this I
think we need a lot more documentation about the situation.

Can you please share more information about your hardware:
1) Manufacturer?
2) CPU?
3) Manufacturer firmware version?
4) If it's AMD what's the AGESA version?

And most importantly do you have the latest system firmware version from
your manufacturer? If not; please upgrade that first.

>
> ---
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 1d857978f5f4..53406dd6cb87 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -671,7 +671,7 @@ static inline void arch_init_invariance_cppc(void) { }
> *
> * Return: 0 for success or negative value for err.
> */
> -int acpi_cppc_processor_probe(struct acpi_processor *pr)
> +int acpi_cppc_processor_probe(struct acpi_processor *pr, bool ignore_osc_cppc_bit)
> {
> struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
> union acpi_object *out_obj, *cpc_obj;
> @@ -686,7 +686,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
>
> if (!osc_sb_cppc2_support_acked) {
> pr_debug("CPPC v2 _OSC not acked\n");
> - if (!cpc_supported_by_cpu()) {
> + if (!ignore_osc_cppc_bit && !cpc_supported_by_cpu()) {
> pr_debug("CPPC is not supported by the CPU\n");
> return -ENODEV;
> }
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 67db60eda370..a183bca6c1c5 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -33,6 +33,11 @@ MODULE_AUTHOR("Paul Diefenbaugh");
> MODULE_DESCRIPTION("ACPI Processor Driver");
> MODULE_LICENSE("GPL");
>
> +static bool ignore_osc_cppc_bit = false;
> +module_param(ignore_osc_cppc_bit, bool, 0);
> +MODULE_PARM_DESC(ignore_osc_cppc_bit,
> + "Ignore _OSC CPPC bit, assume CPPC v2 is present");
> +
> static int acpi_processor_start(struct device *dev);
> static int acpi_processor_stop(struct device *dev);
>
> @@ -170,7 +175,7 @@ static int __acpi_processor_start(struct acpi_device *device)
> if (pr->flags.need_hotplug_init)
> return 0;
>
> - result = acpi_cppc_processor_probe(pr);
> + result = acpi_cppc_processor_probe(pr, ignore_osc_cppc_bit);
> if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
> dev_dbg(&device->dev, "CPPC data invalid or not present\n");
>
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 3f34ebb27525..79fd61b3f537 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -358,10 +358,10 @@ int acpi_map_cpuid(phys_cpuid_t phys_id, u32 acpi_id);
> int acpi_get_cpuid(acpi_handle, int type, u32 acpi_id);
>
> #ifdef CONFIG_ACPI_CPPC_LIB
> -extern int acpi_cppc_processor_probe(struct acpi_processor *pr);
> +extern int acpi_cppc_processor_probe(struct acpi_processor *pr, bool ignore_osc_cppc_bit);
> extern void acpi_cppc_processor_exit(struct acpi_processor *pr);
> #else
> -static inline int acpi_cppc_processor_probe(struct acpi_processor *pr)
> +static inline int acpi_cppc_processor_probe(struct acpi_processor *pr, bool ignore_osc_cppc_bit)
> {
> return 0;
> }