diff mbox series

cpufreq/amd-pstate: Detect preferred core support before driver registration

Message ID 20241210032557.754-1-kprateek.nayak@amd.com
State Accepted
Commit 919bfa9b2dbf3bc0c478afd4e44445836381dacb
Headers show
Series cpufreq/amd-pstate: Detect preferred core support before driver registration | expand

Commit Message

K Prateek Nayak Dec. 10, 2024, 3:25 a.m. UTC 
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>
---
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(-)


base-commit: 1f2f221668b210107f1277901bb757f1d77de842

Comments

Gautham R. Shenoy Dec. 11, 2024, 6:29 a.m. UTC | #1 
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
>
diff mbox series

Patch

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);