Re: [PATCH v8 8/8] xen/cpufreq: Adapt SET/GET_CPUFREQ_CPPC xen_sysctl_pm_op for amd-cppc driver

[Jan Beulich <jbeulich@xxxxxxxx>]

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.)

Jan