diff mbox series

[v3,4/8] target/i386: add AVX10 feature and AVX10 version property

Message ID 20241031085233.425388-5-tao1.su@linux.intel.com (mailing list archive)
State New
Headers show
Series Add AVX10.1 CPUID support and GraniteRapids-v2 model | expand

Commit Message

Tao Su Oct. 31, 2024, 8:52 a.m. UTC 
When AVX10 enable bit is set, the 0x24 leaf will be present as "AVX10
Converged Vector ISA leaf" containing fields for the version number and
the supported vector bit lengths.

Introduce avx10-version property so that avx10 version can be controlled
by user and cpu model. Per spec, avx10 version can never be 0, the default
value of avx10-version is set to 0 to determine whether it is specified by
user.  The default can come from the device model or, for the max model,
from KVM's reported value.

Signed-off-by: Tao Su <tao1.su@linux.intel.com>
Link: https://lore.kernel.org/r/20241028024512.156724-3-tao1.su@linux.intel.com
Link: https://lore.kernel.org/r/20241028024512.156724-4-tao1.su@linux.intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Tested-by: Xuelian Guo <xuelian.guo@intel.com>
---
 target/i386/cpu.c     | 64 ++++++++++++++++++++++++++++++++++++++-----
 target/i386/cpu.h     |  4 +++
 target/i386/kvm/kvm.c |  3 +-
 3 files changed, 63 insertions(+), 8 deletions(-)

Comments

Zhao Liu Oct. 31, 2024, 4:52 p.m. UTC | #1 
> @@ -7578,7 +7607,27 @@ static bool x86_cpu_filter_features(X86CPU *cpu, bool verbose)
>          }
>      }
>  
> -    return x86_cpu_have_filtered_features(cpu);
> +    have_filtered_features = x86_cpu_have_filtered_features(cpu);
> +
> +    if (env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) {
> +        x86_cpu_get_supported_cpuid(0x24, 0,
> +                                    &eax_0, &ebx_0, &ecx_0, &edx_0);
> +        uint8_t version = ebx_0 & 0xff;
> +
> +        if (version < env->avx10_version) {
> +            if (prefix) {
> +                warn_report("%s: avx10.%d. Adjust to avx10.%d",
> +                            prefix, env->avx10_version, version);
> +            }
> +            env->avx10_version = version;
> +            have_filtered_features = true;
> +        }
> +    } else if (env->avx10_version && prefix) {
> +        warn_report("%s: avx10.%d.", prefix, env->avx10_version);
> +        have_filtered_features = true;
> +    }

prefix is just used to print warning. So here we should check prefix
for warn_report.

+    } else if (env->avx10_version) {
+        if (prefix) {
+            warn_report("%s: avx10.%d.", prefix, env->avx10_version);
+        }
+        have_filtered_features = true;
+    }

With this nit fixed,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Thanks.
Zhao
Tao Su Nov. 1, 2024, 2:09 a.m. UTC | #2 
On Fri, Nov 01, 2024 at 12:52:12AM +0800, Zhao Liu wrote:
> > @@ -7578,7 +7607,27 @@ static bool x86_cpu_filter_features(X86CPU *cpu, bool verbose)
> >          }
> >      }
> >  
> > -    return x86_cpu_have_filtered_features(cpu);
> > +    have_filtered_features = x86_cpu_have_filtered_features(cpu);
> > +
> > +    if (env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) {
> > +        x86_cpu_get_supported_cpuid(0x24, 0,
> > +                                    &eax_0, &ebx_0, &ecx_0, &edx_0);
> > +        uint8_t version = ebx_0 & 0xff;
> > +
> > +        if (version < env->avx10_version) {
> > +            if (prefix) {
> > +                warn_report("%s: avx10.%d. Adjust to avx10.%d",
> > +                            prefix, env->avx10_version, version);
> > +            }
> > +            env->avx10_version = version;
> > +            have_filtered_features = true;
> > +        }
> > +    } else if (env->avx10_version && prefix) {
> > +        warn_report("%s: avx10.%d.", prefix, env->avx10_version);
> > +        have_filtered_features = true;
> > +    }
> 
> prefix is just used to print warning. So here we should check prefix
> for warn_report.
> 
> +    } else if (env->avx10_version) {
> +        if (prefix) {
> +            warn_report("%s: avx10.%d.", prefix, env->avx10_version);
> +        }
> +        have_filtered_features = true;
> +    }
> 

Yes, thanks for pointing out! But I see this patch set is already pulled,
not sure whether it is easy to change...
Tao Su Nov. 1, 2024, 2:31 a.m. UTC | #3 
On Fri, Nov 01, 2024 at 10:44:23AM +0800, Zhao Liu wrote:
> > > prefix is just used to print warning. So here we should check prefix
> > > for warn_report.
> > > 
> > > +    } else if (env->avx10_version) {
> > > +        if (prefix) {
> > > +            warn_report("%s: avx10.%d.", prefix, env->avx10_version);
> > > +        }
> > > +        have_filtered_features = true;
> > > +    }
> > > 
> > 
> > Yes, thanks for pointing out! But I see this patch set is already pulled,
> > not sure whether it is easy to change...
> 
> I will soon have another clean-up series, and I can include this part.
> You can send me your patch, or I can help you with it.

Please help me do that, thanks! :-)
Zhao Liu Nov. 1, 2024, 2:44 a.m. UTC | #4 
> > prefix is just used to print warning. So here we should check prefix
> > for warn_report.
> > 
> > +    } else if (env->avx10_version) {
> > +        if (prefix) {
> > +            warn_report("%s: avx10.%d.", prefix, env->avx10_version);
> > +        }
> > +        have_filtered_features = true;
> > +    }
> > 
> 
> Yes, thanks for pointing out! But I see this patch set is already pulled,
> not sure whether it is easy to change...

I will soon have another clean-up series, and I can include this part.
You can send me your patch, or I can help you with it.

-Zhao
Zhao Liu Nov. 1, 2024, 2:57 a.m. UTC | #5 
On Fri, Nov 01, 2024 at 10:31:12AM +0800, Tao Su wrote:
> Date: Fri, 1 Nov 2024 10:31:12 +0800
> From: Tao Su <tao1.su@linux.intel.com>
> Subject: Re: [PATCH v3 4/8] target/i386: add AVX10 feature and AVX10
>  version property
> 
> On Fri, Nov 01, 2024 at 10:44:23AM +0800, Zhao Liu wrote:
> > > > prefix is just used to print warning. So here we should check prefix
> > > > for warn_report.
> > > > 
> > > > +    } else if (env->avx10_version) {
> > > > +        if (prefix) {
> > > > +            warn_report("%s: avx10.%d.", prefix, env->avx10_version);
> > > > +        }
> > > > +        have_filtered_features = true;
> > > > +    }
> > > > 
> > > 
> > > Yes, thanks for pointing out! But I see this patch set is already pulled,
> > > not sure whether it is easy to change...
> > 
> > I will soon have another clean-up series, and I can include this part.
> > You can send me your patch, or I can help you with it.
> 
> Please help me do that, thanks! :-)

Sure, you're welcome.
diff mbox series

Patch

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 8fbbf1fd9e..284ff062df 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -46,6 +46,9 @@ 
 #include "cpu-internal.h"
 
 static void x86_cpu_realizefn(DeviceState *dev, Error **errp);
+static void x86_cpu_get_supported_cpuid(uint32_t func, uint32_t index,
+                                        uint32_t *eax, uint32_t *ebx,
+                                        uint32_t *ecx, uint32_t *edx);
 
 /* Helpers for building CPUID[2] descriptors: */
 
@@ -1132,7 +1135,7 @@  FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
             "avx-vnni-int8", "avx-ne-convert", NULL, NULL,
             "amx-complex", NULL, "avx-vnni-int16", NULL,
             NULL, NULL, "prefetchiti", NULL,
-            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, "avx10",
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
             NULL, NULL, NULL, NULL,
@@ -1967,6 +1970,7 @@  typedef struct X86CPUDefinition {
     int family;
     int model;
     int stepping;
+    uint8_t avx10_version;
     FeatureWordArray features;
     const char *model_id;
     const CPUCaches *const cache_info;
@@ -6309,6 +6313,9 @@  static void x86_cpu_load_model(X86CPU *cpu, X86CPUModel *model)
      */
     object_property_set_str(OBJECT(cpu), "vendor", def->vendor, &error_abort);
 
+    object_property_set_uint(OBJECT(cpu), "avx10-version", def->avx10_version,
+                             &error_abort);
+
     x86_cpu_apply_version_props(cpu, model);
 
     /*
@@ -6837,6 +6844,16 @@  void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         break;
     }
+    case 0x24: {
+        *eax = 0;
+        *ebx = 0;
+        *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;
+        }
+        break;
+    }
     case 0x40000000:
         /*
          * CPUID code in kvm_arch_init_vcpu() ignores stuff
@@ -7417,6 +7434,12 @@  void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
                 ~env->user_features[w] &
                 ~feature_word_info[w].no_autoenable_flags;
         }
+
+        if ((env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) && !env->avx10_version) {
+            uint32_t eax, ebx, ecx, edx;
+            x86_cpu_get_supported_cpuid(0x24, 0, &eax, &ebx, &ecx, &edx);
+            env->avx10_version = ebx & 0xff;
+        }
     }
 
     for (i = 0; i < ARRAY_SIZE(feature_dependencies); i++) {
@@ -7480,6 +7503,11 @@  void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
             x86_cpu_adjust_level(cpu, &env->cpuid_min_level, 0x1F);
         }
 
+        /* Advanced Vector Extensions 10 (AVX10) requires CPUID[0x24] */
+        if (env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) {
+            x86_cpu_adjust_level(cpu, &env->cpuid_min_level, 0x24);
+        }
+
         /* SVM requires CPUID[0x8000000A] */
         if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) {
             x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000000A);
@@ -7530,6 +7558,10 @@  static bool x86_cpu_filter_features(X86CPU *cpu, bool verbose)
     CPUX86State *env = &cpu->env;
     FeatureWord w;
     const char *prefix = NULL;
+    bool have_filtered_features;
+
+    uint32_t eax_0, ebx_0, ecx_0, edx_0;
+    uint32_t eax_1, ebx_1, ecx_1, edx_1;
 
     if (verbose) {
         prefix = accel_uses_host_cpuid()
@@ -7551,13 +7583,10 @@  static bool x86_cpu_filter_features(X86CPU *cpu, bool verbose)
      */
     if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
         kvm_enabled()) {
-        uint32_t eax_0, ebx_0, ecx_0, edx_0_unused;
-        uint32_t eax_1, ebx_1, ecx_1_unused, edx_1_unused;
-
         x86_cpu_get_supported_cpuid(0x14, 0,
-                                    &eax_0, &ebx_0, &ecx_0, &edx_0_unused);
+                                    &eax_0, &ebx_0, &ecx_0, &edx_0);
         x86_cpu_get_supported_cpuid(0x14, 1,
-                                    &eax_1, &ebx_1, &ecx_1_unused, &edx_1_unused);
+                                    &eax_1, &ebx_1, &ecx_1, &edx_1);
 
         if (!eax_0 ||
            ((ebx_0 & INTEL_PT_MINIMAL_EBX) != INTEL_PT_MINIMAL_EBX) ||
@@ -7578,7 +7607,27 @@  static bool x86_cpu_filter_features(X86CPU *cpu, bool verbose)
         }
     }
 
-    return x86_cpu_have_filtered_features(cpu);
+    have_filtered_features = x86_cpu_have_filtered_features(cpu);
+
+    if (env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) {
+        x86_cpu_get_supported_cpuid(0x24, 0,
+                                    &eax_0, &ebx_0, &ecx_0, &edx_0);
+        uint8_t version = ebx_0 & 0xff;
+
+        if (version < env->avx10_version) {
+            if (prefix) {
+                warn_report("%s: avx10.%d. Adjust to avx10.%d",
+                            prefix, env->avx10_version, version);
+            }
+            env->avx10_version = version;
+            have_filtered_features = true;
+        }
+    } else if (env->avx10_version && prefix) {
+        warn_report("%s: avx10.%d.", prefix, env->avx10_version);
+        have_filtered_features = true;
+    }
+
+    return have_filtered_features;
 }
 
 static void x86_cpu_hyperv_realize(X86CPU *cpu)
@@ -8362,6 +8411,7 @@  static Property x86_cpu_properties[] = {
     DEFINE_PROP_UINT32("min-level", X86CPU, env.cpuid_min_level, 0),
     DEFINE_PROP_UINT32("min-xlevel", X86CPU, env.cpuid_min_xlevel, 0),
     DEFINE_PROP_UINT32("min-xlevel2", X86CPU, env.cpuid_min_xlevel2, 0),
+    DEFINE_PROP_UINT8("avx10-version", X86CPU, env.avx10_version, 0),
     DEFINE_PROP_UINT64("ucode-rev", X86CPU, ucode_rev, 0),
     DEFINE_PROP_BOOL("full-cpuid-auto-level", X86CPU, full_cpuid_auto_level, true),
     DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 74886d1580..d845384dcd 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -972,6 +972,8 @@  uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w);
 #define CPUID_7_1_EDX_AMX_COMPLEX       (1U << 8)
 /* PREFETCHIT0/1 Instructions */
 #define CPUID_7_1_EDX_PREFETCHITI       (1U << 14)
+/* Support for Advanced Vector Extensions 10 */
+#define CPUID_7_1_EDX_AVX10             (1U << 19)
 /* Flexible return and event delivery (FRED) */
 #define CPUID_7_1_EAX_FRED              (1U << 17)
 /* Load into IA32_KERNEL_GS_BASE (LKGS) */
@@ -1918,6 +1920,8 @@  typedef struct CPUArchState {
     uint32_t cpuid_vendor3;
     uint32_t cpuid_version;
     FeatureWordArray features;
+    /* AVX10 version */
+    uint8_t avx10_version;
     /* Features that were explicitly enabled/disabled */
     FeatureWordArray user_features;
     uint32_t cpuid_model[12];
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index fd9f198892..8e17942c3b 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1923,7 +1923,8 @@  static uint32_t kvm_x86_build_cpuid(CPUX86State *env,
         case 0x7:
         case 0x14:
         case 0x1d:
-        case 0x1e: {
+        case 0x1e:
+        case 0x24: {
             uint32_t times;
 
             c->function = i;