[PATCH] i386/cpu: Remove FEAT_24_0_EBX for AVX10

Xiaoyao Li posted 1 patch 5 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250704144504.4094810-1-xiaoyao.li@intel.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Zhao Liu <zhao1.liu@intel.com>
There is a newer version of this series
target/i386/cpu.c | 36 +-----------------------------------
target/i386/cpu.h | 12 ------------
2 files changed, 1 insertion(+), 47 deletions(-)
[PATCH] i386/cpu: Remove FEAT_24_0_EBX for AVX10
Posted by Xiaoyao Li 5 months, 1 week ago
Intel AVX10 spec has been updated to make the bit 16-18 of
CPUID.24_0.EBX as reserved at 1 because all the Intel processors with
AVX10 support will support all the vector lengths.

The bits will be reserved at 1 to ensure the compatibility of the
existing software. For QEMU, it makes no sense to allow the
configurability of the bits anymore. So just remove the leaf
FEAT_24_0_EBX and related stuff and hardcore the bits to all 1 when
AVX10 is exposed to guest.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 target/i386/cpu.c | 36 +-----------------------------------
 target/i386/cpu.h | 12 ------------
 2 files changed, 1 insertion(+), 47 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 0d35e95430fe..d0c4a3fa113c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -912,7 +912,6 @@ void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
 #define TCG_SGX_12_0_EAX_FEATURES 0
 #define TCG_SGX_12_0_EBX_FEATURES 0
 #define TCG_SGX_12_1_EAX_FEATURES 0
-#define TCG_24_0_EBX_FEATURES 0
 
 #if defined CONFIG_USER_ONLY
 #define CPUID_8000_0008_EBX_KERNEL_FEATURES (CPUID_8000_0008_EBX_IBPB | \
@@ -1208,20 +1207,6 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
         },
         .tcg_features = TCG_7_2_EDX_FEATURES,
     },
-    [FEAT_24_0_EBX] = {
-        .type = CPUID_FEATURE_WORD,
-        .feat_names = {
-            [16] = "avx10-128",
-            [17] = "avx10-256",
-            [18] = "avx10-512",
-        },
-        .cpuid = {
-            .eax = 0x24,
-            .needs_ecx = true, .ecx = 0,
-            .reg = R_EBX,
-        },
-        .tcg_features = TCG_24_0_EBX_FEATURES,
-    },
     [FEAT_8000_0007_EDX] = {
         .type = CPUID_FEATURE_WORD,
         .feat_names = {
@@ -1839,22 +1824,6 @@ static FeatureDep feature_dependencies[] = {
         .from = { FEAT_7_0_EBX,             CPUID_7_0_EBX_SGX },
         .to = { FEAT_SGX_12_1_EAX,          ~0ull },
     },
-    {
-        .from = { FEAT_24_0_EBX,            CPUID_24_0_EBX_AVX10_128 },
-        .to = { FEAT_24_0_EBX,              CPUID_24_0_EBX_AVX10_256 },
-    },
-    {
-        .from = { FEAT_24_0_EBX,            CPUID_24_0_EBX_AVX10_256 },
-        .to = { FEAT_24_0_EBX,              CPUID_24_0_EBX_AVX10_512 },
-    },
-    {
-        .from = { FEAT_24_0_EBX,            CPUID_24_0_EBX_AVX10_VL_MASK },
-        .to = { FEAT_7_1_EDX,               CPUID_7_1_EDX_AVX10 },
-    },
-    {
-        .from = { FEAT_7_1_EDX,             CPUID_7_1_EDX_AVX10 },
-        .to = { FEAT_24_0_EBX,              ~0ull },
-    },
 };
 
 typedef struct X86RegisterInfo32 {
@@ -4732,9 +4701,6 @@ static const X86CPUDefinition builtin_x86_defs[] = {
                     { "movdiri", "on" },
                     { "movdir64b", "on" },
                     { "avx10", "on" },
-                    { "avx10-128", "on" },
-                    { "avx10-256", "on" },
-                    { "avx10-512", "on" },
                     { "avx10-version", "1" },
                     { "stepping", "1" },
                     { /* end of list */ }
@@ -7720,7 +7686,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *ecx = 0;
         *edx = 0;
         if ((env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) && count == 0) {
-            *ebx = env->features[FEAT_24_0_EBX] | env->avx10_version;
+            *ebx = (0x7U << 16) | env->avx10_version;
         }
         break;
     }
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 51e10139dfdf..7856a882f014 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -671,7 +671,6 @@ typedef enum FeatureWord {
     FEAT_7_1_ECX,       /* CPUID[EAX=7,ECX=1].ECX */
     FEAT_7_1_EDX,       /* CPUID[EAX=7,ECX=1].EDX */
     FEAT_7_2_EDX,       /* CPUID[EAX=7,ECX=2].EDX */
-    FEAT_24_0_EBX,      /* CPUID[EAX=0x24,ECX=0].EBX */
     FEATURE_WORDS,
 } FeatureWord;
 
@@ -1037,17 +1036,6 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w);
 /* Packets which contain IP payload have LIP values */
 #define CPUID_14_0_ECX_LIP              (1U << 31)
 
-/* AVX10 128-bit vector support is present */
-#define CPUID_24_0_EBX_AVX10_128        (1U << 16)
-/* AVX10 256-bit vector support is present */
-#define CPUID_24_0_EBX_AVX10_256        (1U << 17)
-/* AVX10 512-bit vector support is present */
-#define CPUID_24_0_EBX_AVX10_512        (1U << 18)
-/* AVX10 vector length support mask */
-#define CPUID_24_0_EBX_AVX10_VL_MASK    (CPUID_24_0_EBX_AVX10_128 | \
-                                         CPUID_24_0_EBX_AVX10_256 | \
-                                         CPUID_24_0_EBX_AVX10_512)
-
 /* RAS Features */
 #define CPUID_8000_0007_EBX_OVERFLOW_RECOV    (1U << 0)
 #define CPUID_8000_0007_EBX_SUCCOR      (1U << 1)
-- 
2.43.0
Re: [PATCH] i386/cpu: Remove FEAT_24_0_EBX for AVX10
Posted by Zhao Liu 5 months, 1 week ago
On Fri, Jul 04, 2025 at 10:45:04PM +0800, Xiaoyao Li wrote:
> Date: Fri, 4 Jul 2025 22:45:04 +0800
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> Subject: [PATCH] i386/cpu: Remove FEAT_24_0_EBX for AVX10
> X-Mailer: git-send-email 2.43.0
> 
> Intel AVX10 spec has been updated to make the bit 16-18 of
  ^^^^^^^^^^^^^^^^

This "AVX10 spec" is misleading, there are both AVX10.1 spec and AVX10.2
spec, and QEMU currently supports AVX10.1 for the GNR, but your change
is based on AVX10.2 spec.

It would be good to explain something like,

Although AVX10.1 has already marked AVX10/128 as always supported in
revision 3.1 (QEMU did not synchronize this change), in the latest
AVX10.2 spec (revison 5.0), AVX10 roadmap is updated, "AVX10/512 will be
used in all Intel products, supporting vector lengths of 128, 256, and
512 in all product lines". This applies for all AVX10 versions.

> CPUID.24_0.EBX as reserved at 1 because all the Intel processors with
> AVX10 support will support all the vector lengths.
>
> The bits will be reserved at 1 to ensure the compatibility of the
> existing software. For QEMU, it makes no sense to allow the
> configurability of the bits anymore. So just remove the leaf
> FEAT_24_0_EBX and related stuff and hardcore the bits to all 1 when
> AVX10 is exposed to guest.

Add the doc link here if thers's next version.

> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  target/i386/cpu.c | 36 +-----------------------------------
>  target/i386/cpu.h | 12 ------------
>  2 files changed, 1 insertion(+), 47 deletions(-)

...

> @@ -7720,7 +7686,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          *ecx = 0;
>          *edx = 0;
>          if ((env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) && count == 0) {
> -            *ebx = env->features[FEAT_24_0_EBX] | env->avx10_version;
> +            *ebx = (0x7U << 16) | env->avx10_version;

Instead of hardcode, it's better to keep the macros and encoded this
like:

*ebx = CPUID_24_0_EBX_AVX10_VL_MASK | env->avx10_version;

and leave a comment: all processors supporting Intel AVX10 will include
support for all vector lengths.

>          }
>          break;
>      }
Re: [PATCH] i386/cpu: Remove FEAT_24_0_EBX for AVX10
Posted by Xiaoyao Li 5 months, 1 week ago
On 7/6/2025 6:06 PM, Zhao Liu wrote:
> On Fri, Jul 04, 2025 at 10:45:04PM +0800, Xiaoyao Li wrote:
>> Date: Fri, 4 Jul 2025 22:45:04 +0800
>> From: Xiaoyao Li <xiaoyao.li@intel.com>
>> Subject: [PATCH] i386/cpu: Remove FEAT_24_0_EBX for AVX10
>> X-Mailer: git-send-email 2.43.0
>>
>> Intel AVX10 spec has been updated to make the bit 16-18 of
>    ^^^^^^^^^^^^^^^^
> 
> This "AVX10 spec" is misleading, there are both AVX10.1 spec and AVX10.2
> spec, and QEMU currently supports AVX10.1 for the GNR, but your change
> is based on AVX10.2 spec.
> 
> It would be good to explain something like,
> 
> Although AVX10.1 has already marked AVX10/128 as always supported in
> revision 3.1 (QEMU did not synchronize this change), in the latest
> AVX10.2 spec (revison 5.0), AVX10 roadmap is updated, "AVX10/512 will be
> used in all Intel products, supporting vector lengths of 128, 256, and
> 512 in all product lines". This applies for all AVX10 versions.

AVX10.1 spec should be updates as well. I will follow it up to internal 
architects to update the avx10.1 spec.

>> CPUID.24_0.EBX as reserved at 1 because all the Intel processors with
>> AVX10 support will support all the vector lengths.
>>
>> The bits will be reserved at 1 to ensure the compatibility of the
>> existing software. For QEMU, it makes no sense to allow the
>> configurability of the bits anymore. So just remove the leaf
>> FEAT_24_0_EBX and related stuff and hardcore the bits to all 1 when
>> AVX10 is exposed to guest.
> 
> Add the doc link here if thers's next version.
> 
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>>   target/i386/cpu.c | 36 +-----------------------------------
>>   target/i386/cpu.h | 12 ------------
>>   2 files changed, 1 insertion(+), 47 deletions(-)
> 
> ...
> 
>> @@ -7720,7 +7686,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>>           *ecx = 0;
>>           *edx = 0;
>>           if ((env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) && count == 0) {
>> -            *ebx = env->features[FEAT_24_0_EBX] | env->avx10_version;
>> +            *ebx = (0x7U << 16) | env->avx10_version;
> 
> Instead of hardcode, it's better to keep the macros and encoded this
> like:
> 
> *ebx = CPUID_24_0_EBX_AVX10_VL_MASK | env->avx10_version;
> 
> and leave a comment: all processors supporting Intel AVX10 will include
> support for all vector lengths.

The goal of Intel is trying to remove from the spec that there are 
enumeration bits for 128/256/512 bits. It wants to reassure people that 
all the processors support AVX10 support all the length.

Intel wants to disassociate the meaning of bits 16-18 from the 
enumeration of vector length as far as possible. But there might be SW 
already, who checks the bit 16-18 for checking the support of a 
specified version. So Intel has to add the footnote in the spec to 
explain that "Earlier versions of this specification documented this bit 
as enumerating VLxxx support"

We want QEMU to honor Intel's purpose and want QEMU to associate them 
with the enumeration of the vector length.

If you do want a comment, I think just add something like

	/* Bit 16-18 are reserved at 1 */

People interested in why they are reserved at 1 can go read the Intel 
sepc (SDM/ISE/AVX *) to get further information.

>>           }
>>           break;
>>       }
Re: [PATCH] i386/cpu: Remove FEAT_24_0_EBX for AVX10
Posted by Xiaoyao Li 5 months, 1 week ago
On 7/4/2025 10:45 PM, Xiaoyao Li wrote:
> Intel AVX10 spec has been updated to make the bit 16-18 of
> CPUID.24_0.EBX as reserved at 1 because all the Intel processors with
> AVX10 support will support all the vector lengths.

FYI. The latest AVX10 spec:

https://cdrdv2.intel.com/v1/dl/getContent/828965