Re: [PATCH v4 1/2] cpuid: Add a helper in scattered.c to return cpuid leaf info

[Borislav Petkov <bp@alien8.de>]

On Tue, Nov 08, 2016 at 04:52:27PM +0800, He Chen wrote:
> Some sparse cpuid leafs are gathered in a fake leaf to save size of

s/cpuid/CPUID/

> x86_capability array in current code, but sometimes, kernel or other
> modules (e.g. KVM cpuid enumeration) may need actual hardware leaf
> information.
> 
> This patch adds a helper get_scattered_cpuid_leaf to rebuild actual

			   get_scattered_cpuid_leaf()

> cpuid leaf, and it can be called outside by modules.

s/cpuid/CPUID/

> Also, export
> enum cpuid_regs in pt.c and scattered.c to enum cpuid_regs_idx in
> processor.h.

No need for that last sentence - it is obvious when looking at the diff
itself.

> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 984a7bf..e7f8c62 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -137,6 +137,17 @@ struct cpuinfo_x86 {
>  	u32			microcode;
>  };
>  
> +struct cpuid_regs {
> +	u32 eax, ebx, ecx, edx;
> +};

Why do you export this? It is used in cpuid.c only.

> +enum cpuid_regs_idx {
> +	CPUID_EAX = 0,
> +	CPUID_EBX,
> +	CPUID_ECX,
> +	CPUID_EDX,
> +};
> +
>  #define X86_VENDOR_INTEL	0
>  #define X86_VENDOR_CYRIX	1
>  #define X86_VENDOR_AMD		2
> @@ -178,6 +189,9 @@ extern void identify_secondary_cpu(struct cpuinfo_x86 *);
>  extern void print_cpu_info(struct cpuinfo_x86 *);
>  void print_cpu_msr(struct cpuinfo_x86 *);
>  extern void init_scattered_cpuid_features(struct cpuinfo_x86 *c);
> +extern u32 get_scattered_cpuid_leaf(unsigned int level,
> +				    unsigned int sub_leaf,
> +				    enum cpuid_regs_idx reg);
>  extern unsigned int init_intel_cacheinfo(struct cpuinfo_x86 *c);
>  extern void init_amd_cacheinfo(struct cpuinfo_x86 *c);
>  
> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index 1db8dc4..ef131ea 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -17,11 +17,17 @@ struct cpuid_bit {
>  	u32 sub_leaf;
>  };
>  
> -enum cpuid_regs {
> -	CR_EAX = 0,
> -	CR_ECX,
> -	CR_EDX,
> -	CR_EBX
> +/* Please keep the leaf sorted by cpuid_bit.level for faster search. */
> +static const struct cpuid_bit cpuid_bits[] = {
> +	{ X86_FEATURE_APERFMPERF,       CPUID_ECX,  0, 0x00000006, 0 },
> +	{ X86_FEATURE_EPB,              CPUID_ECX,  3, 0x00000006, 0 },
> +	{ X86_FEATURE_INTEL_PT,         CPUID_EBX, 25, 0x00000007, 0 },
> +	{ X86_FEATURE_AVX512_4VNNIW,    CPUID_EDX,  2, 0x00000007, 0 },
> +	{ X86_FEATURE_AVX512_4FMAPS,    CPUID_EDX,  3, 0x00000007, 0 },
> +	{ X86_FEATURE_HW_PSTATE,        CPUID_EDX,  7, 0x80000007, 0 },
> +	{ X86_FEATURE_CPB,              CPUID_EDX,  9, 0x80000007, 0 },
> +	{ X86_FEATURE_PROC_FEEDBACK,    CPUID_EDX, 11, 0x80000007, 0 },
> +	{ 0, 0, 0, 0, 0 }
>  };
>  
>  void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
> @@ -30,18 +36,6 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
>  	u32 regs[4];
>  	const struct cpuid_bit *cb;
>  
> -	static const struct cpuid_bit cpuid_bits[] = {
> -		{ X86_FEATURE_INTEL_PT,		CR_EBX,25, 0x00000007, 0 },
> -		{ X86_FEATURE_AVX512_4VNNIW,	CR_EDX, 2, 0x00000007, 0 },
> -		{ X86_FEATURE_AVX512_4FMAPS,	CR_EDX, 3, 0x00000007, 0 },
> -		{ X86_FEATURE_APERFMPERF,	CR_ECX, 0, 0x00000006, 0 },
> -		{ X86_FEATURE_EPB,		CR_ECX, 3, 0x00000006, 0 },
> -		{ X86_FEATURE_HW_PSTATE,	CR_EDX, 7, 0x80000007, 0 },
> -		{ X86_FEATURE_CPB,		CR_EDX, 9, 0x80000007, 0 },
> -		{ X86_FEATURE_PROC_FEEDBACK,	CR_EDX,11, 0x80000007, 0 },
> -		{ 0, 0, 0, 0, 0 }
> -	};
> -
>  	for (cb = cpuid_bits; cb->feature; cb++) {
>  
>  		/* Verify that the level is valid */

@tip guys: this will conflict with the CAT changes. I've resolved it
this way by keeping the cpuid_bit.level sorted.

@@ -17,11 +17,20 @@ struct cpuid_bit {
        u32 sub_leaf;
 };
 
-enum cpuid_regs {
-       CR_EAX = 0,
-       CR_ECX,
-       CR_EDX,
-       CR_EBX
+/* Please keep the leaf sorted by cpuid_bit.level for faster search. */
+static const struct cpuid_bit cpuid_bits[] = {
+       { X86_FEATURE_APERFMPERF,       CPUID_ECX,  0, 0x00000006, 0 },
+       { X86_FEATURE_EPB,              CPUID_ECX,  3, 0x00000006, 0 },
+       { X86_FEATURE_INTEL_PT,         CPUID_EBX, 25, 0x00000007, 0 },
+       { X86_FEATURE_AVX512_4VNNIW,    CPUID_EDX,  2, 0x00000007, 0 },
+       { X86_FEATURE_AVX512_4FMAPS,    CPUID_EDX,  3, 0x00000007, 0 },
+       { X86_FEATURE_CAT_L3,           CPUID_EBX,  1, 0x00000010, 0 },
+       { X86_FEATURE_CAT_L2,           CPUID_EBX,  2, 0x00000010, 0 },
+       { X86_FEATURE_CDP_L3,           CPUID_ECX,  2, 0x00000010, 1 },
+       { X86_FEATURE_HW_PSTATE,        CPUID_EDX,  7, 0x80000007, 0 },
+       { X86_FEATURE_CPB,              CPUID_EDX,  9, 0x80000007, 0 },
+       { X86_FEATURE_PROC_FEEDBACK,    CPUID_EDX, 11, 0x80000007, 0 },
+       { 0, 0, 0, 0, 0 }
 };
 
 void init_scattered_cpuid_features(struct cpuinfo_x86 *c)


-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.