[PATCH] x86/cpufeatures: Move the definition of X86_FEATURE_AMX_* to the word 18

[PATCH] x86/cpufeatures: Move the definition of X86_FEATURE_AMX_* to the word 18

[Like Xu]

From: Like Xu <likexu@tencent.com>

We have defined the word 18 for Intel-defined CPU features from CPUID level
0x00000007:0 (EDX). Let's move the definitions of X86_FEATURE_AMX_* to the
right entry to prevent misinterpretation. No functional change intended.

Signed-off-by: Like Xu <likexu@tencent.com>
---
 arch/x86/include/asm/cpufeatures.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 6db4e2932b3d..5cd22090e53d 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -299,9 +299,6 @@
 /* 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_AMX_BF16		(18*32+22) /* AMX bf16 Support */
-#define X86_FEATURE_AMX_TILE		(18*32+24) /* AMX tile Support */
-#define X86_FEATURE_AMX_INT8		(18*32+25) /* AMX int8 Support */
 
 /* AMD-defined CPU features, CPUID level 0x80000008 (EBX), word 13 */
 #define X86_FEATURE_CLZERO		(13*32+ 0) /* CLZERO instruction */
@@ -390,7 +387,10 @@
 #define X86_FEATURE_TSXLDTRK		(18*32+16) /* TSX Suspend Load Address Tracking */
 #define X86_FEATURE_PCONFIG		(18*32+18) /* Intel PCONFIG */
 #define X86_FEATURE_ARCH_LBR		(18*32+19) /* Intel ARCH LBR */
+#define X86_FEATURE_AMX_BF16		(18*32+22) /* AMX bf16 Support */
 #define X86_FEATURE_AVX512_FP16		(18*32+23) /* AVX512 FP16 */
+#define X86_FEATURE_AMX_TILE		(18*32+24) /* AMX tile Support */
+#define X86_FEATURE_AMX_INT8		(18*32+25) /* AMX int8 Support */
 #define X86_FEATURE_SPEC_CTRL		(18*32+26) /* "" Speculation Control (IBRS + IBPB) */
 #define X86_FEATURE_INTEL_STIBP		(18*32+27) /* "" Single Thread Indirect Branch Predictors */
 #define X86_FEATURE_FLUSH_L1D		(18*32+28) /* Flush L1D cache */
-- 
2.33.1

Re: [PATCH] x86/cpufeatures: Move the definition of X86_FEATURE_AMX_* to the word 18

[Paolo Bonzini]

On 1/17/22 07:23, Like Xu wrote:
> We have defined the word 18 for Intel-defined CPU features from CPUID level
> 0x00000007:0 (EDX). Let's move the definitions of X86_FEATURE_AMX_* to the
> right entry to prevent misinterpretation. No functional change intended.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [PATCH] x86/cpufeatures: Move the definition of X86_FEATURE_AMX_* to the word 18

[Dave Hansen]

What tree is this against?  I see BF16 and INT8 in some old versions of
Chang's patches, but not current kernels.  All I see right now in
tip/master is:

> #define X86_FEATURE_AMX_TILE            (18*32+24) /* AMX tile ...

It's still in the wrong spot, but the other two features aren't there.

> We have defined the word 18 for Intel-defined CPU features from CPUID level> 0x00000007:0 (EDX). Let's move the definitions of X86_FEATURE_AMX_* to
the> right entry to prevent misinterpretation. No functional change
intended.
Please, no "we's" in changelogs.  Don't say, "let's move".  Just say:
"Move..."

The subject could probably also be trimmed a bit.  Perhaps:

	x86/cpu: Move AMX CPU feature defines to correct word location

Re: [PATCH] x86/cpufeatures: Move the definition of X86_FEATURE_AMX_* to the word 18

[Paolo Bonzini]

On 1/18/22 18:11, Dave Hansen wrote:
> What tree is this against?  I see BF16 and INT8 in some old versions of
> Chang's patches, but not current kernels.  All I see right now in
> tip/master is:
> 
>> #define X86_FEATURE_AMX_TILE            (18*32+24) /* AMX tile ...
> 
> It's still in the wrong spot, but the other two features aren't there.

It was added for the KVM side of AMX (commit 690a757d610e, "kvm: x86: 
Add CPUID support for Intel AMX") and is in Linus's tree.

Paolo


>> We have defined the word 18 for Intel-defined CPU features from CPUID level> 0x00000007:0 (EDX). Let's move the definitions of X86_FEATURE_AMX_* to
> the> right entry to prevent misinterpretation. No functional change
> intended.
> Please, no "we's" in changelogs.  Don't say, "let's move".  Just say:
> "Move..."
> 
> The subject could probably also be trimmed a bit.  Perhaps:
> 
> 	x86/cpu: Move AMX CPU feature defines to correct word location
> 
> 

Re: [PATCH] x86/cpufeatures: Move the definition of X86_FEATURE_AMX_* to the word 18

[Like Xu]

On 19/1/2022 1:15 am, Paolo Bonzini wrote:
> On 1/18/22 18:11, Dave Hansen wrote:
>> What tree is this against?  I see BF16 and INT8 in some old versions of
>> Chang's patches, but not current kernels.  All I see right now in
>> tip/master is:
>>
>>> #define X86_FEATURE_AMX_TILE            (18*32+24) /* AMX tile ...
>>
>> It's still in the wrong spot, but the other two features aren't there.
> 
> It was added for the KVM side of AMX (commit 690a757d610e, "kvm: x86: Add CPUID 
> support for Intel AMX") and is in Linus's tree.
> 
> Paolo
> 
> 
>>> We have defined the word 18 for Intel-defined CPU features from CPUID level> 
>>> 0x00000007:0 (EDX). Let's move the definitions of X86_FEATURE_AMX_* to
>> the> right entry to prevent misinterpretation. No functional change
>> intended.
>> Please, no "we's" in changelogs.  Don't say, "let's move".  Just say:
>> "Move..." >>
>> The subject could probably also be trimmed a bit.  Perhaps:
>>
>>     x86/cpu: Move AMX CPU feature defines to correct word location
>>
>>
> 

Thanks Dave and Paolo.  Just for your convenience:

 From 588c2221999c1f5860188a7cbaeb0d4f80c6d727 Mon Sep 17 00:00:00 2001
From: Like Xu <likexu@tencent.com>
Date: Mon, 17 Jan 2022 14:23:44 +0800
Subject: [PATCH v2] x86/cpufeatures: Move AMX CPU feature defines to correct
  word location

From: Like Xu <likexu@tencent.com>

The word 18 for Intel-defined CPU features from CPUID level 0x00000007:0 (EDX)
has been defined in the same file. Move the definitions of X86_FEATURE_AMX_* to
the right entry to prevent misinterpretation. No functional change intended.

Signed-off-by: Like Xu <likexu@tencent.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
v1 -> v2 Changelog:
- Refine the commit message and subject; (Dave)

  arch/x86/include/asm/cpufeatures.h | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 6db4e2932b3d..5cd22090e53d 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -299,9 +299,6 @@
  /* 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_AMX_BF16		(18*32+22) /* AMX bf16 Support */
-#define X86_FEATURE_AMX_TILE		(18*32+24) /* AMX tile Support */
-#define X86_FEATURE_AMX_INT8		(18*32+25) /* AMX int8 Support */

  /* AMD-defined CPU features, CPUID level 0x80000008 (EBX), word 13 */
  #define X86_FEATURE_CLZERO		(13*32+ 0) /* CLZERO instruction */
@@ -390,7 +387,10 @@
  #define X86_FEATURE_TSXLDTRK		(18*32+16) /* TSX Suspend Load Address Tracking */
  #define X86_FEATURE_PCONFIG		(18*32+18) /* Intel PCONFIG */
  #define X86_FEATURE_ARCH_LBR		(18*32+19) /* Intel ARCH LBR */
+#define X86_FEATURE_AMX_BF16		(18*32+22) /* AMX bf16 Support */
  #define X86_FEATURE_AVX512_FP16		(18*32+23) /* AVX512 FP16 */
+#define X86_FEATURE_AMX_TILE		(18*32+24) /* AMX tile Support */
+#define X86_FEATURE_AMX_INT8		(18*32+25) /* AMX int8 Support */
  #define X86_FEATURE_SPEC_CTRL		(18*32+26) /* "" Speculation Control (IBRS + 
IBPB) */
  #define X86_FEATURE_INTEL_STIBP		(18*32+27) /* "" Single Thread Indirect 
Branch Predictors */
  #define X86_FEATURE_FLUSH_L1D		(18*32+28) /* Flush L1D cache */
-- 
2.33.1

Re: [PATCH] x86/cpufeatures: Move the definition of X86_FEATURE_AMX_* to the word 18

[Borislav Petkov]

On Wed, Jan 19, 2022 at 11:41:31AM +0800, Like Xu wrote:
> Thanks Dave and Paolo.  Just for your convenience:
> 
> From 588c2221999c1f5860188a7cbaeb0d4f80c6d727 Mon Sep 17 00:00:00 2001
> From: Like Xu <likexu@tencent.com>
> Date: Mon, 17 Jan 2022 14:23:44 +0800
> Subject: [PATCH v2] x86/cpufeatures: Move AMX CPU feature defines to correct
>  word location
> 
> From: Like Xu <likexu@tencent.com>
> 
> The word 18 for Intel-defined CPU features from CPUID level 0x00000007:0 (EDX)
> has been defined in the same file. Move the definitions of X86_FEATURE_AMX_* to
> the right entry to prevent misinterpretation. No functional change intended.
> 
> Signed-off-by: Like Xu <likexu@tencent.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> v1 -> v2 Changelog:
> - Refine the commit message and subject; (Dave)
> 
>  arch/x86/include/asm/cpufeatures.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

$ test-apply.sh /tmp/urgent.01
checking file arch/x86/include/asm/cpufeatures.h
Hunk #1 FAILED at 299.
patch: **** malformed patch at line 45: IBPB) */

Don't ever send patches from

User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.5.0

That thing mangles them.

See Documentation/process/email-clients.rst for further info.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86/cpufeatures: Move the definition of X86_FEATURE_AMX_* to the word 18

[Like Xu]

On 29/1/2022 10:19 pm, Borislav Petkov wrote:
> On Wed, Jan 19, 2022 at 11:41:31AM +0800, Like Xu wrote:
>> Thanks Dave and Paolo.  Just for your convenience:
>>
>>  From 588c2221999c1f5860188a7cbaeb0d4f80c6d727 Mon Sep 17 00:00:00 2001
>> From: Like Xu <likexu@tencent.com>
>> Date: Mon, 17 Jan 2022 14:23:44 +0800
>> Subject: [PATCH v2] x86/cpufeatures: Move AMX CPU feature defines to correct
>>   word location
>>
>> From: Like Xu <likexu@tencent.com>
>>
>> The word 18 for Intel-defined CPU features from CPUID level 0x00000007:0 (EDX)
>> has been defined in the same file. Move the definitions of X86_FEATURE_AMX_* to
>> the right entry to prevent misinterpretation. No functional change intended.
>>
>> Signed-off-by: Like Xu <likexu@tencent.com>
>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> v1 -> v2 Changelog:
>> - Refine the commit message and subject; (Dave)
>>
>>   arch/x86/include/asm/cpufeatures.h | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> $ test-apply.sh /tmp/urgent.01

Emm, are you willing to make "test-apply.sh" public (or have done) and let 
others benefit ?

> checking file arch/x86/include/asm/cpufeatures.h
> Hunk #1 FAILED at 299.
> patch: **** malformed patch at line 45: IBPB) */
> 
> Don't ever send patches from

Got you.

> 
> User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.5.0
> 
> That thing mangles them.
> 
> See Documentation/process/email-clients.rst for further info.
> 

Sorry for the late reply, and here's a new version from git-send-email:

https://lore.kernel.org/lkml/20220208080103.8119-1-likexu@tencent.com/

Re: [PATCH] x86/cpufeatures: Move the definition of X86_FEATURE_AMX_* to the word 18

[Borislav Petkov]

On Tue, Feb 08, 2022 at 04:08:17PM +0800, Like Xu wrote:
> Emm, are you willing to make "test-apply.sh" public (or have done) and let
> others benefit ?

It is nothing special - just a simple script which does first

patch -p1 --dry-run -l -i [patch file]

to see whether it would even apply.

> Sorry for the late reply, and here's a new version from git-send-email:

Yeah, someone already took care of this:

https://git.kernel.org/tip/ae75fa54228162ecd65341f9780886f21f557cc4

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86/cpufeatures: Move the definition of X86_FEATURE_AMX_* to the word 18

[Like Xu]

On 8/2/2022 4:51 pm, Borislav Petkov wrote:
> On Tue, Feb 08, 2022 at 04:08:17PM +0800, Like Xu wrote:
>> Emm, are you willing to make "test-apply.sh" public (or have done) and let
>> others benefit ?
> 
> It is nothing special - just a simple script which does first
> 
> patch -p1 --dry-run -l -i [patch file]
> 
> to see whether it would even apply.
> 
>> Sorry for the late reply, and here's a new version from git-send-email:
> 
> Yeah, someone already took care of this:
> 
> https://git.kernel.org/tip/ae75fa54228162ecd65341f9780886f21f557cc4
> 

Uh, it's Jim and we have worked together on many KVM issues.

BTW, I'm not sure why we prefer:

#define X86_FEATURE_AVX512_FP16		(18*32+23) /* AVX512 FP16 */
#define X86_FEATURE_SPEC_CTRL		(18*32+26) /* "" Speculation Control (IBRS + IBPB) */
+#define X86_FEATURE_AMX_BF16		(18*32+22) /* AMX bf16 Support */
+#define X86_FEATURE_AMX_TILE		(18*32+24) /* AMX tile Support */
+#define X86_FEATURE_AMX_INT8		(18*32+25) /* AMX int8 Support */

rather than:

+#define X86_FEATURE_AMX_BF16        (18*32+22) /* AMX bf16 Support */
  #define X86_FEATURE_AVX512_FP16		(18*32+23) /* AVX512 FP16 */
+#define X86_FEATURE_AMX_TILE        (18*32+24) /* AMX tile Support */
+#define X86_FEATURE_AMX_INT8        (18*32+25) /* AMX int8 Support */

It would make more sense to put them in order, what do you think? Need v3 to 
sort it ?

Thanks,
Like Xu

Re: [PATCH] x86/cpufeatures: Move the definition of X86_FEATURE_AMX_* to the word 18

[Borislav Petkov]

On Tue, Feb 08, 2022 at 05:06:04PM +0800, Like Xu wrote:
> It would make more sense to put them in order, what do you think?

Ah, good catch, thanks.

I'll edit the patch directly.

-- 
Regards/Gruss,
    Boris.

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