linux-kernel - Re: [PATCH v4 6/6] cpufreq: CPPC: Support for autonomous selection in cppc

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: <f89fc07a-1c65-4d1e-9ad8-76c6c9a15b25@huawei.com>
Date: Thu, 16 Jan 2025 09:26:37 +0800
From: "zhenglifeng (A)" <zhenglifeng1@...wei.com>
To: "Gautham R. Shenoy" <gautham.shenoy@....com>
CC: <rafael@...nel.org>, <lenb@...nel.org>, <robert.moore@...el.com>,
	<viresh.kumar@...aro.org>, <mario.limonciello@....com>, <ray.huang@....com>,
	<pierre.gondois@....com>, <acpica-devel@...ts.linux.dev>,
	<linux-acpi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-pm@...r.kernel.org>, <linuxarm@...wei.com>,
	<jonathan.cameron@...wei.com>, <zhanjie9@...ilicon.com>,
	<lihuisong@...wei.com>, <hepeng68@...wei.com>, <fanghao11@...wei.com>
Subject: Re: [PATCH v4 6/6] cpufreq: CPPC: Support for autonomous selection in
 cppc_cpufreq

On 2025/1/15 22:51, Gautham R. Shenoy wrote:

> Hello Lifeng,
> 
> 
> On Mon, Jan 13, 2025 at 08:21:04PM +0800, Lifeng Zheng wrote:
>> Add sysfs interfaces for CPPC autonomous selection in the cppc_cpufreq
>> driver.
>>
> 
> [..snip..]
> 
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index bd8f75accfa0..ea6c6a5bbd8c 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -814,10 +814,119 @@ static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
>>  
>>  	return cpufreq_show_cpus(cpu_data->shared_cpu_map, buf);
>>  }
>> +
>> +static ssize_t show_auto_select(struct cpufreq_policy *policy, char *buf)
>> +{
>> +	bool val;
>> +	int ret;
>> +
>> +	ret = cppc_get_auto_sel(policy->cpu, &val);
>> +
>> +	/* show "<unsupported>" when this register is not supported by cpc */
>> +	if (ret == -EOPNOTSUPP)
>> +		return sysfs_emit(buf, "%s\n", "<unsupported>");
>> +
>> +	if (ret)
>> +		return ret;
>> +
>> +	return sysfs_emit(buf, "%d\n", val);
>> +}
>> +
>> +static ssize_t store_auto_select(struct cpufreq_policy *policy,
>> +				 const char *buf, size_t count)
>> +{
>> +	bool val;
>> +	int ret;
>> +
>> +	ret = kstrtobool(buf, &val);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = cppc_set_auto_sel(policy->cpu, val);
> 
> When the auto_select register is not supported, since
> cppc_set_reg_val() doesn't have the !CPC_SUPPORTED(reg) check, that
> function won't return an error, and thus this store function won't
> return an error either. Should there be a !CPC_SUPPORTED(reg) check in
> cppc_set_reg_val() as well? Or should the store function call
> cppc_get_auto_sel() to figure out if the register is supported or not?

In patch 2, I have this check in cppc_set_reg_val():

+	/* if a register is writeable, it must be a buffer */
+	if ((reg->type != ACPI_TYPE_BUFFER) ||
+	    (IS_OPTIONAL_CPC_REG(reg_idx) && IS_NULL_REG(&reg->cpc_entry.reg))) {
+		pr_debug("CPC register (reg_idx=%d) is not supported\n", reg_idx);
+		return -EOPNOTSUPP;
+	}

If a register is not a cpc supported one, it must be either an integer type
or a null one. So it won't pass this check I think.

> 
> --
> Thanks and Regards
> gautham.