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

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

[Collin Walling]

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.
+#
 # Note: When a non-migration-safe CPU model is expanded in static
 #     mode, some features enabled by the CPU model may be omitted,
 #     because they can't be implemented by a static CPU model
@@ -55,7 +61,7 @@
 # Since: 2.8
 ##
 { 'enum': 'CpuModelExpansionType',
-  'data': [ 'static', 'full' ] }
+  'data': [ 'static', 'full', 'static-recommended' ] }
 
 ##
 # @CpuModelCompareResult:
diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index d28eb65845..efafc9711c 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -212,6 +212,20 @@ void s390_feat_bitmap_to_ascii(const S390FeatBitmap features, void *opaque,
     };
 }
 
+void s390_get_deprecated_features(S390FeatBitmap features)
+{
+    static const int feats[] = {
+         /* CSSKE is deprecated on newer generations */
+         S390_FEAT_CONDITIONAL_SSKE,
+         S390_FEAT_BPB,
+    };
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(feats); i++) {
+        set_bit(feats[i], features);
+    }
+}
+
 #define FEAT_GROUP_INIT(_name, _group, _desc)        \
     {                                                \
         .name = _name,                               \
diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
index 87463f064d..5421762db5 100644
--- a/target/s390x/cpu_features.h
+++ b/target/s390x/cpu_features.h
@@ -68,6 +68,7 @@ void s390_add_from_feat_block(S390FeatBitmap features, S390FeatType type,
                           uint8_t *data);
 void s390_feat_bitmap_to_ascii(const S390FeatBitmap features, void *opaque,
                                void (*fn)(const char *name, void *opaque));
+void s390_get_deprecated_features(S390FeatBitmap features);
 
 /* Definition of a CPU feature group */
 typedef struct {
diff --git a/target/s390x/cpu_models_sysemu.c b/target/s390x/cpu_models_sysemu.c
index 63981bf36b..1aa3d076b4 100644
--- a/target/s390x/cpu_models_sysemu.c
+++ b/target/s390x/cpu_models_sysemu.c
@@ -176,7 +176,7 @@ static void qdict_add_enabled_feat(const char *name, void *opaque)
 
 /* 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)
 {
     QDict *qdict = qdict_new();
     S390FeatBitmap bitmap;
@@ -198,6 +198,13 @@ 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);
         }
+
+        /* features flagged as deprecated */
+        if (disable_dep_feats) {
+            bitmap_zero(bitmap, S390_FEAT_MAX);
+            s390_get_deprecated_features(bitmap);
+            s390_feat_bitmap_to_ascii(bitmap, qdict, qdict_add_disabled_feat);
+        }
     } else {
         /* expand all features */
         s390_feat_bitmap_to_ascii(model->features, qdict,
@@ -221,6 +228,7 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
     CpuModelExpansionInfo *expansion_info = NULL;
     S390CPUModel s390_model;
     bool delta_changes = false;
+    bool disable_dep_feats = false;
 
     /* convert it to our internal representation */
     cpu_model_from_info(&s390_model, model, &err);
@@ -229,9 +237,16 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
         return NULL;
     }
 
-    if (type == CPU_MODEL_EXPANSION_TYPE_STATIC) {
+    switch (type) {
+    case CPU_MODEL_EXPANSION_TYPE_STATIC_RECOMMENDED:
+        disable_dep_feats = true;
+        /* fall through */
+    case CPU_MODEL_EXPANSION_TYPE_STATIC:
         delta_changes = true;
-    } else if (type != CPU_MODEL_EXPANSION_TYPE_FULL) {
+        break;
+    case CPU_MODEL_EXPANSION_TYPE_FULL:
+        break;
+    default:
         error_setg(errp, "The requested expansion type is not supported.");
         return NULL;
     }
@@ -239,7 +254,8 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
     /* convert it back to a static representation */
     expansion_info = g_new0(CpuModelExpansionInfo, 1);
     expansion_info->model = g_malloc0(sizeof(*expansion_info->model));
-    cpu_info_from_model(expansion_info->model, &s390_model, delta_changes);
+    cpu_info_from_model(expansion_info->model, &s390_model,
+                        delta_changes, disable_dep_feats);
     return expansion_info;
 }
 
@@ -388,7 +404,7 @@ CpuModelBaselineInfo *qmp_query_cpu_model_baseline(CpuModelInfo *infoa,
 
     baseline_info = g_new0(CpuModelBaselineInfo, 1);
     baseline_info->model = g_malloc0(sizeof(*baseline_info->model));
-    cpu_info_from_model(baseline_info->model, &model, true);
+    cpu_info_from_model(baseline_info->model, &model, true, false);
     return baseline_info;
 }
 
-- 
2.41.0

[PATCH v1 2/2] target/s390x: flag te and cte as deprecated

[Collin Walling]

Add the CONSTRAINT_TRANSACTIONAL_EXE (cte) and TRANSACTIONAL_EXE (te)
under the list of deprecated features.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
---
 target/s390x/cpu_features.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index efafc9711c..cb4e2b8920 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -218,6 +218,9 @@ void s390_get_deprecated_features(S390FeatBitmap features)
          /* CSSKE is deprecated on newer generations */
          S390_FEAT_CONDITIONAL_SSKE,
          S390_FEAT_BPB,
+         /* Deprecated on z16 */
+         S390_FEAT_CONSTRAINT_TRANSACTIONAL_EXE,
+         S390_FEAT_TRANSACTIONAL_EXE
     };
     int i;
 
-- 
2.41.0

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

Re: [PATCH v1 2/2] target/s390x: flag te and cte as deprecated

[David Hildenbrand]

On 11.09.23 22:52, Collin Walling wrote:
> Add the CONSTRAINT_TRANSACTIONAL_EXE (cte) and TRANSACTIONAL_EXE (te)
> under the list of deprecated features.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>   target/s390x/cpu_features.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index efafc9711c..cb4e2b8920 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -218,6 +218,9 @@ void s390_get_deprecated_features(S390FeatBitmap features)
>            /* CSSKE is deprecated on newer generations */
>            S390_FEAT_CONDITIONAL_SSKE,
>            S390_FEAT_BPB,
> +         /* Deprecated on z16 */
> +         S390_FEAT_CONSTRAINT_TRANSACTIONAL_EXE,
> +         S390_FEAT_TRANSACTIONAL_EXE
>       };
>       int i;
>   

We don't have to consider them in init_ignored_base_feat, because they 
have never been part of base model I guess?

-- 
Cheers,

David / dhildenb

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

[Collin Walling]

On 9/12/23 02:57, David Hildenbrand wrote:
> 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?
> 
> [...]
> 

So instead of a "type": "static-recommended" add an entirely new
(optional) parameter key-value to the command?
"disable-deprecated-features": "true|false"?

>>   /* 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" ?
> 
> 

Good call. Tricky one to short-hand :)

With respect to labeling this as "no-deprecated-features", I think that
may also be misleading: it sounds like an exclusion, when in fact we
*want* the deprecated feats to show up paired with the false value. So I
think "disable-deprecated-features" is a better label. Would you agree?

-- 
Regards,
  Collin