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

[Mario Limonciello <mario.limonciello@amd.com>]

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;
>   }