Re: [PATCH v3 4/6] i386: Mask and report unavailable multi-bit feature values

[Xiaoyao Li]

On 2/9/2023 12:21 PM, Wang, Lei wrote:
> > > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > > > index 88aa780566..e638a31d34 100644
> > > > > --- a/target/i386/cpu.c
> > > > > +++ b/target/i386/cpu.c
> > > > > @@ -4377,6 +4377,28 @@ static bool x86_cpu_have_filtered_features(X86CPU *cpu)
> > > > >        return false;
> > > > >    }
> > > > >
> > > > > +static void mark_unavailable_multi_bit_default(X86CPU *cpu, FeatureWord w,
> > > > > +                                               int index,
> > > > > +                                               const char *verbose_prefix)
> > > > > +{
> > > > > +    FeatureWordInfo *f = &feature_word_info[w];
> > > > > +    g_autofree char *feat_word_str = feature_word_description(f);
> > > > > +    uint64_t host_feat = x86_cpu_get_supported_feature_word(w, false);
> > > > > +    MultiBitFeatureInfo mf = f->multi_bit_features[index];
> > > > > +
> > > > > +    if ((cpu->env.features[w] & mf.mask) &&
> > > > With this checking bits are all 0 but covered by mf.mask's range are skippped,
> > > > even if they're different from the host_feat, please check whether it's desried
> > > > behavior.
> > > This is the intended design because there are quite a number of multi-bit CPUIDs
> > > should support passing all 0 to them.
> > you didn't answer the question. The question is why the handling can be skipped
> > when the value of multi-bit feature is 0.
> I think the default function should handle the most common case, which is,
> passing all 0 multi-bits to KVM is valid and shouldn't be warned. E.g, when we
> are using some earlier CPU models which doesn't support AMX, we shouldn't be
> warned that all 0 values don't match the CPUID reported by KVM.

passing value 0 to KVM is valid, is not the reason why the handling can 
be skipped.

The correct reason is that when value is 0, it means the multi-bit 
feature is not enabled/requested. It's always legal and doesn't need any 
handling. So it can be just skipped.