Re: [PATCH v7 12/13] xen/cpufreq: bypass governor-related para for amd-cppc-epp

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.08.2025 12:52, Penny Zheng wrote:
> --- a/tools/misc/xenpm.c
> +++ b/tools/misc/xenpm.c
> @@ -832,9 +832,13 @@ static void print_cppc_para(unsigned int cpuid,
>  /* print out parameters about cpu frequency */
>  static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para 
> *p_cpufreq)
>  {
> -    bool hwp = strcmp(p_cpufreq->scaling_driver, XEN_HWP_DRIVER_NAME) == 0;
> +    bool is_goverless = false;

I first thought this would be a typo just in the description. Please make
this is_governor_less, or - if you absolutely want to have a shorter
identifier - is_govless (albeit then you might also consider to drop the
is_ prefix).

> --- a/xen/drivers/cpufreq/cpufreq.c
> +++ b/xen/drivers/cpufreq/cpufreq.c
> @@ -956,3 +956,17 @@ int __init cpufreq_register_driver(const struct 
> cpufreq_driver *driver_data)
>  
>      return 0;
>  }
> +
> +#ifdef CONFIG_PM_OP
> +/*
> + * Governor-less cpufreq driver indicates the driver doesn't rely on Xen
> + * governor to do performance tuning, mostly it has hardware built-in
> + * algorithm to calculate runtime workload and adjust cores frequency
> + * automatically. like Intel HWP, or CPPC in AMD.

Nit: Capital letter please at the start of a sentence.

> + */
> +bool cpufreq_is_governorless(unsigned int cpuid)
> +{
> +    return processor_pminfo[cpuid]->init && (hwp_active() ||
> +                                             cpufreq_driver.setpolicy);
> +}
> +#endif /* CONFIG_PM_OP */

Aiui the #ifdef is to please Misra, but that's not very nice to have. Does
any of the constituents stand in the way of this becoming an inline function
in a suitable header file?

Jan