2023-07-27 12:35:09

by Steffen Eiden

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

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 <[email protected]>
---
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



2023-07-27 16:11:32

by Claudio Imbrenda

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

On Thu, 27 Jul 2023 14:20:52 +0200
Steffen Eiden <[email protected]> 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 <[email protected]>
> ---
> 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)) {