| Message ID | 20241210032557.754-1-kprateek.nayak@amd.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Mario Limonciello |
| Headers | show |
| Series | cpufreq/amd-pstate: Detect preferred core support before driver registration | expand |
On 12/9/2024 21:25, 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 the fix. I'll apply this to my fixes branch, do some further testing and will include this in a future 6.13-fixes PR. Acked-by: Mario Limonciello <mario.limonciello@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(-) > > 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
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 --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);
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