Re: [Qemu-devel] [PATCH/s390-next 2/3] s390x/cpumodel: add zpci, aen and ais facilities

[Cornelia Huck]

On Thu, 13 Jul 2017 12:40:28 +0200
Christian Borntraeger <address@hidden> wrote:

> From: Yi Min Zhao <address@hidden>
> 
> zPCI instructions and facilities are available since IBM zEnterprise
> EC12. To support z/PCI in QEMU we enable zpci, aen and ais facilities
> starting with zEC12 GA1. And we always set zpci and aen bits in max cpu
> model. Later they might be switched off due to applied real cpu model.
> For ais bit, we only provide it in the full cpu model beginning with
> zEC12 and defer its enablement in the default cpu model to a later point
> in time. At the same time, disable them for 2.9 and older machines.
> 
> Because of introducing AIS facility, we could check if it's enabled to
> initialize flic->ais_supported with the real value.
> 
> Signed-off-by: Yi Min Zhao <address@hidden>
> Signed-off-by: Christian Borntraeger <address@hidden>
> ---
>  hw/intc/s390_flic.c             | 3 ++-
>  hw/intc/s390_flic_kvm.c         | 3 ---
>  hw/s390x/s390-virtio-ccw.c      | 3 +++
>  target/s390x/cpu_features.c     | 3 +++
>  target/s390x/cpu_features_def.h | 3 +++
>  target/s390x/gen-features.c     | 5 +++++
>  target/s390x/kvm.c              | 7 +++++++
>  7 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
> index ff6e4ec..6e7c610 100644
> --- a/hw/intc/s390_flic.c
> +++ b/hw/intc/s390_flic.c
> @@ -163,9 +163,10 @@ static void s390_flic_common_realize(DeviceState *dev, 
> Error **errp)
>      if (max_batch > ADAPTER_ROUTES_MAX_GSI) {
>          error_setg(errp, "flic property adapter_routes_max_batch too big"
>                     " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI);
> +        return;

Hunk should go into a previous patch?

(And does it really matter?)

>      }
>  
> -    fs->ais_supported = true;
> +    fs->ais_supported = s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION);
>  }
>  
>  static void s390_flic_class_init(ObjectClass *oc, void *data)

> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 78ebe83..1901153 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -302,6 +302,9 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>          }
>      }
>  
> +    /* Try to enable AIS facility */
> +    kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0);

What happens if you fail to enable it? You probably don't want to allow
the feature bit, then?

> +
>      qemu_mutex_init(&qemu_sigp_mutex);
>  
>      return 0;

Let's summarize to make sure that I'm not confused:

- Starting with zEC12 GA1, we provide zpci, aen, ais in the full model
- Starting with zEC12 GA1, we provide zpci and aen in the default model
- In the host model, we add zpci and aen; they might be switched off
  after applying the found model
- Compat for 2.9 and earlier switches off zpci, aen, ais
- We unconditionally enable the kvm part of ais

I'm still not sure what's supposed to happen with new qemu + old kernel
(no ais) + full zEC12 GA1 or later model.