On Thu, Oct 31, 2024 at 03:18:37PM +0800, Zhao Liu wrote:
> > > > > > > @@ -7674,13 +7682,21 @@ static bool
> > > > > > > x86_cpu_filter_features(X86CPU *cpu, bool verbose)
> > > > > > > &eax_0, &ebx_0, &ecx_0,
> > > > > > > &edx_0);
> > > > > > > uint8_t version = ebx_0 & 0xff;
> > > > > > >
> > > > > > > - if (version < env->avx10_version) {
> > > > > > > + if (!env->avx10_version) {
> > > > > > > + env->avx10_version = version;
> > > > > >
> > > > > > x86_cpu_filter_features() is not a good place to assign
> > > > > > avx10_version, I
> > > > > > still tend to set it in max_x86_cpu_realize().
> > > > >
> > > > > It's not proper to get the host's version when AVX10 cannot be
> > > > > enabled,
> > > > > even maybe host doesn't support AVX10.
> > > > >
> > > > > As you found out earlier, max_x86_cpu_realize doesn't know if AVX10
> > > > > can
> > > > > be enabled or not.
> > > > >
> > > >
> > > > How about moving to x86_cpu_expand_features()? We can set when checking
> > > > cpu->max_features.
> > >
> > > The feature bit set in x86_cpu_expand_features() is unstable since it
> > > may be masked later in x86_cpu_filter_features(). :)
> > >
> >
> > A lot of feature bits are set in x86_cpu_expand_features() with reported
> > value, so I think avx10_version can also be set to reported value there.
> >
> > I mainly want to let avx10_version be assigned only when -cpu host or max,
> > so that it can be distinguished from the cpu model. This should also be
> > Paolo's original intention in v2.
>
> OK. In this case, extend avx10-version is also consistent with the
> semantics of this function. Even if host doesn't support avx10, then in
> principle it's ok to read unimplemented avx10-version as 0.
>
> Pls go ahead. :)
I will submit v3 based on all your comments, thanks for review :)
