Re: Kernel 6.9 regression: X86: Bogus messages from topology detection

[Christian Heusel]

On 24/05/30 06:24PM, Thomas Gleixner wrote:
> On Thu, May 30 2024 at 17:53, Thomas Gleixner wrote:
>
> > Let me figure out how to fix that sanely.
> 
> The proper fix is obviously to unlock CPUID on Intel _before_ anything
> which depends on cpuid_level is evaluated.
> 
> Thanks,
> 
>         tglx

Hey Thomas,

as reported on the other mail the proposed fix broke the build (see
below) due to get_cpu_cap() becoming static but still being used in
other parts of the code.

One of the reporters in the Arch Bugtracker with an Intel Core i7-7700k
has tested a modified version of this fix[0] with the static change
reversed on top of the 6.9.2 stable kernel and reports that the patch
does not fix the issue for them. I have attached their output for the
patched (dmesg6.9.2-1.5.log) and nonpatched (dmesg6.9.2-1.log) kernel.

Should we also get them to test the mainline version or do you need any
other debug output?

Cheers,
gromit

[0]: https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/issues/57#note_189079

> ---
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -969,7 +969,7 @@ static void init_speculation_control(str
>  	}
>  }
>  
> -void get_cpu_cap(struct cpuinfo_x86 *c)
> +static void get_cpu_cap(struct cpuinfo_x86 *c)

making this function static breaks the build for me:

arch/x86/xen/enlighten_pv.c: In function ‘xen_start_kernel’:
arch/x86/xen/enlighten_pv.c:1388:9: error: implicit declaration of function ‘get_cpu_cap’; did you mean ‘set_cpu_cap’? [-Wimplicit-function-declaration]
 1388 |         get_cpu_cap(&boot_cpu_data);
    ¦ |         ^~~~~~~~~~~
    ¦ |         set_cpu_cap


>  {
>  	u32 eax, ebx, ecx, edx;
>  
> @@ -1585,6 +1585,7 @@ static void __init early_identify_cpu(st
>  	if (have_cpuid_p()) {
>  		cpu_detect(c);
>  		get_cpu_vendor(c);
> +		intel_unlock_cpuid_leafs(c);
>  		get_cpu_cap(c);
>  		setup_force_cpu_cap(X86_FEATURE_CPUID);
>  		get_cpu_address_sizes(c);
> @@ -1744,7 +1745,7 @@ static void generic_identify(struct cpui
>  	cpu_detect(c);
>  
>  	get_cpu_vendor(c);
> -
> +	intel_unlock_cpuid_leafs(c);
>  	get_cpu_cap(c);
>  
>  	get_cpu_address_sizes(c);
> --- a/arch/x86/kernel/cpu/cpu.h
> +++ b/arch/x86/kernel/cpu/cpu.h
> @@ -61,14 +61,15 @@ extern __ro_after_init enum tsx_ctrl_sta
>  
>  extern void __init tsx_init(void);
>  void tsx_ap_init(void);
> +void intel_unlock_cpuid_leafs(struct cpuinfo_x86 *c);
>  #else
>  static inline void tsx_init(void) { }
>  static inline void tsx_ap_init(void) { }
> +static inline void intel_unlock_cpuid_leafs(struct cpuinfo_x86 *c) { }
>  #endif /* CONFIG_CPU_SUP_INTEL */
>  
>  extern void init_spectral_chicken(struct cpuinfo_x86 *c);
>  
> -extern void get_cpu_cap(struct cpuinfo_x86 *c);
>  extern void get_cpu_address_sizes(struct cpuinfo_x86 *c);
>  extern void cpu_detect_cache_sizes(struct cpuinfo_x86 *c);
>  extern void init_scattered_cpuid_features(struct cpuinfo_x86 *c);
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -269,19 +269,26 @@ static void detect_tme_early(struct cpui
>  	c->x86_phys_bits -= keyid_bits;
>  }
>  
> +void intel_unlock_cpuid_leafs(struct cpuinfo_x86 *c)
> +{
> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> +		return;
> +
> +	if (c->x86 < 6 || (c->x86 == 6 && c->x86_model < 0xd))
> +		return;
> +
> +	/*
> +	 * The BIOS can have limited CPUID to leaf 2, which breaks feature
> +	 * enumeration. Unlock it and update the maximum leaf info.
> +	 */
> +	if (msr_clear_bit(MSR_IA32_MISC_ENABLE, MSR_IA32_MISC_ENABLE_LIMIT_CPUID_BIT) > 0)
> +		c->cpuid_level = cpuid_eax(0);
> +}
> +
>  static void early_init_intel(struct cpuinfo_x86 *c)
>  {
>  	u64 misc_enable;
>  
> -	/* Unmask CPUID levels if masked: */
> -	if (c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xd)) {
> -		if (msr_clear_bit(MSR_IA32_MISC_ENABLE,
> -				  MSR_IA32_MISC_ENABLE_LIMIT_CPUID_BIT) > 0) {
> -			c->cpuid_level = cpuid_eax(0);
> -			get_cpu_cap(c);
> -		}
> -	}
> -
>  	if ((c->x86 == 0xf && c->x86_model >= 0x03) ||
>  		(c->x86 == 0x6 && c->x86_model >= 0x0e))
>  		set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
>

Attachment: signature.asc
Description: PGP signature