Re: [PATCH] ACPI: CPPC: Gate desired performance getter on ACPI <6.6

Zhongqiu Han <[email protected]> Wed, 29 Jul 2026 12:03:16 +0800
Newsgroups gmane.linux.acpi.devel,gmane.linux.power-management.general,gmane.linux.kernel
Message-ID <[email protected]>
Hello Christian,
Thanks for the patch.

On 7/29/2026 12:05 AM, Christian Loehle wrote:
> When CPPC feedback counters cannot provide a usable sample, cppc-cpufreq
> calls cppc_get_desired_perf() because some platforms repurpose Desired
> Performance to report actual delivered performance.
> 
> ACPI 6.6 changed the register's Optional Attribute from Read/Write to
> Write. The existing cppc-cpufreq error path now falls back to its
> cached OSPM request on ACPI >=6.6.
> The original (and existing) workaround was for a specific HiSilicon
> platform which isn't expected to report >=6.6.
> 
> Fixes: c47195631960 ("cppc_cpufreq: Use desired perf if feedback ctrs are 0 or unchanged")

missing S-o-b? 🙂

> ---
>   drivers/acpi/cppc_acpi.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 53d09ca98f06..218fdea09c1c 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1321,10 +1321,17 @@ static int cppc_set_reg_val(int cpu, enum cppc_regs reg_idx, u64 val)
>    * @cpunum: CPU from which to get desired performance.
>    * @desired_perf: Return address.
>    *
> - * Return: 0 for success, -EIO otherwise.
> + * Return: 0 for success, -EOPNOTSUPP for ACPI 6.6 or later, and a negative
> + * errno otherwise.
>    */
>   int cppc_get_desired_perf(int cpunum, u64 *desired_perf)
>   {
> +	/* ACPI 6.6 no longer specifies Desired Performance as readable. */
> +	if (acpi_gbl_FADT.header.revision > 6 ||
> +	    (acpi_gbl_FADT.header.revision == 6 &&
> +	     acpi_gbl_FADT.minor_revision >= 6))
> +		return -EOPNOTSUPP;
> +

Just for context, a separate topic: it seems that cppc_get_perf() also
performs such a read. However, the value is subsequently overwritten in
cppc_cpufreq_cpu_init(). Since this all takes place during
initialization, it should be harmless. But It might be worth cleaning
this up as well, in case reading a write-only register leads to
potential undefined behavior?

>   	return cppc_get_reg_val(cpunum, DESIRED_PERF, desired_perf);
>   }
>   EXPORT_SYMBOL_GPL(cppc_get_desired_perf);


-- 
Thx and BRs,
Zhongqiu Han