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

[Markus Armbruster]

David Hildenbrand <address@hidden> writes:

> 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!

I think I can guess how this could work for reporting features (I
haven't checked my guess against the code), which is what the
query-cpu-model-* do: suppress individual features when their group is
complete.

But "'-cpu' setup" doesn't seem to be about reporting features.  Am I
confused?

While testing, I noticed that

    $ s390x-softmmu/qemu-system-s390x

flashes a window at me, then terminates successfully, without printing
anything.  With -S, it behaves like other targets.  Bug?