Re: [PATCH 02/49] x86/cpu: Describe hybrid CPUs in cpuinfo_x86
On Mon, Feb 08, 2021 at 11:10:18AM -0800, Luck, Tony wrote: > > +u32 x86_read_hybrid_type(void) > > +{ > > + if (cpu_feature_enabled(X86_FEATURE_HYBRID_CPU)) > > + return cpuid_eax(0x001a); > > + > > + return 0; > > +} > > Machine check logging will want to include this in "struct mce". > > But ok to pick it up with a function like you describe above. Sure, that looks ok. We can always lift it up into cpuinfo_x86 later, when it is needed on the majority of machines but right now it is only a small subset of machines which have this. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 02/49] x86/cpu: Describe hybrid CPUs in cpuinfo_x86
On Mon, Feb 08, 2021 at 02:04:24PM -0500, Liang, Kan wrote: > On 2/8/2021 12:56 PM, Borislav Petkov wrote: > > I think it's good enough for perf, but I'm not sure whether other codes need > the CPU type information. > > Ricardo, do you know? > > Maybe we should implement a generic function as below for this? > (Not test. Just use as an example.) > > diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c > index a66c1fd..679f5fe 100644 > --- a/arch/x86/kernel/cpu/common.c > +++ b/arch/x86/kernel/cpu/common.c > @@ -2056,3 +2056,11 @@ void arch_smt_update(void) > /* Check whether IPI broadcasting can be enabled */ > apic_smt_update(); > } > + > +u32 x86_read_hybrid_type(void) > +{ > + if (cpu_feature_enabled(X86_FEATURE_HYBRID_CPU)) > + return cpuid_eax(0x001a); > + > + return 0; > +} Machine check logging will want to include this in "struct mce". But ok to pick it up with a function like you describe above. -Tony
Re: [PATCH 02/49] x86/cpu: Describe hybrid CPUs in cpuinfo_x86
On 2/8/2021 12:56 PM, Borislav Petkov wrote: On Mon, Feb 08, 2021 at 07:24:59AM -0800, kan.li...@linux.intel.com wrote: diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index c20a52b..1f25ac9 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -139,6 +139,16 @@ struct cpuinfo_x86 { u32 microcode; /* Address space bits used by the cache internally */ u8 x86_cache_bits; + /* +* In hybrid processors, there is a CPU type and a native model ID. The +* CPU type (x86_cpu_type[31:24]) describes the type of micro- +* architecture families. The native model ID (x86_cpu_type[23:0]) +* describes a specific microarchitecture version. Combining both +* allows to uniquely identify a CPU. +* +* Please note that the native model ID is not related to x86_model. +*/ + u32 x86_cpu_type; Why are you adding it here instead of simply using X86_FEATURE_HYBRID_CPU at the call site? How many uses in this patchset? /me searches... Exactly one. Just query X86_FEATURE_HYBRID_CPU at the call site and read what you need from CPUID and use it there - no need for this. I think it's good enough for perf, but I'm not sure whether other codes need the CPU type information. Ricardo, do you know? Maybe we should implement a generic function as below for this? (Not test. Just use as an example.) diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index a66c1fd..679f5fe 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -2056,3 +2056,11 @@ void arch_smt_update(void) /* Check whether IPI broadcasting can be enabled */ apic_smt_update(); } + +u32 x86_read_hybrid_type(void) +{ + if (cpu_feature_enabled(X86_FEATURE_HYBRID_CPU)) + return cpuid_eax(0x001a); + + return 0; +} Thanks, Kan
Re: [PATCH 02/49] x86/cpu: Describe hybrid CPUs in cpuinfo_x86
On Mon, Feb 08, 2021 at 07:24:59AM -0800, kan.li...@linux.intel.com wrote: > diff --git a/arch/x86/include/asm/processor.h > b/arch/x86/include/asm/processor.h > index c20a52b..1f25ac9 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -139,6 +139,16 @@ struct cpuinfo_x86 { > u32 microcode; > /* Address space bits used by the cache internally */ > u8 x86_cache_bits; > + /* > + * In hybrid processors, there is a CPU type and a native model ID. The > + * CPU type (x86_cpu_type[31:24]) describes the type of micro- > + * architecture families. The native model ID (x86_cpu_type[23:0]) > + * describes a specific microarchitecture version. Combining both > + * allows to uniquely identify a CPU. > + * > + * Please note that the native model ID is not related to x86_model. > + */ > + u32 x86_cpu_type; Why are you adding it here instead of simply using X86_FEATURE_HYBRID_CPU at the call site? How many uses in this patchset? /me searches... Exactly one. Just query X86_FEATURE_HYBRID_CPU at the call site and read what you need from CPUID and use it there - no need for this. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette