On 11.07.2025 05:51, Penny Zheng wrote:
> --- a/xen/drivers/acpi/pm-op.c
> +++ b/xen/drivers/acpi/pm-op.c
> @@ -152,7 +152,15 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> else
> strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
>
> - ret = get_cpufreq_cppc(op->cpuid, &op->u.get_para.u.cppc_para);
> + /*
> + * When cpufreq driver in cppc passive mode, it has both cppc and
> governor
> + * info. Users could only rely on "get-cpufreq-cppc" to acquire CPPC
> info.
> + * And it returns governor info in "get-cpufreq-para"
> + */
Which of the two they need to invoke to obtain a complete picture? Both?
I'm confused by you bypassing get_cpufreq_cppc() (i.e. get_hwp_para())
for yet another reason, when - aiui - some information there is relevant
in both active and passive modes.
> + if ( cpufreq_in_cppc_passive_mode(op->cpuid) )
> + ret = -ENODEV;
> + else
> + ret = get_cpufreq_cppc(op->cpuid, &op->u.get_para.u.cppc_para);
Any reason the extra check isn't put in get_cpufreq_cppc(), alongside the
hwp_active() one?
> --- a/xen/drivers/cpufreq/cpufreq.c
> +++ b/xen/drivers/cpufreq/cpufreq.c
> @@ -962,3 +962,9 @@ int __init cpufreq_register_driver(const struct
> cpufreq_driver *driver_data)
>
> return 0;
> }
> +
> +bool cpufreq_in_cppc_passive_mode(unsigned int cpuid)
> +{
> + return processor_pminfo[cpuid]->init & XEN_CPPC_INIT &&
Nit: Please use parentheses when using & and && together.
Also, isn't this function going to become unreachable when PM_OP=n, thus
violating Misra rule 2.1?
Jan
