linux-kernel - Re: [PATCH v6 6/9] ACPI: CPPC: add APIs and sysfs interface for min/max

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1d75dd4f-0eaa-447f-99b1-4c6c9d39a87d@arm.com>
Date: Mon, 26 Jan 2026 11:51:36 +0100
From: Pierre Gondois <pierre.gondois@....com>
To: Sumit Gupta <sumitg@...dia.com>, rafael@...nel.org,
 viresh.kumar@...aro.org, zhenglifeng1@...wei.com, ionela.voinescu@....com,
 lenb@...nel.org, robert.moore@...el.com, corbet@....net,
 rdunlap@...radead.org, ray.huang@....com, gautham.shenoy@....com,
 mario.limonciello@....com, perry.yuan@....com, zhanjie9@...ilicon.com,
 linux-pm@...r.kernel.org, linux-acpi@...r.kernel.org,
 linux-doc@...r.kernel.org, acpica-devel@...ts.linux.dev,
 linux-kernel@...r.kernel.org
Cc: linux-tegra@...r.kernel.org, treding@...dia.com, jonathanh@...dia.com,
 vsethi@...dia.com, ksitaraman@...dia.com, sanjayc@...dia.com,
 nhartman@...dia.com, bbasu@...dia.com
Subject: Re: [PATCH v6 6/9] ACPI: CPPC: add APIs and sysfs interface for
 min/max_perf


On 1/24/26 21:32, Sumit Gupta wrote:
>
> On 22/01/26 17:06, Pierre Gondois wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 1/20/26 15:56, Sumit Gupta wrote:
>>> Add cppc_get/set_min_perf() and cppc_get/set_max_perf() APIs to read 
>>> and
>>> write the MIN_PERF and MAX_PERF registers.
>>>
>>> Also add sysfs interfaces (min_perf, max_perf) in cppc_cpufreq driver
>>> to expose these controls to userspace. The sysfs values are in 
>>> frequency
>>> (kHz) for consistency with other cpufreq sysfs files.
>>>
>>> A mutex is used to serialize sysfs store operations to ensure hardware
>>> register writes and perf_ctrls updates are atomic.
>>>
>>> Signed-off-by: Sumit Gupta <sumitg@...dia.com>
>>> ---
>>>   drivers/acpi/cppc_acpi.c       |  44 +++++++++
>>>   drivers/cpufreq/cppc_cpufreq.c | 157 
>>> +++++++++++++++++++++++++++++++++
>>>   include/acpi/cppc_acpi.h       |  20 +++++
>>>   3 files changed, 221 insertions(+)
>>>
>>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>>> index 45c6bd6ec24b..46bf45f8b0f3 100644
>>> --- a/drivers/acpi/cppc_acpi.c
>>> +++ b/drivers/acpi/cppc_acpi.c
>>> @@ -1743,6 +1743,50 @@ int cppc_set_auto_sel(int cpu, bool enable)
>>>   }
>>>   EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
>>>
>>> +/**
>>> + * cppc_get_min_perf - Read minimum performance register.
>>> + * @cpu: CPU from which to read register.
>>> + * @min_perf: Return address.
>>> + */
>>> +int cppc_get_min_perf(int cpu, u64 *min_perf)
>>> +{
>>> +     return cppc_get_reg_val(cpu, MIN_PERF, min_perf);
>>> +}
>>> +EXPORT_SYMBOL_GPL(cppc_get_min_perf);
>>> +
>>> +/**
>>> + * cppc_set_min_perf - Write minimum performance register.
>>> + * @cpu: CPU to which to write register.
>>> + * @min_perf: the desired minimum performance value to be updated.
>>> + */
>>> +int cppc_set_min_perf(int cpu, u32 min_perf)
>>> +{
>>> +     return cppc_set_reg_val(cpu, MIN_PERF, min_perf);
>>> +}
>>> +EXPORT_SYMBOL_GPL(cppc_set_min_perf);
>>> +
>>> +/**
>>> + * cppc_get_max_perf - Read maximum performance register.
>>> + * @cpu: CPU from which to read register.
>>> + * @max_perf: Return address.
>>> + */
>>> +int cppc_get_max_perf(int cpu, u64 *max_perf)
>>> +{
>>> +     return cppc_get_reg_val(cpu, MAX_PERF, max_perf);
>>> +}
>>> +EXPORT_SYMBOL_GPL(cppc_get_max_perf);
>>> +
>>> +/**
>>> + * cppc_set_max_perf - Write maximum performance register.
>>> + * @cpu: CPU to which to write register.
>>> + * @max_perf: the desired maximum performance value to be updated.
>>> + */
>>> +int cppc_set_max_perf(int cpu, u32 max_perf)
>>> +{
>>> +     return cppc_set_reg_val(cpu, MAX_PERF, max_perf);
>>> +}
>>> +EXPORT_SYMBOL_GPL(cppc_set_max_perf);
>>> +
>>>   /**
>>>    * cppc_set_enable - Set to enable CPPC on the processor by 
>>> writing the
>>>    * Continuous Performance Control package EnableRegister field.
>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c 
>>> b/drivers/cpufreq/cppc_cpufreq.c
>>> index 229880c4eedb..66e183b45fb0 100644
>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>> @@ -28,6 +28,8 @@
>>>
>>>   static struct cpufreq_driver cppc_cpufreq_driver;
>>>
>>> +static DEFINE_MUTEX(cppc_cpufreq_autonomous_lock);
>>> +
>>
>> Shouldn't concurrent access be handled by the policy->rwsem ?
>>
>> I think this can be checked using either:
>> - lockdep_assert_held_write(&policy->rwsem)
>> - lockdep_assert_held_read(&policy->rwsem)
>>
>> in store/show_max_perf() for instance.
>>
>
> You're right. The cpufreq sysfs already holds policy->rwsem for
> show/store callbacks. I'll remove the mutex and add lockdep
> assertions for the expected locking.

I think it's ok not to have lockde assertions.

It seems that it is a common assumption sysfs files cannot be modified

concurrently. None of the cpufreq driver seems to use lockdep assertion.


> --------
> File: drivers/cpufreq/cpufreq.c
>   static ssize_t store(struct kobject *kobj, struct attribute *attr,
>                      const char *buf, size_t count)
>   {
>       struct cpufreq_policy *policy = to_policy(kobj);
>       ....
>       guard(cpufreq_policy_write)(policy);
> --------
>
> Thank you,
> Sumit Gupta
>