RE: [PATCH v4 15/15] xen/xenpm: Adapt SET/GET_CPUFREQ_CPPC xen_sysctl_pm_op for amd-cppc driver

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

[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: Thursday, May 22, 2025 2:51 PM
> To: Penny, Zheng <penny.zheng@xxxxxxx>
> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v4 15/15] xen/xenpm: Adapt SET/GET_CPUFREQ_CPPC
> xen_sysctl_pm_op for amd-cppc driver
>
> On 22.05.2025 07:59, Penny, Zheng wrote:
> > [Public]
> >
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@xxxxxxxx>
> >> Sent: Wednesday, April 30, 2025 11:02 PM
> >> To: Penny, Zheng <penny.zheng@xxxxxxx>
> >> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Andrew Cooper
> >> <andrew.cooper3@xxxxxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>;
> >> xen- devel@xxxxxxxxxxxxxxxxxxxx
> >> Subject: Re: [PATCH v4 15/15] xen/xenpm: Adapt
> SET/GET_CPUFREQ_CPPC
> >> xen_sysctl_pm_op for amd-cppc driver
> >>
> >> On 14.04.2025 09:40, Penny Zheng wrote:
> >>> +    /* Only allow values if params bit is set. */
> >>> +    if ( (!(set_cppc->set_params & XEN_SYSCTL_CPPC_SET_DESIRED)
> &&
> >>> +          set_cppc->desired) ||
> >>> +         (!(set_cppc->set_params & XEN_SYSCTL_CPPC_SET_MINIMUM)
> &&
> >>> +          set_cppc->minimum) ||
> >>> +         (!(set_cppc->set_params & XEN_SYSCTL_CPPC_SET_MAXIMUM)
> &&
> >>> +          set_cppc->maximum) ||
> >>> +         (!(set_cppc->set_params &
> >> XEN_SYSCTL_CPPC_SET_ENERGY_PERF) &&
> >>> +          set_cppc->energy_perf) )
> >>> +        return -EINVAL;
> >>> +
> >>> +    /* Activity window not supported in MSR */
> >>> +    if ( set_cppc->set_params &
> XEN_SYSCTL_CPPC_SET_ACT_WINDOW )
> >>> +        return -EOPNOTSUPP;
> >>> +
> >>> +    /* Return if there is nothing to do. */
> >>> +    if ( set_cppc->set_params == 0 )
> >>> +        return 0;
> >>> +
> >>> +    epp = per_cpu(epp_init, cpu);
> >>> +    /*
> >>> +     * Apply presets:
> >>> +     * XEN_SYSCTL_CPPC_SET_DESIRED reflects whether desired perf is
> >>> + set,
> >> which
> >>> +     * is also the flag to distiguish between passive mode and active 
> >>> mode.
> >>> +     * When it is set, CPPC in passive mode, only
> >>> +     * XEN_SYSCTL_CPPC_SET_PRESET_NONE could be chosen, where
> >> min_perf =
> >>> +     * lowest_nonlinear_perf to ensures performance in P-state range.
> >>> +     * when it is not set, CPPC in active mode, three different profile
> >>> +     *
> >>
> XEN_SYSCTL_CPPC_SET_PRESET_POWERSAVE/PERFORMANCE/BALANC
> >> E are provided,
> >>> +     * where min_perf = lowest_perf having T-state range of performance.
> >>> +     */
> >>
> >> I fear I'm struggling to parse some of this, making it difficult to
> >> suggest possible adjustments (as I can't derive what is meant to be
> >> said). Plus where's the term T- state coming from all of the sudden?
> >>
> >
> > Pasting description on "lowest_perf" and "nonlinear_lowest_perf":
> > "
> > Lowest Nonlinear Performance is the lowest performance level at which 
> > nonlinear
> power savings are achieved, for example, due to the combined effects of 
> voltage
> and frequency scaling. Above this threshold, lower performance levels should 
> be
> generally more energy efficient than higher performance levels. In 
> traditional terms,
> this represents the P-state range of performance levels.
> >
> > Lowest Performance is the absolute lowest performance level of the platform.
> Selecting a performance level lower than the lowest nonlinear performance 
> level
> may actually cause an efficiency penalty, but should reduce the instantaneous
> power consumption of the processor. In traditional terms, this represents the 
> T-state
> range of performance levels.
> > "
>
> And again "T-state". The only T-ish thing I'm aware of is something that was 
> never
> implemented in Xen's power management. Hence I fear I continue to be confused.
> (And btw, I searched PM vol 2 for the term, and it doesn't occur there at 
> all.) As a
> result ...
>

Sorry, I didn't add the reference:
https://uefi.org/specs/ACPI/6.5/08_Processor_Configuration_and_Control.html?highlight=cppc#lowest-nonlinear-performance

> > IMO, in passive mode, we rely on Xen governor to do performance
> > tuning. And Xen governor is thinking based on P-states. So I take non-linear
> lowest perf as min_perf While in active mode, hardware itself is calculating 
> suitable
> performance/frequency automatically based on workload, thermal, voltage, and 
> etc.
> So maybe setting min_perf with lowest perf is a better choice?
>
> ...answering this question isn't possible for me (yet).
>
> >>> +        ret = get_amd_cppc_para(policy->cpu,
> >>> + &op->u.get_para.u.cppc_para);
> >>> +
> >>> +    if ( strncmp(op->u.get_para.scaling_driver, XEN_HWP_DRIVER_NAME,
> >>> +                 CPUFREQ_NAME_LEN) &&
> >>> +         strncmp(op->u.get_para.scaling_driver,
> >> XEN_AMD_CPPC_EPP_DRIVER_NAME,
> >>> +                 CPUFREQ_NAME_LEN) )
> >>>      {
> >>>          if ( !(scaling_available_governors =
> >>>                 xzalloc_array(char, gov_num * CPUFREQ_NAME_LEN)) )
> >>
> >> Isn't it the non-EPP driver which is governor-independent?
> >>
> >
> > EPP driver is governor-independent, indicating active mode.
>
> I'm confused here as well then: Early in the series, before the EPP driver is
> introduced (i.e. as preparation for the non-EPP one) you suppress use and
> reporting of the governor. Whereas for the EPP driver while you (also) don't 
> use the

Are you mentioning the commit of " xen/cpufreq: only set gov NULL when 
cpufreq_driver.setpolicy is NULL " ?
The commit previously is added before the introduction of amd-cppc in active 
mode (EPP-mode), and in the commit message,
It says " amd-cppc on active mode bypasses the scaling governor layer ...."
IMO, cpufreq driver in Xen inherited from Linux, and in Linux, for hooks in 
cpufreq driver, we defined ->setpolicy() and ->target(), these
two are exclusive to each other. We have " !driver_data->target == 
!driver_data->setpolicy " checking in cpufreq_register_driver().
->targe() hook is for cpufreq driver needing Xen governor, and ->setpolicy() is 
for governor-less cpufreq driver.
Although we already defined these hooks, EPP-mode is the first driver which 
actually implements ->setpolicy hook

> governor as such, you use the choice of governor to determine the EPP 
> setting. In
> that sense to me the EPP driver is less governor- independent than the non-EPP
> one. That's what I tried to express in my earlier reply.
>

Sorry for bringing confusion here.
Like I addressed in previous email, the reason why I tried to build connection 
between governor and EPP-mode is  for those users,
who chooses EPP-mode, and tries use governor to deliver their performance 
policy, such as defining such cpufreq cmdline
"cpufreq=performance,amd-cppc,active"
Quoting what I replied in another thread:

The only useful info in governor for epp mode, IMO, is its name.
I deduce which performance policy user wants to apply through which governor 
they choose.
If user chooses performance governor, they want maximum performance.
If user chooses powersave governor, they want the least power consumption I 
could only provide appropriate epp value in above two scenarios.
ondemand and userspace are forbidden choices, and if users specify such options 
in cmdline, I shall give warning message to say  that amd-cppc in active mode 
is governor-less, and users could set epp values on runtime to specify bias 
towards performance or efficiency.

> Janr
>
> > Hardware itself will automatically calculate and set frequency. User only 
> > shall set
> min_perf, max_perf, and EPP at the beginning.
> > EPP is a preference ratio value towards performance over powersave, on
> > the scale of 0-255, 0 as 100% percentage favoring performance, while 255 as
> 100% percentage favoring powersave Non-EPP driver is still relying on Xen
> governor to do the tuning.
> >
> >> Jan