Re: [PATCH v10 7/8] xen/cpufreq: Adapt SET/GET_CPUFREQ_CPPC xen_sysctl_pm_op for amd-cppc driver

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.09.2025 06:38, 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] )

amd_cppc_prepare_policy() may leave ->governor set to NULL afaics, so I
think you need to add a NULL check here alongside with pulling this out
of ...

> +        strlcpy(op->u.get_para.s.scaling_governor,
> +                policy->governor->name, CPUFREQ_NAME_LEN);
> +    else
> +        strlcpy(op->u.get_para.s.scaling_governor, "Unknown",
> +                CPUFREQ_NAME_LEN);
> +
>      if ( !cpufreq_is_governorless(op->cpuid) )
>      {

... this conditional.

The description also continues to not mention the effect for HWP. I'm
actually somewhat confused, I suppose (Jason, question mainly to you):
HWP falls in the governor-less category, iirc. Yet it doesn't supply
a .setpolicy hook, hence __cpufreq_set_policy() goes through the normal
governor setting logic. What's the deal here? The answer may affect
whether I'd deem the pulling out of the conditional correct (or at least
benign) here as to HWP.

Jan

> @@ -178,13 +191,6 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
>          op->u.get_para.s.scaling_max_freq = policy->max;
>          op->u.get_para.s.scaling_min_freq = policy->min;
>  
> -        if ( policy->governor->name[0] )
> -            strlcpy(op->u.get_para.s.scaling_governor,
> -                    policy->governor->name, CPUFREQ_NAME_LEN);
> -        else
> -            strlcpy(op->u.get_para.s.scaling_governor, "Unknown",
> -                    CPUFREQ_NAME_LEN);
> -
>          /* governor specific para */
>          if ( !strncasecmp(op->u.get_para.s.scaling_governor,
>                            "userspace", CPUFREQ_NAME_LEN) )