Re: [PATCH v3] target/s390x: filter deprecated properties based on model expansion type

[Markus Armbruster]

Collin Walling <walling@linux.ibm.com> writes:

> Currently, there is no way to execute the query-cpu-model-expansion
> command to retrieve a comprehenisve list of deprecated properties, as
> the result is dependent per-model. To enable this, the expansion output
> is modified as such:
>
> When reporting a "full" CPU model, show the *entire* list of deprecated
> properties regardless if they are supported on the model. A full
> expansion outputs all known CPU model properties anyway, so it makes
> sense to report all deprecated properties here too.
>
> This allows management apps to query a single model (e.g. host) to
> acquire the full list of deprecated properties.
>
> Additionally, when reporting a "static" CPU model, the command will
> only show deprecated properties that are a subset of the model's
> *enabled* properties. This is more accurate than how the query was
> handled before, which blindly reported deprecated properties that
> were never otherwise introduced for certain models.
>
> Acked-by: David Hildenbrand <david@redhat.com>
> Suggested-by: Jiri Denemark <jdenemar@redhat.com>
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>
> Changelog:
>
>     v3
>     - Removed the 'note' and cleaned up documentation
>     - Revised commit message
>
>     v2
>     - Changed commit message
>     - Added documentation reflecting this change
>     - Made code changes that more accurately filter the deprecated
>         properties based on expansion type.  This change makes it
>         so that the deprecated-properties reported for a static model
>         expansion are a subset of the model's properties instead of
>         the model's full-definition properties.
>
> ---
>  qapi/machine-target.json         |  5 +++--
>  target/s390x/cpu_models_sysemu.c | 16 +++++++++-------
>  2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index a8d9ec87f5..67086f006f 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -21,8 +21,9 @@
>  # @props: a dictionary of QOM properties to be applied
>  #
>  # @deprecated-props: a list of properties that are flagged as deprecated
> -#     by the CPU vendor.  These props are a subset of the full model's
> -#     definition list of properties. (since 9.1)
> +#     by the CPU vendor.  These properties are either a subset of the
> +#     properties enabled on the CPU model, or a set of properties
> +#     deprecated across all models for the architecture.


When is it "a subset of the properties enabled on the CPU model", and
when is it "a set of properties deprecated across all models for the
architecture"?

My guess based on the commit message: it's the former when
query-cpu-model-expansion's type is "static", and the latter when it's
"full".

>  #
>  # Since: 2.8
>  ##
> diff --git a/target/s390x/cpu_models_sysemu.c 
> b/target/s390x/cpu_models_sysemu.c
> index 977fbc6522..e28ecf7ab9 100644
> --- a/target/s390x/cpu_models_sysemu.c
> +++ b/target/s390x/cpu_models_sysemu.c
> @@ -174,11 +174,15 @@ static void cpu_info_from_model(CpuModelInfo *info, 
> const S390CPUModel *model,
>                                  bool delta_changes)
>  {
>      QDict *qdict = qdict_new();
> -    S390FeatBitmap bitmap;
> +    S390FeatBitmap bitmap, deprecated;
>  
>      /* always fallback to the static base model */
>      info->name = g_strdup_printf("%s-base", model->def->name);
>  
> +    /* features flagged as deprecated */
> +    bitmap_zero(deprecated, S390_FEAT_MAX);
> +    s390_get_deprecated_features(deprecated);
> +
>      if (delta_changes) {
>          /* features deleted from the base feature set */
>          bitmap_andnot(bitmap, model->def->base_feat, model->features,
> @@ -193,6 +197,9 @@ static void cpu_info_from_model(CpuModelInfo *info, const 
> S390CPUModel *model,
>          if (!bitmap_empty(bitmap, S390_FEAT_MAX)) {
>              s390_feat_bitmap_to_ascii(bitmap, qdict, qdict_add_enabled_feat);
>          }
> +
> +        /* deprecated features that are a subset of the model's enabled 
> features */

Recommend to wrap this line for legibility.

> +        bitmap_and(deprecated, deprecated, model->features, S390_FEAT_MAX);
>      } else {
>          /* expand all features */
>          s390_feat_bitmap_to_ascii(model->features, qdict,
> @@ -207,12 +214,7 @@ static void cpu_info_from_model(CpuModelInfo *info, 
> const S390CPUModel *model,
>          info->props = QOBJECT(qdict);
>      }
>  
> -    /* features flagged as deprecated */
> -    bitmap_zero(bitmap, S390_FEAT_MAX);
> -    s390_get_deprecated_features(bitmap);
> -
> -    bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX);
> -    s390_feat_bitmap_to_ascii(bitmap, &info->deprecated_props, 
> list_add_feat);
> +    s390_feat_bitmap_to_ascii(deprecated, &info->deprecated_props, 
> list_add_feat);
>      info->has_deprecated_props = !!info->deprecated_props;
>  }