On 26.08.2025 10:21, Penny, Zheng wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeul...@suse.com>
>> Sent: Monday, August 25, 2025 11:37 PM
>>
>> On 22.08.2025 12:52, Penny Zheng wrote:
>>> --- a/tools/libs/ctrl/xc_pm.c
>>> +++ b/tools/libs/ctrl/xc_pm.c
>>> @@ -288,7 +288,6 @@ int xc_get_cpufreq_para(xc_interface *xch, int cpuid,
>>> CHK_FIELD(s.scaling_min_freq);
>>> CHK_FIELD(s.u.userspace);
>>> CHK_FIELD(s.u.ondemand);
>>> - CHK_FIELD(cppc_para);
>>>
>>> #undef CHK_FIELD
>>
>> What is done here is already less than what could be done; I think ...
>>
>
> Emm, maybe because we define two different cpufreq para structures for user
> space and sysctl, struct xc_get_cpufreq_para and struct xen_get_cppc_para.
> But for cppc para, it is an alias:
> typedef struct xen_get_cppc_para xc_cppc_para_t;
Oh. Then ...
> So ...
>
>>> @@ -366,6 +365,33 @@ int xc_set_cpufreq_cppc(xc_interface *xch, int cpuid,
>>> return ret;
>>> }
>>>
>>> +int xc_get_cppc_para(xc_interface *xch, unsigned int cpuid,
>>> + xc_cppc_para_t *cppc_para) {
>>> + int ret;
>>> + struct xen_sysctl sysctl = {};
>>> + struct xen_get_cppc_para *sys_cppc_para =
>>> +&sysctl.u.pm_op.u.get_cppc;
>>> +
>>> + if ( !xch || !cppc_para )
>>> + {
>>> + errno = EINVAL;
>>> + return -1;
>>> + }
>>> +
>>> + sysctl.cmd = XEN_SYSCTL_pm_op;
>>> + sysctl.u.pm_op.cmd = GET_CPUFREQ_CPPC;
>>> + sysctl.u.pm_op.cpuid = cpuid;
>>> +
>>> + ret = xc_sysctl(xch, &sysctl);
>>> + if ( ret )
>>> + return ret;
>>> +
>>> + BUILD_BUG_ON(sizeof(*cppc_para) != sizeof(*sys_cppc_para));
... why is this here, when ...
>>> + memcpy(cppc_para, sys_cppc_para, sizeof(*sys_cppc_para));
>>
>> ... you minimally want to apply as much checking here.
... a better effect can be had by
cppc_para = sys_cppc_para;
?
Jan
