Re: git: 6badb512a94d - main - Prefer CPUID leaf 1Fh for Intel CPU topology detection.

From: Rodney W. Grimes <freebsd_at_gndrsh.dnsmgr.net>
Date: Sat, 06 Nov 2021 15:16:56 UTC
> The branch main has been updated by mav:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=6badb512a94df667f0df1484fb288ece186305bd
> 
> commit 6badb512a94df667f0df1484fb288ece186305bd
> Author:     Alexander Motin <mav@FreeBSD.org>
> AuthorDate: 2021-11-06 04:48:37 +0000
> Commit:     Alexander Motin <mav@FreeBSD.org>
> CommitDate: 2021-11-06 04:53:52 +0000
> 
>     Prefer CPUID leaf 1Fh for Intel CPU topology detection.
>     
>     Leaf 1Fh is a prefered extended version of 0Bh.  It is supported by
>     new Lader Lake CPUs, though does not report anything new so far.
>     
>     MFC after:      2 weeks
> ---
>  sys/x86/x86/mp_x86.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/sys/x86/x86/mp_x86.c b/sys/x86/x86/mp_x86.c
> index 7a72c501ff25..3b0e25172d0d 100644
> --- a/sys/x86/x86/mp_x86.c
> +++ b/sys/x86/x86/mp_x86.c
> @@ -380,7 +380,7 @@ topo_probe_intel_0x4(void)
>  
>  /*
>   * Determine topology of processing units for Intel CPUs
> - * using CPUID Leaf 11, if supported.
> + * using CPUID Leaf 1Fh or 0Bh, if supported.
>   * See:
>   *  - Intel 64 Architecture Processor Topology Enumeration
>   *  - Intel 64 and IA-32 ArchitecturesSoftware Developer?s Manual,
> @@ -390,13 +390,23 @@ topo_probe_intel_0x4(void)
>  static void
>  topo_probe_intel_0xb(void)
>  {
> -	u_int p[4];
> +	u_int leaf;
> +	u_int p[4] = { 0 };
>  	int bits;
>  	int type;
>  	int i;
>  
> -	/* Fall back if CPU leaf 11 doesn't really exist. */
> -	cpuid_count(0x0b, 0, p);
> +	/* Prefer leaf 1Fh (V2 Extended Topology Enumeration). */
> +	if (cpu_high >= 0x1f) {
> +		leaf = 0x1f;
> +		cpuid_count(leaf, 0, p);
> +	}
> +	/* Fall back to leaf 0Bh (Extended Topology Enumeration). */
> +	if (p[1] == 0) {
> +		leaf = 0x0b;
> +		cpuid_count(leaf, 0, p);
> +	}
> +	/* Fall back to leaf 04h (Deterministic Cache Parameters). */
>  	if (p[1] == 0) {
>  		topo_probe_intel_0x4();
>  		return;
> @@ -404,7 +414,7 @@ topo_probe_intel_0xb(void)
>  
>  	/* We only support three levels for now. */
>  	for (i = 0; ; i++) {
> -		cpuid_count(0x0b, i, p);
> +		cpuid_count(leaf, i, p);
>  
>  		bits = p[0] & 0x1f;
>  		type = (p[2] >> 8) & 0xff;
> @@ -412,13 +422,12 @@ topo_probe_intel_0xb(void)
>  		if (type == 0)
>  			break;
>  
> -		/* TODO: check for duplicate (re-)assignment */
>  		if (type == CPUID_TYPE_SMT)
>  			core_id_shift = bits;
>  		else if (type == CPUID_TYPE_CORE)
>  			pkg_id_shift = bits;
> -		else
> -			printf("unknown CPU level type %d\n", type);

Why do we loose this potential diagnostic message,
and isnt its trailing \n needed to close the probe message?
It would need to be come an else of the if (bootverbose) if
I am reading things correctly.

> +		else if (bootverbose)
> +			printf("Topology level type %d shift: %d\n", type, bits);
>  	}
>  
>  	if (pkg_id_shift < core_id_shift) {
> 

-- 
Rod Grimes                                                 rgrimes@freebsd.org