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: <83ed733b-f7d2-42c3-ab0a-9b9fc76ecb4a@nvidia.com>
Date: Tue, 27 Jan 2026 16:52:24 +0530
From: Sumit Gupta <sumitg@...dia.com>
To: Pierre Gondois <pierre.gondois@....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, sumitg@...dia.com
Subject: Re: [PATCH v6 6/9] ACPI: CPPC: add APIs and sysfs interface for
 min/max_perf


>>> 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.
>

Sure. I will just remove the mutex, keeping it consistent with other 
cpufreq drivers.

Thank you,
Sumit Gupta


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