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

[David Hildenbrand]

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)

I think we should do stable backports.

Reviewed-by: David Hildenbrand <address@hidden>

> 
> Fixes: 782417446279717aa85320191a519b51f6d5dd31
> Cc: Halil Pasic <address@hidden>
> Cc: Cornelia Huck <address@hidden>
> Cc: Christian Borntraeger <address@hidden>
> Cc: Richard Henderson <address@hidden>
> Cc: David Hildenbrand <address@hidden>
> Cc: address@hidden
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  target/s390x/cpu_features_def.inc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/cpu_features_def.inc.h 
> b/target/s390x/cpu_features_def.inc.h
> index 31dff0d84e..a8d562d688 100644
> --- a/target/s390x/cpu_features_def.inc.h
> +++ b/target/s390x/cpu_features_def.inc.h
> @@ -310,7 +310,7 @@ DEF_FEAT(PCC_CMAC_ETDEA_192, "pcc-cmac-etdea-128", PCC, 
> 10, "PCC Compute-Last-Bl
>  DEF_FEAT(PCC_CMAC_TDEA, "pcc-cmac-etdea-192", PCC, 11, "PCC 
> Compute-Last-Block-CMAC-Using-EncryptedTDEA-192")
>  DEF_FEAT(PCC_CMAC_AES_128, "pcc-cmac-aes-128", PCC, 18, "PCC 
> Compute-Last-Block-CMAC-Using-AES-128")
>  DEF_FEAT(PCC_CMAC_AES_192, "pcc-cmac-aes-192", PCC, 19, "PCC 
> Compute-Last-Block-CMAC-Using-AES-192")
> -DEF_FEAT(PCC_CMAC_AES_256, "pcc-cmac-eaes-256", PCC, 20, "PCC 
> Compute-Last-Block-CMAC-Using-AES-256")
> +DEF_FEAT(PCC_CMAC_AES_256, "pcc-cmac-aes-256", PCC, 20, "PCC 
> Compute-Last-Block-CMAC-Using-AES-256")
>  DEF_FEAT(PCC_CMAC_EAES_128, "pcc-cmac-eaes-128", PCC, 26, "PCC 
> Compute-Last-Block-CMAC-Using-Encrypted-AES-128")
>  DEF_FEAT(PCC_CMAC_EAES_192, "pcc-cmac-eaes-192", PCC, 27, "PCC 
> Compute-Last-Block-CMAC-Using-Encrypted-AES-192")
>  DEF_FEAT(PCC_CMAC_EAES_256, "pcc-cmac-eaes-256", PCC, 28, "PCC 
> Compute-Last-Block-CMAC-Using-Encrypted-AES-256")
> 


-- 
Thanks,

David / dhildenb