[Public]
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: Friday, July 4, 2025 5:46 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; Andryuk,
> Jason
> <jason.andr...@amd.com>
> Subject: Re: [PATCH v5 14/18] xen/cpufreq: introduce GET_CPUFREQ_CPPC sub-
> cmd
>
> On 04.07.2025 10:56, Penny, Zheng wrote:
> > [Public]
> >
> >> -----Original Message-----
> >> From: Jan Beulich <jbeul...@suse.com>
> >> Sent: Friday, July 4, 2025 4:33 PM
> >> To: Penny, Zheng <penny.zh...@amd.com>; Andryuk, Jason
> >> <jason.andr...@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 04.07.2025 10:13, Penny, Zheng wrote:
> >>> [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.
> >>
> >> How is the xenpm command "get-cpufreq-para" related to the sysctl
> >> interface struct size? If you need to invoke two sysctl sub-ops to
> >> produce all relevant "get-cpufreq- para" output, so be it I would say.
> >>
> >
> > Because we are limiting each sysctl-subcmd-struct, such as " struct
> xen_sysctl_pm_op ", 128 bytes in "struct xen_sysctl"，They are all combined as
> a
> union.
> > Also, in "struct xen_sysctl_pm_op", its descending sub-op structs,
> > including "struct xen_get_cpufreq_para", are all combined as union too
> > ``` struct xen_sysctl_pm_op {
> > ... ...
> > union {
> > struct xen_get_cpufreq_para get_para;
> > struct xen_set_cpufreq_gov set_gov;
> > struct xen_set_cpufreq_para set_para;
> > struct xen_set_cppc_para set_cppc;
> > uint64_aligned_t get_avgfreq;
> > uint32_t set_sched_opt_smt;
> > #define XEN_SYSCTL_CX_UNLIMITED 0xffffffffU
> > uint32_t get_max_cstate;
> > uint32_t set_max_cstate;
> > } u;
> > }
> > ```
> > It could deduce that "struct xen_get_cpufreq_para" is limited to 128 bytes
> > (I think
> actual limit is smaller than 128)....
>
> And that implies what? In my earlier reply I already said that you may then
> simply
> need to invoke more than one sysctl to get all the data you need. (As one of
> the
> options, that is.)
>
Okay, I only explained why I couldn't use one command "get-cpufreq-para" to
output all info
> Jan
