mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ACPI: CPPC: Skip writes to unsupported performance controls
@ 2026-07-24 10:40 Christian Loehle
  2026-07-24 10:57 ` Sudeep Holla
  2026-07-27 11:14 ` zhenglifeng (A)
  0 siblings, 2 replies; 6+ messages in thread
From: Christian Loehle @ 2026-07-24 10:40 UTC (permalink / raw)
  To: Rafael J . Wysocki, Viresh Kumar
  Cc: linux-pm, linux-acpi, linux-kernel, Len Brown, Jie Zhan,
	Lifeng Zheng, Pierre Gondois, Sumit Gupta, Sudeep Holla,
	Ionela Voinescu, Christian Loehle

MIN_PERF and MAX_PERF are optional CPPC controls. DESIRED_PERF is also
optional with CPPC2 when autonomous selection is supported.

The cppc-cpufreq target callbacks populate both limits for every request
without checking whether the controls are implemented. cppc_set_perf()
consequently passes NULL register descriptors to cpc_write(). The writes
fail width validation and their return values are ignored, so the failed
access paths are repeated on every target request. An autonomous-only
platform can take the same path for DESIRED_PERF.

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

Fixes: ea3db45ae476 ("cpufreq: cppc: Update MIN_PERF/MAX_PERF in target callbacks")
Reviewed-by: Sumit Gupta <sumitg@nvidia.com>
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
v2: Also added desired_perf check (Sumit)

 drivers/acpi/cppc_acpi.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 1d3a94100491..53d09ca98f06 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1963,16 +1963,17 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 		cpc_desc->write_cmd_status = 0;
 	}
 
-	cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
+	if (CPC_SUPPORTED(desired_reg))
+		cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
 
 	/*
 	 * Only write if min_perf and max_perf not zero. Some drivers pass zero
 	 * 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI: CPPC: Skip writes to unsupported performance controls
  2026-07-24 10:40 [PATCH] ACPI: CPPC: Skip writes to unsupported performance controls Christian Loehle
@ 2026-07-24 10:57 ` Sudeep Holla
  2026-07-24 12:01   ` Christian Loehle
  2026-07-27 11:14 ` zhenglifeng (A)
  1 sibling, 1 reply; 6+ messages in thread
From: Sudeep Holla @ 2026-07-24 10:57 UTC (permalink / raw)
  To: Christian Loehle
  Cc: Rafael J . Wysocki, Viresh Kumar, linux-pm, linux-acpi,
	linux-kernel, Len Brown, Jie Zhan, Lifeng Zheng, Pierre Gondois,
	Sumit Gupta, Ionela Voinescu

On Fri, Jul 24, 2026 at 11:40:42AM +0100, Christian Loehle wrote:
> MIN_PERF and MAX_PERF are optional CPPC controls. DESIRED_PERF is also
> optional with CPPC2 when autonomous selection is supported.
> 
> The cppc-cpufreq target callbacks populate both limits for every request
> without checking whether the controls are implemented. cppc_set_perf()
> consequently passes NULL register descriptors to cpc_write(). The writes
> fail width validation and their return values are ignored, so the failed
> access paths are repeated on every target request. An autonomous-only
> platform can take the same path for DESIRED_PERF.
> 
> Check that each performance control is supported before calling
> cpc_write().
> 
> Fixes: ea3db45ae476 ("cpufreq: cppc: Update MIN_PERF/MAX_PERF in target callbacks")
> Reviewed-by: Sumit Gupta <sumitg@nvidia.com>
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
> v2: Also added desired_perf check (Sumit)
> 
>  drivers/acpi/cppc_acpi.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 1d3a94100491..53d09ca98f06 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1963,16 +1963,17 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>  		cpc_desc->write_cmd_status = 0;
>  	}
>  
> -	cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
> +	if (CPC_SUPPORTED(desired_reg))
> +		cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
>  

In general, can't be set perf_ctrls->desired_perf if and only if
CPC_SUPPORTED(desired_reg) ?

I see cppc_get_perf() does that but I assume you are catering for some
other access that may happen before ?

-- 
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI: CPPC: Skip writes to unsupported performance controls
  2026-07-24 10:57 ` Sudeep Holla
@ 2026-07-24 12:01   ` Christian Loehle
  0 siblings, 0 replies; 6+ messages in thread
From: Christian Loehle @ 2026-07-24 12:01 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Rafael J . Wysocki, Viresh Kumar, linux-pm, linux-acpi,
	linux-kernel, Len Brown, Jie Zhan, Lifeng Zheng, Pierre Gondois,
	Sumit Gupta, Ionela Voinescu

On 7/24/26 11:57, Sudeep Holla wrote:
> On Fri, Jul 24, 2026 at 11:40:42AM +0100, Christian Loehle wrote:
>> MIN_PERF and MAX_PERF are optional CPPC controls. DESIRED_PERF is also
>> optional with CPPC2 when autonomous selection is supported.
>>
>> The cppc-cpufreq target callbacks populate both limits for every request
>> without checking whether the controls are implemented. cppc_set_perf()
>> consequently passes NULL register descriptors to cpc_write(). The writes
>> fail width validation and their return values are ignored, so the failed
>> access paths are repeated on every target request. An autonomous-only
>> platform can take the same path for DESIRED_PERF.
>>
>> Check that each performance control is supported before calling
>> cpc_write().
>>
>> Fixes: ea3db45ae476 ("cpufreq: cppc: Update MIN_PERF/MAX_PERF in target callbacks")
>> Reviewed-by: Sumit Gupta <sumitg@nvidia.com>
>> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
>> ---
>> v2: Also added desired_perf check (Sumit)
>>
>>  drivers/acpi/cppc_acpi.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>> index 1d3a94100491..53d09ca98f06 100644
>> --- a/drivers/acpi/cppc_acpi.c
>> +++ b/drivers/acpi/cppc_acpi.c
>> @@ -1963,16 +1963,17 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>>  		cpc_desc->write_cmd_status = 0;
>>  	}
>>  
>> -	cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
>> +	if (CPC_SUPPORTED(desired_reg))
>> +		cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
>>  
> 
> In general, can't be set perf_ctrls->desired_perf if and only if
> CPC_SUPPORTED(desired_reg) ?
> 
> I see cppc_get_perf() does that but I assume you are catering for some
> other access that may happen before ?
> 

cppc-cpufreq sets desired_perf regardless, I guess the check could also move up
there, but down here is fine as well IMO.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI: CPPC: Skip writes to unsupported performance controls
  2026-07-24 10:40 [PATCH] ACPI: CPPC: Skip writes to unsupported performance controls Christian Loehle
  2026-07-24 10:57 ` Sudeep Holla
@ 2026-07-27 11:14 ` zhenglifeng (A)
  2026-07-27 12:27   ` Christian Loehle
  1 sibling, 1 reply; 6+ messages in thread
From: zhenglifeng (A) @ 2026-07-27 11:14 UTC (permalink / raw)
  To: Christian Loehle, Rafael J . Wysocki, Viresh Kumar
  Cc: linux-pm, linux-acpi, linux-kernel, Len Brown, Jie Zhan,
	Pierre Gondois, Sumit Gupta, Sudeep Holla, Ionela Voinescu

On 7/24/2026 6:40 PM, Christian Loehle wrote:
> MIN_PERF and MAX_PERF are optional CPPC controls. DESIRED_PERF is also
> optional with CPPC2 when autonomous selection is supported.
> 
> The cppc-cpufreq target callbacks populate both limits for every request
> without checking whether the controls are implemented. cppc_set_perf()
> consequently passes NULL register descriptors to cpc_write(). The writes
> fail width validation and their return values are ignored, so the failed
> access paths are repeated on every target request. An autonomous-only
> platform can take the same path for DESIRED_PERF.
> 
> Check that each performance control is supported before calling
> cpc_write().
> 
> Fixes: ea3db45ae476 ("cpufreq: cppc: Update MIN_PERF/MAX_PERF in target callbacks")
> Reviewed-by: Sumit Gupta <sumitg@nvidia.com>
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
> v2: Also added desired_perf check (Sumit)
> 
>  drivers/acpi/cppc_acpi.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 1d3a94100491..53d09ca98f06 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1963,16 +1963,17 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>  		cpc_desc->write_cmd_status = 0;
>  	}
>  
> -	cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
> +	if (CPC_SUPPORTED(desired_reg))
> +		cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
>  
>  	/*
>  	 * Only write if min_perf and max_perf not zero. Some drivers pass zero
>  	 * 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))

LGTM.

Reviewed-by: Lifeng Zheng <zhenglifeng1@huawei.com>

Apologize for replying to v1 before seeing v2.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI: CPPC: Skip writes to unsupported performance controls
  2026-07-27 11:14 ` zhenglifeng (A)
@ 2026-07-27 12:27   ` Christian Loehle
  2026-07-27 13:21     ` Rafael J. Wysocki (Intel)
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Loehle @ 2026-07-27 12:27 UTC (permalink / raw)
  To: zhenglifeng (A), Rafael J . Wysocki, Viresh Kumar
  Cc: linux-pm, linux-acpi, linux-kernel, Len Brown, Jie Zhan,
	Pierre Gondois, Sumit Gupta, Sudeep Holla, Ionela Voinescu

On 7/27/26 12:14, zhenglifeng (A) wrote:
> On 7/24/2026 6:40 PM, Christian Loehle wrote:
>> MIN_PERF and MAX_PERF are optional CPPC controls. DESIRED_PERF is also
>> optional with CPPC2 when autonomous selection is supported.
>>
>> The cppc-cpufreq target callbacks populate both limits for every request
>> without checking whether the controls are implemented. cppc_set_perf()
>> consequently passes NULL register descriptors to cpc_write(). The writes
>> fail width validation and their return values are ignored, so the failed
>> access paths are repeated on every target request. An autonomous-only
>> platform can take the same path for DESIRED_PERF.
>>
>> Check that each performance control is supported before calling
>> cpc_write().
>>
>> Fixes: ea3db45ae476 ("cpufreq: cppc: Update MIN_PERF/MAX_PERF in target callbacks")
>> Reviewed-by: Sumit Gupta <sumitg@nvidia.com>
>> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
>> ---
>> v2: Also added desired_perf check (Sumit)
>>
>>  drivers/acpi/cppc_acpi.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>> index 1d3a94100491..53d09ca98f06 100644
>> --- a/drivers/acpi/cppc_acpi.c
>> +++ b/drivers/acpi/cppc_acpi.c
>> @@ -1963,16 +1963,17 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>>  		cpc_desc->write_cmd_status = 0;
>>  	}
>>  
>> -	cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
>> +	if (CPC_SUPPORTED(desired_reg))
>> +		cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
>>  
>>  	/*
>>  	 * Only write if min_perf and max_perf not zero. Some drivers pass zero
>>  	 * 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))
> 
> LGTM.
> 
> Reviewed-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> 
> Apologize for replying to v1 before seeing v2.
> 

Thanks for taking a look!
You can blame it on me for messing up the title and forgetting to put v2 there :/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI: CPPC: Skip writes to unsupported performance controls
  2026-07-27 12:27   ` Christian Loehle
@ 2026-07-27 13:21     ` Rafael J. Wysocki (Intel)
  0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-07-27 13:21 UTC (permalink / raw)
  To: Christian Loehle
  Cc: zhenglifeng (A),
	Rafael J . Wysocki, Viresh Kumar, linux-pm, linux-acpi,
	linux-kernel, Len Brown, Jie Zhan, Pierre Gondois, Sumit Gupta,
	Sudeep Holla, Ionela Voinescu

On Mon, Jul 27, 2026 at 2:27 PM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 7/27/26 12:14, zhenglifeng (A) wrote:
> > On 7/24/2026 6:40 PM, Christian Loehle wrote:
> >> MIN_PERF and MAX_PERF are optional CPPC controls. DESIRED_PERF is also
> >> optional with CPPC2 when autonomous selection is supported.
> >>
> >> The cppc-cpufreq target callbacks populate both limits for every request
> >> without checking whether the controls are implemented. cppc_set_perf()
> >> consequently passes NULL register descriptors to cpc_write(). The writes
> >> fail width validation and their return values are ignored, so the failed
> >> access paths are repeated on every target request. An autonomous-only
> >> platform can take the same path for DESIRED_PERF.
> >>
> >> Check that each performance control is supported before calling
> >> cpc_write().
> >>
> >> Fixes: ea3db45ae476 ("cpufreq: cppc: Update MIN_PERF/MAX_PERF in target callbacks")
> >> Reviewed-by: Sumit Gupta <sumitg@nvidia.com>
> >> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> >> ---
> >> v2: Also added desired_perf check (Sumit)
> >>
> >>  drivers/acpi/cppc_acpi.c | 7 ++++---
> >>  1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> >> index 1d3a94100491..53d09ca98f06 100644
> >> --- a/drivers/acpi/cppc_acpi.c
> >> +++ b/drivers/acpi/cppc_acpi.c
> >> @@ -1963,16 +1963,17 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
> >>              cpc_desc->write_cmd_status = 0;
> >>      }
> >>
> >> -    cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
> >> +    if (CPC_SUPPORTED(desired_reg))
> >> +            cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
> >>
> >>      /*
> >>       * Only write if min_perf and max_perf not zero. Some drivers pass zero
> >>       * 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))
> >
> > LGTM.
> >
> > Reviewed-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> >
> > Apologize for replying to v1 before seeing v2.
> >
>
> Thanks for taking a look!
> You can blame it on me for messing up the title and forgetting to put v2 there :/

Applied as 7.2-rc material, thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-27 13:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-24 10:40 [PATCH] ACPI: CPPC: Skip writes to unsupported performance controls Christian Loehle
2026-07-24 10:57 ` Sudeep Holla
2026-07-24 12:01   ` Christian Loehle
2026-07-27 11:14 ` zhenglifeng (A)
2026-07-27 12:27   ` Christian Loehle
2026-07-27 13:21     ` Rafael J. Wysocki (Intel)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®