Re: [PATCH v6 17/19] xen/cpufreq: introduce helper cpufreq_in_cppc_passive_mode()

[Jan Beulich <jbeulich@xxxxxxxx>]

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