[PATCH v4 0/6] x86: KVM: Advertise CPUID of new Intel platform instructions to user space

[PATCH v4 0/6] x86: KVM: Advertise CPUID of new Intel platform instructions to user space

[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 patch series advertises KVM support of these CPUIDs to host
userspace.

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

v4:
 - Put CPUID_7_1_EAX back to cpuid_leaf[], considering more bits will be
   defined in the future for this leaf.

v3:
https://lore.kernel.org/lkml/20221110015252.202566-1-jiaxi.chen@linux.intel.com/
 - Remain CPUID_8000_001F_EAX in the last leaf of cpuid_leaf[]
 - Replace CPUID_7_1_EAX with CPUID_LNX_5, waiting for future new CPUIDs

v2:
https://lore.kernel.org/all/20221103025030.78371-1-jiaxi.chen@linux.intel.com/
 - Remove vague descriptions in the changelogs, including pronouns and
   "this patch" kind of things.
 - Move the two CPUIDs of cpuid_leaf[12] CPUID_7_1_EAX to kvm-only
   subleaves.
 - Replace cpuid_leaf[12] CPUID_7_1_EAX with the last leaf
   CPUID_8000_001F_EAX to shorten array length.
 - Change the newly-added CPUID leaf [CPUID_7_1_EDX] in v1 into kvm-only
   subleaves. 
 
v1: https://lore.kernel.org/all/20221019084734.3590760-1-jiaxi.chen@linux.intel.com/

Chang S. Bae (1):
  x86: KVM: Advertise AMX-FP16 CPUID to user space

Jiaxi Chen (5):
  x86: KVM: Advertise CMPccXADD CPUID to user space
  x86: KVM: Advertise AVX-IFMA CPUID to user space
  x86: KVM: Advertise AVX-VNNI-INT8 CPUID to user space
  x86: KVM: Advertise AVX-NE-CONVERT CPUID to user space
  x86: KVM: Advertise PREFETCHIT0/1 CPUID to user space

 arch/x86/include/asm/cpufeatures.h | 3 +++
 arch/x86/kvm/cpuid.c               | 8 ++++++--
 arch/x86/kvm/reverse_cpuid.h       | 7 +++++++
 3 files changed, 16 insertions(+), 2 deletions(-)


base-commit: f0c4d9fc9cc9462659728d168387191387e903cc
-- 
2.27.0

[PATCH v4 1/6] x86: KVM: Advertise CMPccXADD CPUID to user space

[Jiaxi Chen]

CMPccXADD is a new set of instructions in the latest Intel platform
Sierra Forest. This new instruction set 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 CPUID is exposed to userspace. Besides, there is no other VMX
control for this instruction.

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 b71f4f2ecdd5..19db3940f262 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 62bc7a01cecc..7ab7cc717b1c 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 v4 2/6] x86: KVM: Advertise AMX-FP16 CPUID to user space

[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 CPUID is exposed to user space. Besides, there is no other VMX
control for this instruction.

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 19db3940f262..df4a7f7505a9 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 7ab7cc717b1c..2a334d4cd04e 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 v4 3/6] x86: KVM: Advertise AVX-IFMA CPUID to user space

[Jiaxi Chen]

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

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

This CPUID is exposed to user space. Besides, there is no other VMX
control for this instruction.

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 df4a7f7505a9..159f8b9898bf 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 2a334d4cd04e..5726afb2d14c 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 v4 4/6] x86: KVM: Advertise AVX-VNNI-INT8 CPUID to user space

[Jiaxi Chen]

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

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

This CPUID is exposed to user space. Besides, there is no other VMX
control for this instruction.

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

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 5726afb2d14c..e2b8e5485474 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_init_scattered(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..8357480b75d6 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,9 @@ 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 (EDX) */
+#define X86_FEATURE_AVX_VNNI_INT8       KVM_X86_FEATURE(CPUID_7_1_EDX, 4)
+
 struct cpuid_reg {
 	u32 function;
 	u32 index;
@@ -48,6 +52,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 v4 5/6] x86: KVM: Advertise AVX-NE-CONVERT CPUID to user space

[Jiaxi Chen]

AVX-NE-CONVERT is a new set of instructions which can convert low
precision floating point like BF16/FP16 to high precision floating point
FP32, and 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 CPUID is exposed to user space. Besides, there is no other VMX
control for this instruction.

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

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index e2b8e5485474..f87292ae69d2 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_init_scattered(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
diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
index 8357480b75d6..d0fc43c3b2fd 100644
--- a/arch/x86/kvm/reverse_cpuid.h
+++ b/arch/x86/kvm/reverse_cpuid.h
@@ -27,6 +27,7 @@ enum kvm_only_cpuid_leafs {
 
 /* Intel-defined sub-features, CPUID level 0x00000007:1 (EDX) */
 #define X86_FEATURE_AVX_VNNI_INT8       KVM_X86_FEATURE(CPUID_7_1_EDX, 4)
+#define X86_FEATURE_AVX_NE_CONVERT      KVM_X86_FEATURE(CPUID_7_1_EDX, 5)
 
 struct cpuid_reg {
 	u32 function;
-- 
2.27.0

[PATCH v4 6/6] x86: KVM: Advertise PREFETCHIT0/1 CPUID to user space

[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 CPUID is exposed to user space. Besides, there is no other VMX
control for this instruction.

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

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index f87292ae69d2..35eaee40c1bb 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_init_scattered(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,
diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
index d0fc43c3b2fd..49bb06163fb4 100644
--- a/arch/x86/kvm/reverse_cpuid.h
+++ b/arch/x86/kvm/reverse_cpuid.h
@@ -28,6 +28,7 @@ enum kvm_only_cpuid_leafs {
 /* Intel-defined sub-features, CPUID level 0x00000007:1 (EDX) */
 #define X86_FEATURE_AVX_VNNI_INT8       KVM_X86_FEATURE(CPUID_7_1_EDX, 4)
 #define X86_FEATURE_AVX_NE_CONVERT      KVM_X86_FEATURE(CPUID_7_1_EDX, 5)
+#define X86_FEATURE_PREFETCHITI         KVM_X86_FEATURE(CPUID_7_1_EDX, 14)
 
 struct cpuid_reg {
 	u32 function;
-- 
2.27.0

Re: [PATCH v4 0/6] x86: KVM: Advertise CPUID of new Intel platform instructions to user space

[Borislav Petkov]

On Fri, Nov 18, 2022 at 10:15:03PM +0800, Jiaxi Chen wrote:
> Chang S. Bae (1):
>   x86: KVM: Advertise AMX-FP16 CPUID to user space
> 
> Jiaxi Chen (5):
>   x86: KVM: Advertise CMPccXADD CPUID to user space
>   x86: KVM: Advertise AVX-IFMA CPUID to user space
>   x86: KVM: Advertise AVX-VNNI-INT8 CPUID to user space
>   x86: KVM: Advertise AVX-NE-CONVERT CPUID to user space
>   x86: KVM: Advertise PREFETCHIT0/1 CPUID to user space
> 
>  arch/x86/include/asm/cpufeatures.h | 3 +++

For the x86 bits:

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v4 3/6] x86: KVM: Advertise AVX-IFMA CPUID to user space

[Sean Christopherson]

On Fri, Nov 18, 2022, Jiaxi Chen wrote:
> AVX-IFMA is a new instruction in the latest Intel platform Sierra
> Forest. This instruction packed multiplies unsigned 52-bit integers and
> adds the low/high 52-bit products to Qword Accumulators.
> 
> The bit definition:
> CPUID.(EAX=7,ECX=1):EAX[bit 23]
> 
> This CPUID is exposed to user space. Besides, there is no other VMX
> control for this instruction.
> 
> 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 df4a7f7505a9..159f8b9898bf 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 2a334d4cd04e..5726afb2d14c 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));

Please keep the terminating paranthesis+semicolon on a separate line.  KVM isn't
100% consistent (as usual), but I would rather "fix" the cases that don't put
the terminators on their own line. 

Re: [PATCH v4 1/6] x86: KVM: Advertise CMPccXADD CPUID to user space

[Dave Hansen]

On 11/18/22 06:15, Jiaxi Chen wrote:
> CMPccXADD is a new set of instructions in the latest Intel platform
> Sierra Forest. This new instruction set 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 CPUID is exposed to userspace. Besides, there is no other VMX
> control for this instruction.

The last time you posted these, I asked:

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

I think you're alluding to that in your comment about VMX contols.
Could you be more explicit here and include *all* of your logic about
why this feature is OK to pass through to guests?

Also, do we *want* this showing up in /proc/cpuinfo?

There are also two distinct kinds of features that you're adding here.
These:

> +#define X86_FEATURE_CMPCCXADD           (12*32+ 7) /* CMPccXADD instructions */

and these:

+#define X86_FEATURE_PREFETCHITI         KVM_X86_FEATURE(CPUID_7_1_EDX, 14)

Could you also please include a sentence or two about why the feature
was treated on way versus another?  That's frankly a lot more important
than telling us which random Intel codename this shows up on first, or
wasting space on telling us what the CPUID bit definition is.  We can
kinda get that from the patch.

Another nit on these:

> This CPUID is exposed to userspace. Besides, there is no other VMX
> control for this instruction.

Please remember to use imperative voice when describing what the patch
in question does.  Using passive voice like that makes it seem like
you're describing the state of the art rather than the patch.

For example, that should probably be:

	Expose CMPCCXADD to KVM userspace.  This is safe because there
	are no new VMX controls or host enabling required for guests to
	use this feature.

See how that first sentence is giving orders?  It's *telling* you what
to do.  That's imperative voice and that's what you use to describe the
actions of *this* patch.

Re: [PATCH v4 4/6] x86: KVM: Advertise AVX-VNNI-INT8 CPUID to user space

[Sean Christopherson]

[-- Attachment #1: Type: text/plain, Size: 1694 bytes --]

On Fri, Nov 18, 2022, Jiaxi Chen wrote:
> AVX-VNNI-INT8 is a new set of instructions in the latest Intel platform
> Sierra Forest, aims for the platform to have superior AI capabilities.
> This instruction multiplies the individual bytes of two unsigned or
> unsigned source operands, then adds and accumulates the results into the
> destination dword element size operand.
> 
> The bit definition:
> CPUID.(EAX=7,ECX=1):EDX[bit 4]
> 
> This CPUID is exposed to user space. Besides, there is no other VMX
> control for this instruction.
> 
> Signed-off-by: Jiaxi Chen <jiaxi.chen@linux.intel.com>
> ---
>  arch/x86/kvm/cpuid.c         | 5 ++++-
>  arch/x86/kvm/reverse_cpuid.h | 5 +++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 5726afb2d14c..e2b8e5485474 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_init_scattered(CPUID_7_1_EDX,

Ah, this is going to be confusing and potentially error prone.  AVX_VNNI_INT8
isn't actually scattered, i.e. kvm_cpu_cap_init_scattered() is poorly named.  And
using SF() would be _really_ broken as boot_cpu_has() would consume garbage and
potentially leak kernel state to userspace.

To address these issue and also document how to add KVM-only features, can you
slot in the two attached patches at the begining of this series?

Thanks!

> +		F(AVX_VNNI_INT8));

Terminators on a separate line please.

>  	kvm_cpu_cap_mask(CPUID_D_1_EAX,
>  		F(XSAVEOPT) | F(XSAVEC) | F(XGETBV1) | F(XSAVES) | f_xfd
>  	);

[-- Attachment #2: 0001-KVM-x86-Add-BUILD_BUG_ON-to-detect-bad-usage-of-scat.patch --]
[-- Type: text/x-diff, Size: 1710 bytes --]

From d913e35721688aca42056e57a261fa4baad0c45e Mon Sep 17 00:00:00 2001
From: Sean Christopherson <seanjc@google.com>
Date: Fri, 18 Nov 2022 08:17:55 -0800
Subject: [PATCH 1/2] KVM: x86: Add BUILD_BUG_ON() to detect bad usage of
 "scattered" flags

Add a compile-time assert in the SF() macro to detect improper usage,
i.e. to detect passing in an X86_FEATURE_* flag that isn't actually
scattered by the kernel.  Upcoming feature flags will be 100% KVM-only
and will have X86_FEATURE_* macros that point at a kvm_only_cpuid_leafs
word, not a kernel-defined word.  Using SF() and thus boot_cpu_has() for
such feature flags would access memory beyond x86_capability[NCAPINTS]
and at best incorrectly hide a feature, and at worst leak kernel state to
userspace.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/cpuid.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 6b5912578edd..ff2e9734e5c1 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -65,7 +65,13 @@ u32 xstate_required_size(u64 xstate_bv, bool compacted)
 #define KVM_X86_FEATURE_AMD_PSFD	(13*32+28) /* Predictive Store Forwarding Disable */
 
 #define F feature_bit
-#define SF(name) (boot_cpu_has(X86_FEATURE_##name) ? F(name) : 0)
+
+/* Scattered Flag - For features that are scattered by cpufeatures.h. */
+#define SF(name)						\
+({								\
+	BUILD_BUG_ON(X86_FEATURE_##name >= MAX_CPU_FEATURES);	\
+	(boot_cpu_has(X86_FEATURE_##name) ? F(name) : 0);	\
+})
 
 /*
  * Magic value used by KVM when querying userspace-provided CPUID entries and

base-commit: d663b8a285986072428a6a145e5994bc275df994
-- 
2.38.1.584.g0f3c55d4c2-goog


[-- Attachment #3: 0002-KVM-x86-Update-KVM-only-leaf-handling-to-allow-for-1.patch --]
[-- Type: text/x-diff, Size: 3595 bytes --]

From 565a06e1d6e1ea40daa113bc2b3d10e7b2a8a508 Mon Sep 17 00:00:00 2001
From: Sean Christopherson <seanjc@google.com>
Date: Fri, 18 Nov 2022 08:52:28 -0800
Subject: [PATCH 2/2] KVM: x86: Update KVM-only leaf handling to allow for 100%
 KVM-only leafs

Rename kvm_cpu_cap_init_scattered() to kvm_cpu_cap_init_kvm_defined() in
anticipation of adding KVM-only CPUID leafs that aren't recognized by the
kernel and thus not scattered, i.e. for leafs that are 100% KVM-defined.

Adjust/add comments to kvm_only_cpuid_leafs and KVM_X86_FEATURE to
document how to create new kvm_only_cpuid_leafs entries for scattered
features as well as features that are entirely unknown to the kernel.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/cpuid.c         |  8 ++++----
 arch/x86/kvm/reverse_cpuid.h | 18 +++++++++++++++---
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index ff2e9734e5c1..73c3c6dc6e7b 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -549,9 +549,9 @@ static __always_inline void __kvm_cpu_cap_mask(unsigned int leaf)
 }
 
 static __always_inline
-void kvm_cpu_cap_init_scattered(enum kvm_only_cpuid_leafs leaf, u32 mask)
+void kvm_cpu_cap_init_kvm_defined(enum kvm_only_cpuid_leafs leaf, u32 mask)
 {
-	/* Use kvm_cpu_cap_mask for non-scattered leafs. */
+	/* Use kvm_cpu_cap_mask for leafs that aren't KVM-only. */
 	BUILD_BUG_ON(leaf < NCAPINTS);
 
 	kvm_cpu_caps[leaf] = mask;
@@ -561,7 +561,7 @@ void kvm_cpu_cap_init_scattered(enum kvm_only_cpuid_leafs leaf, u32 mask)
 
 static __always_inline void kvm_cpu_cap_mask(enum cpuid_leafs leaf, u32 mask)
 {
-	/* Use kvm_cpu_cap_init_scattered for scattered leafs. */
+	/* Use kvm_cpu_cap_init_kvm_defined for KVM-only leafs. */
 	BUILD_BUG_ON(leaf >= NCAPINTS);
 
 	kvm_cpu_caps[leaf] &= mask;
@@ -670,7 +670,7 @@ void kvm_set_cpu_caps(void)
 		F(XSAVEOPT) | F(XSAVEC) | F(XGETBV1) | F(XSAVES) | f_xfd
 	);
 
-	kvm_cpu_cap_init_scattered(CPUID_12_EAX,
+	kvm_cpu_cap_init_kvm_defined(CPUID_12_EAX,
 		SF(SGX1) | SF(SGX2)
 	);
 
diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
index a19d473d0184..443a6b3e66c0 100644
--- a/arch/x86/kvm/reverse_cpuid.h
+++ b/arch/x86/kvm/reverse_cpuid.h
@@ -7,9 +7,9 @@
 #include <asm/cpufeatures.h>
 
 /*
- * Hardware-defined CPUID leafs that are scattered in the kernel, but need to
- * be directly used by KVM.  Note, these word values conflict with the kernel's
- * "bug" caps, but KVM doesn't use those.
+ * Hardware-defined CPUID leafs that are either scattered by the kernel or are
+ * unknown to the kernel, but need to be directly used by KVM.  Note, these
+ * word values conflict with the kernel's "bug" caps, but KVM doesn't use those.
  */
 enum kvm_only_cpuid_leafs {
 	CPUID_12_EAX	 = NCAPINTS,
@@ -18,6 +18,18 @@ enum kvm_only_cpuid_leafs {
 	NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
 };
 
+/*
+ * Define a KVM-only feature flag.
+ *
+ * For features that are scattered by cpufeatures.h, __feature_translate() also
+ * needs to be updated to translate the kernel-defined feature into the
+ * KVM-defined feature.
+ *
+ * For features that are 100% KVM-only, i.e. not defined by cpufeatures.h,
+ * forego the intermediate KVM_X86_FEATURE and directly define X86_FEATURE_* so
+ * that X86_FEATURE_* can be used in KVM.  No __feature_translate() handling is
+ * needed in this case.
+ */
 #define KVM_X86_FEATURE(w, f)		((w)*32 + (f))
 
 /* Intel-defined SGX sub-features, CPUID level 0x12 (EAX). */
-- 
2.38.1.584.g0f3c55d4c2-goog

Re: [PATCH v4 1/6] x86: KVM: Advertise CMPccXADD CPUID to user space

[Borislav Petkov]

On Fri, Nov 18, 2022 at 08:47:55AM -0800, Dave Hansen wrote:
> Also, do we *want* this showing up in /proc/cpuinfo?

Yeah, I was wondering about that. Currently, we try to add feature bits
to /proc/cpuinfo only when the kernel has done any enablement for them.
For other needs, people should use tools/arch/x86/kcpuid/

If we say that adding those bits so that guests can export them doesn't
count as a real enablement, then sure we can hide them initially.

I wanna say yes here...

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v4 1/6] x86: KVM: Advertise CMPccXADD CPUID to user space

[Jiaxi Chen]

On 11/19/2022 12:47 AM, Dave Hansen wrote:
> On 11/18/22 06:15, Jiaxi Chen wrote:
>> CMPccXADD is a new set of instructions in the latest Intel platform
>> Sierra Forest. This new instruction set 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 CPUID is exposed to userspace. Besides, there is no other VMX
>> control for this instruction.
> 
> The last time you posted these, I asked:
> 
>> Intel folks, when you add these bits, can you please include information
>> about the "vetting" that you performed?
> 
> I think you're alluding to that in your comment about VMX contols.
> Could you be more explicit here and include *all* of your logic about
> why this feature is OK to pass through to guests?
> 
Yes, that's very rigorous. Will check and add these for all features in
this patch series.

> Also, do we *want* this showing up in /proc/cpuinfo?
>
> There are also two distinct kinds of features that you're adding here.
> These:
> 
>> +#define X86_FEATURE_CMPCCXADD           (12*32+ 7) /* CMPccXADD instructions */
> 
> and these:
> 
> +#define X86_FEATURE_PREFETCHITI         KVM_X86_FEATURE(CPUID_7_1_EDX, 14)
> 
> Could you also please include a sentence or two about why the feature
> was treated on way versus another?  That's frankly a lot more important
> than telling us which random Intel codename this shows up on first, or
> wasting space on telling us what the CPUID bit definition is.  We can
> kinda get that from the patch.
Yes. A few words of description is necessary here.

Features which has been enabled in kernel usually should be added to
/proc/cpuinfo.

The first way is often used for bit whose leaf has many other bits in
use. It's very simple to do, just adding one line for each feature based
on existing words in can get the effect.

For those bits whose leaf has just a few bits in use, they should be
defined in a 'scattered' way. However, this kind of features in this
patch series have no other kernel usage and they just need to be
advertised to kvm userspace. Therefore, define them in a kvm-only way is
more explicit.

> 
> Another nit on these:
> 
>> This CPUID is exposed to userspace. Besides, there is no other VMX
>> control for this instruction.
> 
> Please remember to use imperative voice when describing what the patch
> in question does.  Using passive voice like that makes it seem like
> you're describing the state of the art rather than the patch.
> 
> For example, that should probably be:
> 
> 	Expose CMPCCXADD to KVM userspace.  This is safe because there
> 	are no new VMX controls or host enabling required for guests to
> 	use this feature.
> 
> See how that first sentence is giving orders?  It's *telling* you what
> to do.  That's imperative voice and that's what you use to describe the
> actions of *this* patch.

Appreciate your very detailed suggestions. Thanks very much!
-- 
Regards,
Jiaxi

Re: [PATCH v4 3/6] x86: KVM: Advertise AVX-IFMA CPUID to user space

[Jiaxi Chen]

On 11/19/2022 12:08 AM, Sean Christopherson wrote:
> On Fri, Nov 18, 2022, Jiaxi Chen wrote:
>> AVX-IFMA is a new instruction in the latest Intel platform Sierra
>> Forest. This instruction packed multiplies unsigned 52-bit integers and
>> adds the low/high 52-bit products to Qword Accumulators.
>>
>> The bit definition:
>> CPUID.(EAX=7,ECX=1):EAX[bit 23]
>>
>> This CPUID is exposed to user space. Besides, there is no other VMX
>> control for this instruction.
>>
>> 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 df4a7f7505a9..159f8b9898bf 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 2a334d4cd04e..5726afb2d14c 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));
> 
> Please keep the terminating paranthesis+semicolon on a separate line.  KVM isn't
> 100% consistent (as usual), but I would rather "fix" the cases that don't put
> the terminators on their own line. 

That's very careful. Thank you~
-- 
Regards,
Jiaxi

Re: [PATCH v4 4/6] x86: KVM: Advertise AVX-VNNI-INT8 CPUID to user space

[Jiaxi Chen]

On 11/19/2022 1:17 AM, Sean Christopherson wrote:
> On Fri, Nov 18, 2022, Jiaxi Chen wrote:
>> AVX-VNNI-INT8 is a new set of instructions in the latest Intel platform
>> Sierra Forest, aims for the platform to have superior AI capabilities.
>> This instruction multiplies the individual bytes of two unsigned or
>> unsigned source operands, then adds and accumulates the results into the
>> destination dword element size operand.
>>
>> The bit definition:
>> CPUID.(EAX=7,ECX=1):EDX[bit 4]
>>
>> This CPUID is exposed to user space. Besides, there is no other VMX
>> control for this instruction.
>>
>> Signed-off-by: Jiaxi Chen <jiaxi.chen@linux.intel.com>
>> ---
>>  arch/x86/kvm/cpuid.c         | 5 ++++-
>>  arch/x86/kvm/reverse_cpuid.h | 5 +++++
>>  2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>> index 5726afb2d14c..e2b8e5485474 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_init_scattered(CPUID_7_1_EDX,
> 
> Ah, this is going to be confusing and potentially error prone.  AVX_VNNI_INT8
> isn't actually scattered, i.e. kvm_cpu_cap_init_scattered() is poorly named.  And
> using SF() would be _really_ broken as boot_cpu_has() would consume garbage and
> potentially leak kernel state to userspace.
> 
> To address these issue and also document how to add KVM-only features, can you
> slot in the two attached patches at the begining of this series?
> 
Sure. Thanks for your kind contribution.
> Thanks!
> 
>> +		F(AVX_VNNI_INT8));
> 
> Terminators on a separate line please.
> 
>>  	kvm_cpu_cap_mask(CPUID_D_1_EAX,
>>  		F(XSAVEOPT) | F(XSAVEC) | F(XGETBV1) | F(XSAVES) | f_xfd
>>  	);

Got it. Thanks~
-- 
Regards,
Jiaxi

Re: [PATCH v4 1/6] x86: KVM: Advertise CMPccXADD CPUID to user space

[Dave Hansen]

On 11/21/22 06:46, Jiaxi Chen wrote:
> Features which has been enabled in kernel usually should be added to
> /proc/cpuinfo.

Features that the kernel *itself* is actually using always get in there.
 Things like "smep".

But, things that the kernel "enables" but that only get used by
userspace don't generally show up in /proc/cpuinfo.

KVM is kinda a weird case.  The kernel is making the feature available
to guests, but it's not _using_ it in any meaningful way.  To me, this
seems much more akin to the features that are just available to
userspace than something that the kernel is truly using.

Also, these feature names are just long and ugly, and the "flags" line
is already a human-*un*readable mess.  I think we should just leave them
out.

Re: [PATCH v4 1/6] x86: KVM: Advertise CMPccXADD CPUID to user space

[Borislav Petkov]

On Mon, Nov 21, 2022 at 10:46:21PM +0800, Jiaxi Chen wrote:
> Features which has been enabled in kernel usually should be added to
> /proc/cpuinfo.

No, pls read this first: Documentation/x86/cpuinfo.rst

If something's not clear, we will extend it so that it is.

/proc/cpuinfo - a user ABI - is not a dumping ground for CPUID bits.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v4 1/6] x86: KVM: Advertise CMPccXADD CPUID to user space

[Sean Christopherson]

On Mon, Nov 21, 2022, Dave Hansen wrote:
> On 11/21/22 06:46, Jiaxi Chen wrote:
> > Features which has been enabled in kernel usually should be added to
> > /proc/cpuinfo.
> 
> Features that the kernel *itself* is actually using always get in there.
>  Things like "smep".
> 
> But, things that the kernel "enables" but that only get used by
> userspace don't generally show up in /proc/cpuinfo.
> 
> KVM is kinda a weird case.  The kernel is making the feature available
> to guests, but it's not _using_ it in any meaningful way.  To me, this
> seems much more akin to the features that are just available to
> userspace than something that the kernel is truly using.

Actually, for these features that don't require additional KVM enabling, KVM isn't
making the feature avaiable to the guest.  KVM is just advertising to userspace
that KVM "supports" these features.  Userspace ultimately controls guest CPUID;
outside of a few special cases, KVM neither rejects nor filters unsupported bits
in CPUID.

Most VMMs will only enable features that KVM "officially" supports, but for these
features, nothing stops userspace from enumerating support to the guest without
waiting for KVM.

> Also, these feature names are just long and ugly, and the "flags" line
> is already a human-*un*readable mess.  I think we should just leave them
> out.

I have no strong opinion, either way works for me.

Re: [PATCH v4 1/6] x86: KVM: Advertise CMPccXADD CPUID to user space

[Borislav Petkov]

On Mon, Nov 21, 2022 at 03:48:06PM +0000, Sean Christopherson wrote:
> Actually, for these features that don't require additional KVM enabling, KVM isn't
> making the feature avaiable to the guest.  KVM is just advertising to userspace
> that KVM "supports" these features.  Userspace ultimately controls guest CPUID;
> outside of a few special cases, KVM neither rejects nor filters unsupported bits
> in CPUID.

So is there any point to those "enable it in KVM" patches streaming constantly?

AFAICT, the only reason is to "pass through" the CPUID bit to the guest
in case KVM is not showing it in the intercepted CPUID...

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v4 1/6] x86: KVM: Advertise CMPccXADD CPUID to user space

[Sean Christopherson]

On Mon, Nov 21, 2022, Borislav Petkov wrote:
> On Mon, Nov 21, 2022 at 03:48:06PM +0000, Sean Christopherson wrote:
> > Actually, for these features that don't require additional KVM enabling, KVM isn't
> > making the feature avaiable to the guest.  KVM is just advertising to userspace
> > that KVM "supports" these features.  Userspace ultimately controls guest CPUID;
> > outside of a few special cases, KVM neither rejects nor filters unsupported bits
> > in CPUID.
> 
> So is there any point to those "enable it in KVM" patches streaming constantly?

Yes.  Most userspace VMMs sanitize their CPUID models based on KVM_GET_SUPPORTED_CPUID,
e.g. by default, QEMU will refuse to enable features in guest CPUID that aren't
reported as supported by KVM.

Another use case is for userspace to blindly use the result of KVM_GET_SUPPORTED_CPUID
as the guest's CPUID model, e.g. for using KVM to isolate code as opposed to standing
up a traditional virtual machine.  For that use case, userspace again relies on KVM to
enumerate support.

What I was trying to call out in the above is that the KVM "enabling" technically
doesn't expose the feature to the guest.  E.g. a clever guest could ignore CPUID
and probe the relevant instructions manually by seeing whether or not they #UD.

Re: [PATCH v4 1/6] x86: KVM: Advertise CMPccXADD CPUID to user space

[Borislav Petkov]

On Mon, Nov 21, 2022 at 05:28:39PM +0000, Sean Christopherson wrote:
> Yes.  Most userspace VMMs sanitize their CPUID models based on KVM_GET_SUPPORTED_CPUID,
> e.g. by default, QEMU will refuse to enable features in guest CPUID that aren't
> reported as supported by KVM.
> 
> Another use case is for userspace to blindly use the result of KVM_GET_SUPPORTED_CPUID
> as the guest's CPUID model, e.g. for using KVM to isolate code as opposed to standing
> up a traditional virtual machine.  For that use case, userspace again relies on KVM to
> enumerate support.

Ah ok, thx.

/me makes a mental note.

> What I was trying to call out in the above is that the KVM "enabling" technically
> doesn't expose the feature to the guest.  E.g. a clever guest could ignore CPUID
> and probe the relevant instructions manually by seeing whether or not they #UD.

As can a nasty userspace on baremetal. That's why /proc/cpuinfo is not
really the authority of what's supported and we're going away from
treating it that way.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v4 1/6] x86: KVM: Advertise CMPccXADD CPUID to user space

[Jiaxi Chen]

On 11/21/2022 11:29 PM, Dave Hansen wrote:
> On 11/21/22 06:46, Jiaxi Chen wrote:
>> Features which has been enabled in kernel usually should be added to
>> /proc/cpuinfo.
> 
> Features that the kernel *itself* is actually using always get in there.
>  Things like "smep".
> 
> But, things that the kernel "enables" but that only get used by
> userspace don't generally show up in /proc/cpuinfo.
> 
> KVM is kinda a weird case.  The kernel is making the feature available
> to guests, but it's not _using_ it in any meaningful way.  To me, this
> seems much more akin to the features that are just available to
> userspace than something that the kernel is truly using.
> 
> Also, these feature names are just long and ugly, and the "flags" line
> is already a human-*un*readable mess.  I think we should just leave them
> out.

True and agree. As for these cpuids are not truly used by kernel except
for advertising to kvm userspace, we can hide them in /proc/cpuinfo by
overriding their name with "".

-- 
Regards,
Jiaxi

Re: [PATCH v4 1/6] x86: KVM: Advertise CMPccXADD CPUID to user space

[Jiaxi Chen]

On 11/21/2022 11:38 PM, Borislav Petkov wrote:
> On Mon, Nov 21, 2022 at 10:46:21PM +0800, Jiaxi Chen wrote:
>> Features which has been enabled in kernel usually should be added to
>> /proc/cpuinfo.
> 
> No, pls read this first: Documentation/x86/cpuinfo.rst
> 
> If something's not clear, we will extend it so that it is.
> 
> /proc/cpuinfo - a user ABI - is not a dumping ground for CPUID bits.
> 

Thanks. Sorry for the miss understanding.

For those feature bits who have truly kernel usage, their flags should
appear in /proc/cpuinfo. For others, they are not generally show up
here, it depends.

As for features in this patch series:

The first-way defined bits are on an expected-dense cpuid leaf[1] and
some of their siblings have kernel usages[2]. Given that, define them
like X86_FEATURE_* in arch/x86/include/asm/cpufeatures.h. But due to
their complicated and unreadable feature name[3], prefer to hide them in
/proc/cpuinfo.

The second-way defined bits are on a new and sparse cpuid leaf. Besides,
these bits have no turly kernel use case. Therefore, move these new bits
to kvm-only leaves to achieve the purpose for advertising these bits to
kvm userspace[4]. Then of course they will not show up in /proc/cpuinfo.

[1] https://lore.kernel.org/all/Y3O7UYWfOLfJkwM%2F@zn.tnic/
[2]
https://lore.kernel.org/all/f8607d23-afaa-2670-dd03-2ae8ec1e79a0@intel.com/
[3]
https://lore.kernel.org/all/6d7fae50-ef3c-dc1e-336c-691095007117@intel.com/
[4] https://lore.kernel.org/all/Y1ATKF2xjERFbspn@google.com/

-- 
Regards,
Jiaxi