On Mon, 3 Feb 2020 08:19:28 -0500
Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:
> From: Janosch Frank <frankja@xxxxxxxxxxxxx>
>
> This contains 3 main changes:
> 1. changes in SIE control block handling for secure guests
> 2. helper functions for create/destroy/unpack secure guests
> 3. KVM_S390_PV_COMMAND ioctl to allow userspace dealing with secure
> machines
>
> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
> ---
> arch/s390/include/asm/kvm_host.h | 24 ++-
> arch/s390/include/asm/uv.h | 60 ++++++++
> arch/s390/kvm/Makefile | 2 +-
> arch/s390/kvm/kvm-s390.c | 198 ++++++++++++++++++++++++-
> arch/s390/kvm/kvm-s390.h | 45 ++++++
> arch/s390/kvm/pv.c | 246 +++++++++++++++++++++++++++++++
> include/uapi/linux/kvm.h | 33 +++++
> 7 files changed, 604 insertions(+), 4 deletions(-)
> create mode 100644 arch/s390/kvm/pv.c
>
(...)
> @@ -80,6 +95,32 @@ struct uv_cb_init {
>
> } __packed __aligned(8);
>
> +struct uv_cb_cgc {
Given that we now have a bunch of structs of the form uv_cb_TLA, can we
add a comment to each for what uv call they are?
> + struct uv_cb_header header;
> + u64 reserved08[2];
> + u64 guest_handle;
> + u64 conf_base_stor_origin;
> + u64 conf_var_stor_origin;
> + u64 reserved30;
> + u64 guest_stor_origin;
> + u64 guest_stor_len;
> + u64 guest_sca;
> + u64 guest_asce;
> + u64 reserved60[5];
> +} __packed __aligned(8);
(...)
> +#ifdef CONFIG_KVM_S390_PROTECTED_VIRTUALIZATION_HOST
> +static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
> +{
> + int r = 0;
> + void __user *argp = (void __user *)cmd->data;
> +
> + switch (cmd->cmd) {
> + case KVM_PV_VM_CREATE: {
> + r = -EINVAL;
> + if (kvm_s390_pv_is_protected(kvm))
> + break;
> +
> + r = kvm_s390_pv_alloc_vm(kvm);
> + if (r)
> + break;
> +
> + mutex_lock(&kvm->lock);
> + kvm_s390_vcpu_block_all(kvm);
> + /* FMT 4 SIE needs esca */
> + r = sca_switch_to_extended(kvm);
> + if (!r)
> + r = kvm_s390_pv_create_vm(kvm);
If sca_switch_to_extended() fails, you don't call
kvm_s390_pv_dealloc_vm(). Also, kvm_s390_pv_create_vm() _does_ call
_dealloc_vm() on failure, which seems a bit surprising. I'd probably
move the _dealloc_vm() out of the error path of _create_vm() and call
it here for r != 0.
> + kvm_s390_vcpu_unblock_all(kvm);
> + mutex_unlock(&kvm->lock);
> + break;
> + }
(...)
