Re: [PATCH v3 1/7] x86: KVM: Move existing x86 CPUID leaf [CPUID_7_1_EAX] to kvm-only leaf

[Xiaoyao Li <xiaoyao.li@xxxxxxxxx>]

On 11/10/2022 9:52 AM, Jiaxi Chen wrote:
> cpuid_leaf[12] CPUID_7_1_EAX has only two bits are in use currently:
>
>   - AVX-VNNI CPUID.(EAX=7,ECX=1):EAX[bit 4]
>   - AVX512-BF16 CPUID.(EAX=7,ECX=1):EAX[bit 5]
>
> These two bits have no other kernel usages other than the guest
> CPUID advertisement in KVM. Given that and to save space for kernel
> feature bits, move these two bits to the kvm-only subleaves. The
> existing leaf cpuid_leafs[12] is set to CPUID_LNX_5 so future feature
> can pick it. This basically reverts:
>
>   - commit b85a0425d805 ("Enumerate AVX Vector Neural Network
> instructions")
>   - commit b302e4b176d0 ("x86/cpufeatures: Enumerate the new AVX512
> BFLOAT16 instructions")
>   - commit 1085a6b585d7 ("KVM: Expose AVX_VNNI instruction to guset")

FYI, LAM support has been queued in tip 
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=aa387b1b1e666cacffc0b7ac7e0949a68013b2d9

It adds

+#define X86_FEATURE_LAM			(12*32+26) /* Linear Address Masking */

and conflict with this patch.

Seen from the ISE, there are more bits defined in CPUID_7_1_EAX. And I 
believe Intel will define more and it's likely some of them will be used 
by kernel just like LAM.

> Signed-off-by: Jiaxi Chen <jiaxi.chen@xxxxxxxxxxxxxxx>
> ---
>   arch/x86/include/asm/cpufeature.h  | 2 +-
>   arch/x86/include/asm/cpufeatures.h | 4 +---
>   arch/x86/kernel/cpu/common.c       | 6 ------
>   arch/x86/kernel/cpu/cpuid-deps.c   | 1 -
>   arch/x86/kvm/cpuid.c               | 2 +-
>   arch/x86/kvm/reverse_cpuid.h       | 5 +++++
>   6 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 1a85e1fb0922..b2905ddd7ab4 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -24,7 +24,7 @@ enum cpuid_leafs
>   	CPUID_7_0_EBX,
>   	CPUID_D_1_EAX,
>   	CPUID_LNX_4,
> -	CPUID_7_1_EAX,
> +	CPUID_LNX_5,
>   	CPUID_8000_0008_EBX,
>   	CPUID_6_EAX,
>   	CPUID_8000_000A_EDX,
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index b71f4f2ecdd5..ec468d6eaf06 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -305,9 +305,7 @@
>   #define X86_FEATURE_USE_IBPB_FW		(11*32+16) /* "" Use IBPB during runtime firmware calls */
>   #define X86_FEATURE_RSB_VMEXIT_LITE	(11*32+17) /* "" Fill RSB on VM exit when EIBRS is enabled */
>   
> -/* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
> -#define X86_FEATURE_AVX_VNNI		(12*32+ 4) /* AVX VNNI instructions */
> -#define X86_FEATURE_AVX512_BF16		(12*32+ 5) /* AVX512 BFLOAT16 instructions */
> +/* Linux-defined mapping, word 12 */
>   
>   /* AMD-defined CPU features, CPUID level 0x80000008 (EBX), word 13 */
>   #define X86_FEATURE_CLZERO		(13*32+ 0) /* CLZERO instruction */
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 3e508f239098..0c19c84d7ba0 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1026,12 +1026,6 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
>   		c->x86_capability[CPUID_7_0_EBX] = ebx;
>   		c->x86_capability[CPUID_7_ECX] = ecx;
>   		c->x86_capability[CPUID_7_EDX] = edx;
> -
> -		/* Check valid sub-leaf index before accessing it */
> -		if (eax >= 1) {
> -			cpuid_count(0x00000007, 1, &eax, &ebx, &ecx, &edx);
> -			c->x86_capability[CPUID_7_1_EAX] = eax;
> -		}
>   	}
>   
>   	/* Extended state features: level 0x0000000d */
> diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
> index c881bcafba7d..a88e0e8c39fa 100644
> --- a/arch/x86/kernel/cpu/cpuid-deps.c
> +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> @@ -68,7 +68,6 @@ static const struct cpuid_dep cpuid_deps[] = {
>   	{ X86_FEATURE_CQM_OCCUP_LLC,		X86_FEATURE_CQM_LLC   },
>   	{ X86_FEATURE_CQM_MBM_TOTAL,		X86_FEATURE_CQM_LLC   },
>   	{ X86_FEATURE_CQM_MBM_LOCAL,		X86_FEATURE_CQM_LLC   },
> -	{ X86_FEATURE_AVX512_BF16,		X86_FEATURE_AVX512VL  },
>   	{ X86_FEATURE_AVX512_FP16,		X86_FEATURE_AVX512BW  },
>   	{ X86_FEATURE_ENQCMD,			X86_FEATURE_XSAVES    },
>   	{ X86_FEATURE_PER_THREAD_MBA,		X86_FEATURE_MBA       },
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 7065462378e2..89f5e7f0402b 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -656,7 +656,7 @@ void kvm_set_cpu_caps(void)
>   	if (boot_cpu_has(X86_FEATURE_AMD_SSBD))
>   		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL_SSBD);
>   
> -	kvm_cpu_cap_mask(CPUID_7_1_EAX,
> +	kvm_cpu_cap_init_scattered(CPUID_7_1_EAX,
>   		F(AVX_VNNI) | F(AVX512_BF16)
>   	);
>   
> diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
> index a19d473d0184..674de5b24f8d 100644
> --- a/arch/x86/kvm/reverse_cpuid.h
> +++ b/arch/x86/kvm/reverse_cpuid.h
> @@ -13,6 +13,7 @@
>    */
>   enum kvm_only_cpuid_leafs {
>   	CPUID_12_EAX	 = NCAPINTS,
> +	CPUID_7_1_EAX,
>   	NR_KVM_CPU_CAPS,
>   
>   	NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
> @@ -24,6 +25,10 @@ enum kvm_only_cpuid_leafs {
>   #define KVM_X86_FEATURE_SGX1		KVM_X86_FEATURE(CPUID_12_EAX, 0)
>   #define KVM_X86_FEATURE_SGX2		KVM_X86_FEATURE(CPUID_12_EAX, 1)
>   
> +/* Intel-defined sub-features, CPUID level 0x00000007:1 (EAX) */
> +#define X86_FEATURE_AVX_VNNI            KVM_X86_FEATURE(CPUID_7_1_EAX, 4)
> +#define X86_FEATURE_AVX512_BF16         KVM_X86_FEATURE(CPUID_7_1_EAX, 5)
> +
>   struct cpuid_reg {
>   	u32 function;
>   	u32 index;