MIN_PERF and MAX_PERF are optional CPPC controls. The cppc-cpufreq
target callbacks nevertheless populate both values for every request
without checking whether the controls are implemented.

cppc_set_perf() consequently passes a NULL register descriptor to
cpc_write(). The write fails width validation and its return value is
ignored, so the failed access path is repeated on every target request.

Check that each optional limit control is supported before calling
cpc_write().

Fixes: ea3db45ae476 ("cpufreq: cppc: Update MIN_PERF/MAX_PERF in target callbacks")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 drivers/acpi/cppc_acpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 1d3a94100491..7bac6d5da24b 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1970,9 +1970,9 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 	 * value to min and max perf, but they don't mean to set the zero value,
 	 * they just don't want to write to those registers.
 	 */
-	if (perf_ctrls->min_perf)
+	if (perf_ctrls->min_perf && CPC_SUPPORTED(min_perf_reg))
 		cpc_write(cpu, min_perf_reg, perf_ctrls->min_perf);
-	if (perf_ctrls->max_perf)
+	if (perf_ctrls->max_perf && CPC_SUPPORTED(max_perf_reg))
 		cpc_write(cpu, max_perf_reg, perf_ctrls->max_perf);
 
 	if (CPC_IN_PCC(desired_reg) || CPC_IN_PCC(min_perf_reg) || CPC_IN_PCC(max_perf_reg))
-- 
2.34.1

On 24/07/26 03:26, Christian Loehle wrote:
> External email: Use caution opening links or attachments
>
>
> MIN_PERF and MAX_PERF are optional CPPC controls. The cppc-cpufreq
> target callbacks nevertheless populate both values for every request
> without checking whether the controls are implemented.
>
> cppc_set_perf() consequently passes a NULL register descriptor to
> cpc_write(). The write fails width validation and its return value is
> ignored, so the failed access path is repeated on every target request.
>
> Check that each optional limit control is supported before calling
> cpc_write().

Looks good.

DESIRED_PERF is also optional when CPPC2 and autonomous selection are
enabled. Should its write also be guarded with CPC_SUPPORTED()?

Reviewed-by: Sumit Gupta <sumitg@nvidia.com>

>
> Fixes: ea3db45ae476 ("cpufreq: cppc: Update MIN_PERF/MAX_PERF in target callbacks")
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
>   drivers/acpi/cppc_acpi.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 1d3a94100491..7bac6d5da24b 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1970,9 +1970,9 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>           * value to min and max perf, but they don't mean to set the zero value,
>           * they just don't want to write to those registers.
>           */
> -       if (perf_ctrls->min_perf)
> +       if (perf_ctrls->min_perf && CPC_SUPPORTED(min_perf_reg))
>                  cpc_write(cpu, min_perf_reg, perf_ctrls->min_perf);
> -       if (perf_ctrls->max_perf)
> +       if (perf_ctrls->max_perf && CPC_SUPPORTED(max_perf_reg))
>                  cpc_write(cpu, max_perf_reg, perf_ctrls->max_perf);
>
>          if (CPC_IN_PCC(desired_reg) || CPC_IN_PCC(min_perf_reg) || CPC_IN_PCC(max_perf_reg))
> --
> 2.34.1
>