Re: [PATCH 4/8] target/i386: add AVX10 feature and AVX10 version property

[Zhao Liu <zhao1.liu@intel.com>]

On Wed, Oct 30, 2024 at 11:05:23AM +0800, Tao Su wrote:
> Date: Wed, 30 Oct 2024 11:05:23 +0800
> From: Tao Su <tao1.su@linux.intel.com>
> Subject: Re: [PATCH 4/8] target/i386: add AVX10 feature and AVX10 version
>  property
> 
> On Tue, Oct 29, 2024 at 04:18:54PM +0100, Paolo Bonzini wrote:
> > From: Tao Su <tao1.su@linux.intel.com>
> 
> [ ... ]
> 
> >  static void max_x86_cpu_realize(DeviceState *dev, Error **errp)
> >  {
> >      Object *obj = OBJECT(dev);
> > +    X86CPU *cpu = X86_CPU(dev);
> > +    CPUX86State *env = &cpu->env;
> >  
> >      if (!object_property_get_int(obj, "family", &error_abort)) {
> >          if (X86_CPU(obj)->env.features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
> > @@ -5351,6 +5357,14 @@ static void max_x86_cpu_realize(DeviceState *dev, Error **errp)
> >          }
> >      }
> >  
> > +    if ((env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10) && !env->avx10_version) {
> 
> CPUID_7_1_EDX_AVX10 is not set now and will be set in x86_cpu_realizefn().
> How about just checking !env->avx10_version? Because cpu_x86_cpuid will
> also check (env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10).

If you explicitly turn on avx10 via -cpu max,+avx10, then CPUID_7_1_EDX_AVX10
will be there. But I agree, this is not a good place to check avx10 and
avx10_version.