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

Enabling AP-passthrough(AP-pt) for PV-guest by 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.

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         |  2 +
 target/s390x/kvm/kvm.c              | 73 +++++++++++++++++++++++++++++
 5 files changed, 82 insertions(+)

On 8/10/23 14:47, Steffen Eiden wrote:
> Enabling AP-passthrough(AP-pt) for PV-guest by 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.
> 
> 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         |  2 +
>   target/s390x/kvm/kvm.c              | 73 +++++++++++++++++++++++++++++
>   5 files changed, 82 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,

You've named them UV features in the KVM patches.
None of this is "call" related.

Also you made a point of having the word "guest" in the KVM features

>   } 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..9a2c5a9dfc 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 interpretation for secure guests")

*INTERRUPTION* support

The AP instructions are nearly fully interpreted in secure guests for 
security reasons. The only time we see an AP instruction is for defining 
the notification byte for the interrupt support since we need to pin the 
page on which the byte resides.

Have a look what the feature for STFLE 65 says and either keep the INTR 
or use the suffix that's defined there

> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 42b52afdb4..a7900a8a5c 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..6ae10a2cd8 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,
>   };
>   
>   
> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index a7e2cdf668..500b9d894d 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -2307,6 +2307,42 @@ static bool ap_enabled(const S390FeatBitmap features)
>       return test_bit(S390_FEAT_AP, features);
>   }
>   
> +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()) {

Can this be made into an early return?

> +        if (prop.ap) {
> +            set_bit(S390_FEAT_UV_CALL_AP, features);
> +        }
> +        if (prop.ap_intr) {
> +            set_bit(S390_FEAT_UV_CALL_AP_INTR, features);
> +        }
> +    }
> +    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 },
> @@ -2501,11 +2537,41 @@ 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 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)) {

Same early return check please

> +        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 :
> @@ -2569,6 +2635,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)