On 23.09.2025 06:38, Penny Zheng wrote:
> We extract cppc info from "struct xen_get_cpufreq_para", where it acts as
> a member of union, and share the space with governor info.
> However, it may fail in amd-cppc passive mode, in which governor info and
> CPPC info could co-exist, and both need to be printed together via xenpm tool.
> If we tried to still put it in "struct xen_get_cpufreq_para" (e.g. just move
> out of union), "struct xen_get_cpufreq_para" will enlarge too much to further
> make xen_sysctl.u exceed 128 bytes.
>
> So we introduce a new sub-field GET_CPUFREQ_CPPC to dedicatedly acquire
> CPPC-related para, and make get-cpufreq-para invoke GET_CPUFREQ_CPPC
> if available.
> New helpers print_cppc_para() and get_cpufreq_cppc() are introduced to
> extract CPPC-related parameters process from cpufreq para.
>
> Signed-off-by: Penny Zheng <[email protected]>
> Acked-by: Jan Beulich <[email protected]> # hypervisor
> Acked-by: Anthony PERARD <[email protected]>
> ---
> v4 -> v5:
> - new commit
> ---
> v5 -> v6:
> - remove the changes for get-cpufreq-para
> ---
> v6 -> v7:
> - make get-cpufreq-para invoke GET_CPUFREQ_CPPC
> ---
> v7 -> v8:
> - use structure assignment as it is a alias
> - add errno info to the error print
> ---
> v9 -> v10
> - drop the outer union
In the interest of getting this series in I think we will want to take
this patch as is (I yet have to see the other, related patch though),
but ...
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -478,22 +478,19 @@ struct xen_get_cpufreq_para {
> uint32_t cpuinfo_cur_freq;
> uint32_t cpuinfo_max_freq;
> uint32_t cpuinfo_min_freq;
> - union {
> - struct {
> - uint32_t scaling_cur_freq;
> -
> - char scaling_governor[CPUFREQ_NAME_LEN];
> - uint32_t scaling_max_freq;
> - uint32_t scaling_min_freq;
> -
> - /* for specific governor */
> - union {
> - struct xen_userspace userspace;
> - struct xen_ondemand ondemand;
> - } u;
> - } s;
> - struct xen_get_cppc_para cppc_para;
> - } u;
> + struct {
> + uint32_t scaling_cur_freq;
> +
> + char scaling_governor[CPUFREQ_NAME_LEN];
> + uint32_t scaling_max_freq;
> + uint32_t scaling_min_freq;
> +
> + /* for specific governor */
> + union {
> + struct xen_userspace userspace;
> + struct xen_ondemand ondemand;
> + } u;
> + } s;
... I don't quite see why we'd need to retain the nested struct now
either. Imo we ought to be cleaning this up for 4.22.
Jan
