On Tue, 25 Feb 2020 16:48:22 -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>
> [borntraeger@xxxxxxxxxx: patch merging, splitting, fixing]
> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> ---
> arch/s390/include/asm/kvm_host.h | 24 ++-
> arch/s390/include/asm/uv.h | 69 ++++++++
> arch/s390/kvm/Makefile | 2 +-
> arch/s390/kvm/kvm-s390.c | 209 +++++++++++++++++++++++-
> arch/s390/kvm/kvm-s390.h | 33 ++++
> arch/s390/kvm/pv.c | 269 +++++++++++++++++++++++++++++++
> include/uapi/linux/kvm.h | 31 ++++
> 7 files changed, 633 insertions(+), 4 deletions(-)
> create mode 100644 arch/s390/kvm/pv.c
> +int kvm_s390_pv_init_vm(struct kvm *kvm, u16 *rc, u16 *rrc)
> +{
> + struct uv_cb_cgc uvcb = {
Broken indentation.
> + .header.cmd = UVC_CMD_CREATE_SEC_CONF,
> + .header.len = sizeof(uvcb)
> + };
> + int cc, ret;
> + u16 dummy;
> +
> + ret = kvm_s390_pv_alloc_vm(kvm);
> + if (ret)
> + return ret;
> +
> + /* Inputs */
> + uvcb.guest_stor_origin = 0; /* MSO is 0 for KVM */
What is 'MSO'? (i.e., where is that 'M' coming from?)
> + uvcb.guest_stor_len = kvm->arch.pv.guest_len;
> + uvcb.guest_asce = kvm->arch.gmap->asce;
> + uvcb.guest_sca = (unsigned long)kvm->arch.sca;
> + uvcb.conf_base_stor_origin = (u64)kvm->arch.pv.stor_base;
> + uvcb.conf_virt_stor_origin = (u64)kvm->arch.pv.stor_var;
> +
> + cc = uv_call(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);
> +
> + /* Outputs */
> + kvm->arch.pv.handle = uvcb.guest_handle;
Is this valid if the call failed?
> +
> + if (cc) {
> + if (uvcb.header.rc & UVC_RC_NEED_DESTROY)
> + kvm_s390_pv_deinit_vm(kvm, &dummy, &dummy);
> + else
> + kvm_s390_pv_dealloc_vm(kvm);
> + return -EIO;
> + }
> + kvm->arch.gmap->guest_handle = uvcb.guest_handle;
...especially as you assign that handle only down here.
> + atomic_set(&kvm->mm->context.is_protected, 1);
> + return 0;
> +}
> +
> +int kvm_s390_pv_set_sec_parms(struct kvm *kvm, void *hdr, u64 length, u16 *rc,
> + u16 *rrc)
> +{
> + struct uv_cb_ssc uvcb = {
> + .header.cmd = UVC_CMD_SET_SEC_CONF_PARAMS,
> + .header.len = sizeof(uvcb),
> + .sec_header_origin = (u64)hdr,
> + .sec_header_len = length,
> + .guest_handle = kvm_s390_pv_get_handle(kvm),
> + };
> + int cc = uv_call(0, (u64)&uvcb);
> +
> + *rc = uvcb.header.rc;
> + *rrc = uvcb.header.rrc;
> + KVM_UV_EVENT(kvm, 3, "PROTVIRT VM SET PARMS: rc %x rrc %x",
> + *rc, *rrc);
> + if (cc)
> + return -EINVAL;
> + return 0;
Maybe
return cc ? -EINVAL : 0;
(I assume none of the possible rcs in this case could indicate
something that does not map to -EINVAL?)
> +}
