Skip to contentSkip to conversation

target/s390x/cpu_models: Set some additional feature bits for the "qemu" CPU

RFC PATCH In review Thomas Huth 4 participants 12 messages May 17, 2017 – May 18, 2017

lore
Share & tools
lore

Thomas Huth

author
#0 lore raw

RFC PATCH 1/1 target/s390x/cpu_models: Set some additional feature bits for the "qemu" CPU

Currently we only present the plain z900 feature bits to the guest, but QEMU already emulates some additional features (but not all of the next CPU generation, so we can not use the next CPU level as default yet). Since newer Linux kernels are checking the feature bits and refuse to work if a required feature is missing, we should present as much of the supported features as possible when we are running with the default "qemu" CPU.
This patch now adds the "stfle", "extended immediate" and "stckf" facility bits to the "qemu" CPU, which are already supported facilities. It is unfortunately still not enough to run e.g. recent Fedora kernels, but at least it's a first step into the right direction.

Signed-off-by Thomas Huth <[email protected]>
---
 target/s390x/cpu_models.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)
target/s390x/cpu_models.c +26 -3
index 8d27363..18789ab 100644
@@ -658,6 +658,24 @@ static void check_compatibility(const S390CPUModel *max_model,
                   "available in the configuration: ");
 }
 
+/**
+ * The base TCG CPU model "qemu" is based on the z900. However, we already
+ * can also emulate some additional features of later CPU generations, so
+ * we add these additional feature bits here.
+ */
+static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
+{
+    int i, feats[] = {
+        S390_FEAT_STFLE,
+        S390_FEAT_EXTENDED_IMMEDIATE,
+        S390_FEAT_STORE_CLOCK_FAST
+    };
+
+    for (i = 0; i < ARRAY_SIZE(feats); i++) {
+        set_bit(feats[i], fbm);
+    }
+}
+
 static S390CPUModel *get_max_cpu_model(Error **errp)
 {
     static S390CPUModel max_model;

@@ -670,10 +688,11 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
     if (kvm_enabled()) {
         kvm_s390_get_host_cpu_model(&max_model, errp);
     } else {
-        /* TCG emulates a z900 */
+        /* TCG emulates a z900 (with some additional features) */
         max_model.def = &s390_cpu_defs[0];
         bitmap_copy(max_model.features, max_model.def->default_feat,
                     S390_FEAT_MAX);
+        add_qemu_cpu_model_features(max_model.features);
     }
     if (!*errp) {
         cached = true;

@@ -925,11 +944,15 @@ static void s390_host_cpu_model_initfn(Object *obj)
 
 static void s390_qemu_cpu_model_initfn(Object *obj)
 {
+    static S390CPUDef s390_qemu_cpu_defs;
     S390CPU *cpu = S390_CPU(obj);
 
     cpu->model = g_malloc0(sizeof(*cpu->model));
-    /* TCG emulates a z900 */
-    cpu->model->def = &s390_cpu_defs[0];
+    /* TCG emulates a z900 (with some additional features) */
+    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
+    add_qemu_cpu_model_features(s390_qemu_cpu_defs.default_feat);
+    add_qemu_cpu_model_features(s390_qemu_cpu_defs.full_feat);
+    cpu->model->def = &s390_qemu_cpu_defs;
     bitmap_copy(cpu->model->features, cpu->model->def->default_feat,
                 S390_FEAT_MAX);
 }
Signature · 1 line
1.8.3.1
Reviewed-by on v1 1/1
#1 lore raw
Quoted 11 lines · Thomas Huth

Currently we only present the plain z900 feature bits to the guest, but QEMU already emulates some additional features (but not all of the next CPU generation, so we can not use the next CPU level as default yet). Since newer Linux kernels are checking the feature bits and refuse to work if a required feature is missing, we should present as much of the supported features as possible when we are running with the default "qemu" CPU.
This patch now adds the "stfle", "extended immediate" and "stckf" facility bits to the "qemu" CPU, which are already supported facilities. It is unfortunately still not enough to run e.g. recent Fedora kernels, but at least it's a first step into the right direction.

This indeed looks like a step in a good direction. At least it makes the recently added STFLE emulation useful :-).

Quoted 14 lines
Signed-off-by Thomas Huth <[email protected]>
---
 target/s390x/cpu_models.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)
target/s390x/cpu_models.c +1 -0
index 8d27363..18789ab 100644
@@ -658,6 +658,24 @@ static void check_compatibility(const S390CPUModel *max_model,
                   "available in the configuration: ");
 }
 
+/**

target/s390x/cpu_models.c static void check_compatibility(const S390CPUModel *max_model, · line 671

Quoted 10 lines
target/s390x/cpu_models.c +10 -0
+ * The base TCG CPU model "qemu" is based on the z900. However, we already
+ * can also emulate some additional features of later CPU generations, so
+ * we add these additional feature bits here.
+ */
+static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
+{
+    int i, feats[] = {
+        S390_FEAT_STFLE,
+        S390_FEAT_EXTENDED_IMMEDIATE,
+        S390_FEAT_STORE_CLOCK_FAST

According to my list, QEMU also fully implements the following facilities:

S390_FEAT_LONG_DISPLACEMENT
S390_FEAT_GENERAL_INSTRUCTIONS_EXT
S390_FEAT_EXECUTE_EXT
S390_FEAT_STFLE_45

Quoted 31 lines
target/s390x/cpu_models.c +10 -1
+    };
+
+    for (i = 0; i < ARRAY_SIZE(feats); i++) {
+        set_bit(feats[i], fbm);
+    }
+}
+
 static S390CPUModel *get_max_cpu_model(Error **errp)
 {
     static S390CPUModel max_model;

@@ -670,10 +688,11 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
     if (kvm_enabled()) {
         kvm_s390_get_host_cpu_model(&max_model, errp);
     } else {
-        /* TCG emulates a z900 */
+        /* TCG emulates a z900 (with some additional features) */
         max_model.def = &s390_cpu_defs[0];
         bitmap_copy(max_model.features, max_model.def->default_feat,
                     S390_FEAT_MAX);
+        add_qemu_cpu_model_features(max_model.features);
     }
     if (!*errp) {
         cached = true;

@@ -925,11 +944,15 @@ static void s390_host_cpu_model_initfn(Object *obj)
 
 static void s390_qemu_cpu_model_initfn(Object *obj)
 {
+    static S390CPUDef s390_qemu_cpu_defs;
     S390CPU *cpu = S390_CPU(obj);
 
     cpu->model = g_malloc0(sizeof(*cpu->model));

target/s390x/cpu_models.c static void s390_host_cpu_model_initfn(Object *obj) · line 958

Quoted 10 lines
target/s390x/cpu_models.c +5 -2
-    /* TCG emulates a z900 */
-    cpu->model->def = &s390_cpu_defs[0];
+    /* TCG emulates a z900 (with some additional features) */
+    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
+    add_qemu_cpu_model_features(s390_qemu_cpu_defs.default_feat);
+    add_qemu_cpu_model_features(s390_qemu_cpu_defs.full_feat);
+    cpu->model->def = &s390_qemu_cpu_defs;
     bitmap_copy(cpu->model->features, cpu->model->def->default_feat,
                 S390_FEAT_MAX);
 }

Otherwise it looks fine to me.

Reviewed-by Aurelien Jarno <[email protected]>
Signature · 2 lines
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
[email protected]                 http://www.aurel32.net
author
#2 lore raw
Quoted 44 lines · Aurelien Jarno
Quoted 11 lines · Thomas Huth

Currently we only present the plain z900 feature bits to the guest, but QEMU already emulates some additional features (but not all of the next CPU generation, so we can not use the next CPU level as default yet). Since newer Linux kernels are checking the feature bits and refuse to work if a required feature is missing, we should present as much of the supported features as possible when we are running with the default "qemu" CPU.
This patch now adds the "stfle", "extended immediate" and "stckf" facility bits to the "qemu" CPU, which are already supported facilities. It is unfortunately still not enough to run e.g. recent Fedora kernels, but at least it's a first step into the right direction.

This indeed looks like a step in a good direction. At least it makes the recently added STFLE emulation useful :-).

Quoted 14 lines
Signed-off-by Thomas Huth <[email protected]>
---
 target/s390x/cpu_models.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)
target/s390x/cpu_models.c +1 -0
index 8d27363..18789ab 100644
@@ -658,6 +658,24 @@ static void check_compatibility(const S390CPUModel *max_model,
                   "available in the configuration: ");
 }
 
+/**

target/s390x/cpu_models.c static void check_compatibility(const S390CPUModel *max_model, · line 671

Quoted 10 lines
target/s390x/cpu_models.c +10 -0
+ * The base TCG CPU model "qemu" is based on the z900. However, we already
+ * can also emulate some additional features of later CPU generations, so
+ * we add these additional feature bits here.
+ */
+static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
+{
+    int i, feats[] = {
+        S390_FEAT_STFLE,
+        S390_FEAT_EXTENDED_IMMEDIATE,
+        S390_FEAT_STORE_CLOCK_FAST

According to my list, QEMU also fully implements the following facilities:

Quoted 4 lines

S390_FEAT_LONG_DISPLACEMENT
S390_FEAT_GENERAL_INSTRUCTIONS_EXT
S390_FEAT_EXECUTE_EXT
S390_FEAT_STFLE_45

Ok, thanks, I'll respin and add them to the list!

Quoted 44 lines
Quoted 31 lines
target/s390x/cpu_models.c +10 -1
+    };
+
+    for (i = 0; i < ARRAY_SIZE(feats); i++) {
+        set_bit(feats[i], fbm);
+    }
+}
+
 static S390CPUModel *get_max_cpu_model(Error **errp)
 {
     static S390CPUModel max_model;

@@ -670,10 +688,11 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
     if (kvm_enabled()) {
         kvm_s390_get_host_cpu_model(&max_model, errp);
     } else {
-        /* TCG emulates a z900 */
+        /* TCG emulates a z900 (with some additional features) */
         max_model.def = &s390_cpu_defs[0];
         bitmap_copy(max_model.features, max_model.def->default_feat,
                     S390_FEAT_MAX);
+        add_qemu_cpu_model_features(max_model.features);
     }
     if (!*errp) {
         cached = true;

@@ -925,11 +944,15 @@ static void s390_host_cpu_model_initfn(Object *obj)
 
 static void s390_qemu_cpu_model_initfn(Object *obj)
 {
+    static S390CPUDef s390_qemu_cpu_defs;
     S390CPU *cpu = S390_CPU(obj);
 
     cpu->model = g_malloc0(sizeof(*cpu->model));

target/s390x/cpu_models.c static void s390_host_cpu_model_initfn(Object *obj) · line 958

Quoted 10 lines
target/s390x/cpu_models.c +5 -2
-    /* TCG emulates a z900 */
-    cpu->model->def = &s390_cpu_defs[0];
+    /* TCG emulates a z900 (with some additional features) */
+    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
+    add_qemu_cpu_model_features(s390_qemu_cpu_defs.default_feat);
+    add_qemu_cpu_model_features(s390_qemu_cpu_defs.full_feat);
+    cpu->model->def = &s390_qemu_cpu_defs;
     bitmap_copy(cpu->model->features, cpu->model->def->default_feat,
                 S390_FEAT_MAX);
 }

Otherwise it looks fine to me.

Quoted 1 line
Reviewed-by Aurelien Jarno <[email protected]>

Thanks!

Thomas

#3 lore raw
In reply to Aurelien Jarno
Quoted 44 lines · Aurelien Jarno
Quoted 11 lines · Thomas Huth

Currently we only present the plain z900 feature bits to the guest, but QEMU already emulates some additional features (but not all of the next CPU generation, so we can not use the next CPU level as default yet). Since newer Linux kernels are checking the feature bits and refuse to work if a required feature is missing, we should present as much of the supported features as possible when we are running with the default "qemu" CPU.
This patch now adds the "stfle", "extended immediate" and "stckf" facility bits to the "qemu" CPU, which are already supported facilities. It is unfortunately still not enough to run e.g. recent Fedora kernels, but at least it's a first step into the right direction.

This indeed looks like a step in a good direction. At least it makes the recently added STFLE emulation useful :-).

Quoted 14 lines
Signed-off-by Thomas Huth <[email protected]>
---
 target/s390x/cpu_models.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)
target/s390x/cpu_models.c +1 -0
index 8d27363..18789ab 100644
@@ -658,6 +658,24 @@ static void check_compatibility(const S390CPUModel *max_model,
                   "available in the configuration: ");
 }
 
+/**

target/s390x/cpu_models.c static void check_compatibility(const S390CPUModel *max_model, · line 671

Quoted 10 lines
target/s390x/cpu_models.c +10 -0
+ * The base TCG CPU model "qemu" is based on the z900. However, we already
+ * can also emulate some additional features of later CPU generations, so
+ * we add these additional feature bits here.
+ */
+static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
+{
+    int i, feats[] = {
+        S390_FEAT_STFLE,
+        S390_FEAT_EXTENDED_IMMEDIATE,
+        S390_FEAT_STORE_CLOCK_FAST

According to my list, QEMU also fully implements the following facilities:

Quoted 5 lines

S390_FEAT_LONG_DISPLACEMENT
S390_FEAT_GENERAL_INSTRUCTIONS_EXT
S390_FEAT_EXECUTE_EXT
S390_FEAT_STFLE_45

If S390_FEAT_LONG_DISPLACEMENT is supported, so is S390_FEAT_LONG_DISPLACEMENT_FAST (because TCG obvious has high performance ;) )

Signature · 3 lines
Thanks,

David
#4 lore raw
In reply to Thomas Huth
Quoted 12 lines · Thomas Huth

Currently we only present the plain z900 feature bits to the guest, but QEMU already emulates some additional features (but not all of the next CPU generation, so we can not use the next CPU level as default yet). Since newer Linux kernels are checking the feature bits and refuse to work if a required feature is missing, we should present as much of the supported features as possible when we are running with the default "qemu" CPU.
This patch now adds the "stfle", "extended immediate" and "stckf" facility bits to the "qemu" CPU, which are already supported facilities. It is unfortunately still not enough to run e.g. recent Fedora kernels, but at least it's a first step into the right direction.

Three things:

1. Should we care about backwards compatibility? I think so. This should be fixed up using compat machine properties. (qemu model is a migration-safe model and could e.g. be used in KVM setups, too).

2. I would recommend to not enable STFLE for now. Why?

It is/was an indication that the system is running on a z9 (and implicitly has the basic features). This was not only done because people were lazy, but because this bit was implicitly connected to other machine properties.

One popular example is the "DAT-enhancement facility 2". It introduced the "LOAD PAGE TABLE ENTRY ADDRESS" instruction, but no facility bit was introduced. SO there is no way to check if the instruction is available and actually working.

I heard rumors that if the STFLE is available, this is also an indication for DAT-enhancement facility 2. target/s390x/kvm.c uses the same heuristic in kvm_s390_get_host_cpu_model().

Please note that we added a feature representation for this facility, because this would allow us later on to at least model removal of such a facility (if HW actually would drop it) on a CPU model level.

3. This introduces some inconsistency. s390x/cpu_models.c:set_feature() explicitly tests for such inconsistencies.

So your QEMU CPU model would have a feature, but you would not be able to run that model using QEMU when manually specifying it on the command line. Especially, expanding the "qemu" model and feeding it back to QEMU will fail.

So I am not sure if we should introduce such inconsistencies at that point. Rather fix up the basics and then move the CPU model to a consistent model.

Quoted 66 lines
Signed-off-by Thomas Huth <[email protected]>
---
 target/s390x/cpu_models.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)
target/s390x/cpu_models.c +26 -3
index 8d27363..18789ab 100644
@@ -658,6 +658,24 @@ static void check_compatibility(const S390CPUModel *max_model,
                   "available in the configuration: ");
 }
 
+/**
+ * The base TCG CPU model "qemu" is based on the z900. However, we already
+ * can also emulate some additional features of later CPU generations, so
+ * we add these additional feature bits here.
+ */
+static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
+{
+    int i, feats[] = {
+        S390_FEAT_STFLE,
+        S390_FEAT_EXTENDED_IMMEDIATE,
+        S390_FEAT_STORE_CLOCK_FAST
+    };
+
+    for (i = 0; i < ARRAY_SIZE(feats); i++) {
+        set_bit(feats[i], fbm);
+    }
+}
+
 static S390CPUModel *get_max_cpu_model(Error **errp)
 {
     static S390CPUModel max_model;

@@ -670,10 +688,11 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
     if (kvm_enabled()) {
         kvm_s390_get_host_cpu_model(&max_model, errp);
     } else {
-        /* TCG emulates a z900 */
+        /* TCG emulates a z900 (with some additional features) */
         max_model.def = &s390_cpu_defs[0];
         bitmap_copy(max_model.features, max_model.def->default_feat,
                     S390_FEAT_MAX);
+        add_qemu_cpu_model_features(max_model.features);
     }
     if (!*errp) {
         cached = true;

@@ -925,11 +944,15 @@ static void s390_host_cpu_model_initfn(Object *obj)
 
 static void s390_qemu_cpu_model_initfn(Object *obj)
 {
+    static S390CPUDef s390_qemu_cpu_defs;
     S390CPU *cpu = S390_CPU(obj);
 
     cpu->model = g_malloc0(sizeof(*cpu->model));
-    /* TCG emulates a z900 */
-    cpu->model->def = &s390_cpu_defs[0];
+    /* TCG emulates a z900 (with some additional features) */
+    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
+    add_qemu_cpu_model_features(s390_qemu_cpu_defs.default_feat);
+    add_qemu_cpu_model_features(s390_qemu_cpu_defs.full_feat);
+    cpu->model->def = &s390_qemu_cpu_defs;
     bitmap_copy(cpu->model->features, cpu->model->def->default_feat,
                 S390_FEAT_MAX);
 }
Signature · 3 lines
Thanks,

David
author
#5 lore raw
Quoted 14 lines · David Hildenbrand
Quoted 12 lines · Thomas Huth

Currently we only present the plain z900 feature bits to the guest, but QEMU already emulates some additional features (but not all of the next CPU generation, so we can not use the next CPU level as default yet). Since newer Linux kernels are checking the feature bits and refuse to work if a required feature is missing, we should present as much of the supported features as possible when we are running with the default "qemu" CPU.
This patch now adds the "stfle", "extended immediate" and "stckf" facility bits to the "qemu" CPU, which are already supported facilities. It is unfortunately still not enough to run e.g. recent Fedora kernels, but at least it's a first step into the right direction.

Quoted 5 lines

Three things:

1. Should we care about backwards compatibility? I think so. This should be fixed up using compat machine properties. (qemu model is a migration-safe model and could e.g. be used in KVM setups, too).

Theoretically, I agree, but do we really care about backwards compatibility at this point in time? All major distro kernels (except Debian, I think) currently do not work in QEMU, so there is currently not that much that can be migrated...
And currently, the "qemu" CPU is the very same as the "z900" CPU, so you might also get along with simply using "-cpu z900" on the destination instead, I guess.

Quoted 6 lines

2. I would recommend to not enable STFLE for now. Why?

It is/was an indication that the system is running on a z9 (and implicitly has the basic features). This was not only done because people were lazy, but because this bit was implicitly connected to other machine properties.

Uh, that's ugly!

Quoted 4 lines

One popular example is the "DAT-enhancement facility 2". It introduced the "LOAD PAGE TABLE ENTRY ADDRESS" instruction, but no facility bit was introduced. SO there is no way to check if the instruction is available and actually working.

Does the Linux kernel use this instruction at all? I just grep'ed through the kernel sources and did not find it. If the Linux kernel does not use it, I think we should ignore this interdependency and just provide the STFLE feature bit to the guest - since recent Linux kernels depend on it.

Quoted 3 lines

Please note that we added a feature representation for this facility, because this would allow us later on to at least model removal of such a facility (if HW actually would drop it) on a CPU model level.

What about STFLE bit 78, according to my version of the POP, it says:

"The enhanced-DAT facility 2 is installed in the
z/Architecture architectural mode."

?

Quoted 7 lines

3. This introduces some inconsistency. s390x/cpu_models.c:set_feature() explicitly tests for such inconsistencies.

So your QEMU CPU model would have a feature, but you would not be able to run that model using QEMU when manually specifying it on the command line. Especially, expanding the "qemu" model and feeding it back to QEMU will fail.

I've checked that I can also successfully disable the features again at the command line, using "-cpu qemu,eimm=false" for example, so not sure what exactly you're talking about here. Could you please elaborate?

Quoted 3 lines

So I am not sure if we should introduce such inconsistencies at that point. Rather fix up the basics and then move the CPU model to a consistent model.

I think we're very far away from being able to use the next official CPU model generation in QEMU TCG, so having at least something that let's us run other recent distro kernels apart from the Debian ones would be very helpful. I also understand the "qemu" CPU this way: "Simply give me the best CPU features that TCG currently can provide". If you want to have a "consistent" CPU state, you can simply use an official model like "z900" instead.

Thomas

#6 lore raw
Quoted 22 lines · Thomas Huth
Quoted 7 lines · David Hildenbrand

On 17.05.2017 17:35, Thomas Huth wrote:
2. I would recommend to not enable STFLE for now. Why?

It is/was an indication that the system is running on a z9 (and implicitly has the basic features). This was not only done because people were lazy, but because this bit was implicitly connected to other machine properties.

Uh, that's ugly!

Quoted 4 lines

One popular example is the "DAT-enhancement facility 2". It introduced the "LOAD PAGE TABLE ENTRY ADDRESS" instruction, but no facility bit was introduced. SO there is no way to check if the instruction is available and actually working.

Does the Linux kernel use this instruction at all? I just grep'ed through the kernel sources and did not find it. If the Linux kernel does not use it, I think we should ignore this interdependency and just provide the STFLE feature bit to the guest - since recent Linux kernels depend on it.

Quoted 8 lines
Quoted 3 lines

Please note that we added a feature representation for this facility, because this would allow us later on to at least model removal of such a facility (if HW actually would drop it) on a CPU model level.

What about STFLE bit 78, according to my version of the POP, it says:

"The enhanced-DAT facility 2 is installed in the
z/Architecture architectural mode."

No that is different. IBM has chosen very confusing names for the DAT related facilities:
- DAT-Enhancement Facility 1 => bits 3, 4 & 5
- DAT-Enhancement Facility 2
- Enhanced-DAT Facility 1 => bit 8
- Enhanced-DAT Facility 2 => bit 78

Aurelien

Signature · 2 lines
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
[email protected]                 http://www.aurel32.net
#7 lore raw
In reply to Thomas Huth
Quoted 21 lines · Thomas Huth
Quoted 14 lines · David Hildenbrand
Quoted 12 lines · Thomas Huth

Currently we only present the plain z900 feature bits to the guest, but QEMU already emulates some additional features (but not all of the next CPU generation, so we can not use the next CPU level as default yet). Since newer Linux kernels are checking the feature bits and refuse to work if a required feature is missing, we should present as much of the supported features as possible when we are running with the default "qemu" CPU.
This patch now adds the "stfle", "extended immediate" and "stckf" facility bits to the "qemu" CPU, which are already supported facilities. It is unfortunately still not enough to run e.g. recent Fedora kernels, but at least it's a first step into the right direction.

Quoted 5 lines

Three things:

1. Should we care about backwards compatibility? I think so. This should be fixed up using compat machine properties. (qemu model is a migration-safe model and could e.g. be used in KVM setups, too).

Quoted 7 lines

Theoretically, I agree, but do we really care about backwards compatibility at this point in time? All major distro kernels (except Debian, I think) currently do not work in QEMU, so there is currently not that much that can be migrated...
And currently, the "qemu" CPU is the very same as the "z900" CPU, so you might also get along with simply using "-cpu z900" on the destination instead, I guess.

If possible, I would like to avoid changing migration safe CPU model. And I guess it shouldn't be too hard for now (unless we really change the base model to e.g. a z9, then some more work might have to be done)

I think for now, setting "stfle=off" on "s390-cpu-qemu" for compat machines should do the trick.

Quoted 15 lines
Quoted 7 lines

2. I would recommend to not enable STFLE for now. Why?

It is/was an indication that the system is running on a z9 (and implicitly has the basic features). This was not only done because people were lazy, but because this bit was implicitly connected to other machine properties.

Uh, that's ugly!

Quoted 4 lines

One popular example is the "DAT-enhancement facility 2". It introduced the "LOAD PAGE TABLE ENTRY ADDRESS" instruction, but no facility bit was introduced. SO there is no way to check if the instruction is available and actually working.

Quoted 5 lines

Does the Linux kernel use this instruction at all? I just grep'ed through the kernel sources and did not find it. If the Linux kernel does not use it, I think we should ignore this interdependency and just provide the STFLE feature bit to the guest - since recent Linux kernels depend on it.

Yes, current linux doesn't use it, I don't remember if previous versions did. Most likely not. The question is if they relied on the stfle==z9 assumption. The STFLE facility really is special in that sense.

Quoted 11 lines
Quoted 4 lines

Please note that we added a feature representation for this facility, because this would allow us later on to at least model removal of such a facility (if HW actually would drop it) on a CPU model level.

What about STFLE bit 78, according to my version of the POP, it says:

"The enhanced-DAT facility 2 is installed in the
z/Architecture architectural mode."

?

As Aurelien already mentioned, there seemed to be different ways to enhance DAT :) enhanced-DAT facility 2 is 2GB page support.

Quoted 4 lines
Quoted 4 lines

3. This introduces some inconsistency. s390x/cpu_models.c:set_feature() explicitly tests for such inconsistencies.

Quoted 8 lines
Quoted 4 lines

So your QEMU CPU model would have a feature, but you would not be able to run that model using QEMU when manually specifying it on the command line. Especially, expanding the "qemu" model and feeding it back to QEMU will fail.

I've checked that I can also successfully disable the features again at the command line, using "-cpu qemu,eimm=false" for example, so not sure what exactly you're talking about here. Could you please elaborate?

Assume libvirt/the user expands the CPU model name "qemu" via "qmp-expand-cpu-model "qemu", you will get something like

"z900-base,.....,stfle=on"

If you feed that to QEMU using "-cpu z900-base,...,stfle=on", QEMU will detect the inconsistency when setting the property and abort. However, "-cpu qemu" will succeed. Please note that these checks actually make sense for KVM:

If you're on a z13 and configure a zEC12, you might be tempted to add e.g. "vx=on". However, IBC (Instruction Blocking Control) in the machine will block any attempt to execute a vx instruction. So these checks make sure that only facilities really supported for a machine generation can be enabled.

If we really want that, we might decide to drop such checks for models < e.g. z9, because nobody will most likely care.

Quoted 5 lines
Quoted 4 lines

So I am not sure if we should introduce such inconsistencies at that point. Rather fix up the basics and then move the CPU model to a consistent model.

Quoted 7 lines

I think we're very far away from being able to use the next official CPU model generation in QEMU TCG, so having at least something that let's us run other recent distro kernels apart from the Debian ones would be very helpful. I also understand the "qemu" CPU this way: "Simply give me the best CPU features that TCG currently can provide". If you want to have a "consistent" CPU state, you can simply use an official model like "z900" instead.

"qemu" is just like what "host" is for kvm. A consistent model, because it is the default.

However, KVM folks also have the requirement to allow "unfiltered"/"inconsistent" models, e.g. for development purposes.

There was the idea to introduce a CPU model "-cpu off", that would act as before, without any CPU model support: Blindly enable anything we can.

This model would of course not be static, not migration-safe, and one would not be able to modify features. This would map to an "unfiltered qemu" model under TCG. We could blindly enable anything there.

Quoted 3 lines

Thomas

Signature · 3 lines
Thanks,

David
#8 lore raw
Quoted 93 lines · David Hildenbrand
Quoted 21 lines · Thomas Huth
Quoted 14 lines · David Hildenbrand
Quoted 12 lines · Thomas Huth

Currently we only present the plain z900 feature bits to the guest, but QEMU already emulates some additional features (but not all of the next CPU generation, so we can not use the next CPU level as default yet). Since newer Linux kernels are checking the feature bits and refuse to work if a required feature is missing, we should present as much of the supported features as possible when we are running with the default "qemu" CPU.
This patch now adds the "stfle", "extended immediate" and "stckf" facility bits to the "qemu" CPU, which are already supported facilities. It is unfortunately still not enough to run e.g. recent Fedora kernels, but at least it's a first step into the right direction.

Quoted 5 lines

Three things:

1. Should we care about backwards compatibility? I think so. This should be fixed up using compat machine properties. (qemu model is a migration-safe model and could e.g. be used in KVM setups, too).

Quoted 7 lines

Theoretically, I agree, but do we really care about backwards compatibility at this point in time? All major distro kernels (except Debian, I think) currently do not work in QEMU, so there is currently not that much that can be migrated...
And currently, the "qemu" CPU is the very same as the "z900" CPU, so you might also get along with simply using "-cpu z900" on the destination instead, I guess.

If possible, I would like to avoid changing migration safe CPU model. And I guess it shouldn't be too hard for now (unless we really change the base model to e.g. a z9, then some more work might have to be done)

I think for now, setting "stfle=off" on "s390-cpu-qemu" for compat machines should do the trick.

Quoted 15 lines
Quoted 7 lines

2. I would recommend to not enable STFLE for now. Why?

It is/was an indication that the system is running on a z9 (and implicitly has the basic features). This was not only done because people were lazy, but because this bit was implicitly connected to other machine properties.

Uh, that's ugly!

Quoted 4 lines

One popular example is the "DAT-enhancement facility 2". It introduced the "LOAD PAGE TABLE ENTRY ADDRESS" instruction, but no facility bit was introduced. SO there is no way to check if the instruction is available and actually working.

Quoted 5 lines

Does the Linux kernel use this instruction at all? I just grep'ed through the kernel sources and did not find it. If the Linux kernel does not use it, I think we should ignore this interdependency and just provide the STFLE feature bit to the guest - since recent Linux kernels depend on it.

Yes, current linux doesn't use it, I don't remember if previous versions did. Most likely not. The question is if they relied on the stfle==z9 assumption. The STFLE facility really is special in that sense.

Quoted 11 lines
Quoted 4 lines

Please note that we added a feature representation for this facility, because this would allow us later on to at least model removal of such a facility (if HW actually would drop it) on a CPU model level.

What about STFLE bit 78, according to my version of the POP, it says:

"The enhanced-DAT facility 2 is installed in the
z/Architecture architectural mode."

?

As Aurelien already mentioned, there seemed to be different ways to enhance DAT :) enhanced-DAT facility 2 is 2GB page support.

Quoted 4 lines
Quoted 4 lines

3. This introduces some inconsistency. s390x/cpu_models.c:set_feature() explicitly tests for such inconsistencies.

Quoted 8 lines
Quoted 4 lines

So your QEMU CPU model would have a feature, but you would not be able to run that model using QEMU when manually specifying it on the command line. Especially, expanding the "qemu" model and feeding it back to QEMU will fail.

I've checked that I can also successfully disable the features again at the command line, using "-cpu qemu,eimm=false" for example, so not sure what exactly you're talking about here. Could you please elaborate?

Assume libvirt/the user expands the CPU model name "qemu" via "qmp-expand-cpu-model "qemu", you will get something like

Quoted 7 lines

"z900-base,.....,stfle=on"

If you feed that to QEMU using "-cpu z900-base,...,stfle=on", QEMU will detect the inconsistency when setting the property and abort. However, "-cpu qemu" will succeed. Please note that these checks actually make sense for KVM:

Jason (now on cc) has a patch prepared for other reasons that disabled features for given machines. I kept the ESOP example in that patch.
That would allow us to disable STFLE for old machines but enable it for 2.10

copy/pasted and hand edited to get rid of the unrelated changes:

hw/s390x/s390-virtio-ccw.c +2 -0
index 3c12735..26b0ac9 100644
@@ -30,6 +30,7 @@
 #include "ipl.h"
 #include "hw/s390x/s390-virtio-ccw.h"
 #include "hw/s390x/css-bridge.h"
+#include "cpu_models.h"
 
 static const char *const reset_dev_types[] = {
     TYPE_VIRTUAL_CSS_BRIDGE,

@@ -481,6 +482,7 @@ DEFINE_CCW_MACHINE(2_10, "2.10", true);
 static void ccw_machine_2_9_instance_options(MachineState *machine)
 {
     ccw_machine_2_10_instance_options(machine);
+    s390_cpudef_featoff_greater(12, 1, S390_FEAT_ESOP);
 }
 
 static void ccw_machine_2_9_class_options(MachineClass *mc)
target/s390x/cpu_models.c +26 -0
index 71ddb6c..5f295a5 100644
@@ -78,6 +78,32 @@ static S390CPUDef s390_cpu_defs[] = {
     CPUDEF_INIT(0x3906, 14, 1, 47, 0x08000000U, "z14", "IBM z14 GA1"),
 };
 
+void s390_cpudef_featoff(uint8_t gen, uint8_t ec_ga, S390Feat feat)
+{
+    const S390CPUDef *def;
+
+    def = s390_find_cpu_def(0, gen, ec_ga, NULL);
+    clear_bit(feat, (unsigned long *)&def->default_feat);
+}
+
+void s390_cpudef_featoff_greater(uint8_t gen, uint8_t ec_ga, S390Feat feat)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(s390_cpu_defs); i++) {
+        const S390CPUDef *def = &s390_cpu_defs[i];
+
+        if (def->gen < gen) {
+            continue;
+        }
+        if (def->gen == gen && def->ec_ga < ec_ga) {
+            continue;
+        }
+
+        clear_bit(feat, (unsigned long *)&def->default_feat);
+    }
+}
+
 uint32_t s390_get_hmfai(void)
 {
     static S390CPU *cpu;
target/s390x/cpu_models.h +2 -0
index 136a602..881eb65 100644
@@ -69,6 +69,8 @@ typedef struct S390CPUModel {
 #define ibc_gen(x)        (x == 0 ? 0 : ((x >> 4) + S390_GEN_Z10))
 #define ibc_ec_ga(x)      (x & 0xf)
 
+void s390_cpudef_featoff(uint8_t gen, uint8_t ec_ga, S390Feat feat);
+void s390_cpudef_featoff_greater(uint8_t gen, uint8_t ec_ga, S390Feat feat);
 uint32_t s390_get_hmfai(void);
 uint8_t s390_get_mha_pow(void);
 uint32_t s390_get_ibc_val(void);
target/s390x/gen-features.c +1 -0
index 3efd5dd..105e5f5 100644
@@ -515,6 +515,7 @@ static uint16_t default_GEN12_GA1[] = {
     S390_FEAT_ADAPTER_EVENT_NOTIFICATION,
     S390_FEAT_ADAPTER_INT_SUPPRESSION,
     S390_FEAT_EDAT_2,
+    S390_FEAT_ESOP,
 };

Maybe we should split that out and merge such a patch sooner than the (yet in development) other changes?

Quoted 40 lines

If you're on a z13 and configure a zEC12, you might be tempted to add e.g. "vx=on". However, IBC (Instruction Blocking Control) in the machine will block any attempt to execute a vx instruction. So these checks make sure that only facilities really supported for a machine generation can be enabled.

If we really want that, we might decide to drop such checks for models < e.g. z9, because nobody will most likely care.

Quoted 5 lines
Quoted 4 lines

So I am not sure if we should introduce such inconsistencies at that point. Rather fix up the basics and then move the CPU model to a consistent model.

Quoted 7 lines

I think we're very far away from being able to use the next official CPU model generation in QEMU TCG, so having at least something that let's us run other recent distro kernels apart from the Debian ones would be very helpful. I also understand the "qemu" CPU this way: "Simply give me the best CPU features that TCG currently can provide". If you want to have a "consistent" CPU state, you can simply use an official model like "z900" instead.

"qemu" is just like what "host" is for kvm. A consistent model, because it is the default.

However, KVM folks also have the requirement to allow "unfiltered"/"inconsistent" models, e.g. for development purposes.

There was the idea to introduce a CPU model "-cpu off", that would act as before, without any CPU model support: Blindly enable anything we can.

This model would of course not be static, not migration-safe, and one would not be able to modify features. This would map to an "unfiltered qemu" model under TCG. We could blindly enable anything there.

Quoted 3 lines

Thomas

author
#9 lore raw
Quoted 105 lines · Christian Borntraeger
Quoted 93 lines · David Hildenbrand
Quoted 21 lines · Thomas Huth
Quoted 14 lines · David Hildenbrand
Quoted 12 lines · Thomas Huth

Currently we only present the plain z900 feature bits to the guest, but QEMU already emulates some additional features (but not all of the next CPU generation, so we can not use the next CPU level as default yet). Since newer Linux kernels are checking the feature bits and refuse to work if a required feature is missing, we should present as much of the supported features as possible when we are running with the default "qemu" CPU.
This patch now adds the "stfle", "extended immediate" and "stckf" facility bits to the "qemu" CPU, which are already supported facilities. It is unfortunately still not enough to run e.g. recent Fedora kernels, but at least it's a first step into the right direction.

Quoted 5 lines

Three things:

1. Should we care about backwards compatibility? I think so. This should be fixed up using compat machine properties. (qemu model is a migration-safe model and could e.g. be used in KVM setups, too).

Quoted 7 lines

Theoretically, I agree, but do we really care about backwards compatibility at this point in time? All major distro kernels (except Debian, I think) currently do not work in QEMU, so there is currently not that much that can be migrated...
And currently, the "qemu" CPU is the very same as the "z900" CPU, so you might also get along with simply using "-cpu z900" on the destination instead, I guess.

If possible, I would like to avoid changing migration safe CPU model. And I guess it shouldn't be too hard for now (unless we really change the base model to e.g. a z9, then some more work might have to be done)

I think for now, setting "stfle=off" on "s390-cpu-qemu" for compat machines should do the trick.

Quoted 15 lines
Quoted 7 lines

2. I would recommend to not enable STFLE for now. Why?

It is/was an indication that the system is running on a z9 (and implicitly has the basic features). This was not only done because people were lazy, but because this bit was implicitly connected to other machine properties.

Uh, that's ugly!

Quoted 4 lines

One popular example is the "DAT-enhancement facility 2". It introduced the "LOAD PAGE TABLE ENTRY ADDRESS" instruction, but no facility bit was introduced. SO there is no way to check if the instruction is available and actually working.

Quoted 5 lines

Does the Linux kernel use this instruction at all? I just grep'ed through the kernel sources and did not find it. If the Linux kernel does not use it, I think we should ignore this interdependency and just provide the STFLE feature bit to the guest - since recent Linux kernels depend on it.

Yes, current linux doesn't use it, I don't remember if previous versions did. Most likely not. The question is if they relied on the stfle==z9 assumption. The STFLE facility really is special in that sense.

Quoted 11 lines
Quoted 4 lines

Please note that we added a feature representation for this facility, because this would allow us later on to at least model removal of such a facility (if HW actually would drop it) on a CPU model level.

What about STFLE bit 78, according to my version of the POP, it says:

"The enhanced-DAT facility 2 is installed in the
z/Architecture architectural mode."

?

As Aurelien already mentioned, there seemed to be different ways to enhance DAT :) enhanced-DAT facility 2 is 2GB page support.

Quoted 4 lines
Quoted 4 lines

3. This introduces some inconsistency. s390x/cpu_models.c:set_feature() explicitly tests for such inconsistencies.

Quoted 8 lines
Quoted 4 lines

So your QEMU CPU model would have a feature, but you would not be able to run that model using QEMU when manually specifying it on the command line. Especially, expanding the "qemu" model and feeding it back to QEMU will fail.

I've checked that I can also successfully disable the features again at the command line, using "-cpu qemu,eimm=false" for example, so not sure what exactly you're talking about here. Could you please elaborate?

Assume libvirt/the user expands the CPU model name "qemu" via "qmp-expand-cpu-model "qemu", you will get something like

Quoted 7 lines

"z900-base,.....,stfle=on"

If you feed that to QEMU using "-cpu z900-base,...,stfle=on", QEMU will detect the inconsistency when setting the property and abort. However, "-cpu qemu" will succeed. Please note that these checks actually make sense for KVM:

Jason (now on cc) has a patch prepared for other reasons that disabled features for given machines. I kept the ESOP example in that patch.
That would allow us to disable STFLE for old machines but enable it for 2.10

[...]

Quoted 2 lines

Maybe we should split that out and merge such a patch sooner than the (yet in development) other changes?

Yes, that sounds like a good idea, I think we could use the same mechanism here, too, so please split it out and submit it earlier!

Thanks a lot,
Thomas

#10 lore raw
Quoted 107 lines · Thomas Huth
Quoted 105 lines · Christian Borntraeger
Quoted 93 lines · David Hildenbrand
Quoted 21 lines · Thomas Huth
Quoted 14 lines · David Hildenbrand
Quoted 12 lines · Thomas Huth

Currently we only present the plain z900 feature bits to the guest, but QEMU already emulates some additional features (but not all of the next CPU generation, so we can not use the next CPU level as default yet). Since newer Linux kernels are checking the feature bits and refuse to work if a required feature is missing, we should present as much of the supported features as possible when we are running with the default "qemu" CPU.
This patch now adds the "stfle", "extended immediate" and "stckf" facility bits to the "qemu" CPU, which are already supported facilities. It is unfortunately still not enough to run e.g. recent Fedora kernels, but at least it's a first step into the right direction.

Quoted 5 lines

Three things:

1. Should we care about backwards compatibility? I think so. This should be fixed up using compat machine properties. (qemu model is a migration-safe model and could e.g. be used in KVM setups, too).

Quoted 7 lines

Theoretically, I agree, but do we really care about backwards compatibility at this point in time? All major distro kernels (except Debian, I think) currently do not work in QEMU, so there is currently not that much that can be migrated...
And currently, the "qemu" CPU is the very same as the "z900" CPU, so you might also get along with simply using "-cpu z900" on the destination instead, I guess.

If possible, I would like to avoid changing migration safe CPU model. And I guess it shouldn't be too hard for now (unless we really change the base model to e.g. a z9, then some more work might have to be done)

I think for now, setting "stfle=off" on "s390-cpu-qemu" for compat machines should do the trick.

Quoted 15 lines
Quoted 7 lines

2. I would recommend to not enable STFLE for now. Why?

It is/was an indication that the system is running on a z9 (and implicitly has the basic features). This was not only done because people were lazy, but because this bit was implicitly connected to other machine properties.

Uh, that's ugly!

Quoted 4 lines

One popular example is the "DAT-enhancement facility 2". It introduced the "LOAD PAGE TABLE ENTRY ADDRESS" instruction, but no facility bit was introduced. SO there is no way to check if the instruction is available and actually working.

Quoted 5 lines

Does the Linux kernel use this instruction at all? I just grep'ed through the kernel sources and did not find it. If the Linux kernel does not use it, I think we should ignore this interdependency and just provide the STFLE feature bit to the guest - since recent Linux kernels depend on it.

Yes, current linux doesn't use it, I don't remember if previous versions did. Most likely not. The question is if they relied on the stfle==z9 assumption. The STFLE facility really is special in that sense.

Quoted 11 lines
Quoted 4 lines

Please note that we added a feature representation for this facility, because this would allow us later on to at least model removal of such a facility (if HW actually would drop it) on a CPU model level.

What about STFLE bit 78, according to my version of the POP, it says:

"The enhanced-DAT facility 2 is installed in the
z/Architecture architectural mode."

?

As Aurelien already mentioned, there seemed to be different ways to enhance DAT :) enhanced-DAT facility 2 is 2GB page support.

Quoted 4 lines
Quoted 4 lines

3. This introduces some inconsistency. s390x/cpu_models.c:set_feature() explicitly tests for such inconsistencies.

Quoted 8 lines
Quoted 4 lines

So your QEMU CPU model would have a feature, but you would not be able to run that model using QEMU when manually specifying it on the command line. Especially, expanding the "qemu" model and feeding it back to QEMU will fail.

I've checked that I can also successfully disable the features again at the command line, using "-cpu qemu,eimm=false" for example, so not sure what exactly you're talking about here. Could you please elaborate?

Assume libvirt/the user expands the CPU model name "qemu" via "qmp-expand-cpu-model "qemu", you will get something like

Quoted 7 lines

"z900-base,.....,stfle=on"

If you feed that to QEMU using "-cpu z900-base,...,stfle=on", QEMU will detect the inconsistency when setting the property and abort. However, "-cpu qemu" will succeed. Please note that these checks actually make sense for KVM:

Jason (now on cc) has a patch prepared for other reasons that disabled features for given machines. I kept the ESOP example in that patch.
That would allow us to disable STFLE for old machines but enable it for 2.10

[...]

Quoted 9 lines
Quoted 2 lines

Maybe we should split that out and merge such a patch sooner than the (yet in development) other changes?

Yes, that sounds like a good idea, I think we could use the same mechanism here, too, so please split it out and submit it earlier!

Thanks a lot,
Thomas

I think this is useful but a different use case:

What Christian/Jason have here is a way to fixup default models (e.g. z900, ZEC12...). This is necessary when introducing new features / movinf features from FULL into DEFAULT.

We don't want to fixup default models but the s390x-cpu-qemu.

Signature · 3 lines
Thanks,

David
author
#11 lore raw
Quoted 114 lines · David Hildenbrand
Quoted 106 lines · Thomas Huth
Quoted 102 lines · Christian Borntraeger
Quoted 93 lines · David Hildenbrand
Quoted 21 lines · Thomas Huth
Quoted 14 lines · David Hildenbrand
Quoted 12 lines · Thomas Huth

Currently we only present the plain z900 feature bits to the guest, but QEMU already emulates some additional features (but not all of the next CPU generation, so we can not use the next CPU level as default yet). Since newer Linux kernels are checking the feature bits and refuse to work if a required feature is missing, we should present as much of the supported features as possible when we are running with the default "qemu" CPU.
This patch now adds the "stfle", "extended immediate" and "stckf" facility bits to the "qemu" CPU, which are already supported facilities. It is unfortunately still not enough to run e.g. recent Fedora kernels, but at least it's a first step into the right direction.

Quoted 5 lines

Three things:

1. Should we care about backwards compatibility? I think so. This should be fixed up using compat machine properties. (qemu model is a migration-safe model and could e.g. be used in KVM setups, too).

Quoted 7 lines

Theoretically, I agree, but do we really care about backwards compatibility at this point in time? All major distro kernels (except Debian, I think) currently do not work in QEMU, so there is currently not that much that can be migrated...
And currently, the "qemu" CPU is the very same as the "z900" CPU, so you might also get along with simply using "-cpu z900" on the destination instead, I guess.

If possible, I would like to avoid changing migration safe CPU model. And I guess it shouldn't be too hard for now (unless we really change the base model to e.g. a z9, then some more work might have to be done)

I think for now, setting "stfle=off" on "s390-cpu-qemu" for compat machines should do the trick.

Quoted 15 lines
Quoted 7 lines

2. I would recommend to not enable STFLE for now. Why?

It is/was an indication that the system is running on a z9 (and implicitly has the basic features). This was not only done because people were lazy, but because this bit was implicitly connected to other machine properties.

Uh, that's ugly!

Quoted 4 lines

One popular example is the "DAT-enhancement facility 2". It introduced the "LOAD PAGE TABLE ENTRY ADDRESS" instruction, but no facility bit was introduced. SO there is no way to check if the instruction is available and actually working.

Quoted 5 lines

Does the Linux kernel use this instruction at all? I just grep'ed through the kernel sources and did not find it. If the Linux kernel does not use it, I think we should ignore this interdependency and just provide the STFLE feature bit to the guest - since recent Linux kernels depend on it.

Yes, current linux doesn't use it, I don't remember if previous versions did. Most likely not. The question is if they relied on the stfle==z9 assumption. The STFLE facility really is special in that sense.

Quoted 11 lines
Quoted 4 lines

Please note that we added a feature representation for this facility, because this would allow us later on to at least model removal of such a facility (if HW actually would drop it) on a CPU model level.

What about STFLE bit 78, according to my version of the POP, it says:

"The enhanced-DAT facility 2 is installed in the
z/Architecture architectural mode."

?

As Aurelien already mentioned, there seemed to be different ways to enhance DAT :) enhanced-DAT facility 2 is 2GB page support.

Quoted 4 lines
Quoted 4 lines

3. This introduces some inconsistency. s390x/cpu_models.c:set_feature() explicitly tests for such inconsistencies.

Quoted 8 lines
Quoted 4 lines

So your QEMU CPU model would have a feature, but you would not be able to run that model using QEMU when manually specifying it on the command line. Especially, expanding the "qemu" model and feeding it back to QEMU will fail.

I've checked that I can also successfully disable the features again at the command line, using "-cpu qemu,eimm=false" for example, so not sure what exactly you're talking about here. Could you please elaborate?

Assume libvirt/the user expands the CPU model name "qemu" via "qmp-expand-cpu-model "qemu", you will get something like

Quoted 7 lines

"z900-base,.....,stfle=on"

If you feed that to QEMU using "-cpu z900-base,...,stfle=on", QEMU will detect the inconsistency when setting the property and abort. However, "-cpu qemu" will succeed. Please note that these checks actually make sense for KVM:

Quoted 3 lines

Jason (now on cc) has a patch prepared for other reasons that disabled features for given machines. I kept the ESOP example in that patch.
That would allow us to disable STFLE for old machines but enable it for 2.10

Quoted 7 lines

[...]

Quoted 2 lines

Maybe we should split that out and merge such a patch sooner than the (yet in development) other changes?

Yes, that sounds like a good idea, I think we could use the same mechanism here, too, so please split it out and submit it earlier!

Quoted 7 lines

I think this is useful but a different use case:

What Christian/Jason have here is a way to fixup default models (e.g. z900, ZEC12...). This is necessary when introducing new features / movinf features from FULL into DEFAULT.

We don't want to fixup default models but the s390x-cpu-qemu.

Looking at the functions again, I think you're right, David... it's similar at a first glance, but I'd need slightly different functions here. Ok, so never mind, I guess I have to do it in a different way here...

Thomas