Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes,eaes}-256

[David Hildenbrand]

On 30.04.20 20:22, Markus Armbruster wrote:
> David Hildenbrand <address@hidden> writes:
> 
>> On 28.04.20 18:34, Markus Armbruster wrote:
>>> Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and
>>> s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is
>>> "pcc-cmac-eaes-256".  The former is obviously a pasto.
>>>
>>> Impact:
>>>
>>> * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256
>>>   as "pcc-cmac-eaes-256".  Affects QMP commands query-cpu-definitions,
>>>   query-cpu-model-expansion, query-cpu-model-baseline,
>>>   query-cpu-model-comparison, and the error message when
>>>   s390_realize_cpu_model() fails in check_compatibility().
>>>
>>> * s390_realize_cpu_model() misidentifies it in check_consistency()
>>>   warnings.
>>>
>>> * s390_cpu_list() likewise.  Affects -cpu help.
>>>
>>> * s390_cpu_model_register_props() creates CPU property
>>>   "pcc-cmac-eaes-256" twice.  The second one fails, but the error is
>>>   ignored (a later commit will change that).  Results in a single
>>>   property "pcc-cmac-eaes-256" with the description for
>>>   S390_FEAT_PCC_CMAC_AES_256, and no property for
>>>   S390_FEAT_PCC_CMAC_EAES_256.  CPU properties are visible in CLI -cpu
>>>   and -device, QMP & HMP device_add, QMP device-list-properties, and
>>>   QOM introspection.
>>>
>>> Fix by deleting the wayward 'e'.
>>
>> Very nice catch - thanks!
> 
> :)
> 
>> While this sounds very bad, it's luckily not that bad in practice
>> (currently).
>>
>> The feature (or rather, both features) is part of the feature group
>> "msa4". As long as we have all sub-features part of that group (which is
>> usually the case), we will always indicate "msa4" to the user, instead
>> of all the separate sub-features. So, expansion, baseline, comparison
>> will usually only work with "msa4".
>>
>> (in addition, current KVM is not capable of actually masking off these
>> sub-features, so it will still, always see the feature, even if not
>> explicitly specified via "-cpu X,pcc-cmac-aes-256=on)
> 
> Would you like to propose an commit message improvements?

Maybe something like

"Both affected features are part of the feature group msa4. In current
setups, we will always see the msa4 feature instead of the separate
contained sub-features (because all sub-features are around). Therefore,
both features are currently never passed from/to the user explicitly
(e.g., via cpu model expansion, comparison, baseline and '-cpu' setup)."

Thanks!

-- 
Thanks,

David / dhildenb