[Intel-gfx] [core-for-CI] x86/topology: fix erroneous smp_num_siblings on Intel Hybrid platform

[Saarinen, Jani]

Hi, 
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Imre Deak
> Sent: maanantai 27. maaliskuuta 2023 15.11
> To: intel-gfx at lists.freedesktop.org
> Subject: [Intel-gfx] [core-for-CI] x86/topology: fix erroneous smp_num_siblings on
> Intel Hybrid platform
> 
> From: Zhang Rui <rui.zhang at intel.com>
> 
> The SMT siblings value returned by CPUID.1F SMT level EBX differs among CPUs on
> Intel Hybrid platforms like AlderLake and MeteorLake.
> It returns 2 for Pcore CPUs which have SMT siblings and returns 1 for Ecore CPUs
> which do not have SMT siblings.
> 
> Today, the CPU boot code sets the global variable smp_num_siblings when every
> CPU thread is brought up. The last thread to boot will overwrite it with the number
> of siblings of *that* thread. That last thread to boot will "win". If the thread is a
> Pcore, smp_num_siblings == 2.  If it is an Ecore, smp_num_siblings == 1.
> 
> smp_num_siblings describes if the *system* supports SMT.  It should specify the
> maximum number of SMT threads among all cores.
> 
> Ensure that smp_num_siblings represents the system-wide maximum number of
> siblings by always increasing its value. Never allow it to decrease.
> 
> On MeteorLake-P platform, this fixes a problem that the Ecore CPUs are not updated
> in any cpu sibling map because the system is treated as an UP system when probing
> Ecore CPUs.
> 
> Below shows part of the CPU topology information before and after the fix, for both
> Pcore and Ecore CPU (cpu0 is Pcore, cpu 12 is Ecore).

Tested-By: Jani Saarinen <jani.saarinen at intel.com> on local MTL setup. Also tested earlier on other CI systems 
by: https://patchwork.freedesktop.org/series/115601/ trybot series. 
For this there is https://gitlab.freedesktop.org/drm/intel/-/issues/8317 

Br,
Jani

> ...
> -/sys/devices/system/cpu/cpu0/topology/package_cpus:000fff
> -/sys/devices/system/cpu/cpu0/topology/package_cpus_list:0-11
> +/sys/devices/system/cpu/cpu0/topology/package_cpus:3fffff
> +/sys/devices/system/cpu/cpu0/topology/package_cpus_list:0-21
> ...
> -/sys/devices/system/cpu/cpu12/topology/package_cpus:001000
> -/sys/devices/system/cpu/cpu12/topology/package_cpus_list:12
> +/sys/devices/system/cpu/cpu12/topology/package_cpus:3fffff
> +/sys/devices/system/cpu/cpu12/topology/package_cpus_list:0-21
> 
> And this also breaks userspace tools like lscpu
> -Core(s) per socket:  1
> -Socket(s):           11
> +Core(s) per socket:  16
> +Socket(s):           1
> 
> CC: stable at kernel.org
> Fixes: bbb65d2d365e ("x86: use cpuid vector 0xb when available for detecting cpu
> topology")
> Fixes: 95f3d39ccf7a ("x86/cpu/topology: Provide
> detect_extended_topology_early()")
> Suggested-by: Len Brown <len.brown at intel.com>
> Signed-off-by: Zhang Rui <rui.zhang at intel.com>
> Acked-by: Peter Zijlstra (Intel) <peterz at infradead.org>
> [Imre: resend for core-for-CI]
> References: https://lore.kernel.org/lkml/20230323015640.27906-1-
> rui.zhang at intel.com
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/8317
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>  arch/x86/kernel/cpu/topology.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/topology.c b/arch/x86/kernel/cpu/topology.c index
> 5e868b62a7c4e..0270925fe013b 100644
> --- a/arch/x86/kernel/cpu/topology.c
> +++ b/arch/x86/kernel/cpu/topology.c
> @@ -79,7 +79,7 @@ int detect_extended_topology_early(struct cpuinfo_x86 *c)
>  	 * initial apic id, which also represents 32-bit extended x2apic id.
>  	 */
>  	c->initial_apicid = edx;
> -	smp_num_siblings = LEVEL_MAX_SIBLINGS(ebx);
> +	smp_num_siblings = max_t(int, smp_num_siblings,
> +LEVEL_MAX_SIBLINGS(ebx));
>  #endif
>  	return 0;
>  }
> @@ -109,7 +109,8 @@ int detect_extended_topology(struct cpuinfo_x86 *c)
>  	 */
>  	cpuid_count(leaf, SMT_LEVEL, &eax, &ebx, &ecx, &edx);
>  	c->initial_apicid = edx;
> -	core_level_siblings = smp_num_siblings = LEVEL_MAX_SIBLINGS(ebx);
> +	core_level_siblings = LEVEL_MAX_SIBLINGS(ebx);
> +	smp_num_siblings = max_t(int, smp_num_siblings,
> +LEVEL_MAX_SIBLINGS(ebx));
>  	core_plus_mask_width = ht_mask_width = BITS_SHIFT_NEXT_LEVEL(eax);
>  	die_level_siblings = LEVEL_MAX_SIBLINGS(ebx);
>  	pkg_mask_width = die_plus_mask_width = BITS_SHIFT_NEXT_LEVEL(eax);
> --
> 2.37.2