Re: [PATCH v1 1/2] target/s390x: introduce "host-recommended" option for model expansion

[David Hildenbrand]

On 11.09.23 22:52, Collin Walling wrote:

Patch subject is wrong (should contain "static-recommended")

> Newer S390 machines may drop support for features completely, rendering
> guests operating with older CPU models incapable of running on said
> machines. A manual effort to disable certain CPU features would be
> required.
>
> To alleviate this issue, a list of "deprecated" features are now
> retained within QEMU, and a new "static-recommended" CPU model expansion
> type has been created to allow a query of the host-model with deprecated
> features explicitly disabled.
>
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>   qapi/machine-target.json         |  8 +++++++-
>   target/s390x/cpu_features.c      | 14 ++++++++++++++
>   target/s390x/cpu_features.h      |  1 +
>   target/s390x/cpu_models_sysemu.c | 26 +++++++++++++++++++++-----
>   4 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index f0a6b72414..4dc891809d 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -42,6 +42,12 @@
>   #     to be migration-safe, but allows tooling to get an insight and
>   #     work with model details.
>   #
> +# @static-recommended: Expand to a static CPU model with property
> +#     changes suggested by the architecture. This is useful for
> +#     expanding a CPU model expected to operate in mixed
> +#     CPU-generation environments. The @static-recommended CPU
> +#     models are migration-safe.
> +#

Can we instead make this a new parameter for query-cpu-model-expansion 
("no-deprecated-features" ? ), that properly gets rejected from other 
archs when not supported?

[...]

>   /* convert S390CPUDef into a static CpuModelInfo */
>   static void cpu_info_from_model(CpuModelInfo *info, const S390CPUModel *model,
> -                                bool delta_changes)
> +                                bool delta_changes, bool disable_dep_feats)

"dep" can be misleading ("depended")

"no_deprecated_feat" ?


--
Cheers,

David / dhildenb