[PATCH v2] x86/cpu: Enable SD_ASYM_PACKING for PKG domain on systems with AMD preferred cores

[PATCH v2] x86/cpu: Enable SD_ASYM_PACKING for PKG domain on systems with AMD preferred cores

[Mario Limonciello]

For the scheduler to use and prefer AMD preferred core rankings set
SD_ASYM_PACKING for x86_die_flags().

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v2:
 * Fix c23 compatibility issue reported by LKP
---
 arch/x86/kernel/smpboot.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index b5a8f0891135b..6a38cf3feb1a9 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -62,6 +62,8 @@
 #include <linux/mc146818rtc.h>
 #include <linux/acpi.h>
 
+#include <acpi/cppc_acpi.h>
+
 #include <asm/acpi.h>
 #include <asm/cacheinfo.h>
 #include <asm/desc.h>
@@ -501,6 +503,15 @@ static int x86_die_flags(void)
 	    cpu_feature_enabled(X86_FEATURE_AMD_HETEROGENEOUS_CORES))
 		return x86_sched_itmt_flags();
 
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD ||
+	    boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
+		bool prefcore = false;
+
+		amd_detect_prefcore(&prefcore);
+		if (prefcore)
+			return x86_sched_itmt_flags();
+	}
+
 	return 0;
 }
 

base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
-- 
2.43.0

Re: [PATCH v2] x86/cpu: Enable SD_ASYM_PACKING for PKG domain on systems with AMD preferred cores

[Peter Jung]

This does fix the preferred core behavior now and it works now as it should.
Tested with a 9950X on several games, and the scheduler now picks the 
correct cores, which results into a heavy performance improvement, due 
not migrating through the second ccd.

Thanks!

Tested-by: Peter Jung <ptr1337@cachyos.org>


On 03.12.24 21:11, Mario Limonciello wrote:
> For the scheduler to use and prefer AMD preferred core rankings set
> SD_ASYM_PACKING for x86_die_flags().
> 
> Signed-off-by: Mario Limonciello<mario.limonciello@amd.com>
> ---
> v2:
>   * Fix c23 compatibility issue reported by LKP
> ---
>   arch/x86/kernel/smpboot.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index b5a8f0891135b..6a38cf3feb1a9 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -62,6 +62,8 @@
>   #include <linux/mc146818rtc.h>
>   #include <linux/acpi.h>
>   
> +#include <acpi/cppc_acpi.h>
> +
>   #include <asm/acpi.h>
>   #include <asm/cacheinfo.h>
>   #include <asm/desc.h>
> @@ -501,6 +503,15 @@ static int x86_die_flags(void)
>   	    cpu_feature_enabled(X86_FEATURE_AMD_HETEROGENEOUS_CORES))
>   		return x86_sched_itmt_flags();
>   
> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD ||
> +	    boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
> +		bool prefcore = false;
> +
> +		amd_detect_prefcore(&prefcore);
> +		if (prefcore)
> +			return x86_sched_itmt_flags();
> +	}
> +
>   	return 0;
>   }
>   
> 
> base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
> -- 2.43.0
> 

Re: [PATCH v2] x86/cpu: Enable SD_ASYM_PACKING for PKG domain on systems with AMD preferred cores

[Borislav Petkov]

On Tue, Dec 03, 2024 at 02:11:29PM -0600, Mario Limonciello wrote:
> For the scheduler to use and prefer AMD preferred core rankings set
> SD_ASYM_PACKING for x86_die_flags().
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v2:
>  * Fix c23 compatibility issue reported by LKP
> ---
>  arch/x86/kernel/smpboot.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index b5a8f0891135b..6a38cf3feb1a9 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -62,6 +62,8 @@
>  #include <linux/mc146818rtc.h>
>  #include <linux/acpi.h>
>  
> +#include <acpi/cppc_acpi.h>
> +
>  #include <asm/acpi.h>
>  #include <asm/cacheinfo.h>
>  #include <asm/desc.h>
> @@ -501,6 +503,15 @@ static int x86_die_flags(void)
>  	    cpu_feature_enabled(X86_FEATURE_AMD_HETEROGENEOUS_CORES))
>  		return x86_sched_itmt_flags();
>  
> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD ||
> +	    boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {

You're going to call this on *every* AMD and on Hygon?

So that whole effort with X86_FEATURE_s was for nothing?

What's up?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v2] x86/cpu: Enable SD_ASYM_PACKING for PKG domain on systems with AMD preferred cores

[Mario Limonciello]

On 12/3/2024 15:54, Borislav Petkov wrote:
> On Tue, Dec 03, 2024 at 02:11:29PM -0600, Mario Limonciello wrote:
>> For the scheduler to use and prefer AMD preferred core rankings set
>> SD_ASYM_PACKING for x86_die_flags().
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> v2:
>>   * Fix c23 compatibility issue reported by LKP
>> ---
>>   arch/x86/kernel/smpboot.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
>> index b5a8f0891135b..6a38cf3feb1a9 100644
>> --- a/arch/x86/kernel/smpboot.c
>> +++ b/arch/x86/kernel/smpboot.c
>> @@ -62,6 +62,8 @@
>>   #include <linux/mc146818rtc.h>
>>   #include <linux/acpi.h>
>>   
>> +#include <acpi/cppc_acpi.h>
>> +
>>   #include <asm/acpi.h>
>>   #include <asm/cacheinfo.h>
>>   #include <asm/desc.h>
>> @@ -501,6 +503,15 @@ static int x86_die_flags(void)
>>   	    cpu_feature_enabled(X86_FEATURE_AMD_HETEROGENEOUS_CORES))
>>   		return x86_sched_itmt_flags();
>>   
>> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD ||
>> +	    boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
> 
> You're going to call this on *every* AMD and on Hygon?
> 
> So that whole effort with X86_FEATURE_s was for nothing?
> 
> What's up?
> 

Preferred core classifications are available on "non-heterogenous" 
designs for a few generations.  There isn't an indication they're 
supported which is why amd_detect_prefcore() was made.

That's already called during the boot either way because that is used
to identify the boost numerator.  The boolean value it finds is cached, 
and the next call will use the cached value.  So I don't expect this 
change affects boot speed.

Re: [PATCH v2] x86/cpu: Enable SD_ASYM_PACKING for PKG domain on systems with AMD preferred cores

[Borislav Petkov]

On Tue, Dec 03, 2024 at 04:02:29PM -0600, Mario Limonciello wrote:
> Preferred core classifications are available on "non-heterogenous" designs
> for a few generations.  There isn't an indication they're supported which is
> why amd_detect_prefcore() was made.

Not surprised.

> That's already called during the boot either way because that is used
> to identify the boost numerator.  The boolean value it finds is cached, and
> the next call will use the cached value.  So I don't expect this change
> affects boot speed.

So with this addition, amd_detect_prefcore() would get called *three* times
where you need to call it exactly and only once. And all it gives you a one
bit of information which states that the system has preferred cores.

So why don't you define a synthetic X86_FEATURE:

	X86_FEATURE_AMD_PREFERRED_CORES

run this code *once* and *early* enough so that every user can have the result
ready, if you have preferred cores, do

	setup_force_cpu_cap(X86_FEATURE_AMD_PREFERRED_CORES)

(it needs to run before alternatives) and then in each user, you do

	if (cpu_feature_enabled(X86_FEATURE_AMD_PREFERRED_CORES))
		/* do things */

instead of what you have now?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v2] x86/cpu: Enable SD_ASYM_PACKING for PKG domain on systems with AMD preferred cores

[Mario Limonciello]

On 12/3/2024 16:17, Borislav Petkov wrote:
> On Tue, Dec 03, 2024 at 04:02:29PM -0600, Mario Limonciello wrote:
>> Preferred core classifications are available on "non-heterogenous" designs
>> for a few generations.  There isn't an indication they're supported which is
>> why amd_detect_prefcore() was made.
> 
> Not surprised.
> 
>> That's already called during the boot either way because that is used
>> to identify the boost numerator.  The boolean value it finds is cached, and
>> the next call will use the cached value.  So I don't expect this change
>> affects boot speed.
> 
> So with this addition, amd_detect_prefcore() would get called *three* times
> where you need to call it exactly and only once. And all it gives you a one
> bit of information which states that the system has preferred cores.
> 
> So why don't you define a synthetic X86_FEATURE:
> 
> 	X86_FEATURE_AMD_PREFERRED_CORES
> 
> run this code *once* and *early* enough so that every user can have the result
> ready, if you have preferred cores, do
> 
> 	setup_force_cpu_cap(X86_FEATURE_AMD_PREFERRED_CORES)
> 
> (it needs to run before alternatives) and then in each user, you do
> 
> 	if (cpu_feature_enabled(X86_FEATURE_AMD_PREFERRED_CORES))
> 		/* do things */
> 
> instead of what you have now?
> 
> Thx.
> 

Sure, that sounds reasonable, I'll try to find the most appropriate call 
site for it.