linux-kernel - Re: [PATCH v2 3/5] cpufreq/amd-pstate: Add support for platform profile class

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: <f9953826-1324-4df2-9252-b10d0c9e5679@amd.com>
Date: Tue, 18 Mar 2025 14:07:00 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: "Gautham R. Shenoy" <gautham.shenoy@....com>
Cc: Mario Limonciello <superm1@...nel.org>, Perry Yuan <perry.yuan@....com>,
 Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>,
 "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
 <linux-kernel@...r.kernel.org>,
 "open list:CPU FREQUENCY SCALING FRAMEWORK" <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v2 3/5] cpufreq/amd-pstate: Add support for platform
 profile class

On 3/10/2025 00:09, Gautham R. Shenoy wrote:
> [...snip...]
> 
> On Fri, Mar 07, 2025 at 10:30:25PM -0600, Mario Limonciello wrote:
>> On 3/7/2025 10:55, Mario Limonciello wrote:
>>> On 3/7/2025 10:22, Gautham R. Shenoy wrote:
>>>>> +static int amd_pstate_profile_set(struct device *dev,
>>>>> +                  enum platform_profile_option profile)
>>>>> +{
>>>>> +    struct amd_cpudata *cpudata = dev_get_drvdata(dev);
>>>>> +    struct cpufreq_policy *policy __free(put_cpufreq_policy) =
>>>>> cpufreq_cpu_get(cpudata->cpu);
>>>>> +    int ret;
>>>>> +
>>>>> +    switch (profile) {
>>>>> +    case PLATFORM_PROFILE_LOW_POWER:
>>>>> +        if (cpudata->policy != CPUFREQ_POLICY_POWERSAVE)
>>>>> +            cpudata->policy = CPUFREQ_POLICY_POWERSAVE;
>>>>
>>>> So prior to the patch, cpudata->policy is supposed to mirror
>>>> policy->policy.  With this patch, this assumption is no longer
>>>> true. So it is possible for the user to again override the choice of
>>>> EPP set via platform profile by changing the cpufreq governor ?
>>>>
>>>> Is this the expected behaviour?
>>>>
>>>> The bigger concern is, if the governor was previously "performance"
>>>> and then the platform profile requested "low power", "cat
>>>> /sys/devices/system/cpu/cpuX/cpufreq/scaling_governor" would still
>>>> show "performance", which is inconsistent with the behaviour.
>>>>
>>>>
>>>
>>> This ties back to the previous patches for dynamic EPP.  My expectation
>>> was that when dynamic EPP is enabled that users can't manually set the
>>> EPP anymore (it will return -EBUSY) and likewise turning on dynamic EPP
>>> should keep the governor as powersave.
>>>
>>> I'll double check all those are properly enforced; but that's at least
>>> the intent.
>>
>> FWIW - I double checked and confirmed that this is working as intended.
>> * I couldn't change from powersave to performance when dynamic_epp was
>> enabled (-EBUSY)
>> * I couldn't change energy_performance_preference when dynamic_epp was
>> enabled (-EBUSY)
> 
> Thanks for double checking this.

The other option is to create a "3rd" cpufreq driver for amd-pstate when 
dynamic epp is in use.

The upside is this one could then exclude 
"energy_performance_preference" and 
"energy_performance_available_preferences" so no need to report -EBUSY 
for that case since you can't manually change EPP.

The downside is that you couldn't discover what the kernel was doing 
with EPP without ftrace.

> 
> 
>>
>>>
>>> IMO this "should" all work because turning on Dynamic EPP sysfs file
>>> forces the driver to go through a state transition that it will tear
>>> everything down and back up.  The policy will come back up in
>>> "powersave" even if it was previously in "performance" when the dynamic
>>> EPP sysfs file was turned on.
>>>
>>> Longer term; I also envision the scheduler influencing EPP values when
>>> dynamic_epp is turned on.  The "platform profile" would be an "input" to
>>> that decision making process (maybe giving a weighting?), not the only
>>> lever.
> 
> Yes, the scheduler influencing the EPP values is something that I have
> been wanting to explore as well. My idea was to use the nature of the
> task + the load on the rq to determine the EPP value.
> 
>>>
>>> I haven't given any serious look at how to do this with the scheduler, I
>>> wanted to lay the foundation first being dynamic EPP and raw EPP.
>>>
>>> So even if dynamic_epp isn't interesting "right now" for server because
>>> the focus is around behavior for AC/DC, don't write it off just yet.
>>
> 
> Fair enough.
> 
>