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

[Wang, Lei]

On 2/9/2023 1:59 PM, Xiaoyao Li wrote:
> 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.

Currently, we can say yes, but I doubt if there will be a multi-bit field in the
future that only accepts the non-zero value specified.