Re: [PATCH 02/49] x86/cpu: Describe hybrid CPUs in cpuinfo_x86

2021-02-08 Thread Borislav Petkov
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

2021-02-08 Thread Luck, Tony
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

2021-02-08 Thread Liang, Kan




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

2021-02-08 Thread Borislav Petkov
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