[3/3] target/s390x: AP-passthrough for PV guests

Enabling AP-passthrough(AP-pt) for PV-guest via using the new CPU
features for PV-AP-pt of kvm.

As usual QEMU first checks which CPU features are available and then
sets them if available and selected by user. An additional check is done
to verify that PV-AP can only be enabled if "regular" AP-pt is enabled
as well. Note that KVM itself does not enforce this restriction.

If regular AP-pt is enabled and kvm/firmware supports PV-AP-pt it is
also turned on by default.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
 target/s390x/cpu_features.h         |  1 +
 target/s390x/cpu_features_def.h.inc |  4 ++
 target/s390x/cpu_models.c           |  2 +
 target/s390x/gen-features.c         |  4 ++
 target/s390x/kvm/kvm.c              | 73 +++++++++++++++++++++++++++++
 5 files changed, 84 insertions(+)

On 27/07/2023 14.25, Steffen Eiden wrote:
> Enabling AP-passthrough(AP-pt) for PV-guest via using the new CPU

Either "via the new" or "by using the new", but "via using" sounds weird to me.

> features for PV-AP-pt of kvm.
> 
> As usual QEMU first checks which CPU features are available and then
> sets them if available and selected by user. An additional check is done
> to verify that PV-AP can only be enabled if "regular" AP-pt is enabled
> as well. Note that KVM itself does not enforce this restriction.
> 
> If regular AP-pt is enabled and kvm/firmware supports PV-AP-pt it is
> also turned on by default.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
>   target/s390x/cpu_features.h         |  1 +
>   target/s390x/cpu_features_def.h.inc |  4 ++
>   target/s390x/cpu_models.c           |  2 +
>   target/s390x/gen-features.c         |  4 ++
>   target/s390x/kvm/kvm.c              | 73 +++++++++++++++++++++++++++++
>   5 files changed, 84 insertions(+)
> 
> diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
> index 87463f064d..40928c60e9 100644
> --- a/target/s390x/cpu_features.h
> +++ b/target/s390x/cpu_features.h
> @@ -43,6 +43,7 @@ typedef enum {
>       S390_FEAT_TYPE_KDSA,
>       S390_FEAT_TYPE_SORTL,
>       S390_FEAT_TYPE_DFLTCC,
> +    S390_FEAT_TYPE_UV_CALL,
>   } S390FeatType;
>   
>   /* Definition of a CPU feature */
> diff --git a/target/s390x/cpu_features_def.h.inc b/target/s390x/cpu_features_def.h.inc
> index e3cfe63735..4b659d4064 100644
> --- a/target/s390x/cpu_features_def.h.inc
> +++ b/target/s390x/cpu_features_def.h.inc
> @@ -379,3 +379,7 @@ DEF_FEAT(DEFLATE_GHDT, "dfltcc-gdht", DFLTCC, 1, "DFLTCC GDHT")
>   DEF_FEAT(DEFLATE_CMPR, "dfltcc-cmpr", DFLTCC, 2, "DFLTCC CMPR")
>   DEF_FEAT(DEFLATE_XPND, "dfltcc-xpnd", DFLTCC, 4, "DFLTCC XPND")
>   DEF_FEAT(DEFLATE_F0, "dfltcc-f0", DFLTCC, 192, "DFLTCC format 0 parameter-block")
> +
> +/* Features exposed via the UV-CALL instruction */
> +DEF_FEAT(UV_CALL_AP, "appv", UV_CALL, 4, "AP instructions installed for secure guests")
> +DEF_FEAT(UV_CALL_AP_INTR, "appvi", UV_CALL, 5, "AP instructions interpretion for secure guests")
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index ae8880e81d..6d703b3c55 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -483,6 +483,8 @@ static void check_consistency(const S390CPUModel *model)
>           { S390_FEAT_DIAG_318, S390_FEAT_EXTENDED_LENGTH_SCCB },
>           { S390_FEAT_NNPA, S390_FEAT_VECTOR },
>           { S390_FEAT_RDP, S390_FEAT_LOCAL_TLB_CLEARING },
> +        { S390_FEAT_UV_CALL_AP, S390_FEAT_AP },
> +        { S390_FEAT_UV_CALL_AP_INTR, S390_FEAT_UV_CALL_AP },
>       };
>       int i;
>   
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 1e3b7c0dc9..0220864d89 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -576,6 +576,8 @@ static uint16_t full_GEN16_GA1[] = {
>       S390_FEAT_RDP,
>       S390_FEAT_PAI,
>       S390_FEAT_PAIE,
> +    S390_FEAT_UV_CALL_AP,
> +    S390_FEAT_UV_CALL_AP_INTR,
>   };
>   
>   
> @@ -671,6 +673,8 @@ static uint16_t default_GEN16_GA1[] = {
>       S390_FEAT_RDP,
>       S390_FEAT_PAI,
>       S390_FEAT_PAIE,
> +    S390_FEAT_UV_CALL_AP,
> +    S390_FEAT_UV_CALL_AP_INTR,
>   };

If you want to change the default model, you need to disable the bits again 
in older machine types, otherwise we'll run into migration issues later. See 
for example commit 1d41de5f0524b00a3e for how to do this correctly.

You might also want to pull in
  https://lore.kernel.org/qemu-devel/20230718142235.135319-1-cohuck@redhat.com/
to avoid contextual conflicts later.

>   /* QEMU (CPU model) features */
> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index bd62a7f606..cf11bfb0fa 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -2301,6 +2301,39 @@ static bool ap_available(void)
>       return kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, KVM_S390_VM_CRYPTO_ENABLE_APIE);
>   }
>   
> +static bool uv_feat_supported(void)
> +{
> +    return kvm_vm_check_attr(
> +            kvm_state, KVM_S390_VM_CPU_MODEL, KVM_S390_VM_CPU_PROCESSOR_UV_FEAT_GUEST);
> +}
> +
> +static int query_uv_feat_guest(S390FeatBitmap features)
> +{
> +    struct kvm_s390_vm_cpu_uv_feat prop = {};
> +    struct kvm_device_attr attr = {
> +        .group = KVM_S390_VM_CPU_MODEL,
> +        .attr = KVM_S390_VM_CPU_MACHINE_UV_FEAT_GUEST,
> +        .addr = (uint64_t) &prop,
> +    };
> +    int rc;
> +
> +    if (!uv_feat_supported())
> +        return 0;
> +
> +    rc = kvm_vm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
> +    if (rc) {
> +        return  rc;
> +    }
> +
> +    if (ap_available()) {
> +        if (prop.ap)
> +            set_bit(S390_FEAT_UV_CALL_AP, features);
> +        if (prop.ap_intr)
> +            set_bit(S390_FEAT_UV_CALL_AP_INTR, features);

Missing curly braces. Please check your patches with "scripts/checkpatch.pl".

> +    }
> +    return 0;
> +}
> +
>   static int kvm_to_feat[][2] = {
>       { KVM_S390_VM_CPU_FEAT_ESOP, S390_FEAT_ESOP },
>       { KVM_S390_VM_CPU_FEAT_SIEF2, S390_FEAT_SIE_F2 },
> @@ -2495,11 +2528,44 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
>           set_bit(S390_FEAT_DIAG_318, model->features);
>       }
>   
> +    /* Test for Ultravisor features that influence secure guest behavior */
> +    query_uv_feat_guest(model->features);
> +
>       /* strip of features that are not part of the maximum model */
>       bitmap_and(model->features, model->features, model->def->full_feat,
>                  S390_FEAT_MAX);
>   }
>   
> +static bool ap_enabled(const S390FeatBitmap features)
> +{
> +    return test_bit(S390_FEAT_AP, features);
> +}
> +
> +static int configure_uv_feat_guest(const S390FeatBitmap features, bool interpret)
> +{
> +
> +    struct kvm_s390_vm_cpu_uv_feat uv_feat = {};
> +    struct kvm_device_attr attribute = {
> +        .group = KVM_S390_VM_CPU_MODEL,
> +        .attr = KVM_S390_VM_CPU_PROCESSOR_UV_FEAT_GUEST,
> +        .addr = (__u64)&uv_feat,
> +    };
> +
> +    if (!uv_feat_supported())
> +        return 0;
> +
> +    if (ap_enabled(features)) {
> +        if (test_bit(S390_FEAT_UV_CALL_AP, features)) {
> +            uv_feat.ap = 1;
> +        }
> +        if (test_bit(S390_FEAT_UV_CALL_AP_INTR, features) && interpret) {
> +            uv_feat.ap_intr = 1;
> +        }
> +    }
> +
> +    return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attribute);
> +}
> +
>   static void kvm_s390_configure_apie(bool interpret)
>   {
>       uint64_t attr = interpret ? KVM_S390_VM_CRYPTO_ENABLE_APIE :
> @@ -2563,6 +2629,13 @@ void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
>       if (ap_enabled(model->features)) {
>           kvm_s390_configure_apie(true);
>       }
> +
> +    /* configure uv features for the guest indicated via query / test_bit*/
> +    rc = configure_uv_feat_guest(model->features, true);
> +    if (rc) {
> +        error_setg(errp, "KVM: Error configuring CPU UV features %d", rc);
> +        return;
> +    }
>   }
>   
>   void kvm_s390_restart_interrupt(S390CPU *cpu)

  Thomas