Re: [Qemu-devel] [RFC PATCH] target/s390x/cpu_models: Set some additional feature bits for the "qemu" CPU

[Aurelien Jarno]

On 2017-05-17 17:35, 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, 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 :-).

> Signed-off-by: Thomas Huth <address@hidden>
> ---
>  target/s390x/cpu_models.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 8d27363..18789ab 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -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

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

> +    };
> +
> +    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);
>  }

Otherwise it looks fine to me.

Reviewed-by: Aurelien Jarno <address@hidden>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net