diff mbox series

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

Message ID 20221118141509.489359-5-jiaxi.chen@linux.intel.com
State New
Headers
Series x86: KVM: Advertise CPUID of new Intel platform instructions to user space |

Commit Message

Jiaxi Chen Nov. 18, 2022, 2:15 p.m. UTC 
  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(-)

Comments

Sean Christopherson Nov. 18, 2022, 5:17 p.m. UTC | #1 
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
>  	);
Jiaxi Chen Nov. 21, 2022, 3:06 p.m. UTC | #2 
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~
diff mbox series

Patch

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},
 };
 
 /*