[PATCH 0/6] x86: KVM: Expose CPUID to guest for new Intel platform instructions

[PATCH 0/6] x86: KVM: Expose CPUID to guest for new Intel platform instructions

[Jiaxi Chen]

Latest Intel platform Granite Rapids/Sierra Forest has introduced below
new instructions and CPUIDs:

 - CMPccXADD CPUID.(EAX=7,ECX=1):EAX[bit 7]
 - AMX-FP16 CPUID.(EAX=7,ECX=1):EAX[bit 21]
 - AVX-IFMA CPUID.(EAX=7,ECX=1):EAX[bit 23]
 - AVX-VNNI-INT8 CPUID.(EAX=7,ECX=1):EDX[bit 4]
 - AVX-NE-CONVERT CPUID.(EAX=7,ECX=1):EDX[bit 5]
 - PREFETCHITI CPUID.(EAX=7,ECX=1):EDX[bit 14]

Details can be found in recent Intel ISE (Instruction Set
Extensions)[1].

This series enables related CPUID bit in the kernel feature bits and
expose them to KVM guest OS.

[1] Intel ISE: https://cdrdv2.intel.com/v1/dl/getContent/671368

Chang S. Bae (1):
  x86: KVM: Enable AMX-FP16 CPUID and expose it to guest

Jiaxi Chen (5):
  x86: KVM: Enable CMPccXADD CPUID and expose it to guest
  x86: KVM: Enable AVX-IFMA CPUID and expose it to guest
  x86: KVM: Enable AVX-VNNI-INT8 CPUID and expose it to guest
  x86: KVM: Enable AVX-NE-CONVERT CPUID and expose it to guest
  x86: KVM: Enable PREFETCHIT0/1 CPUID and expose it to guest

 arch/x86/include/asm/cpufeature.h        |  7 +++++--
 arch/x86/include/asm/cpufeatures.h       | 10 +++++++++-
 arch/x86/include/asm/disabled-features.h |  3 ++-
 arch/x86/include/asm/required-features.h |  3 ++-
 arch/x86/kernel/cpu/common.c             |  1 +
 arch/x86/kvm/cpuid.c                     |  8 ++++++--
 arch/x86/kvm/reverse_cpuid.h             |  1 +
 7 files changed, 26 insertions(+), 7 deletions(-)


base-commit: e18d6152ff0f41b7f01f9817372022df04e0d354
-- 
2.27.0

[PATCH 1/6] x86: KVM: Enable CMPccXADD CPUID and expose it to guest

[Jiaxi Chen]

CMPccXADD is a new set of instructions in the latest Intel platform Sierra
Forest. It includes a semaphore operation that can compare and add the
operands if condition is met, which can improve database performance.

The bit definition:
CPUID.(EAX=7,ECX=1):EAX[bit 7]

This patch enables this CPUID in the kernel feature bits and expose it to
guest OS.

Signed-off-by: Jiaxi Chen <jiaxi.chen@linux.intel.com>
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/kvm/cpuid.c               | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index ef4775c6db01..445626cb5779 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -308,6 +308,7 @@
 /* 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 */
+#define X86_FEATURE_CMPCCXADD           (12*32+ 7) /* CMPccXADD instructions */
 
 /* AMD-defined CPU features, CPUID level 0x80000008 (EBX), word 13 */
 #define X86_FEATURE_CLZERO		(13*32+ 0) /* CLZERO instruction */
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 7065462378e2..3f745f6fdc43 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -657,7 +657,7 @@ void kvm_set_cpu_caps(void)
 		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL_SSBD);
 
 	kvm_cpu_cap_mask(CPUID_7_1_EAX,
-		F(AVX_VNNI) | F(AVX512_BF16)
+		F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD)
 	);
 
 	kvm_cpu_cap_mask(CPUID_D_1_EAX,
-- 
2.27.0

[PATCH 2/6] x86: KVM: Enable AMX-FP16 CPUID and expose it to guest

[Jiaxi Chen]

From: "Chang S. Bae" <chang.seok.bae@intel.com>

Latest Intel platform Granite Rapids has introduced a new instruction -
AMX-FP16, which performs dot-products of two FP16 tiles and accumulates
the results into a packed single precision tile. This instruction adds
FP16 capability and also allows a FP16 GPU trained model to run faster
without loss of accuracy or added SW overhead.

The bit definition:
CPUID.(EAX=7,ECX=1):EAX[bit 21]

This patch enables this CPUID in the kernel feature bits and expose it
to guest OS.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Signed-off-by: Jiaxi Chen <jiaxi.chen@linux.intel.com>
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/kvm/cpuid.c               | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 445626cb5779..9313240e3cdd 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -309,6 +309,7 @@
 #define X86_FEATURE_AVX_VNNI		(12*32+ 4) /* AVX VNNI instructions */
 #define X86_FEATURE_AVX512_BF16		(12*32+ 5) /* AVX512 BFLOAT16 instructions */
 #define X86_FEATURE_CMPCCXADD           (12*32+ 7) /* CMPccXADD instructions */
+#define X86_FEATURE_AMX_FP16		(12*32+21) /* AMX fp16 Support */
 
 /* AMD-defined CPU features, CPUID level 0x80000008 (EBX), word 13 */
 #define X86_FEATURE_CLZERO		(13*32+ 0) /* CLZERO instruction */
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 3f745f6fdc43..d983ddb974ba 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -657,7 +657,7 @@ void kvm_set_cpu_caps(void)
 		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL_SSBD);
 
 	kvm_cpu_cap_mask(CPUID_7_1_EAX,
-		F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD)
+		F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD) | F(AMX_FP16)
 	);
 
 	kvm_cpu_cap_mask(CPUID_D_1_EAX,
-- 
2.27.0

[PATCH 3/6] x86: KVM: Enable AVX-IFMA CPUID and expose it to guest

[Jiaxi Chen]

AVX-IFMA is a new instruction in the latest Intel platform Sierra Forest.
AVX-IFMA packed multiplies unsigned 52-bit integers and add the low/high
52-bit products to Qword Accumulators.

The bit definition:
CPUID.(EAX=7,ECX=1):EAX[bit 23]

This patch enables this CPUID in the kernel feature bits and expose it
to guest OS.

Signed-off-by: Jiaxi Chen <jiaxi.chen@linux.intel.com>
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/kvm/cpuid.c               | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 9313240e3cdd..a682f646243f 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -310,6 +310,7 @@
 #define X86_FEATURE_AVX512_BF16		(12*32+ 5) /* AVX512 BFLOAT16 instructions */
 #define X86_FEATURE_CMPCCXADD           (12*32+ 7) /* CMPccXADD instructions */
 #define X86_FEATURE_AMX_FP16		(12*32+21) /* AMX fp16 Support */
+#define X86_FEATURE_AVX_IFMA            (12*32+23) /* Support for VPMADD52[H,L]UQ */
 
 /* AMD-defined CPU features, CPUID level 0x80000008 (EBX), word 13 */
 #define X86_FEATURE_CLZERO		(13*32+ 0) /* CLZERO instruction */
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index d983ddb974ba..837bcd1373e5 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -657,8 +657,8 @@ void kvm_set_cpu_caps(void)
 		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL_SSBD);
 
 	kvm_cpu_cap_mask(CPUID_7_1_EAX,
-		F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD) | F(AMX_FP16)
-	);
+		F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD) | F(AMX_FP16) |
+		F(AVX_IFMA));
 
 	kvm_cpu_cap_mask(CPUID_D_1_EAX,
 		F(XSAVEOPT) | F(XSAVEC) | F(XGETBV1) | F(XSAVES) | f_xfd
-- 
2.27.0

[PATCH 4/6] x86: KVM: Enable AVX-VNNI-INT8 CPUID and expose it to guest

[Jiaxi Chen]

AVX-VNNI-INT8 is a new set of instructions in the latest Intel platform
Sierra Forest. It multiplies the individual bytes of two unsigned or
unsigned source operands, then add and accumulate the results into the
destination dword element size operand. This instruction allows for the
platform to have superior AI capabilities.

The bit definition:
CPUID.(EAX=7,ECX=1):EDX[bit 4]

This patch enables this CPUID in the kernel feature bits and expose it to
guest OS. Since the CPUID involves a bit of EDX (EAX=7,ECX=1) which has not
been enumerated yet, this patch adds CPUID_7_1_EDX to CPUID subleaves. At
the same time, word 20 is newly-defined in CPU features for CPUID level
0x00000007:1 (EDX).

Signed-off-by: Jiaxi Chen <jiaxi.chen@linux.intel.com>
---
 arch/x86/include/asm/cpufeature.h        | 7 +++++--
 arch/x86/include/asm/cpufeatures.h       | 5 ++++-
 arch/x86/include/asm/disabled-features.h | 3 ++-
 arch/x86/include/asm/required-features.h | 3 ++-
 arch/x86/kernel/cpu/common.c             | 1 +
 arch/x86/kvm/cpuid.c                     | 5 ++++-
 arch/x86/kvm/reverse_cpuid.h             | 1 +
 7 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 1a85e1fb0922..d46b802930b0 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -32,6 +32,7 @@ enum cpuid_leafs
 	CPUID_8000_0007_EBX,
 	CPUID_7_EDX,
 	CPUID_8000_001F_EAX,
+	CPUID_7_1_EDX,
 };
 
 #define X86_CAP_FMT_NUM "%d:%d"
@@ -94,8 +95,9 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
 	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 17, feature_bit) ||	\
 	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 18, feature_bit) ||	\
 	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 19, feature_bit) ||	\
+	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 20, feature_bit) ||    \
 	   REQUIRED_MASK_CHECK					  ||	\
-	   BUILD_BUG_ON_ZERO(NCAPINTS != 20))
+	   BUILD_BUG_ON_ZERO(NCAPINTS != 21))
 
 #define DISABLED_MASK_BIT_SET(feature_bit)				\
 	 ( CHECK_BIT_IN_MASK_WORD(DISABLED_MASK,  0, feature_bit) ||	\
@@ -118,8 +120,9 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
 	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 17, feature_bit) ||	\
 	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 18, feature_bit) ||	\
 	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 19, feature_bit) ||	\
+	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 20, feature_bit) ||    \
 	   DISABLED_MASK_CHECK					  ||	\
-	   BUILD_BUG_ON_ZERO(NCAPINTS != 20))
+	   BUILD_BUG_ON_ZERO(NCAPINTS != 21))
 
 #define cpu_has(c, bit)							\
 	(__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 :	\
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index a682f646243f..b2aa761ea110 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -13,7 +13,7 @@
 /*
  * Defines x86 CPU feature bits
  */
-#define NCAPINTS			20	   /* N 32-bit words worth of info */
+#define NCAPINTS			21	   /* N 32-bit words worth of info */
 #define NBUGINTS			1	   /* N 32-bit bug flags */
 
 /*
@@ -423,6 +423,9 @@
 #define X86_FEATURE_V_TSC_AUX		(19*32+ 9) /* "" Virtual TSC_AUX */
 #define X86_FEATURE_SME_COHERENT	(19*32+10) /* "" AMD hardware-enforced cache coherency */
 
+/* Intel-defined CPU features, CPUID level 0x00000007:1 (EDX), word 20 */
+#define X86_FEATURE_AVX_VNNI_INT8       (20*32+ 4) /* Support for VPDPB[SU,UU,SS]D[,S] */
+
 /*
  * BUG word(s)
  */
diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
index 33d2cd04d254..2db6929ca868 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -111,6 +111,7 @@
 #define DISABLED_MASK17	0
 #define DISABLED_MASK18	0
 #define DISABLED_MASK19	0
-#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
+#define DISABLED_MASK20 0
+#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 21)
 
 #endif /* _ASM_X86_DISABLED_FEATURES_H */
diff --git a/arch/x86/include/asm/required-features.h b/arch/x86/include/asm/required-features.h
index aff774775c67..3b5522b3f051 100644
--- a/arch/x86/include/asm/required-features.h
+++ b/arch/x86/include/asm/required-features.h
@@ -98,6 +98,7 @@
 #define REQUIRED_MASK17	0
 #define REQUIRED_MASK18	0
 #define REQUIRED_MASK19	0
-#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
+#define REQUIRED_MASK20 0
+#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 21)
 
 #endif /* _ASM_X86_REQUIRED_FEATURES_H */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 3e508f239098..7c659127ed89 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1031,6 +1031,7 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 		if (eax >= 1) {
 			cpuid_count(0x00000007, 1, &eax, &ebx, &ecx, &edx);
 			c->x86_capability[CPUID_7_1_EAX] = eax;
+			c->x86_capability[CPUID_7_1_EDX] = edx;
 		}
 	}
 
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 837bcd1373e5..b1b53a5c788a 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -660,6 +660,9 @@ void kvm_set_cpu_caps(void)
 		F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD) | F(AMX_FP16) |
 		F(AVX_IFMA));
 
+	kvm_cpu_cap_mask(CPUID_7_1_EDX,
+		F(AVX_VNNI_INT8));
+
 	kvm_cpu_cap_mask(CPUID_D_1_EAX,
 		F(XSAVEOPT) | F(XSAVEC) | F(XGETBV1) | F(XSAVES) | f_xfd
 	);
@@ -913,9 +916,9 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 				goto out;
 
 			cpuid_entry_override(entry, CPUID_7_1_EAX);
+			cpuid_entry_override(entry, CPUID_7_1_EDX);
 			entry->ebx = 0;
 			entry->ecx = 0;
-			entry->edx = 0;
 		}
 		break;
 	case 0xa: { /* Architectural Performance Monitoring */
diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
index a19d473d0184..fb4f4bffad5c 100644
--- a/arch/x86/kvm/reverse_cpuid.h
+++ b/arch/x86/kvm/reverse_cpuid.h
@@ -48,6 +48,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
 	[CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},
 	[CPUID_12_EAX]        = {0x00000012, 0, CPUID_EAX},
 	[CPUID_8000_001F_EAX] = {0x8000001f, 0, CPUID_EAX},
+	[CPUID_7_1_EDX]       = {         7, 1, CPUID_EDX},
 };
 
 /*
-- 
2.27.0

[PATCH 5/6] x86: KVM: Enable AVX-NE-CONVERT CPUID and expose it to guest

[Jiaxi Chen]

AVX-NE-CONVERT is a new set of instructions in the latest Intel platform
Sierra Forest, which can convert low precision floating point like
BF16/FP16 to high precision floating point FP32. It can also convert
FP32 elements to BF16. This instruction allows the platform to have
improved AI capabilities and better compatibility.

The bit definition:
CPUID.(EAX=7,ECX=1):EDX[bit 5]

This patch enables this CPUID in the kernel feature bits and expose it
to guest OS.

Signed-off-by: Jiaxi Chen <jiaxi.chen@linux.intel.com>
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/kvm/cpuid.c               | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index b2aa761ea110..4e0bbffedbb4 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -425,6 +425,7 @@
 
 /* Intel-defined CPU features, CPUID level 0x00000007:1 (EDX), word 20 */
 #define X86_FEATURE_AVX_VNNI_INT8       (20*32+ 4) /* Support for VPDPB[SU,UU,SS]D[,S] */
+#define X86_FEATURE_AVX_NE_CONVERT      (20*32+ 5) /* AVX NE CONVERT Instructions */
 
 /*
  * BUG word(s)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index b1b53a5c788a..fcd00c68e546 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -661,7 +661,8 @@ void kvm_set_cpu_caps(void)
 		F(AVX_IFMA));
 
 	kvm_cpu_cap_mask(CPUID_7_1_EDX,
-		F(AVX_VNNI_INT8));
+		F(AVX_VNNI_INT8) | F(AVX_NE_CONVERT)
+	);
 
 	kvm_cpu_cap_mask(CPUID_D_1_EAX,
 		F(XSAVEOPT) | F(XSAVEC) | F(XGETBV1) | F(XSAVES) | f_xfd
-- 
2.27.0

[PATCH 6/6] x86: KVM: Enable PREFETCHIT0/1 CPUID and expose it to guest

[Jiaxi Chen]

Latest Intel platform Granite Rapids has introduced a new instruction -
PREFETCHIT0/1, which moves code to memory (cache) closer to the processor
depending on specific hints.

The bit definition:
CPUID.(EAX=7,ECX=1):EDX[bit 14]

This patch enables this CPUID in the kernel feature bits and expose it
to guest OS.

Signed-off-by: Jiaxi Chen <jiaxi.chen@linux.intel.com>
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/kvm/cpuid.c               | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 4e0bbffedbb4..de8aa62bbdb1 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -426,6 +426,7 @@
 /* Intel-defined CPU features, CPUID level 0x00000007:1 (EDX), word 20 */
 #define X86_FEATURE_AVX_VNNI_INT8       (20*32+ 4) /* Support for VPDPB[SU,UU,SS]D[,S] */
 #define X86_FEATURE_AVX_NE_CONVERT      (20*32+ 5) /* AVX NE CONVERT Instructions */
+#define X86_FEATURE_PREFETCHITI         (20*32+14) /* PREFETCHIT0/1 Instructions */
 
 /*
  * BUG word(s)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index fcd00c68e546..e73307732d10 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -661,7 +661,7 @@ void kvm_set_cpu_caps(void)
 		F(AVX_IFMA));
 
 	kvm_cpu_cap_mask(CPUID_7_1_EDX,
-		F(AVX_VNNI_INT8) | F(AVX_NE_CONVERT)
+		F(AVX_VNNI_INT8) | F(AVX_NE_CONVERT) | F(PREFETCHITI)
 	);
 
 	kvm_cpu_cap_mask(CPUID_D_1_EAX,
-- 
2.27.0

Re: [PATCH 4/6] x86: KVM: Enable AVX-VNNI-INT8 CPUID and expose it to guest

[Borislav Petkov]

On Wed, Oct 19, 2022 at 04:47:32PM +0800, Jiaxi Chen wrote:
> AVX-VNNI-INT8 is a new set of instructions in the latest Intel platform
> Sierra Forest. It multiplies the individual bytes of two unsigned or
> unsigned source operands, then add and accumulate the results into the
> destination dword element size operand. This instruction allows for the
> platform to have superior AI capabilities.
> 
> The bit definition:
> CPUID.(EAX=7,ECX=1):EDX[bit 4]
> 
> This patch enables this CPUID in the kernel feature bits and expose it to
> guest OS. Since the CPUID involves a bit of EDX (EAX=7,ECX=1) which has not
> been enumerated yet, this patch adds CPUID_7_1_EDX to CPUID subleaves. At
> the same time, word 20 is newly-defined in CPU features for CPUID level

For all your commit messages:

Avoid having "This patch" or "This commit" in the commit message. It is
tautologically useless.

Also, do

$ git grep 'This patch' Documentation/process

for more details.

For this particular one, use scattered.c instead of adding a new leaf.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 4/6] x86: KVM: Enable AVX-VNNI-INT8 CPUID and expose it to guest

[Sean Christopherson]

On Wed, Oct 19, 2022, Borislav Petkov wrote:
> On Wed, Oct 19, 2022 at 04:47:32PM +0800, Jiaxi Chen wrote:
> > AVX-VNNI-INT8 is a new set of instructions in the latest Intel platform
> > Sierra Forest. It multiplies the individual bytes of two unsigned or
> > unsigned source operands, then add and accumulate the results into the
> > destination dword element size operand. This instruction allows for the
> > platform to have superior AI capabilities.
> > 
> > The bit definition:
> > CPUID.(EAX=7,ECX=1):EDX[bit 4]
>
> For this particular one, use scattered.c instead of adding a new leaf.

Unless the kernel wants to use X86_FEATURE_AVX_VNNI_INT8, which seems unlikely,
there's no need to create a scattered entry.  This can be handled in KVM by adding
a KVM-only leaf entry (which will be needed no matter what), plus a #define for
X86_FEATURE_AVX_VNNI_INT8 to direct it to the KVM entry.

E.g. 

diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
index a19d473d0184..25e7bfc61607 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_EDX,
        NR_KVM_CPU_CAPS,
 
        NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
@@ -24,6 +25,16 @@ 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)
 
+#define KVM_X86_FEATURE_AVX_VNNI_INT8  KVM_X86_FEATURE(CPUID_7_1_EDX, 4)
+
+/*
+ * Alias X86_FEATURE_* to the KVM variant for features in KVM-only leafs that
+ * aren't scattered by cpufeatures.h so that X86_FEATURE_* can be used in KVM,
+ * e.g. to query guest CPUID.  As a bonus, no translation is needed for these
+ * features in __feature_translate().
+ */
+#define X86_FEATURE_AVX_VNNI_INT8      KVM_X86_FEATURE_AVX_VNNI_INT8
+
 struct cpuid_reg {
        u32 function;
        u32 index;
@@ -48,6 +59,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
        [CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},
        [CPUID_12_EAX]        = {0x00000012, 0, CPUID_EAX},
        [CPUID_8000_001F_EAX] = {0x8000001f, 0, CPUID_EAX},
+       [CPUID_7_1_EDX]       = {         7, 1, CPUID_EDX},
 };
 
 /*

Re: [PATCH 4/6] x86: KVM: Enable AVX-VNNI-INT8 CPUID and expose it to guest

[Sean Christopherson]

On Wed, Oct 19, 2022, Sean Christopherson wrote:
> On Wed, Oct 19, 2022, Borislav Petkov wrote:
> > On Wed, Oct 19, 2022 at 04:47:32PM +0800, Jiaxi Chen wrote:
> > > AVX-VNNI-INT8 is a new set of instructions in the latest Intel platform
> > > Sierra Forest. It multiplies the individual bytes of two unsigned or
> > > unsigned source operands, then add and accumulate the results into the
> > > destination dword element size operand. This instruction allows for the
> > > platform to have superior AI capabilities.
> > > 
> > > The bit definition:
> > > CPUID.(EAX=7,ECX=1):EDX[bit 4]
> >
> > For this particular one, use scattered.c instead of adding a new leaf.
> 
> Unless the kernel wants to use X86_FEATURE_AVX_VNNI_INT8, which seems unlikely,
> there's no need to create a scattered entry.  This can be handled in KVM by adding
> a KVM-only leaf entry (which will be needed no matter what), plus a #define for
> X86_FEATURE_AVX_VNNI_INT8 to direct it to the KVM entry.
> 
> E.g. 
> 
> diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
> index a19d473d0184..25e7bfc61607 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_EDX,
>         NR_KVM_CPU_CAPS,
>  
>         NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
> @@ -24,6 +25,16 @@ 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)
>  
> +#define KVM_X86_FEATURE_AVX_VNNI_INT8  KVM_X86_FEATURE(CPUID_7_1_EDX, 4)
> +
> +/*
> + * Alias X86_FEATURE_* to the KVM variant for features in KVM-only leafs that
> + * aren't scattered by cpufeatures.h so that X86_FEATURE_* can be used in KVM,
> + * e.g. to query guest CPUID.  As a bonus, no translation is needed for these
> + * features in __feature_translate().
> + */
> +#define X86_FEATURE_AVX_VNNI_INT8      KVM_X86_FEATURE_AVX_VNNI_INT8

Actually, there's no need for KVM_X86_FEATURE_AVX_VNNI_INT8 in this case, just
#define X86_FEATURE_AVX_VNNI_INT8 directly.  The KVM_ prefixed macro exists purely
to redirect the non-KVM_ version, but that's unnecessary in this case.

diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
index a19d473d0184..38adafb03490 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_EDX,
        NR_KVM_CPU_CAPS,
 
        NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
@@ -24,6 +25,13 @@ 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)
 
+/*
+ * Omit the KVM_ prefix for features in KVM-only leafs that aren't scattered by
+ * cpufeatures.h so that X86_FEATURE_* can be used in KVM,* e.g. to query guest
+ * CPUID.  As a bonus, no handling in __feature_translate() is needed.
+ */
+#define X86_FEATURE_AVX_VNNI_INT8      KVM_X86_FEATURE(CPUID_7_1_EDX, 4)
+
 struct cpuid_reg {
        u32 function;
        u32 index;
@@ -48,6 +56,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
        [CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},
        [CPUID_12_EAX]        = {0x00000012, 0, CPUID_EAX},
        [CPUID_8000_001F_EAX] = {0x8000001f, 0, CPUID_EAX},
+       [CPUID_7_1_EDX]       = {         7, 1, CPUID_EDX},
 };

Re: [PATCH 1/6] x86: KVM: Enable CMPccXADD CPUID and expose it to guest

[Sean Christopherson]

For all the shortlogs, "expose it to guest" is technically wrong.  Adding
recognition in kvm/cpuid.c advertises KVM support to host userspace.  Whether or
not a feature is exposed to the guest is up to the userspace VMM.

On Wed, Oct 19, 2022, Jiaxi Chen wrote:
> CMPccXADD is a new set of instructions in the latest Intel platform Sierra
> Forest. It includes a semaphore operation that can compare and add the

In general, avoid pronouns in changelogs, it's not clear what "it" refers to here.

And for all of these changelogs, please explicitly state that there are no VMX
controls for these instructions, assuming that's actually true.  From a KVM
perspective, that's far more interesting than the details of the instruction(s).

> operands if condition is met, which can improve database performance.
> 
> The bit definition:
> CPUID.(EAX=7,ECX=1):EAX[bit 7]
> 
> This patch enables this CPUID in the kernel feature bits and expose it to
> guest OS.

Same thing here, KVM doesn't decide whether or not to expose the feature to the
guest.

> Signed-off-by: Jiaxi Chen <jiaxi.chen@linux.intel.com>
> ---
>  arch/x86/include/asm/cpufeatures.h | 1 +
>  arch/x86/kvm/cpuid.c               | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index ef4775c6db01..445626cb5779 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -308,6 +308,7 @@
>  /* 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 */
> +#define X86_FEATURE_CMPCCXADD           (12*32+ 7) /* CMPccXADD instructions */

Boris,

What do you think about moving CPUID_7_1_EAX to be a KVM-only leaf too?  AFAICT,
KVM passthrough is the only reason the existing features are defined.

Re: [PATCH 4/6] x86: KVM: Enable AVX-VNNI-INT8 CPUID and expose it to guest

[Chen, Jiaxi]

在 2022/10/19 16:52, Borislav Petkov 写道:
> On Wed, Oct 19, 2022 at 04:47:32PM +0800, Jiaxi Chen wrote:
>> AVX-VNNI-INT8 is a new set of instructions in the latest Intel platform
>> Sierra Forest. It multiplies the individual bytes of two unsigned or
>> unsigned source operands, then add and accumulate the results into the
>> destination dword element size operand. This instruction allows for the
>> platform to have superior AI capabilities.
>>
>> The bit definition:
>> CPUID.(EAX=7,ECX=1):EDX[bit 4]
>>
>> This patch enables this CPUID in the kernel feature bits and expose it to
>> guest OS. Since the CPUID involves a bit of EDX (EAX=7,ECX=1) which has not
>> been enumerated yet, this patch adds CPUID_7_1_EDX to CPUID subleaves. At
>> the same time, word 20 is newly-defined in CPU features for CPUID level
> 
> For all your commit messages:
> 
> Avoid having "This patch" or "This commit" in the commit message. It is
> tautologically useless.

Will follow this in the future. Thanks.
> 
> Also, do
> 
> $ git grep 'This patch' Documentation/process
> 
> for more details.
> 
> For this particular one, use scattered.c instead of adding a new leaf.
> 
> Thx.
> 

Re: [PATCH 1/6] x86: KVM: Enable CMPccXADD CPUID and expose it to guest

[Chen, Jiaxi]

在 2022/10/19 23:15, Sean Christopherson 写道:
> For all the shortlogs, "expose it to guest" is technically wrong.  Adding
> recognition in kvm/cpuid.c advertises KVM support to host userspace.  Whether or
> not a feature is exposed to the guest is up to the userspace VMM.

Thanks for reminding. How about to change the subject to this:
x86: KVM: Advertise CMPccXADD CPUID to userspace
> 
> On Wed, Oct 19, 2022, Jiaxi Chen wrote:
>> CMPccXADD is a new set of instructions in the latest Intel platform Sierra
>> Forest. It includes a semaphore operation that can compare and add the
> 
> In general, avoid pronouns in changelogs, it's not clear what "it" refers to here.
> 
Will change it to: 'This new instruction set' here and avoid use pronouns in
the future commit message.

> And for all of these changelogs, please explicitly state that there are no VMX
> controls for these instructions, assuming that's actually true.  From a KVM
> perspective, that's far more interesting than the details of the instruction(s).
> 
Yes, thanks for comments. Will change this patch series to: 
This instruction has no other VMX control except for exposed to userspace.

>> operands if condition is met, which can improve database performance.
>>
>> The bit definition:
>> CPUID.(EAX=7,ECX=1):EAX[bit 7]
>>
>> This patch enables this CPUID in the kernel feature bits and expose it to
>> guest OS.
> 
> Same thing here, KVM doesn't decide whether or not to expose the feature to the
> guest.
> 
Applied.Thanks.
>> Signed-off-by: Jiaxi Chen <jiaxi.chen@linux.intel.com>
>> ---
>>  arch/x86/include/asm/cpufeatures.h | 1 +
>>  arch/x86/kvm/cpuid.c               | 2 +-
>>  2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
>> index ef4775c6db01..445626cb5779 100644
>> --- a/arch/x86/include/asm/cpufeatures.h
>> +++ b/arch/x86/include/asm/cpufeatures.h
>> @@ -308,6 +308,7 @@
>>  /* 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 */
>> +#define X86_FEATURE_CMPCCXADD           (12*32+ 7) /* CMPccXADD instructions */
> 
> Boris,
> 
> What do you think about moving CPUID_7_1_EAX to be a KVM-only leaf too?  AFAICT,
> KVM passthrough is the only reason the existing features are defined.

Re: [PATCH 4/6] x86: KVM: Enable AVX-VNNI-INT8 CPUID and expose it to guest

[Jiaxi Chen]

On 10/19/2022 11:09 PM, Sean Christopherson wrote:
> On Wed, Oct 19, 2022, Sean Christopherson wrote:
>> On Wed, Oct 19, 2022, Borislav Petkov wrote:
>>> On Wed, Oct 19, 2022 at 04:47:32PM +0800, Jiaxi Chen wrote:
>>>> AVX-VNNI-INT8 is a new set of instructions in the latest Intel platform
>>>> Sierra Forest. It multiplies the individual bytes of two unsigned or
>>>> unsigned source operands, then add and accumulate the results into the
>>>> destination dword element size operand. This instruction allows for the
>>>> platform to have superior AI capabilities.
>>>>
>>>> The bit definition:
>>>> CPUID.(EAX=7,ECX=1):EDX[bit 4]
>>>
>>> For this particular one, use scattered.c instead of adding a new leaf.
>>
>> Unless the kernel wants to use X86_FEATURE_AVX_VNNI_INT8, which seems unlikely,
>> there's no need to create a scattered entry.  This can be handled in KVM by adding
>> a KVM-only leaf entry (which will be needed no matter what), plus a #define for
>> X86_FEATURE_AVX_VNNI_INT8 to direct it to the KVM entry.
>>
>> E.g. 
>>
>> diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
>> index a19d473d0184..25e7bfc61607 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_EDX,
>>         NR_KVM_CPU_CAPS,
>>  
>>         NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
>> @@ -24,6 +25,16 @@ 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)
>>  
>> +#define KVM_X86_FEATURE_AVX_VNNI_INT8  KVM_X86_FEATURE(CPUID_7_1_EDX, 4)
>> +
>> +/*
>> + * Alias X86_FEATURE_* to the KVM variant for features in KVM-only leafs that
>> + * aren't scattered by cpufeatures.h so that X86_FEATURE_* can be used in KVM,
>> + * e.g. to query guest CPUID.  As a bonus, no translation is needed for these
>> + * features in __feature_translate().
>> + */
>> +#define X86_FEATURE_AVX_VNNI_INT8      KVM_X86_FEATURE_AVX_VNNI_INT8
> 
> Actually, there's no need for KVM_X86_FEATURE_AVX_VNNI_INT8 in this case, just
> #define X86_FEATURE_AVX_VNNI_INT8 directly.  The KVM_ prefixed macro exists purely
> to redirect the non-KVM_ version, but that's unnecessary in this case.
> 
> diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
> index a19d473d0184..38adafb03490 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_EDX,
>         NR_KVM_CPU_CAPS,
>  
>         NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
> @@ -24,6 +25,13 @@ 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)
>  
> +/*
> + * Omit the KVM_ prefix for features in KVM-only leafs that aren't scattered by
> + * cpufeatures.h so that X86_FEATURE_* can be used in KVM,* e.g. to query guest
> + * CPUID.  As a bonus, no handling in __feature_translate() is needed.
> + */
> +#define X86_FEATURE_AVX_VNNI_INT8      KVM_X86_FEATURE(CPUID_7_1_EDX, 4)
> +
>  struct cpuid_reg {
>         u32 function;
>         u32 index;
> @@ -48,6 +56,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
>         [CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},
>         [CPUID_12_EAX]        = {0x00000012, 0, CPUID_EAX},
>         [CPUID_8000_001F_EAX] = {0x8000001f, 0, CPUID_EAX},
> +       [CPUID_7_1_EDX]       = {         7, 1, CPUID_EDX},
>  };

Use KVM-only leafs is better for edx in this case. Will follow this suggestion in v2. 
Thanks Sean very much.
-- 
Regards,
Jiaxi

Re: [PATCH 1/6] x86: KVM: Enable CMPccXADD CPUID and expose it to guest

[Jiaxi Chen]

On 10/19/2022 11:15 PM, Sean Christopherson wrote:
> For all the shortlogs, "expose it to guest" is technically wrong.  Adding
> recognition in kvm/cpuid.c advertises KVM support to host userspace.  Whether or
> not a feature is exposed to the guest is up to the userspace VMM.
> 
> On Wed, Oct 19, 2022, Jiaxi Chen wrote:
>> CMPccXADD is a new set of instructions in the latest Intel platform Sierra
>> Forest. It includes a semaphore operation that can compare and add the
> 
> In general, avoid pronouns in changelogs, it's not clear what "it" refers to here.
> 
> And for all of these changelogs, please explicitly state that there are no VMX
> controls for these instructions, assuming that's actually true.  From a KVM
> perspective, that's far more interesting than the details of the instruction(s).
> 
>> operands if condition is met, which can improve database performance.
>>
>> The bit definition:
>> CPUID.(EAX=7,ECX=1):EAX[bit 7]
>>
>> This patch enables this CPUID in the kernel feature bits and expose it to
>> guest OS.
> 
> Same thing here, KVM doesn't decide whether or not to expose the feature to the
> guest.
> 
>> Signed-off-by: Jiaxi Chen <jiaxi.chen@linux.intel.com>
>> ---
>>  arch/x86/include/asm/cpufeatures.h | 1 +
>>  arch/x86/kvm/cpuid.c               | 2 +-
>>  2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
>> index ef4775c6db01..445626cb5779 100644
>> --- a/arch/x86/include/asm/cpufeatures.h
>> +++ b/arch/x86/include/asm/cpufeatures.h
>> @@ -308,6 +308,7 @@
>>  /* 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 */
>> +#define X86_FEATURE_CMPCCXADD           (12*32+ 7) /* CMPccXADD instructions */
> 
> Boris,
> 
> What do you think about moving CPUID_7_1_EAX to be a KVM-only leaf too?  AFAICT,
> KVM passthrough is the only reason the existing features are defined.
Boris,

Since CPUID_7_1_EAX has only 5 features now, it is a big waste, should we move it to 
KVM-only leaf as Sean suggested. What's your opinion about this? 
-- 
Regards,
Jiaxi

Re: [PATCH 1/6] x86: KVM: Enable CMPccXADD CPUID and expose it to guest

[Borislav Petkov]

On Wed, Oct 26, 2022 at 11:40:31AM +0800, Jiaxi Chen wrote:
> > What do you think about moving CPUID_7_1_EAX to be a KVM-only leaf too?  AFAICT,
> > KVM passthrough is the only reason the existing features are defined.

Yap, looking at the patches which added those 2 feature flags upstream,
they don't look like some particular use was the goal but rather to
expose it to guests. Besides, AVX512 apps do their own CPUID detection.

> Since CPUID_7_1_EAX has only 5 features now, it is a big waste,       
> should we move it to KVM-only leaf as Sean suggested. What's your     
> opinion about this?                                                   

Yes, pls do.

And when you do, make sure to undo what

  b302e4b176d0 ("x86/cpufeatures: Enumerate the new AVX512 BFLOAT16 instructions")

added.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 1/6] x86: KVM: Enable CMPccXADD CPUID and expose it to guest

[Jiaxi Chen]

On 10/27/2022 1:15 AM, Borislav Petkov wrote:
> On Wed, Oct 26, 2022 at 11:40:31AM +0800, Jiaxi Chen wrote:
>>> What do you think about moving CPUID_7_1_EAX to be a KVM-only leaf too?  AFAICT,
>>> KVM passthrough is the only reason the existing features are defined.
> 
> Yap, looking at the patches which added those 2 feature flags upstream,
> they don't look like some particular use was the goal but rather to
> expose it to guests. Besides, AVX512 apps do their own CPUID detection.
> 
>> Since CPUID_7_1_EAX has only 5 features now, it is a big waste,       
>> should we move it to KVM-only leaf as Sean suggested. What's your     
>> opinion about this?                                                   
> 
> Yes, pls do.
> 
> And when you do, make sure to undo what
> 
>   b302e4b176d0 ("x86/cpufeatures: Enumerate the new AVX512 BFLOAT16 instructions")
> 
> added.
> 
> Thx.
> 

Yes, will do this in v2. Thanks for reminding~
-- 
Regards,
Jiaxi

Re: [PATCH 1/6] x86: KVM: Enable CMPccXADD CPUID and expose it to guest

[Jiaxi Chen]

On 10/27/2022 1:15 AM, Borislav Petkov wrote:
> On Wed, Oct 26, 2022 at 11:40:31AM +0800, Jiaxi Chen wrote:
>>> What do you think about moving CPUID_7_1_EAX to be a KVM-only leaf too?  AFAICT,
>>> KVM passthrough is the only reason the existing features are defined.
> 
> Yap, looking at the patches which added those 2 feature flags upstream,
> they don't look like some particular use was the goal but rather to
> expose it to guests. Besides, AVX512 apps do their own CPUID detection.
> 
>> Since CPUID_7_1_EAX has only 5 features now, it is a big waste,       
>> should we move it to KVM-only leaf as Sean suggested. What's your     
>> opinion about this?                                                   
> 
> Yes, pls do.
> 
> And when you do, make sure to undo what
> 
>   b302e4b176d0 ("x86/cpufeatures: Enumerate the new AVX512 BFLOAT16 instructions")
> 
> added.
> 
> Thx.
> 
Hi Sean and Boris,

Just realized moving CPUID_7_1_EAX to kvm-only leaf will not save space
in enum cpuid_leafs[]. CPUID_7_1_EAX is indeed removed, but someone
else, ie. CPUID_DUMMY needs to take the place, otherwise the cpuid_leafs
array would be deranged. Therefore, the length of x86 cpuid leaves is
not decreased.

Wonder if the intention of moving this leaf to kvm-only is for saving
space in x86_capability[], or just because there's no other use case in
the host kernel side and the cpuflags of this features can be removed.

Hope for your suggestions.

-- 
Regards,
Jiaxi

Re: [PATCH 1/6] x86: KVM: Enable CMPccXADD CPUID and expose it to guest

[Sean Christopherson]

On Tue, Nov 01, 2022, Jiaxi Chen wrote:
> 
> 
> On 10/27/2022 1:15 AM, Borislav Petkov wrote:
> > On Wed, Oct 26, 2022 at 11:40:31AM +0800, Jiaxi Chen wrote:
> >>> What do you think about moving CPUID_7_1_EAX to be a KVM-only leaf too?  AFAICT,
> >>> KVM passthrough is the only reason the existing features are defined.
> > 
> > Yap, looking at the patches which added those 2 feature flags upstream,
> > they don't look like some particular use was the goal but rather to
> > expose it to guests. Besides, AVX512 apps do their own CPUID detection.
> > 
> >> Since CPUID_7_1_EAX has only 5 features now, it is a big waste,       
> >> should we move it to KVM-only leaf as Sean suggested. What's your     
> >> opinion about this?                                                   
> > 
> > Yes, pls do.
> > 
> > And when you do, make sure to undo what
> > 
> >   b302e4b176d0 ("x86/cpufeatures: Enumerate the new AVX512 BFLOAT16 instructions")
> > 
> > added.
> > 
> > Thx.
> > 
> Hi Sean and Boris,
> 
> Just realized moving CPUID_7_1_EAX to kvm-only leaf will not save space
> in enum cpuid_leafs[]. CPUID_7_1_EAX is indeed removed, but someone
> else, ie. CPUID_DUMMY needs to take the place, otherwise the cpuid_leafs
> array would be deranged. Therefore, the length of x86 cpuid leaves is
> not decreased.

The order of "enum cpuid_leafs" is completely arbitrary.

After replacing CPUID_7_1_EAX with CPUID_DUMMY, replace CPUID_DUMMY with the last
leaf, which is currently CPUID_8000_001F_EAX, and update the #defines accordingly.
Alternatively, Boris may prefer skipping the intermediate CPUID_DUMMY step and
just replace CPUID_7_1_EAX with CPUID_8000_001F_EAX straightaway.

Re: [PATCH 2/6] x86: KVM: Enable AMX-FP16 CPUID and expose it to guest

[Dave Hansen]

On 10/19/22 01:47, Jiaxi Chen wrote:
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 445626cb5779..9313240e3cdd 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -309,6 +309,7 @@
>  #define X86_FEATURE_AVX_VNNI		(12*32+ 4) /* AVX VNNI instructions */
>  #define X86_FEATURE_AVX512_BF16		(12*32+ 5) /* AVX512 BFLOAT16 instructions */
>  #define X86_FEATURE_CMPCCXADD           (12*32+ 7) /* CMPccXADD instructions */
> +#define X86_FEATURE_AMX_FP16		(12*32+21) /* AMX fp16 Support */

Please zap these from /proc/cpuinfo by doing this:

#define X86_FEATURE_AMX_FP16	(12*32+21) /* "" AMX fp16 Support */

>  /* AMD-defined CPU features, CPUID level 0x80000008 (EBX), word 13 */
>  #define X86_FEATURE_CLZERO		(13*32+ 0) /* CLZERO instruction */
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 3f745f6fdc43..d983ddb974ba 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -657,7 +657,7 @@ void kvm_set_cpu_caps(void)
>  		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL_SSBD);
>  
>  	kvm_cpu_cap_mask(CPUID_7_1_EAX,
> -		F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD)
> +		F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD) | F(AMX_FP16)
>  	);
>  
>  	kvm_cpu_cap_mask(CPUID_D_1_EAX,

KVM folks, is the idea that every feature that is enumerated to a guest
needs to be in one of these masks?  Or is there something special about
the features in these masks?

Re: [PATCH 2/6] x86: KVM: Enable AMX-FP16 CPUID and expose it to guest

[Paolo Bonzini]

On 11/2/22 19:14, Dave Hansen wrote:
>>   
>>   	kvm_cpu_cap_mask(CPUID_7_1_EAX,
>> -		F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD)
>> +		F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD) | F(AMX_FP16)
>>   	);
>>   
>>   	kvm_cpu_cap_mask(CPUID_D_1_EAX,
>
> KVM folks, is the idea that every feature that is enumerated to a guest
> needs to be in one of these masks?  Or is there something special about
> the features in these masks?

Yes, all features are vetted manually to see whether they require new 
MSRs and the like.  Therefore, anything that userspace can set in the 
guest's CPUID must be in the list.

Paolo

Re: [PATCH 2/6] x86: KVM: Enable AMX-FP16 CPUID and expose it to guest

[Dave Hansen]

On 11/2/22 11:16, Paolo Bonzini wrote:
> On 11/2/22 19:14, Dave Hansen wrote:
>>>         kvm_cpu_cap_mask(CPUID_7_1_EAX,
>>> -        F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD)
>>> +        F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD) | F(AMX_FP16)
>>>       );
>>>         kvm_cpu_cap_mask(CPUID_D_1_EAX,
>>
>> KVM folks, is the idea that every feature that is enumerated to a guest
>> needs to be in one of these masks?  Or is there something special about
>> the features in these masks?
> 
> Yes, all features are vetted manually to see whether they require new
> MSRs and the like.  Therefore, anything that userspace can set in the
> guest's CPUID must be in the list.

Makes sense.

Intel folks, when you add these bits, can you please include information
about the "vetting" that you performed?

For example, it would be handy to say:

	AMX_FP16 is just a new instruction that operates on existing AMX
	tile registers.  It needs no additional enabling on top of the
	existing kernel AMX enabling.

Re: [PATCH 1/6] x86: KVM: Enable CMPccXADD CPUID and expose it to guest

[Jiaxi Chen]

On 11/1/2022 11:07 PM, Sean Christopherson wrote:
> On Tue, Nov 01, 2022, Jiaxi Chen wrote:
>>
>>
>> On 10/27/2022 1:15 AM, Borislav Petkov wrote:
>>> On Wed, Oct 26, 2022 at 11:40:31AM +0800, Jiaxi Chen wrote:
>>>>> What do you think about moving CPUID_7_1_EAX to be a KVM-only leaf too?  AFAICT,
>>>>> KVM passthrough is the only reason the existing features are defined.
>>>
>>> Yap, looking at the patches which added those 2 feature flags upstream,
>>> they don't look like some particular use was the goal but rather to
>>> expose it to guests. Besides, AVX512 apps do their own CPUID detection.
>>>
>>>> Since CPUID_7_1_EAX has only 5 features now, it is a big waste,       
>>>> should we move it to KVM-only leaf as Sean suggested. What's your     
>>>> opinion about this?                                                   
>>>
>>> Yes, pls do.
>>>
>>> And when you do, make sure to undo what
>>>
>>>   b302e4b176d0 ("x86/cpufeatures: Enumerate the new AVX512 BFLOAT16 instructions")
>>>
>>> added.
>>>
>>> Thx.
>>>
>> Hi Sean and Boris,
>>
>> Just realized moving CPUID_7_1_EAX to kvm-only leaf will not save space
>> in enum cpuid_leafs[]. CPUID_7_1_EAX is indeed removed, but someone
>> else, ie. CPUID_DUMMY needs to take the place, otherwise the cpuid_leafs
>> array would be deranged. Therefore, the length of x86 cpuid leaves is
>> not decreased.
> 
> The order of "enum cpuid_leafs" is completely arbitrary.
> 
> After replacing CPUID_7_1_EAX with CPUID_DUMMY, replace CPUID_DUMMY with the last
> leaf, which is currently CPUID_8000_001F_EAX, and update the #defines accordingly.
> Alternatively, Boris may prefer skipping the intermediate CPUID_DUMMY step and
> just replace CPUID_7_1_EAX with CPUID_8000_001F_EAX straightaway.

Yes, thanks for Sean's kind suggestion. I think use CPUID_DUMMY as the
transition leaf will make the code logic and commit message clearer.
Will change it in v2.
-- 
Regards,
Jiaxi

Re: [PATCH 2/6] x86: KVM: Enable AMX-FP16 CPUID and expose it to guest

[Jiaxi Chen]

On 11/3/2022 2:21 AM, Dave Hansen wrote:
> On 11/2/22 11:16, Paolo Bonzini wrote:
>> On 11/2/22 19:14, Dave Hansen wrote:
>>>>         kvm_cpu_cap_mask(CPUID_7_1_EAX,
>>>> -        F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD)
>>>> +        F(AVX_VNNI) | F(AVX512_BF16) | F(CMPCCXADD) | F(AMX_FP16)
>>>>       );
>>>>         kvm_cpu_cap_mask(CPUID_D_1_EAX,
>>>
>>> KVM folks, is the idea that every feature that is enumerated to a guest
>>> needs to be in one of these masks?  Or is there something special about
>>> the features in these masks?
>>
>> Yes, all features are vetted manually to see whether they require new
>> MSRs and the like.  Therefore, anything that userspace can set in the
>> guest's CPUID must be in the list.
> 
> Makes sense.
> 
> Intel folks, when you add these bits, can you please include information
> about the "vetting" that you performed?
> 
> For example, it would be handy to say:
> 
> 	AMX_FP16 is just a new instruction that operates on existing AMX
> 	tile registers.  It needs no additional enabling on top of the
> 	existing kernel AMX enabling.

Thanks for Dave's suggestion.  It makes the commit message much clear
and readable. Will modify it in v2.

-- 
Regards,
Jiaxi