diff mbox series

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

Message ID 20221019084734.3590760-3-jiaxi.chen@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series x86: KVM: Expose CPUID to guest for new Intel platform instructions | expand

Commit Message

Jiaxi Chen Oct. 19, 2022, 8:47 a.m. UTC 
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(-)

Comments

Dave Hansen Nov. 2, 2022, 6:14 p.m. UTC | #1 
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?
Paolo Bonzini Nov. 2, 2022, 6:16 p.m. UTC | #2 
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
Dave Hansen Nov. 2, 2022, 6:21 p.m. UTC | #3 
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.
Jiaxi Chen Nov. 3, 2022, 2:38 a.m. UTC | #4 
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.
diff mbox series

Patch

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,