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
