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

[David Hildenbrand]

On 02.05.20 08:26, Markus Armbruster wrote:
> 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.

Yes. Expand the group to single features on user input, expand the
single features to the group on user output (if all features are enabled).

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

Let me clarify. Any user input would be broken if the two sub-features
would be specified explicitly, instead of the whole "msa4" group. This
applies to any user input, also the user input for query-cpu-model-.

In the usual cases, libvirt will expand a cpu model (e.g., "host",
"z15") and start QEMU with that (-cpu ...). We will only have the
complete msa4 group here in practice.

Yes, if some user would pick and chose such features manually, it would
be broken - it's just not the common on s390x with the huge amount of
features. But that's why I thing stable backports still make sense.

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

Think this is expected.

t480s: ~  $ qemu-system-s390x --nographic
LOADPARM=[        ]
Could not find a suitable boot device (none specified)

The s390-ccw bios will come up, detect that there is nothing to boot and
quit. The bios can only print to the sclp console, not to a graphical
output.

What the others do (e.g., ppc64, x86_64) is boot the bios/firmware and
then halt there.

-- 
Thanks,

David / dhildenb