RE: [PATCH v8 8/8] xen/cpufreq: 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: Wednesday, September 3, 2025 2:22 PM
> To: Penny, Zheng <penny.zheng@xxxxxxx>
> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Anthony PERARD
> <anthony.perard@xxxxxxxxxx>; Andrew Cooper <andrew.cooper3@xxxxxxxxxx>;
> Roger Pau Monné <roger.pau@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> Andryuk, Jason <Jason.Andryuk@xxxxxxx>
> Subject: Re: [PATCH v8 8/8] xen/cpufreq: Adapt SET/GET_CPUFREQ_CPPC
> xen_sysctl_pm_op for amd-cppc driver
>
> On 03.09.2025 05:14, Penny, Zheng wrote:
> > [Public]
> >
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@xxxxxxxx>
> >> Sent: Thursday, August 28, 2025 7:07 PM
> >> To: Penny, Zheng <penny.zheng@xxxxxxx>
> >> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Anthony PERARD
> >> <anthony.perard@xxxxxxxxxx>; Andrew Cooper
> >> <andrew.cooper3@xxxxxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>;
> >> xen-devel@xxxxxxxxxxxxxxxxxxxx
> >> Subject: Re: [PATCH v8 8/8] xen/cpufreq: Adapt SET/GET_CPUFREQ_CPPC
> >> xen_sysctl_pm_op for amd-cppc driver
> >>
> >> On 28.08.2025 12:06, Penny Zheng wrote:
> >>> @@ -154,6 +156,17 @@ static int get_cpufreq_para(struct
> >>> xen_sysctl_pm_op
> >> *op)
> >>>      else
> >>>          strlcpy(op->u.get_para.scaling_driver, "Unknown",
> >>> CPUFREQ_NAME_LEN);
> >>>
> >>> +    /*
> >>> +     * In CPPC active mode, we are borrowing governor field to indicate
> >>> +     * policy info.
> >>> +     */
> >>> +    if ( policy->governor->name[0] )
> >>> +        strlcpy(op->u.get_para.u.s.scaling_governor,
> >>> +                policy->governor->name, CPUFREQ_NAME_LEN);
> >>> +    else
> >>> +        strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown",
> >>> +                CPUFREQ_NAME_LEN);
> >>
> >> Isn't pulling this ...
> >>
> >>>      if ( !cpufreq_is_governorless(op->cpuid) )
> >>>      {
> >>>          if ( !(scaling_available_governors =
> >>
> >> ... out of this if()'s body going to affect HWP? It's not clear to me
> >> whether that would be entirely benign.
> >
> > HWP has its own unique "hwp" governor. So, imo, it may not affect.
>
> How does it matter what (unique or not) governor it uses? The relevant aspect 
> (to
> me) is the !cpufreq_is_governorless() check that previously guarded the 
> copying
> of the name. (It would be another thing if this was benign to HWP, but such 
> would
> need clarifying in the description. Cc-ing Jason just in case.)

Sorry, What I mean is that HWP do have a governor, so such copying of the name 
shall be benign to the HWP. I'll clarify it in the description

>
> Jan