[PATCH 0/3] KVM: s390: Enable AP instructions for pv-guests

[PATCH 0/3] KVM: s390: Enable AP instructions for pv-guests

[Steffen Eiden]

This series enables general QEMU support for AP pass-through for Secure
Execution guests (pv-guests).

To enable AP-PT on pv-guests QEMU has to turn on the corresponding bits
in the KVM CPU-model[1] if the CPU firmware supports it. However, it
only makes sense to turn on AP-PT if the QEMU user enabled (general) AP
for that guest. If AP is turned on (ap=on) QEMU also turns on AP-PT for
secure execution guests(appv=on) if the CPU supports it.

The series consists of three patches:
 1) update kvm-s390 header for this series
 2) small cleanup for kvm_s390_set_attr()
    refactor code to add ap_available()
 3) Add UV_CALL CPU model enablement

There is **one** problem with the current implementation:
If the user does not enable AP in the cpu model QEMU produces the
following warning:
```
qemu-system-s390x: warning: 'appv' requires 'ap'.
```

During `check_consistency()` the model has appv=on and ap=offi, hence
the warning. However, appv is not turned on during the model
realization, as the code checks for AP in beforehand.

appv is in the default z16 model so that it is automatically enabled if
ap=on was specified.

I did not find a concept of dynamic defaults to model this behavior
(ap=on -> appv=on -> appvi=on). So I hope someone on the list can help
me and give some pointers on how to implement that.


Steffen


Steffen Eiden (3):
  linux-headers: update asm-s390/kvm.h
  target/s390x: refractor AP functionalities
  target/s390x: AP-passthrough for PV guests

 linux-headers/asm-s390/kvm.h        | 21 +++++++
 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              | 91 ++++++++++++++++++++++++++---
 6 files changed, 116 insertions(+), 7 deletions(-)

-- 
2.40.1

[PATCH 1/3] linux-headers: update asm-s390/kvm.h

[Steffen Eiden]

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
 linux-headers/asm-s390/kvm.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index e2afd95420..178edb959e 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -159,6 +159,27 @@ struct kvm_s390_vm_cpu_subfunc {
 	__u8 reserved[1728];
 };
 
+#define KVM_S390_VM_CPU_PROCESSOR_UV_FEAT_GUEST	6
+#define KVM_S390_VM_CPU_MACHINE_UV_FEAT_GUEST	7
+
+#define KVM_S390_VM_CPU_UV_FEAT_NR_BITS	64
+struct kvm_s390_vm_cpu_uv_feat {
+	union {
+		struct {
+			__u64 res0 : 4;
+			__u64 ap : 1;		/* bit 4 */
+			__u64 ap_intr : 1;	/* bit 5 */
+		};
+		__u64 feat;
+	};
+};
+
+#define KVM_S390_VM_CPU_UV_FEAT_GUEST_MASK \
+(((struct kvm_s390_vm_cpu_uv_feat) {	\
+	.ap = 1,		\
+	.ap_intr = 1,		\
+}).feat)
+
 /* kvm attributes for crypto */
 #define KVM_S390_VM_CRYPTO_ENABLE_AES_KW	0
 #define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW	1
-- 
2.40.1

[PATCH 2/3] target/s390x: refractor AP functionalities

[Steffen Eiden]

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
 target/s390x/kvm/kvm.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index 3ac7ec9acf..bd62a7f606 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -250,7 +250,7 @@ static void kvm_s390_enable_cmma(void)
     trace_kvm_enable_cmma(rc);
 }
 
-static void kvm_s390_set_attr(uint64_t attr)
+static void kvm_s390_set_crypto_attr(uint64_t attr)
 {
     struct kvm_device_attr attribute = {
         .group = KVM_S390_VM_CRYPTO,
@@ -275,7 +275,7 @@ static void kvm_s390_init_aes_kw(void)
     }
 
     if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {
-            kvm_s390_set_attr(attr);
+            kvm_s390_set_crypto_attr(attr);
     }
 }
 
@@ -289,7 +289,7 @@ static void kvm_s390_init_dea_kw(void)
     }
 
     if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {
-            kvm_s390_set_attr(attr);
+            kvm_s390_set_crypto_attr(attr);
     }
 }
 
@@ -2296,6 +2296,11 @@ static int configure_cpu_subfunc(const S390FeatBitmap features)
     return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
 }
 
+static bool ap_available(void)
+{
+    return kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, KVM_S390_VM_CRYPTO_ENABLE_APIE);
+}
+
 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 },
@@ -2475,8 +2480,7 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
         return;
     }
     /* for now, we can only provide the AP feature with HW support */
-    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO,
-        KVM_S390_VM_CRYPTO_ENABLE_APIE)) {
+    if (ap_available()) {
         set_bit(S390_FEAT_AP, model->features);
     }
 
@@ -2502,7 +2506,7 @@ static void kvm_s390_configure_apie(bool interpret)
                                 KVM_S390_VM_CRYPTO_DISABLE_APIE;
 
     if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {
-        kvm_s390_set_attr(attr);
+        kvm_s390_set_crypto_attr(attr);
     }
 }
 
@@ -2556,7 +2560,7 @@ void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
         kvm_s390_enable_cmma();
     }
 
-    if (test_bit(S390_FEAT_AP, model->features)) {
+    if (ap_enabled(model->features)) {
         kvm_s390_configure_apie(true);
     }
 }
-- 
2.40.1

Re: [PATCH 2/3] target/s390x: refractor AP functionalities

[Thomas Huth]

In the subject: s/refractor/refactor/

Please also add a short patch description why you are doing these changes.

> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
>   target/s390x/kvm/kvm.c | 18 +++++++++++-------
>   1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index 3ac7ec9acf..bd62a7f606 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -250,7 +250,7 @@ static void kvm_s390_enable_cmma(void)
>       trace_kvm_enable_cmma(rc);
>   }
>   
> -static void kvm_s390_set_attr(uint64_t attr)
> +static void kvm_s390_set_crypto_attr(uint64_t attr)
>   {
>       struct kvm_device_attr attribute = {
>           .group = KVM_S390_VM_CRYPTO,
> @@ -275,7 +275,7 @@ static void kvm_s390_init_aes_kw(void)
>       }
>   
>       if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {
> -            kvm_s390_set_attr(attr);
> +            kvm_s390_set_crypto_attr(attr);
>       }
>   }
>   
> @@ -289,7 +289,7 @@ static void kvm_s390_init_dea_kw(void)
>       }
>   
>       if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {
> -            kvm_s390_set_attr(attr);
> +            kvm_s390_set_crypto_attr(attr);
>       }
>   }

I'd maybe move the renaming into a separate patch.

> @@ -2296,6 +2296,11 @@ static int configure_cpu_subfunc(const S390FeatBitmap features)
>       return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
>   }
>   
> +static bool ap_available(void)
> +{
> +    return kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, KVM_S390_VM_CRYPTO_ENABLE_APIE);

The line is already quite long ... maybe put it on two lines to avoid 
checkpatch.pl warnings.

> +}
> +
>   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 },
> @@ -2475,8 +2480,7 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
>           return;
>       }
>       /* for now, we can only provide the AP feature with HW support */
> -    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO,
> -        KVM_S390_VM_CRYPTO_ENABLE_APIE)) {
> +    if (ap_available()) {
>           set_bit(S390_FEAT_AP, model->features);
>       }
>   
> @@ -2502,7 +2506,7 @@ static void kvm_s390_configure_apie(bool interpret)
>                                   KVM_S390_VM_CRYPTO_DISABLE_APIE;
>   
>       if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {
> -        kvm_s390_set_attr(attr);
> +        kvm_s390_set_crypto_attr(attr);
>       }
>   }
>   
> @@ -2556,7 +2560,7 @@ void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
>           kvm_s390_enable_cmma();
>       }
>   
> -    if (test_bit(S390_FEAT_AP, model->features)) {
> +    if (ap_enabled(model->features)) {

You only introduce ap_enabled() in the next patch ... please move it here to 
avoid breaking bisection later.

  Thomas


>           kvm_s390_configure_apie(true);
>       }
>   }

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

[Steffen Eiden]

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(+)

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,
 };
 
 /* 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);
+    }
+    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)
-- 
2.40.1

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

[Thomas Huth]

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

Re: [PATCH 0/3] KVM: s390: Enable AP instructions for pv-guests

[Steffen Eiden]

On 7/27/23 14:25, Steffen Eiden wrote:
> This series enables general QEMU support for AP pass-through for Secure
> Execution guests (pv-guests).
> 
> To enable AP-PT on pv-guests QEMU has to turn on the corresponding bits
> in the KVM CPU-model[1] if the CPU firmware supports it. However, it
> only makes sense to turn on AP-PT if the QEMU user enabled (general) AP
> for that guest. If AP is turned on (ap=on) QEMU also turns on AP-PT for
> secure execution guests(appv=on) if the CPU supports it.
> 
> The series consists of three patches:
>   1) update kvm-s390 header for this series
>   2) small cleanup for kvm_s390_set_attr()
>      refactor code to add ap_available()
>   3) Add UV_CALL CPU model enablement
> 
> There is **one** problem with the current implementation:
> If the user does not enable AP in the cpu model QEMU produces the
> following warning:
> ```
> qemu-system-s390x: warning: 'appv' requires 'ap'.
> ```
> 
> During `check_consistency()` the model has appv=on and ap=offi, hence
> the warning. However, appv is not turned on during the model
> realization, as the code checks for AP in beforehand.
> 
> appv is in the default z16 model so that it is automatically enabled if
> ap=on was specified.
> 
> I did not find a concept of dynamic defaults to model this behavior
> (ap=on -> appv=on -> appvi=on). So I hope someone on the list can help
> me and give some pointers on how to implement that.
> 
> 
> Steffen

[1] v2 KVM series:
https://lore.kernel.org/linux-s390/20230728092341.1131787-1-seiden@linux.ibm.com/T/#t

> 
> Steffen Eiden (3):
>    linux-headers: update asm-s390/kvm.h
>    target/s390x: refractor AP functionalities
>    target/s390x: AP-passthrough for PV guests
> 
>   linux-headers/asm-s390/kvm.h        | 21 +++++++
>   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              | 91 ++++++++++++++++++++++++++---
>   6 files changed, 116 insertions(+), 7 deletions(-)
>