On 15.03.22 16:58, David Hildenbrand wrote:
On 11.03.22 13:44, Christian Borntraeger wrote:
>
>
> Am 11.03.22 um 10:30 schrieb David Hildenbrand:
>> On 11.03.22 05:17, Collin Walling wrote:
>>> The s390x architecture has a growing list of features that will no longer
>>> be supported on future hardware releases. This introduces an issue with
>>> migration such that guests, running on models with these features enabled,
>>> will be rejected outright by machines that do not support these features.
>>>
>>> A current example is the CSSKE feature that has been deprecated for some
time.
>>> It has been publicly announced that gen15 will be the last release to
>>> support this feature, however we have postponed this to gen16a. A possible
>>> solution to remedy this would be to create a new QEMU QMP Response that
allows
>>> users to query for deprecated/unsupported features.
>>>
>>> This presents two parts of the puzzle: how to report deprecated features to
>>> a user (libvirt) and how should libvirt handle this information.
>>>
>>> First, let's discuss the latter. The patch presented alongside this cover
letter
>>> attempts to solve the migration issue by hard-coding the CSSKE feature to be
>>> disabled for all s390x CPU models. This is done by simply appending the
CSSKE
>>> feature with the disabled policy to the host-model.
>>>
>>> libvirt pseudo:
>>>
>>> if arch is s390x
>>> set CSSKE to disabled for host-model
>>
>> That violates host-model semantics and possibly the user intend. There
>> would have to be some toggle to manually specify this, for example, a
>> new model type or a some magical flag.
>
> What we actually want to do is to disable csske completely from QEMU and
> thus from the host-model. Then it would not violate the spec.
> But this has all kind of issues (you cannot migrate from older versions
> of software and machines) although the hardware still can provide the feature.
>
> The hardware guys promised me to deprecate things two generations earlier
> and we usually deprecate things that are not used or where software has a
> runtime switch.
>
> From what I hear from you is that you do not want to modify the host-model
> semantics to something more useful but rather define a new thing (e.g.
"host-sane") ?
My take would be, to keep the host model consistent, meaning, the
semantics in QEMU exactly match the semantics in Libvirt. It defines the
maximum CPU model that's runnable under KVM. If a feature is not
included (e.g., csske) that feature cannot be enabled in any way.
The "host model" has the semantics of resembling the actual host CPU.
This is only partially true, because we support some features the host
might not support (e.g., zPCI IIRC) and obviously don't support all host
features in QEMU.
So instead of playing games on the libvirt side with the host model, I
see the following alternatives:
1. Remove the problematic features from the host model in QEMU, like "we
just don't support this feature". Consequently, any migration of a VM
with csske=on to a new QEMU version will fail, similar to having an
older QEMU version without support for a certain feature.
"host-passthrough" would change between QEMU versions ... which I see as
problematic.
2. Introduce a new CPU model that has these new semantics: "host model"
- deprecated features. Migration of older VMs with csske=on to a new
QEMU version will work. Make libvirt use/expand that new CPU model
It doesn't necessarily have to be an actual new cpu model. We can use a
feature group, like "-cpu host,deprectated-features=false". What's
inside "deprecated-features" will actually change between QEMU versions,
but we don't really care, as the expanded CPU model won't change.
"host-passthrough" won't change between QEMU versions ...
3. As Daniel suggested, don't use the host model, but a CPU model
indicated as "suggested".
The real issue is that in reality, we don't simply always use a model
like "gen15a", but usually want optional features, if they are around.
Prime examples are "sie" and friends.
I tend to prefer 2. With 3. I see issues with optional features like
"sie" and friends. Often, you really want "give me all you got, but
disable deprecated features that might cause problems in the future".
Something as hacky as this:
diff --git a/slirp b/slirp
--- a/slirp
+++ b/slirp
@@ -1 +1 @@
-Subproject commit a88d9ace234a24ce1c17189642ef9104799425e0
+Subproject commit a88d9ace234a24ce1c17189642ef9104799425e0-dirty
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 11e06cc51f..37200989c6 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -708,6 +708,34 @@ static void set_feature_group(Object *obj, Visitor *v, const char
*name,
}
}
+static void set_deprecated_features(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ DeviceState *dev = DEVICE(obj);
+ S390CPU *cpu = S390_CPU(obj);
+ bool value;
+
+ if (dev->realized) {
+ error_setg(errp, "Attempt to set property '%s' on '%s' after
"
+ "it was realized", name, object_get_typename(obj));
+ return;
+ } else if (!cpu->model) {
+ error_setg(errp, "Details about the host CPU model are not available,
"
+ "features cannot be changed.");
+ return;
+ }
+
+ if (!visit_type_bool(v, name, &value, errp)) {
+ return;
+ }
+ if (value) {
+ error_setg(errp, "Group '%s' can only be disabled.", name);
+ return;
+ }
+
+ clear_bit(S390_FEAT_CONDITIONAL_SSKE, cpu->model->features);
+}
+
static void s390_cpu_model_initfn(Object *obj)
{
S390CPU *cpu = S390_CPU(obj);
@@ -823,6 +851,8 @@ void s390_cpu_model_class_register_props(ObjectClass *oc)
object_class_property_add_bool(oc, "static", get_is_static,
NULL);
object_class_property_add_str(oc, "description", get_description, NULL);
+ object_class_property_add(oc, "deprecated-features", "bool",
NULL,
+ set_deprecated_features, NULL, NULL);
for (feat = 0; feat < S390_FEAT_MAX; feat++) {
const S390FeatDef *def = s390_feat_def(feat);
While it's primarily useful for the "host" model, it *might* be useful for
other (older) models as well.
Under TCG:
{ "execute": "query-cpu-model-expansion", "arguments": {
"type": "static", "model": { "name":
"z14" } } }
{"return": {"model": {"name": "z14-base",
"props": {"aen": true, "aefsi": true, "mepoch":
true, "msa8": true, "msa7": true, "msa6": true,
"msa5": true, "msa4": true, "msa3": true, "msa2":
true, "msa1": true, "sthyi": true, "edat": true,
"ri": true, "edat2": true, "vx": true, "ipter":
true, "mepochptff": true, "vxeh": true, "vxpd": true,
"esop": true, "iep": true, "cte": true, "bpb":
true, "gs": true, "ppa15": true, "zpci": true,
"sea_esop2": true, "te": true, "cmm": true}}}}
{ "execute": "query-cpu-model-expansion", "arguments": {
"type": "static", "model": { "name":
"z14", "props": {"deprecated-features":false}} } }
{"return": {"model": {"name": "z14-base",
"props": {"aen": true, "aefsi": true, "csske":
false, "mepoch": true, "msa8": true, "msa7": true,
"msa6": true, "msa5": true, "msa4": true, "msa3":
true, "msa2": true, "msa1": true, "sthyi": true,
"edat": true, "ri": true, "edat2": true, "vx":
true, "ipter": true, "mepochptff": true, "vxeh": true,
"vxpd": true, "esop": true, "iep": true, "cte":
true, "bpb": true, "gs": true, "ppa15": true,
"zpci": true, "sea_esop2": true, "te": true,
"cmm": true}}}}
Note the "csske=false" change.
--
Thanks,
David / dhildenb