[Public]
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: Tuesday, June 17, 2025 6:08 PM
> To: Penny, Zheng <penny.zh...@amd.com>
> Cc: Huang, Ray <ray.hu...@amd.com>; Anthony PERARD
> <anthony.per...@vates.tech>; Juergen Gross <jgr...@suse.com>; Andrew
> Cooper <andrew.coop...@citrix.com>; Orzel, Michal <michal.or...@amd.com>;
> Julien Grall <jul...@xen.org>; Roger Pau Monné <roger....@citrix.com>; Stefano
> Stabellini <sstabell...@kernel.org>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v5 14/18] xen/cpufreq: introduce GET_CPUFREQ_CPPC sub-
> cmd
>
> On 27.05.2025 10:48, Penny Zheng wrote:
> > --- a/tools/misc/xenpm.c
> > +++ b/tools/misc/xenpm.c
> > @@ -69,6 +69,7 @@ void show_help(void)
> > " set-max-cstate <num>|'unlimited'
> > [<num2>|'unlimited']\n"
> > " set the C-State
> > limitation (<num> >= 0) and\n"
> > " optionally the
> > C-sub-state limitation
> (<num2> >= 0)\n"
> > + " get-cpufreq-cppc [cpuid] list cpu cppc parameter
> > of CPU
> <cpuid> or all\n"
> > " set-cpufreq-cppc [cpuid] [balance|performance|powersave]
> <param:val>*\n"
> > " set Hardware P-State
> > (HWP) parameters\n"
> > " on CPU <cpuid> or all if
> > omitted.\n"
> > @@ -812,33 +813,7 @@ static void print_cpufreq_para(int cpuid, struct
> > xc_get_cpufreq_para *p_cpufreq)
> >
> > printf("scaling_driver : %s\n", p_cpufreq->scaling_driver);
> >
> > - if ( hwp )
> > - {
> > - const xc_cppc_para_t *cppc = &p_cpufreq->u.cppc_para;
> > -
> > - printf("cppc variables :\n");
> > - printf(" hardware limits : lowest [%"PRIu32"] lowest nonlinear
> [%"PRIu32"]\n",
> > - cppc->lowest, cppc->lowest_nonlinear);
> > - printf(" : nominal [%"PRIu32"] highest
> > [%"PRIu32"]\n",
> > - cppc->nominal, cppc->highest);
> > - printf(" configured limits : min [%"PRIu32"] max [%"PRIu32"]
> > energy perf
> [%"PRIu32"]\n",
> > - cppc->minimum, cppc->maximum, cppc->energy_perf);
> > -
> > - if ( cppc->features & XEN_SYSCTL_CPPC_FEAT_ACT_WINDOW )
> > - {
> > - unsigned int activity_window;
> > - const char *units;
> > -
> > - activity_window = calculate_activity_window(cppc, &units);
> > - printf(" : activity_window [%"PRIu32"
> > %s]\n",
> > - activity_window, units);
> > - }
> > -
> > - printf(" : desired [%"PRIu32"%s]\n",
> > - cppc->desired,
> > - cppc->desired ? "" : " hw autonomous");
> > - }
> > - else
> > + if ( !hwp )
> > {
> > if ( p_cpufreq->gov_num )
> > printf("scaling_avail_gov : %s\n",
>
> I'm not sure it is a good idea to alter what is being output for
> get-cpufreq-para.
> People may simply miss that output then, without having any indication where
> it
> went.
Hwp is more like amd-cppc in active mode. It also does not rely on Xen governor
to do performance tuning, so in previous design, we could borrow governor filed
to output cppc info
However after introducing amd-cppc passive mode, we have request to output both
governor and CPPC info. And if continuing expanding get-cpufreq-para to include
CPPC info, it will make the parent stuct xen_sysctl.u exceed exceed 128 bytes.
So I'm left to create a new subcmd to specifically deal with CPPC info
I could leave above output for get-cpufreq-para unchanged. Then we will have
duplicate CPPC info in two commands. Or is it fine to do that?
>
>
> Jan
