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

[David Hildenbrand]

[...]

>           1                 "msa4-base": true,
>           1                 "pcc-cmac-aes-256": false,
>           1                 "pcc-cmac-eaes-256": false,
> 
> The grouping and masking you described appears to apply to
> query-cpu-model-expansion with type "static".  With type "full", I can
> see the grouping, but not the masking.  With query-cpu-definitions, I
> can't see either.
> 
> I haven't played with query-cpu-model-comparison and
> query-cpu-model-baseline.

Grouping will be done whenever all features part of a group are to be
reported. Please note that "msa4-base" is part of "msa4".

You chose the only model where we do have msa4-base but none of the
other msa4 features - the qemu model.

So when you do a "query-cpu-definitions" under TCG (which basically maps
to the qemu model and only has the msa4-base feature), then you do have
msa4-base of all relevant models, but none of the other sub-features
they define. That's why you don't see msa4 explicitly, but instead the
subfeatures.

query-cpu-model-expansion and the others behave similar the same way
with the "qemu" model. Note that the qemu model is not really used under
KVM on s390x. There, we use "host" as default.

> 
>>> 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.
> 
> The commit message should be accurate and sufficiently precise.  The
> "sufficiently" gives me some wiggle room to avoid inaccuracy due to my
> ignorance.  Would the following be good enough?
> 
>     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_cpu_list() also misidentifies it.  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.
> 
>     The two features are almost always used via their group msa4.  Such
>     use is not affected by this bug.

Yeah, sounds good, thanks.


-- 
Thanks,

David / dhildenb