[PATCH v5 12/18] xen/cpufreq: get performance policy from governor set via xenpm

Penny Zheng posted 18 patches 5 months, 1 week ago
There is a newer version of this series
[PATCH v5 12/18] xen/cpufreq: get performance policy from governor set via xenpm
Posted by Penny Zheng 5 months, 1 week ago
Even if Xen governor is not used in amd-cppc active mode, we could
somehow deduce which performance policy (CPUFREQ_POLICY_xxx) user wants to
apply through which governor they choose, such as:
If user chooses performance governor, they want maximum performance.
If user chooses powersave governor, they want the least power consumption.
Function cpufreq_policy_from_governor() is responsible for above transition,
and it shall be also effective when users setting new governor through xenpm.

ondemand and userspace are forbidden choices, and if users specify
such options, we shall not only give warning message, but also error out.

Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
---
v4 -> v5:
- new commit
---
 xen/drivers/acpi/pmstat.c     | 8 ++++++++
 xen/drivers/cpufreq/utility.c | 1 +
 2 files changed, 9 insertions(+)

diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c
index 330bc3a1c2..514475cf5c 100644
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -319,6 +319,14 @@ static int set_cpufreq_gov(struct xen_sysctl_pm_op *op)
     if (new_policy.governor == NULL)
         return -EINVAL;
 
+    new_policy.policy = cpufreq_policy_from_governor(new_policy.governor);
+    if ( new_policy.policy == CPUFREQ_POLICY_UNKNOWN )
+    {
+        printk("Failed to get performance policy from %s, and users shall write epp values to deliver preference towards performance over efficiency",
+               new_policy.governor->name);
+        return -EINVAL;
+    }
+
     return __cpufreq_set_policy(old_policy, &new_policy);
 }
 
diff --git a/xen/drivers/cpufreq/utility.c b/xen/drivers/cpufreq/utility.c
index 2617581125..734cd84be1 100644
--- a/xen/drivers/cpufreq/utility.c
+++ b/xen/drivers/cpufreq/utility.c
@@ -456,6 +456,7 @@ int __cpufreq_set_policy(struct cpufreq_policy *data,
     data->min = policy->min;
     data->max = policy->max;
     data->limits = policy->limits;
+    data->policy = policy->policy;
     if (cpufreq_driver.setpolicy)
         return alternative_call(cpufreq_driver.setpolicy, data);
 
-- 
2.34.1
Re: [PATCH v5 12/18] xen/cpufreq: get performance policy from governor set via xenpm
Posted by Jan Beulich 4 months, 2 weeks ago
On 27.05.2025 10:48, Penny Zheng wrote:
> --- a/xen/drivers/acpi/pmstat.c
> +++ b/xen/drivers/acpi/pmstat.c
> @@ -319,6 +319,14 @@ static int set_cpufreq_gov(struct xen_sysctl_pm_op *op)
>      if (new_policy.governor == NULL)
>          return -EINVAL;
>  
> +    new_policy.policy = cpufreq_policy_from_governor(new_policy.governor);
> +    if ( new_policy.policy == CPUFREQ_POLICY_UNKNOWN )
> +    {
> +        printk("Failed to get performance policy from %s, and users shall write epp values to deliver preference towards performance over efficiency",

This message is excessively long and is lacking a newline (i.e. effectively two:
one in the middle and one at the end; yet better would be to find less verbose
wording). What's worse, how would a "user" go about "writing epp values"?

Jan
RE: [PATCH v5 12/18] xen/cpufreq: get performance policy from governor set via xenpm
Posted by Penny, Zheng 3 months, 4 weeks ago
[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, June 17, 2025 12:22 AM
> To: Penny, Zheng <penny.zheng@amd.com>
> Cc: Huang, Ray <Ray.Huang@amd.com>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v5 12/18] xen/cpufreq: get performance policy from governor
> set via xenpm
>
> On 27.05.2025 10:48, Penny Zheng wrote:
> > --- a/xen/drivers/acpi/pmstat.c
> > +++ b/xen/drivers/acpi/pmstat.c
> > @@ -319,6 +319,14 @@ static int set_cpufreq_gov(struct xen_sysctl_pm_op
> *op)
> >      if (new_policy.governor == NULL)
> >          return -EINVAL;
> >
> > +    new_policy.policy = cpufreq_policy_from_governor(new_policy.governor);
> > +    if ( new_policy.policy == CPUFREQ_POLICY_UNKNOWN )
> > +    {
> > +        printk("Failed to get performance policy from %s, and users
> > + shall write epp values to deliver preference towards performance
> > + over efficiency",
>
> This message is excessively long and is lacking a newline (i.e. effectively two:
> one in the middle and one at the end; yet better would be to find less verbose
> wording). What's worse, how would a "user" go about "writing epp values"?
>

Maybe change it to
```
printk("Failed to get performance policy from %s, Try \"xenpm set-cpufreq-cppc\"\n", ...);
```

> Jan
Re: [PATCH v5 12/18] xen/cpufreq: get performance policy from governor set via xenpm
Posted by Jan Beulich 3 months, 4 weeks ago
On 04.07.2025 09:51, Penny, Zheng wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Tuesday, June 17, 2025 12:22 AM
>> To: Penny, Zheng <penny.zheng@amd.com>
>> Cc: Huang, Ray <Ray.Huang@amd.com>; xen-devel@lists.xenproject.org
>> Subject: Re: [PATCH v5 12/18] xen/cpufreq: get performance policy from governor
>> set via xenpm
>>
>> On 27.05.2025 10:48, Penny Zheng wrote:
>>> --- a/xen/drivers/acpi/pmstat.c
>>> +++ b/xen/drivers/acpi/pmstat.c
>>> @@ -319,6 +319,14 @@ static int set_cpufreq_gov(struct xen_sysctl_pm_op
>> *op)
>>>      if (new_policy.governor == NULL)
>>>          return -EINVAL;
>>>
>>> +    new_policy.policy = cpufreq_policy_from_governor(new_policy.governor);
>>> +    if ( new_policy.policy == CPUFREQ_POLICY_UNKNOWN )
>>> +    {
>>> +        printk("Failed to get performance policy from %s, and users
>>> + shall write epp values to deliver preference towards performance
>>> + over efficiency",
>>
>> This message is excessively long and is lacking a newline (i.e. effectively two:
>> one in the middle and one at the end; yet better would be to find less verbose
>> wording). What's worse, how would a "user" go about "writing epp values"?
>>
> 
> Maybe change it to
> ```
> printk("Failed to get performance policy from %s, Try \"xenpm set-cpufreq-cppc\"\n", ...);
> ```

Looks quite a bit better, thanks.

Jan