On 2025-09-02 23: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.
get_hwp_para() writes into the same union:
op->u.get_para.u.cppc_para
op->u.get_para.u.s.scaling_governor
Which is why I avoided it for hwp.
I guess writing scaling_governor first and then overwriting it still
ends up with the same data in cppc_para. Seems a little messy though.
Penny, I'm confused by this comment:
+ /*
+ * In CPPC active mode, we are borrowing governor field to indicate
+ * policy info.
+ */
You have CPPC active and passive modes - which uses a governor and which
uses get_cppc?
It seems like only writing the scaling governor inside
if ( !cpufreq_is_governorless )
should be correct since it's using the union. Am I missing something?
Thanks,
Jason
