Re: [PATCH v6 14/19] xen/cpufreq: get performance policy from governor set via xenpm

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.07.2025 05:51, Penny Zheng wrote:
> 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, then
> the policy shall be CPUFREQ_POLICY_PERFORMANCE
> If user chooses powersave governor, they want the least power consumption,
> then the policy shall be CPUFREQ_POLICY_POWERSAVE
> 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

Is this stale? You permit ...

> such options, we shall not only give warning message to suggest using
> "xenpm set-cpufreq-cppc", but also error out.
> 
> Signed-off-by: Penny Zheng <Penny.Zheng@xxxxxxx>
> ---
> v4 -> v5:
> - new commit
> ---
> v5 -> v6:
> - refactor warning message
> ---
>  xen/drivers/acpi/pm-op.c      | 8 ++++++++
>  xen/drivers/cpufreq/utility.c | 1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/xen/drivers/acpi/pm-op.c b/xen/drivers/acpi/pm-op.c
> index d10f6db0e4..e616c3316a 100644
> --- a/xen/drivers/acpi/pm-op.c
> +++ b/xen/drivers/acpi/pm-op.c
> @@ -205,6 +205,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 )

... CPUFREQ_POLICY_ONDEMAND here now, aiui (as per patch 13).

> --- a/xen/drivers/cpufreq/utility.c
> +++ b/xen/drivers/cpufreq/utility.c
> @@ -250,6 +250,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);

This looks like it would belong in the patch introducing the field. More
generally, is the field left uninitialized in certain cases until here?
That we will want to avoid.

Jan