RE: [PATCH v6 16/19] xen/cpufreq: introduce GET_CPUFREQ_CPPC sub-op

["Penny, Zheng" <penny.zheng@xxxxxxx>]

[Public]

> -----Original Message-----
> From: Jason Andryuk <jason.andryuk@xxxxxxx>
> Sent: Thursday, July 24, 2025 10:17 PM
> To: Jan Beulich <jbeulich@xxxxxxxx>; Penny, Zheng <penny.zheng@xxxxxxx>
> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Anthony PERARD
> <anthony.perard@xxxxxxxxxx>; Juergen Gross <jgross@xxxxxxxx>; Andrew
> Cooper <andrew.cooper3@xxxxxxxxxx>; Orzel, Michal <Michal.Orzel@xxxxxxx>;
> Julien Grall <julien@xxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>;
> Stefano Stabellini <sstabellini@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v6 16/19] xen/cpufreq: introduce GET_CPUFREQ_CPPC
> sub-op
>
> On 2025-07-24 09:31, Jan Beulich wrote:
> > On 11.07.2025 05:51, Penny Zheng wrote:
> >> In amd-cppc passive mode, it's Xen governor which is responsible for
> >> performance tuning, so governor and CPPC could co-exist. That is,
> >> both governor-info and CPPC-info need to be printed together via xenpm 
> >> tool.
> >>
> >> If we tried to still put it in "struct xen_get_cpufreq_para" (e.g.
> >> just move out of union), "struct xen_get_cpufreq_para" will enlarge
> >> too much to further make xen_sysctl.u exceed 128 bytes.
> >> So we introduce a new sub-op GET_CPUFREQ_CPPC to specifically print
> >> CPPC-related para.
> >>
> >> Signed-off-by: Penny Zheng <Penny.Zheng@xxxxxxx>
>
> >>   void scaling_max_freq_func(int argc, char *argv[])
> >>   {
> >>       int cpuid = -1, freq = -1;
> >> @@ -1576,6 +1622,7 @@ struct {
> >>       { "get-cpufreq-average", cpufreq_func },
> >>       { "start", start_gather_func },
> >>       { "get-cpufreq-para", cpufreq_para_func },
> >> +    { "get-cpufreq-cppc", cppc_para_func },
> >
> > Didn't Jason also suggest that we would better not introduce a new
> > command, but rather make get-cpufreq-para invoke GET_CPUFREQ_CPPC as
> > needed? Considering that as per patch 15 the same information is
> > already printed, I think I'm a little lost with the need for this
> > separate operation (and command), and then also with the need for patch 15.
>
> Yes, but I thought I was repeating your suggestion, Jan :)
>
> xenpm's show_cpufreq_para_by_cpuid() would do something like:
>
> show_cpufreq_para_by_cpuid() {
>      xc_get_cpufreq_para()
>      hw_auto = HWP || CPPC
>      if ( hw_auto ) {
>          xc_get_cppc_para()
>          print_cppc_para()
>      } else
>          print_cpufreq_para()
> }
>
> Would that work?
>

Understood, I will re-write as you suggests, thx

> That way the single `xenpm get-cpufreq-para` would return the current cpufreq
> data without the user needed to know what is running.
>
> Regards,
> Jason