On 03.09.2025 08:46, Penny, Zheng wrote:
> [Public]
>
>> -----Original Message-----
>> From: Jan Beulich <jbeul...@suse.com>
>> Sent: Wednesday, September 3, 2025 2:22 PM
>> To: Penny, Zheng <penny.zh...@amd.com>
>> Cc: Huang, Ray <ray.hu...@amd.com>; Anthony PERARD
>> <anthony.per...@vates.tech>; Andrew Cooper <andrew.coop...@citrix.com>;
>> Roger Pau Monné <roger....@citrix.com>; xen-devel@lists.xenproject.org;
>> Andryuk, Jason <jason.andr...@amd.com>
>> 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 <jbeul...@suse.com>
>>>> Sent: Thursday, August 28, 2025 7:07 PM
>>>> To: Penny, Zheng <penny.zh...@amd.com>
>>>> Cc: Huang, Ray <ray.hu...@amd.com>; Anthony PERARD
>>>> <anthony.per...@vates.tech>; Andrew Cooper
>>>> <andrew.coop...@citrix.com>; Roger Pau Monné <roger....@citrix.com>;
>>>> xen-devel@lists.xenproject.org
>>>> 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
FTAOD - "shall" isn't enough, it needs to be (provably) "is".
Jan
