> +static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd) > +{ > + int r = 0; > + u16 dummy; > + void __user *argp = (void __user *)cmd->data; > + > + switch (cmd->cmd) { > + case KVM_PV_ENABLE: { > + r = -EINVAL; > + if (kvm_s390_pv_is_protected(kvm)) > + break; > + > + r = kvm_s390_pv_alloc_vm(kvm); > + if (r) > + break; > + To make this nicer, can we simply merge alloc+create into init /* FMT 4 SIE needs esca */ r = sca_switch_to_extended(kvm); if (r) break; r = kvm_s390_pv_init_vm(); if (r) break; r = kvm_s390_cpus_to_pv(kvm, &cmd->rc, &cmd->rrc); if (r) kvm_s390_pv_deinit_vm(); break; I remember the split dates back to an earlier UAPI interface. Similarly from deinit. The you can just make deinit never fail and handle that freeing special-case in there and add a comment. [...] > +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); empty line. > + *rc = uvcb.header.rc; [...] > +int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size, > + unsigned long tweak, u16 *rc, u16 *rrc) > +{ > + u64 offset = 0; > + int ret = 0; > + > + if (addr & ~PAGE_MASK || !size || size & ~PAGE_MASK) > + return -EINVAL; > + > + KVM_UV_EVENT(kvm, 3, "PROTVIRT VM UNPACK: start addr %lx size %lx", > + addr, size); > + > + while (offset < size) { > + ret = unpack_one(kvm, addr, tweak, offset, rc, rrc); > + if (ret == -EAGAIN) { > + cond_resched(); > + if (fatal_signal_pending(current)) > + break; > + continue; > + } > + if (ret) > + break; > + addr += PAGE_SIZE; > + offset += PAGE_SIZE; > + } Much better :) -- Thanks, David / dhildenb