* [Qemu-devel] [PATCH v2] target/s390x/cpu_models: Allow some additional feature bits for the "qemu" CPU @ 2017-05-25 9:22 Thomas Huth 2017-05-26 9:36 ` Aurelien Jarno 2017-05-26 12:14 ` David Hildenbrand 0 siblings, 2 replies; 5+ messages in thread From: Thomas Huth @ 2017-05-25 9:22 UTC (permalink / raw) To: qemu-devel, Richard Henderson Cc: Alexander Graf, David Hildenbrand, Aurelien Jarno 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, it would be nice to have a way to present more of the supported features when we are running with the "qemu" CPU. This patch now adds the supported features to the "full_feat" bitmap, so that additional features can be enabled on the command line now, for example with: qemu-system-s390x -cpu qemu,stfle=true,ldisp=true,eimm=true,stckf=true Signed-off-by: Thomas Huth <thuth@redhat.com> --- v2: Mark feats array with "static const" target/s390x/cpu_models.c | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index 8d27363..e5e005a 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -658,6 +658,30 @@ 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) +{ + static const int feats[] = { + S390_FEAT_STFLE, + S390_FEAT_EXTENDED_IMMEDIATE, + S390_FEAT_LONG_DISPLACEMENT, + S390_FEAT_LONG_DISPLACEMENT_FAST, + S390_FEAT_STORE_CLOCK_FAST, + S390_FEAT_GENERAL_INSTRUCTIONS_EXT, + S390_FEAT_EXECUTE_EXT, + S390_FEAT_STFLE_45, + }; + int i; + + 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 +694,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 optional 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 +950,14 @@ 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 optional 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.full_feat); + cpu->model->def = &s390_qemu_cpu_defs; bitmap_copy(cpu->model->features, cpu->model->def->default_feat, S390_FEAT_MAX); } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target/s390x/cpu_models: Allow some additional feature bits for the "qemu" CPU 2017-05-25 9:22 [Qemu-devel] [PATCH v2] target/s390x/cpu_models: Allow some additional feature bits for the "qemu" CPU Thomas Huth @ 2017-05-26 9:36 ` Aurelien Jarno 2017-05-26 12:14 ` David Hildenbrand 1 sibling, 0 replies; 5+ messages in thread From: Aurelien Jarno @ 2017-05-26 9:36 UTC (permalink / raw) To: Thomas Huth Cc: qemu-devel, Richard Henderson, Alexander Graf, David Hildenbrand On 2017-05-25 11:22, Thomas Huth wrote: > 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, it would be nice > to have a way to present more of the supported features when we are > running with the "qemu" CPU. > This patch now adds the supported features to the "full_feat" bitmap, > so that additional features can be enabled on the command line now, > for example with: > > qemu-system-s390x -cpu qemu,stfle=true,ldisp=true,eimm=true,stckf=true > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > v2: Mark feats array with "static const" > > target/s390x/cpu_models.c | 34 +++++++++++++++++++++++++++++++--- > 1 file changed, 31 insertions(+), 3 deletions(-) Reviewed-by: Aurelien Jarno <aurelien@aurel32.net> -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target/s390x/cpu_models: Allow some additional feature bits for the "qemu" CPU 2017-05-25 9:22 [Qemu-devel] [PATCH v2] target/s390x/cpu_models: Allow some additional feature bits for the "qemu" CPU Thomas Huth 2017-05-26 9:36 ` Aurelien Jarno @ 2017-05-26 12:14 ` David Hildenbrand 2017-05-26 20:14 ` Thomas Huth 1 sibling, 1 reply; 5+ messages in thread From: David Hildenbrand @ 2017-05-26 12:14 UTC (permalink / raw) To: Thomas Huth, qemu-devel, Richard Henderson; +Cc: Alexander Graf, Aurelien Jarno On 25.05.2017 11:22, Thomas Huth wrote: > 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, it would be nice > to have a way to present more of the supported features when we are > running with the "qemu" CPU. > This patch now adds the supported features to the "full_feat" bitmap, > so that additional features can be enabled on the command line now, > for example with: > > qemu-system-s390x -cpu qemu,stfle=true,ldisp=true,eimm=true,stckf=true> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > v2: Mark feats array with "static const" > > target/s390x/cpu_models.c | 34 +++++++++++++++++++++++++++++++--- > 1 file changed, 31 insertions(+), 3 deletions(-) > > diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c > index 8d27363..e5e005a 100644 > --- a/target/s390x/cpu_models.c > +++ b/target/s390x/cpu_models.c > @@ -658,6 +658,30 @@ 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) > +{ > + static const int feats[] = { > + S390_FEAT_STFLE, > + S390_FEAT_EXTENDED_IMMEDIATE, > + S390_FEAT_LONG_DISPLACEMENT, > + S390_FEAT_LONG_DISPLACEMENT_FAST, > + S390_FEAT_STORE_CLOCK_FAST, > + S390_FEAT_GENERAL_INSTRUCTIONS_EXT, > + S390_FEAT_EXECUTE_EXT, > + S390_FEAT_STFLE_45, > + }; > + int i; > + > + 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 +694,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 optional 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 +950,14 @@ 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 optional 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.full_feat); > + cpu->model->def = &s390_qemu_cpu_defs; > bitmap_copy(cpu->model->features, cpu->model->def->default_feat, > S390_FEAT_MAX); That should work for the general case, at least for now. arch_query_cpu_model_baseline() will still drop the additional featues. Another option would be to directly bump up the CPU model to a z9, with missing base features (for now). Which looks cleaner in my eyes: >From 59c944c6fef87808e4456ba9565ace7772d236d7 Mon Sep 17 00:00:00 2001 From: David Hildenbrand <david@redhat.com> Date: Fri, 26 May 2017 13:54:27 +0200 Subject: [PATCH] s390x/cpu_models: Allow some additional feature bits for the "qemu" CPU Signed-off-by: David Hildenbrand <david@redhat.com> --- hw/s390x/s390-virtio-ccw.c | 13 +++++++++++ target/s390x/cpu.h | 2 ++ target/s390x/cpu_models.c | 55 +++++++++++++++++++++++++++++++++++++++------- 3 files changed, 62 insertions(+), 8 deletions(-) diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index fdd4384..3963a84 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -454,6 +454,19 @@ DEFINE_CCW_MACHINE(2_10, "2.10", true); static void ccw_machine_2_9_instance_options(MachineState *machine) { + static const int feats[] = { + S390_FEAT_GROUP_PLO, + S390_FEAT_ESAN3, + S390_FEAT_ZARCH, + }; + S390FeatBitmap qemu_features = {}; + int i; + + for (i = 0; i < ARRAY_SIZE(feats); i++) { + set_bit(feats[i], qemu_features); + } + s390_set_qemu_cpu_model_compat(0x2064, 7, 1, qemu_features); + ccw_machine_2_10_instance_options(machine); } diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index 240b8a5..8a11104 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -637,6 +637,8 @@ void s390_cpu_model_register_props(Object *obj); void s390_cpu_model_class_register_props(ObjectClass *oc); void s390_realize_cpu_model(CPUState *cs, Error **errp); ObjectClass *s390_cpu_class_by_name(const char *name); +void s390_set_qemu_cpu_model_compat(uint16_t type, uint8_t gen, uint8_t ec_ga, + S390FeatBitmap features); #define EXCP_EXT 1 /* external interrupt */ #define EXCP_SVC 2 /* supervisor call (syscall) */ diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index 8d27363..04643d7 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -658,6 +658,28 @@ static void check_compatibility(const S390CPUModel *max_model, "available in the configuration: "); } +static void get_max_qemu_cpu_model_features(S390FeatBitmap features) +{ + static const int feats[] = { + S390_FEAT_GROUP_PLO, + S390_FEAT_ESAN3, + S390_FEAT_ZARCH, + S390_FEAT_STFLE, + S390_FEAT_EXTENDED_IMMEDIATE, + S390_FEAT_LONG_DISPLACEMENT, + S390_FEAT_LONG_DISPLACEMENT_FAST, + S390_FEAT_STORE_CLOCK_FAST, + S390_FEAT_GENERAL_INSTRUCTIONS_EXT, + S390_FEAT_EXECUTE_EXT, + S390_FEAT_STFLE_45, + }; + int i; + + for (i = 0; i < ARRAY_SIZE(feats); i++) { + set_bit(feats[i], features); + } +} + static S390CPUModel *get_max_cpu_model(Error **errp) { static S390CPUModel max_model; @@ -670,10 +692,9 @@ 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 */ - max_model.def = &s390_cpu_defs[0]; - bitmap_copy(max_model.features, max_model.def->default_feat, - S390_FEAT_MAX); + /* QEMU emulates a z9 but with some missing base features */ + max_model.def = s390_find_cpu_def(0x2094, 9, 1, NULL); + get_max_qemu_cpu_model_features(max_model.features); } if (!*errp) { cached = true; @@ -923,15 +944,27 @@ static void s390_host_cpu_model_initfn(Object *obj) } #endif +static uint16_t qemu_type; +static uint8_t qemu_gen; +static uint8_t qemu_ec_ga; +static S390FeatBitmap qemu_features; + +void s390_set_qemu_cpu_model_compat(uint16_t type, uint8_t gen, uint8_t ec_ga, + S390FeatBitmap features) +{ + qemu_type = type; + qemu_gen = gen; + qemu_ec_ga = ec_ga; + bitmap_copy(qemu_features, features, S390_FEAT_MAX); +} + static void s390_qemu_cpu_model_initfn(Object *obj) { S390CPU *cpu = S390_CPU(obj); cpu->model = g_malloc0(sizeof(*cpu->model)); - /* TCG emulates a z900 */ - cpu->model->def = &s390_cpu_defs[0]; - bitmap_copy(cpu->model->features, cpu->model->def->default_feat, - S390_FEAT_MAX); + cpu->model->def = s390_find_cpu_def(qemu_type, qemu_gen, qemu_ec_ga, NULL); + bitmap_copy(cpu->model->features, qemu_features, S390_FEAT_MAX); } static void s390_cpu_model_finalize(Object *obj) @@ -1064,6 +1097,12 @@ static void register_types(void) s390_cpu_defs[i].full_feat); } + /* QEMU emulates a z9 but with some missing base features */ + qemu_type = 0x2094; + qemu_gen = 9; + qemu_ec_ga = 1; + get_max_qemu_cpu_model_features(qemu_features); + for (i = 0; i < ARRAY_SIZE(s390_cpu_defs); i++) { char *base_name = s390_base_cpu_type_name(s390_cpu_defs[i].name); TypeInfo ti_base = { -- 2.9.3 -- Thanks, David ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target/s390x/cpu_models: Allow some additional feature bits for the "qemu" CPU 2017-05-26 12:14 ` David Hildenbrand @ 2017-05-26 20:14 ` Thomas Huth 2017-05-27 14:42 ` David Hildenbrand 0 siblings, 1 reply; 5+ messages in thread From: Thomas Huth @ 2017-05-26 20:14 UTC (permalink / raw) To: David Hildenbrand, qemu-devel, Richard Henderson Cc: Alexander Graf, Aurelien Jarno On 26.05.2017 14:14, David Hildenbrand wrote: > On 25.05.2017 11:22, Thomas Huth wrote: >> 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, it would be nice >> to have a way to present more of the supported features when we are >> running with the "qemu" CPU. >> This patch now adds the supported features to the "full_feat" bitmap, >> so that additional features can be enabled on the command line now, >> for example with: >> >> qemu-system-s390x -cpu qemu,stfle=true,ldisp=true,eimm=true,stckf=true> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> v2: Mark feats array with "static const" >> >> target/s390x/cpu_models.c | 34 +++++++++++++++++++++++++++++++--- >> 1 file changed, 31 insertions(+), 3 deletions(-) >> >> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c >> index 8d27363..e5e005a 100644 >> --- a/target/s390x/cpu_models.c >> +++ b/target/s390x/cpu_models.c >> @@ -658,6 +658,30 @@ 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) >> +{ >> + static const int feats[] = { >> + S390_FEAT_STFLE, >> + S390_FEAT_EXTENDED_IMMEDIATE, >> + S390_FEAT_LONG_DISPLACEMENT, >> + S390_FEAT_LONG_DISPLACEMENT_FAST, >> + S390_FEAT_STORE_CLOCK_FAST, >> + S390_FEAT_GENERAL_INSTRUCTIONS_EXT, >> + S390_FEAT_EXECUTE_EXT, >> + S390_FEAT_STFLE_45, >> + }; >> + int i; >> + >> + 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 +694,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 optional 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 +950,14 @@ 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 optional 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.full_feat); >> + cpu->model->def = &s390_qemu_cpu_defs; >> bitmap_copy(cpu->model->features, cpu->model->def->default_feat, >> S390_FEAT_MAX); > > That should work for the general case, at least for now. > > arch_query_cpu_model_baseline() will still drop the additional featues. It's only for experimenting with the additional feature right now, not for serious usage yet (since some features are still missing to run Linux kernels that are compiled for something > z900) - so I don't think we need this for QMP already, i.e. IMHO we do not need care too much about arch_query_cpu_model_baseline() yet. > Another option would be to directly bump up the CPU model to a z9, with > missing base features (for now). Which looks cleaner in my eyes: But couldn't this cause issues, too? What if a kernel just assumes that certain features are available with a z9 machine, without having a closer look at the feature bits? Also, what about the features that we already provide, but that were not available in the z9 yet, like S390_FEAT_GENERAL_INSTRUCTIONS_EXT (which was introduced with the z10 as far as I can see)? Don't you have the same issue with arch_query_cpu_model_baseline() there? Thomas ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target/s390x/cpu_models: Allow some additional feature bits for the "qemu" CPU 2017-05-26 20:14 ` Thomas Huth @ 2017-05-27 14:42 ` David Hildenbrand 0 siblings, 0 replies; 5+ messages in thread From: David Hildenbrand @ 2017-05-27 14:42 UTC (permalink / raw) To: Thomas Huth, qemu-devel, Richard Henderson; +Cc: Alexander Graf, Aurelien Jarno On 26.05.2017 22:14, Thomas Huth wrote: > On 26.05.2017 14:14, David Hildenbrand wrote: >> On 25.05.2017 11:22, Thomas Huth wrote: >>> 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, it would be nice >>> to have a way to present more of the supported features when we are >>> running with the "qemu" CPU. >>> This patch now adds the supported features to the "full_feat" bitmap, >>> so that additional features can be enabled on the command line now, >>> for example with: >>> >>> qemu-system-s390x -cpu qemu,stfle=true,ldisp=true,eimm=true,stckf=true> >>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>> --- >>> v2: Mark feats array with "static const" >>> >>> target/s390x/cpu_models.c | 34 +++++++++++++++++++++++++++++++--- >>> 1 file changed, 31 insertions(+), 3 deletions(-) >>> >>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c >>> index 8d27363..e5e005a 100644 >>> --- a/target/s390x/cpu_models.c >>> +++ b/target/s390x/cpu_models.c >>> @@ -658,6 +658,30 @@ 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) >>> +{ >>> + static const int feats[] = { >>> + S390_FEAT_STFLE, >>> + S390_FEAT_EXTENDED_IMMEDIATE, >>> + S390_FEAT_LONG_DISPLACEMENT, >>> + S390_FEAT_LONG_DISPLACEMENT_FAST, >>> + S390_FEAT_STORE_CLOCK_FAST, >>> + S390_FEAT_GENERAL_INSTRUCTIONS_EXT, >>> + S390_FEAT_EXECUTE_EXT, >>> + S390_FEAT_STFLE_45, >>> + }; >>> + int i; >>> + >>> + 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 +694,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 optional 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 +950,14 @@ 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 optional 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.full_feat); >>> + cpu->model->def = &s390_qemu_cpu_defs; >>> bitmap_copy(cpu->model->features, cpu->model->def->default_feat, >>> S390_FEAT_MAX); >> >> That should work for the general case, at least for now. >> >> arch_query_cpu_model_baseline() will still drop the additional featues. > > It's only for experimenting with the additional feature right now, not > for serious usage yet (since some features are still missing to run > Linux kernels that are compiled for something > z900) - so I don't think > we need this for QMP already, i.e. IMHO we do not need care too much > about arch_query_cpu_model_baseline() yet. > Yes, for a short term solution this should be just fine. >> Another option would be to directly bump up the CPU model to a z9, with >> missing base features (for now). Which looks cleaner in my eyes: > > But couldn't this cause issues, too? What if a kernel just assumes that > certain features are available with a z9 machine, without having a > closer look at the feature bits? Also, what about the features that we A tcg guest currently only sees the feature part of CPU models. So e.g. CPUID is not correctly set. So it most likely makes sense to connect that properly to the CPU model, too, before we change the CPU generation. (otherwise we will have guest visible changes once we connect it - not the features will change but the CPU type/generation). > already provide, but that were not available in the z9 yet, like > S390_FEAT_GENERAL_INSTRUCTIONS_EXT (which was introduced with the z10 as > far as I can see)? Don't you have the same issue with > arch_query_cpu_model_baseline() there? Both point are true. In your approach, I don't like creation of fake, duplicate CPU definitions. But as this will be a short term "hack" to allow enabling these features, this should be fine for now. We should just keep in mind that once we want to enable these features permanently, that we should get rid of the hacked up CPU definition and implement proper migration compatibility support. So Acked-by: David Hildenbrand <david@redhat.com> Thanks! > > Thomas > -- Thanks, David ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-05-27 14:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-25 9:22 [Qemu-devel] [PATCH v2] target/s390x/cpu_models: Allow some additional feature bits for the "qemu" CPU Thomas Huth 2017-05-26 9:36 ` Aurelien Jarno 2017-05-26 12:14 ` David Hildenbrand 2017-05-26 20:14 ` Thomas Huth 2017-05-27 14:42 ` David Hildenbrand
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.