Re: [PATCH v3.1 09/37] KVM: s390: protvirt: Add initial vm and cpu lifecycle handling

[David Hildenbrand <david@redhat.com>]

> +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