[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 KVM support for AP pass-through for Secure
Execution guests (pv-guests).

To enable AP inside pv-guests two things have to be done/considered:
	1) set corresponding flags in the Create Secure Configuration UVC ifi
     firmware supports AP for pv-guests (patch 3).
	2) enable/disable AP in pv-guests if the VMM wants this (patch 2).

Steffen

Steffen Eiden (3):
  s390: uv: UV feature check utility
  KVM: s390: Add UV feature negotiation
  KVM: s390: pv:  Allow AP-instructions for pv guests

 arch/s390/include/asm/kvm_host.h |  2 +
 arch/s390/include/asm/uv.h       | 17 +++++++-
 arch/s390/include/uapi/asm/kvm.h | 24 +++++++++++
 arch/s390/kernel/uv.c            |  2 +-
 arch/s390/kvm/kvm-s390.c         | 72 +++++++++++++++++++++++++++++++-
 arch/s390/kvm/pv.c               |  8 +++-
 arch/s390/mm/fault.c             |  2 +-
 7 files changed, 121 insertions(+), 6 deletions(-)

-- 
2.40.1

[PATCH 1/3] s390: uv: UV feature check utility

[Steffen Eiden]

Introduces a function to check the existence of an UV feature.
Refactor feature bit checks to use the new function.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
 arch/s390/include/asm/uv.h | 5 +++++
 arch/s390/kernel/uv.c      | 2 +-
 arch/s390/kvm/kvm-s390.c   | 2 +-
 arch/s390/mm/fault.c       | 2 +-
 4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index d6bb2f4f78d1..338845402324 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -397,6 +397,11 @@ struct uv_info {
 
 extern struct uv_info uv_info;
 
+static inline bool uv_has_feature(u8 feature_bit)
+{
+	return test_bit_inv(feature_bit, &uv_info.uv_feature_indications);
+}
+
 #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
 extern int prot_virt_guest;
 
diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
index d8b25cda5471..c82a667d2113 100644
--- a/arch/s390/kernel/uv.c
+++ b/arch/s390/kernel/uv.c
@@ -249,7 +249,7 @@ static bool should_export_before_import(struct uv_cb_header *uvcb, struct mm_str
 	 * shared page from a different protected VM will automatically also
 	 * transfer its ownership.
 	 */
-	if (test_bit_inv(BIT_UV_FEAT_MISC, &uv_info.uv_feature_indications))
+	if (uv_has_feature(BIT_UV_FEAT_MISC))
 		return false;
 	if (uvcb->cmd == UVC_CMD_UNPIN_PAGE_SHARED)
 		return false;
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 17b81659cdb2..339e3190f6dc 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2402,7 +2402,7 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc)
 	struct kvm_vcpu *vcpu;
 
 	/* Disable the GISA if the ultravisor does not support AIV. */
-	if (!test_bit_inv(BIT_UV_FEAT_AIV, &uv_info.uv_feature_indications))
+	if (!uv_has_feature(BIT_UV_FEAT_AIV))
 		kvm_s390_gisa_disable(kvm);
 
 	kvm_for_each_vcpu(i, vcpu, kvm) {
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index b65144c392b0..b8ef42fac2c4 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -824,7 +824,7 @@ void do_secure_storage_access(struct pt_regs *regs)
 	 * reliable without the misc UV feature so we need to check
 	 * for that as well.
 	 */
-	if (test_bit_inv(BIT_UV_FEAT_MISC, &uv_info.uv_feature_indications) &&
+	if (uv_has_feature(BIT_UV_FEAT_MISC) &&
 	    !test_bit_inv(61, &regs->int_parm_long)) {
 		/*
 		 * When this happens, userspace did something that it
-- 
2.40.1

[PATCH 2/3] KVM: s390: Add UV feature negotiation

[Steffen Eiden]

Add a uv_feature list for pv-guests to the kvm cpu-model.
The feature bits 'AP-interpretation for secure guests' and
'AP-interrupt for secure guests' are available.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |  2 +
 arch/s390/include/uapi/asm/kvm.h | 24 +++++++++++
 arch/s390/kvm/kvm-s390.c         | 70 ++++++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 2bbc3d54959d..7ae57ac352b2 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -817,6 +817,8 @@ struct kvm_s390_cpu_model {
 	__u64 *fac_list;
 	u64 cpuid;
 	unsigned short ibc;
+	/* subset of available uv features for pv-guests enabled by user space */
+	struct kvm_s390_vm_cpu_uv_feat uv_feat_guest;
 };
 
 typedef int (*crypto_hook)(struct kvm_vcpu *vcpu);
diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
index a73cf01a1606..7ea7e4fbd37e 100644
--- a/arch/s390/include/uapi/asm/kvm.h
+++ b/arch/s390/include/uapi/asm/kvm.h
@@ -159,6 +159,30 @@ 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)
+
+#define KVM_S390_VM_CPU_UV_FEAT_GUEST_DEFAULT \
+((struct kvm_s390_vm_cpu_uv_feat) { })
+
 /* kvm attributes for crypto */
 #define KVM_S390_VM_CRYPTO_ENABLE_AES_KW	0
 #define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW	1
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 339e3190f6dc..50c8b85b89ac 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1531,6 +1531,32 @@ static int kvm_s390_set_processor_subfunc(struct kvm *kvm,
 	return 0;
 }
 
+static int kvm_s390_set_uv_feat(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+	struct kvm_s390_vm_cpu_uv_feat data = {};
+	unsigned long filter =
+		uv_info.uv_feature_indications &
+		(unsigned long)KVM_S390_VM_CPU_UV_FEAT_GUEST_MASK;
+
+	if (copy_from_user(&data, (void __user *)attr->addr, sizeof(data)))
+		return -EFAULT;
+	if (!bitmap_subset((unsigned long *)&data, &filter,
+			   KVM_S390_VM_CPU_UV_FEAT_NR_BITS))
+		return -EINVAL;
+
+	mutex_lock(&kvm->lock);
+	if (kvm->created_vcpus) {
+		mutex_unlock(&kvm->lock);
+		return -EBUSY;
+	}
+	kvm->arch.model.uv_feat_guest = data;
+	mutex_unlock(&kvm->lock);
+
+	VM_EVENT(kvm, 3, "SET: guest uv feat: 0x%16.16llx", data.feat);
+
+	return 0;
+}
+
 static int kvm_s390_set_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr)
 {
 	int ret = -ENXIO;
@@ -1545,6 +1571,9 @@ static int kvm_s390_set_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr)
 	case KVM_S390_VM_CPU_PROCESSOR_SUBFUNC:
 		ret = kvm_s390_set_processor_subfunc(kvm, attr);
 		break;
+	case KVM_S390_VM_CPU_PROCESSOR_UV_FEAT_GUEST:
+		ret = kvm_s390_set_uv_feat(kvm, attr);
+		break;
 	}
 	return ret;
 }
@@ -1777,6 +1806,37 @@ static int kvm_s390_get_machine_subfunc(struct kvm *kvm,
 	return 0;
 }
 
+static int kvm_s390_get_processor_uv_feat(struct kvm *kvm,
+					  struct kvm_device_attr *attr)
+{
+	struct kvm_s390_vm_cpu_uv_feat *src = &kvm->arch.model.uv_feat_guest;
+
+	if (copy_to_user((void __user *)attr->addr, src, sizeof(*src)))
+		return -EFAULT;
+	VM_EVENT(kvm, 3, "GET: guest  uv feat: 0x%16.16llx",
+		 kvm->arch.model.uv_feat_guest.feat);
+
+	return 0;
+}
+
+static int kvm_s390_get_machine_uv_feat(struct kvm *kvm,
+					struct kvm_device_attr *attr)
+{
+	struct kvm_s390_vm_cpu_uv_feat data = {
+		.feat = uv_info.uv_feature_indications &
+			KVM_S390_VM_CPU_UV_FEAT_GUEST_MASK
+	};
+
+	BUILD_BUG_ON(sizeof(struct kvm_s390_vm_cpu_uv_feat) !=
+		     sizeof(uv_info.uv_feature_indications));
+	if (copy_to_user((void __user *)attr->addr, &data,
+			 sizeof(struct kvm_s390_vm_cpu_uv_feat)))
+		return -EFAULT;
+	VM_EVENT(kvm, 3, "GET: guest  uv feat: 0x%16.16llx", data.feat);
+
+	return 0;
+}
+
 static int kvm_s390_get_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr)
 {
 	int ret = -ENXIO;
@@ -1800,6 +1860,12 @@ static int kvm_s390_get_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr)
 	case KVM_S390_VM_CPU_MACHINE_SUBFUNC:
 		ret = kvm_s390_get_machine_subfunc(kvm, attr);
 		break;
+	case KVM_S390_VM_CPU_PROCESSOR_UV_FEAT_GUEST:
+		ret = kvm_s390_get_processor_uv_feat(kvm, attr);
+		break;
+	case KVM_S390_VM_CPU_MACHINE_UV_FEAT_GUEST:
+		ret = kvm_s390_get_machine_uv_feat(kvm, attr);
+		break;
 	}
 	return ret;
 }
@@ -1952,6 +2018,8 @@ static int kvm_s390_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
 		case KVM_S390_VM_CPU_MACHINE_FEAT:
 		case KVM_S390_VM_CPU_MACHINE_SUBFUNC:
 		case KVM_S390_VM_CPU_PROCESSOR_SUBFUNC:
+		case KVM_S390_VM_CPU_MACHINE_UV_FEAT_GUEST:
+		case KVM_S390_VM_CPU_PROCESSOR_UV_FEAT_GUEST:
 			ret = 0;
 			break;
 		default:
@@ -3292,6 +3360,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	kvm->arch.model.cpuid = kvm_s390_get_initial_cpuid();
 	kvm->arch.model.ibc = sclp.ibc & 0x0fff;
 
+	kvm->arch.model.uv_feat_guest = KVM_S390_VM_CPU_UV_FEAT_GUEST_DEFAULT;
+
 	kvm_s390_crypto_init(kvm);
 
 	if (IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM)) {
-- 
2.40.1

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

[Steffen Eiden]

Introduces new feature bits and enablement flags for AP and AP IRQ
support.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
 arch/s390/include/asm/uv.h | 12 +++++++++++-
 arch/s390/kvm/pv.c         |  8 ++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index 338845402324..913ccfaa9d76 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -99,6 +99,8 @@ enum uv_cmds_inst {
 enum uv_feat_ind {
 	BIT_UV_FEAT_MISC = 0,
 	BIT_UV_FEAT_AIV = 1,
+	BIT_UV_FEAT_AP = 4,
+	BIT_UV_FEAT_AP_INTR = 5,
 };
 
 struct uv_cb_header {
@@ -159,7 +161,15 @@ struct uv_cb_cgc {
 	u64 guest_handle;
 	u64 conf_base_stor_origin;
 	u64 conf_virt_stor_origin;
-	u64 reserved30;
+	u8  reserved30[6];
+	union {
+		struct {
+			u16 reserved : 14;
+			u16 ap_instr_intr : 1;
+			u16 ap_allow_instr : 1;
+		};
+		u16 raw;
+	} flags;
 	u64 guest_stor_origin;
 	u64 guest_stor_len;
 	u64 guest_sca;
diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
index 899f3b8ac011..103add1894c9 100644
--- a/arch/s390/kvm/pv.c
+++ b/arch/s390/kvm/pv.c
@@ -561,12 +561,16 @@ int kvm_s390_pv_init_vm(struct kvm *kvm, u16 *rc, u16 *rrc)
 	uvcb.conf_base_stor_origin =
 		virt_to_phys((void *)kvm->arch.pv.stor_base);
 	uvcb.conf_virt_stor_origin = (u64)kvm->arch.pv.stor_var;
+	uvcb.flags.ap_allow_instr = uv_has_feature(BIT_UV_FEAT_AP) &&
+				    kvm->arch.model.uv_feat_guest.ap;
+	uvcb.flags.ap_instr_intr = uv_has_feature(BIT_UV_FEAT_AP_INTR) &&
+				   kvm->arch.model.uv_feat_guest.ap_intr;
 
 	cc = uv_call_sched(0, (u64)&uvcb);
 	*rc = uvcb.header.rc;
 	*rrc = uvcb.header.rrc;
-	KVM_UV_EVENT(kvm, 3, "PROTVIRT CREATE VM: handle %llx len %llx rc %x rrc %x",
-		     uvcb.guest_handle, uvcb.guest_stor_len, *rc, *rrc);
+	KVM_UV_EVENT(kvm, 3, "PROTVIRT CREATE VM: handle %llx len %llx rc %x rrc %x flags %04x",
+		     uvcb.guest_handle, uvcb.guest_stor_len, *rc, *rrc, uvcb.flags.raw);
 
 	/* Outputs */
 	kvm->arch.pv.handle = uvcb.guest_handle;
-- 
2.40.1

Re: [PATCH 1/3] s390: uv: UV feature check utility

[Claudio Imbrenda]

On Thu, 27 Jul 2023 14:20:51 +0200
Steffen Eiden <seiden@linux.ibm.com> wrote:

> Introduces a function to check the existence of an UV feature.
> Refactor feature bit checks to use the new function.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  arch/s390/include/asm/uv.h | 5 +++++
>  arch/s390/kernel/uv.c      | 2 +-
>  arch/s390/kvm/kvm-s390.c   | 2 +-
>  arch/s390/mm/fault.c       | 2 +-
>  4 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
> index d6bb2f4f78d1..338845402324 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -397,6 +397,11 @@ struct uv_info {
>  
>  extern struct uv_info uv_info;
>  
> +static inline bool uv_has_feature(u8 feature_bit)
> +{
> +	return test_bit_inv(feature_bit, &uv_info.uv_feature_indications);
> +}
> +
>  #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
>  extern int prot_virt_guest;
>  
> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index d8b25cda5471..c82a667d2113 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
> @@ -249,7 +249,7 @@ static bool should_export_before_import(struct uv_cb_header *uvcb, struct mm_str
>  	 * shared page from a different protected VM will automatically also
>  	 * transfer its ownership.
>  	 */
> -	if (test_bit_inv(BIT_UV_FEAT_MISC, &uv_info.uv_feature_indications))
> +	if (uv_has_feature(BIT_UV_FEAT_MISC))
>  		return false;
>  	if (uvcb->cmd == UVC_CMD_UNPIN_PAGE_SHARED)
>  		return false;
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 17b81659cdb2..339e3190f6dc 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2402,7 +2402,7 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc)
>  	struct kvm_vcpu *vcpu;
>  
>  	/* Disable the GISA if the ultravisor does not support AIV. */
> -	if (!test_bit_inv(BIT_UV_FEAT_AIV, &uv_info.uv_feature_indications))
> +	if (!uv_has_feature(BIT_UV_FEAT_AIV))
>  		kvm_s390_gisa_disable(kvm);
>  
>  	kvm_for_each_vcpu(i, vcpu, kvm) {
> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> index b65144c392b0..b8ef42fac2c4 100644
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -824,7 +824,7 @@ void do_secure_storage_access(struct pt_regs *regs)
>  	 * reliable without the misc UV feature so we need to check
>  	 * for that as well.
>  	 */
> -	if (test_bit_inv(BIT_UV_FEAT_MISC, &uv_info.uv_feature_indications) &&
> +	if (uv_has_feature(BIT_UV_FEAT_MISC) &&
>  	    !test_bit_inv(61, &regs->int_parm_long)) {
>  		/*
>  		 * When this happens, userspace did something that it

Re: [PATCH 2/3] KVM: s390: Add UV feature negotiation

[Claudio Imbrenda]

On Thu, 27 Jul 2023 14:20:52 +0200
Steffen Eiden <seiden@linux.ibm.com> wrote:

> Add a uv_feature list for pv-guests to the kvm cpu-model.
> The feature bits 'AP-interpretation for secure guests' and
> 'AP-interrupt for secure guests' are available.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
>  arch/s390/include/asm/kvm_host.h |  2 +
>  arch/s390/include/uapi/asm/kvm.h | 24 +++++++++++
>  arch/s390/kvm/kvm-s390.c         | 70 ++++++++++++++++++++++++++++++++
>  3 files changed, 96 insertions(+)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 2bbc3d54959d..7ae57ac352b2 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -817,6 +817,8 @@ struct kvm_s390_cpu_model {
>  	__u64 *fac_list;
>  	u64 cpuid;
>  	unsigned short ibc;
> +	/* subset of available uv features for pv-guests enabled by user space */
> +	struct kvm_s390_vm_cpu_uv_feat uv_feat_guest;
>  };
>  
>  typedef int (*crypto_hook)(struct kvm_vcpu *vcpu);
> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
> index a73cf01a1606..7ea7e4fbd37e 100644
> --- a/arch/s390/include/uapi/asm/kvm.h
> +++ b/arch/s390/include/uapi/asm/kvm.h
> @@ -159,6 +159,30 @@ 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 */

I would put padding here for completeness, and maybe a static assert
for the size of struct kvm_s390_vm_cpu_uv_feat

> +		};
> +		__u64 feat;
> +	};
> +};
> +
> +#define KVM_S390_VM_CPU_UV_FEAT_GUEST_MASK \
> +(((struct kvm_s390_vm_cpu_uv_feat) {	\
> +	.ap = 1,		\
> +	.ap_intr = 1,		\
> +}).feat)

please indent the body of the macro, and add tabs on the right so that
the \ are aligned

> +
> +#define KVM_S390_VM_CPU_UV_FEAT_GUEST_DEFAULT \
> +((struct kvm_s390_vm_cpu_uv_feat) { })

please indent the body of the macro

> +
>  /* kvm attributes for crypto */
>  #define KVM_S390_VM_CRYPTO_ENABLE_AES_KW	0
>  #define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW	1
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 339e3190f6dc..50c8b85b89ac 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1531,6 +1531,32 @@ static int kvm_s390_set_processor_subfunc(struct kvm *kvm,
>  	return 0;
>  }
>  
> +static int kvm_s390_set_uv_feat(struct kvm *kvm, struct kvm_device_attr *attr)
> +{
> +	struct kvm_s390_vm_cpu_uv_feat data = {};
> +	unsigned long filter =
> +		uv_info.uv_feature_indications &

I think this can go in the previous line?

> +		(unsigned long)KVM_S390_VM_CPU_UV_FEAT_GUEST_MASK;

the cast is not necessary

without the cast the whole thing can fit in one line

> +
> +	if (copy_from_user(&data, (void __user *)attr->addr, sizeof(data)))
> +		return -EFAULT;
> +	if (!bitmap_subset((unsigned long *)&data, &filter,
> +			   KVM_S390_VM_CPU_UV_FEAT_NR_BITS))

this also fits in one line :)

> +		return -EINVAL;
> +
> +	mutex_lock(&kvm->lock);
> +	if (kvm->created_vcpus) {
> +		mutex_unlock(&kvm->lock);
> +		return -EBUSY;
> +	}
> +	kvm->arch.model.uv_feat_guest = data;
> +	mutex_unlock(&kvm->lock);
> +
> +	VM_EVENT(kvm, 3, "SET: guest uv feat: 0x%16.16llx", data.feat);
> +
> +	return 0;
> +}
> +
>  static int kvm_s390_set_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr)
>  {
>  	int ret = -ENXIO;
> @@ -1545,6 +1571,9 @@ static int kvm_s390_set_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr)
>  	case KVM_S390_VM_CPU_PROCESSOR_SUBFUNC:
>  		ret = kvm_s390_set_processor_subfunc(kvm, attr);
>  		break;
> +	case KVM_S390_VM_CPU_PROCESSOR_UV_FEAT_GUEST:
> +		ret = kvm_s390_set_uv_feat(kvm, attr);
> +		break;
>  	}
>  	return ret;
>  }
> @@ -1777,6 +1806,37 @@ static int kvm_s390_get_machine_subfunc(struct kvm *kvm,
>  	return 0;
>  }
>  
> +static int kvm_s390_get_processor_uv_feat(struct kvm *kvm,
> +					  struct kvm_device_attr *attr)
> +{
> +	struct kvm_s390_vm_cpu_uv_feat *src = &kvm->arch.model.uv_feat_guest;
> +
> +	if (copy_to_user((void __user *)attr->addr, src, sizeof(*src)))
> +		return -EFAULT;
> +	VM_EVENT(kvm, 3, "GET: guest  uv feat: 0x%16.16llx",
> +		 kvm->arch.model.uv_feat_guest.feat);

this can actually fit in one line

> +
> +	return 0;
> +}
> +
> +static int kvm_s390_get_machine_uv_feat(struct kvm *kvm,
> +					struct kvm_device_attr *attr)
> +{
> +	struct kvm_s390_vm_cpu_uv_feat data = {
> +		.feat = uv_info.uv_feature_indications &
> +			KVM_S390_VM_CPU_UV_FEAT_GUEST_MASK

this can fit in one line

> +	};
> +
> +	BUILD_BUG_ON(sizeof(struct kvm_s390_vm_cpu_uv_feat) !=
> +		     sizeof(uv_info.uv_feature_indications));
> +	if (copy_to_user((void __user *)attr->addr, &data,
> +			 sizeof(struct kvm_s390_vm_cpu_uv_feat)))

why not sizeof(data) ?

(also, it can fit in one line)

> +		return -EFAULT;
> +	VM_EVENT(kvm, 3, "GET: guest  uv feat: 0x%16.16llx", data.feat);
> +
> +	return 0;
> +}
> +
>  static int kvm_s390_get_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr)
>  {
>  	int ret = -ENXIO;
> @@ -1800,6 +1860,12 @@ static int kvm_s390_get_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr)
>  	case KVM_S390_VM_CPU_MACHINE_SUBFUNC:
>  		ret = kvm_s390_get_machine_subfunc(kvm, attr);
>  		break;
> +	case KVM_S390_VM_CPU_PROCESSOR_UV_FEAT_GUEST:
> +		ret = kvm_s390_get_processor_uv_feat(kvm, attr);
> +		break;
> +	case KVM_S390_VM_CPU_MACHINE_UV_FEAT_GUEST:
> +		ret = kvm_s390_get_machine_uv_feat(kvm, attr);
> +		break;
>  	}
>  	return ret;
>  }
> @@ -1952,6 +2018,8 @@ static int kvm_s390_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>  		case KVM_S390_VM_CPU_MACHINE_FEAT:
>  		case KVM_S390_VM_CPU_MACHINE_SUBFUNC:
>  		case KVM_S390_VM_CPU_PROCESSOR_SUBFUNC:
> +		case KVM_S390_VM_CPU_MACHINE_UV_FEAT_GUEST:
> +		case KVM_S390_VM_CPU_PROCESSOR_UV_FEAT_GUEST:
>  			ret = 0;
>  			break;
>  		default:
> @@ -3292,6 +3360,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>  	kvm->arch.model.cpuid = kvm_s390_get_initial_cpuid();
>  	kvm->arch.model.ibc = sclp.ibc & 0x0fff;
>  
> +	kvm->arch.model.uv_feat_guest = KVM_S390_VM_CPU_UV_FEAT_GUEST_DEFAULT;
> +
>  	kvm_s390_crypto_init(kvm);
>  
>  	if (IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM)) {

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

[Claudio Imbrenda]

On Thu, 27 Jul 2023 14:20:53 +0200
Steffen Eiden <seiden@linux.ibm.com> wrote:

> Introduces new feature bits and enablement flags for AP and AP IRQ
> support.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
>  arch/s390/include/asm/uv.h | 12 +++++++++++-
>  arch/s390/kvm/pv.c         |  8 ++++++--
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
> index 338845402324..913ccfaa9d76 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -99,6 +99,8 @@ enum uv_cmds_inst {
>  enum uv_feat_ind {
>  	BIT_UV_FEAT_MISC = 0,
>  	BIT_UV_FEAT_AIV = 1,
> +	BIT_UV_FEAT_AP = 4,
> +	BIT_UV_FEAT_AP_INTR = 5,
>  };
>  
>  struct uv_cb_header {
> @@ -159,7 +161,15 @@ struct uv_cb_cgc {
>  	u64 guest_handle;
>  	u64 conf_base_stor_origin;
>  	u64 conf_virt_stor_origin;
> -	u64 reserved30;
> +	u8  reserved30[6];
> +	union {
> +		struct {
> +			u16 reserved : 14;
> +			u16 ap_instr_intr : 1;
> +			u16 ap_allow_instr : 1;
> +		};
> +		u16 raw;
> +	} flags;
>  	u64 guest_stor_origin;
>  	u64 guest_stor_len;
>  	u64 guest_sca;
> diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
> index 899f3b8ac011..103add1894c9 100644
> --- a/arch/s390/kvm/pv.c
> +++ b/arch/s390/kvm/pv.c
> @@ -561,12 +561,16 @@ int kvm_s390_pv_init_vm(struct kvm *kvm, u16 *rc, u16 *rrc)
>  	uvcb.conf_base_stor_origin =
>  		virt_to_phys((void *)kvm->arch.pv.stor_base);
>  	uvcb.conf_virt_stor_origin = (u64)kvm->arch.pv.stor_var;
> +	uvcb.flags.ap_allow_instr = uv_has_feature(BIT_UV_FEAT_AP) &&
> +				    kvm->arch.model.uv_feat_guest.ap;

how can it happen that uv_feat_guest.ap is set but
uv_has_feature(BIT_UV_FEAT_AP) is 0 ?

in the previous patch you check for the feature before allowing the bit
to be set.

> +	uvcb.flags.ap_instr_intr = uv_has_feature(BIT_UV_FEAT_AP_INTR) &&
> +				   kvm->arch.model.uv_feat_guest.ap_intr;

same here

>  
>  	cc = uv_call_sched(0, (u64)&uvcb);
>  	*rc = uvcb.header.rc;
>  	*rrc = uvcb.header.rrc;
> -	KVM_UV_EVENT(kvm, 3, "PROTVIRT CREATE VM: handle %llx len %llx rc %x rrc %x",
> -		     uvcb.guest_handle, uvcb.guest_stor_len, *rc, *rrc);
> +	KVM_UV_EVENT(kvm, 3, "PROTVIRT CREATE VM: handle %llx len %llx rc %x rrc %x flags %04x",
> +		     uvcb.guest_handle, uvcb.guest_stor_len, *rc, *rrc, uvcb.flags.raw);
>  
>  	/* Outputs */
>  	kvm->arch.pv.handle = uvcb.guest_handle;