Re: [PATCH] cpufreq/amd-pstate: Detect preferred core support before driver registration

["Gautham R. Shenoy" <gautham.shenoy@amd.com>]

On Tue, Dec 10, 2024 at 03:25:57AM +0000, K Prateek Nayak wrote:
> Booting with amd-pstate on 3rd Generation EPYC system incorrectly
> enabled ITMT support despite the system not supporting Preferred Core
> ranking. amd_pstate_init_prefcore() called during amd_pstate*_cpu_init()
> requires "amd_pstate_prefcore" to be set correctly however the preferred
> core support is detected only after driver registration which is too
> late.
> 
> Swap the function calls around to detect preferred core support before
> registring the driver via amd_pstate_register_driver(). This ensures
> amd_pstate*_cpu_init() sees the correct value of "amd_pstate_prefcore"
> considering the platform support.
> 
> Fixes: 279f838a61f9 ("x86/amd: Detect preferred cores in amd_get_boost_ratio_numerator()")
> Fixes: ff2653ded4d9 ("cpufreq/amd-pstate: Move registration after static function call update")
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>

Thanks for this fix.

So, the test for this is to check the `sched_domain` flags for
SD_ASYM_PACKING on EPYC, right?

We should add this to our test harness to catch any regressions in the future.

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>


--
Thanks and Regards
gautham.



> ---
> This patch is based on the latest superm1/linux:bleeding-edge and was
> also tested on v6.13-rc2 upstream release. Following is the behavior on
> a 3rd Generation EPYC system with and without this fix:
> 
> o v6.13-rc2
> 
>     # cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_driver
>     amd-pstate
> 
>     # cat /proc/sys/kernel/sched_itmt_enabled
>     1
> 
>     # echo Y > /sys/kernel/debug/sched/verbose
>     # cat /sys/kernel/debug/sched/domains/cpu0/domain*/flags
>     SD_BALANCE_NEWIDLE SD_BALANCE_EXEC SD_BALANCE_FORK SD_WAKE_AFFINE SD_SHARE_CPUCAPACITY SD_SHARE_LLC SD_PREFER_SIBLING
>     SD_BALANCE_NEWIDLE SD_BALANCE_EXEC SD_BALANCE_FORK SD_WAKE_AFFINE SD_SHARE_LLC SD_ASYM_PACKING SD_PREFER_SIBLING
>     ...
> 
> o v6.13-rc2 + this patch
> 
>     # cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_driver
>     amd-pstate
> 
>     # cat /proc/sys/kernel/sched_itmt_enabled
>     cat: /proc/sys/kernel/sched_itmt_enabled: No such file or directory
> 
>     root@yamuna:/home/amd# echo Y > /sys/kernel/debug/sched/verbose
>     root@yamuna:/home/amd# cat /sys/kernel/debug/sched/domains/cpu0/domain*/flags
>     SD_BALANCE_NEWIDLE SD_BALANCE_EXEC SD_BALANCE_FORK SD_WAKE_AFFINE SD_SHARE_CPUCAPACITY SD_SHARE_LLC SD_PREFER_SIBLING
>     SD_BALANCE_NEWIDLE SD_BALANCE_EXEC SD_BALANCE_FORK SD_WAKE_AFFINE SD_SHARE_LLC SD_PREFER_SIBLING
>     ...
> 
> System was booted with "amd_pstate=passive" cmdline.
> ---
>  drivers/cpufreq/amd-pstate.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 66fb7aee95d2..cb03f7d6575c 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1862,18 +1862,18 @@ static int __init amd_pstate_init(void)
>  		static_call_update(amd_pstate_set_epp, shmem_set_epp);
>  	}
>  
> -	ret = amd_pstate_register_driver(cppc_state);
> -	if (ret) {
> -		pr_err("failed to register with return %d\n", ret);
> -		return ret;
> -	}
> -
>  	if (amd_pstate_prefcore) {
>  		ret = amd_detect_prefcore(&amd_pstate_prefcore);
>  		if (ret)
>  			return ret;
>  	}
>  
> +	ret = amd_pstate_register_driver(cppc_state);
> +	if (ret) {
> +		pr_err("failed to register with return %d\n", ret);
> +		return ret;
> +	}
> +
>  	dev_root = bus_get_dev_root(&cpu_subsys);
>  	if (dev_root) {
>  		ret = sysfs_create_group(&dev_root->kobj, &amd_pstate_global_attr_group);
> 
> base-commit: 1f2f221668b210107f1277901bb757f1d77de842
> -- 
> 2.34.1
>