Re: [PATCH v6 31/33] KVM: s390: arm64: Implement vCPU IOCTLs

[Janosch Frank]

On 8/12/26 5:36 PM, Steffen Eiden wrote:
> Implement all required vCPU IOCTLs.
>
> Co-developed-by: Andreas Grapentin <gra@xxxxxxxxxxxxx>
> Signed-off-by: Andreas Grapentin <gra@xxxxxxxxxxxxx>
> Co-developed-by: Nina Schoetterl-Glausch <nsg@xxxxxxxxxxxxx>
> Signed-off-by: Nina Schoetterl-Glausch <nsg@xxxxxxxxxxxxx>
> Signed-off-by: Steffen Eiden <seiden@xxxxxxxxxxxxx>
> ---
>   arch/s390/kvm/arm64/arm.c         | 392 ++++++++++++++++++++++++++++++
>   arch/s390/kvm/arm64/guest.c       |  47 +++-
>   arch/s390/kvm/arm64/handle_exit.c |  31 +++
>   arch/s390/kvm/arm64/handle_exit.h |   9 +
>   arch/s390/kvm/arm64/reset.c       |  73 ++++++
>   arch/s390/kvm/arm64/reset.h       |  11 +
>   arch/s390/kvm/arm64/trace.h       |   2 +-
>   7 files changed, 562 insertions(+), 3 deletions(-)
>   create mode 100644 arch/s390/kvm/arm64/handle_exit.h
>   create mode 100644 arch/s390/kvm/arm64/reset.c
>   create mode 100644 arch/s390/kvm/arm64/reset.h

[...]

> +static int kvm_vcpu_initialize(struct kvm_vcpu *vcpu,
> +			       const struct kvm_vcpu_init *init)
> +{
> +	unsigned long features = init->features[0];
> +	struct kvm *kvm = vcpu->kvm;
> +
> +	scoped_guard(mutex, &kvm->arch.config_lock) {
> +		if (test_bit(KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED,
> +			     &kvm->arch.flags) &&


In this case I'd rather have a long line than having to look at this 
mess. One of the cases where a normal mutex acquisition would have made 
the code more readable.


> +		    kvm_vcpu_init_changed(vcpu, init))
> +			return -EINVAL;
> +
> +		bitmap_copy(kvm->arch.vcpu_features, &features,
> +			    KVM_VCPU_MAX_FEATURES);
> +
> +		kvm_reset_vcpu(vcpu);
> +
> +		set_bit(KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED,
> +			&kvm->arch.flags);
> +		vcpu_set_flag(vcpu, VCPU_INITIALIZED);
> +	}

[...]

> +/** kvm_arch_vcpu_ioctl_run() - run arm64 vCPU
> + *
> + * Execute arm64 guest instructions using SAE.
> + *
> + * Returns:
> + * 1 enter the guest (should not be observed by userspace)
> + * 0 exit to userspace
> + * < 0 exit to userspace, where the return value indicates n error
> + *
> + *
> + */
> +int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> +{
> +	DECLARE_KERNEL_FPU_ONSTACK32(fpu_save);
> +	struct kvm_run *kvm_run = vcpu->run;
> +	int ret;
> +
> +	if (kvm_run->exit_reason == KVM_EXIT_MMIO) {
> +		ret = kvm_handle_mmio_return(vcpu);
> +		if (ret <= 0)
> +			return ret;
> +	}
> +
> +	vcpu_load(vcpu);
> +
> +	kernel_fpu_begin(&fpu_save, KERNEL_FPC | KERNEL_VXR);
> +	load_vx_regs((vcpu->arch.ctxt.vregs));
> +
> +	if (!vcpu->wants_to_run) {
> +		ret = -EINTR;
> +		goto out;
> +	}
> +
> +	kvm_sigset_activate(vcpu);
> +
> +	might_fault();
> +
> +	ret = 1;
> +	do {
> +		if (signal_pending(current)) {
> +			kvm_run->exit_reason = KVM_EXIT_INTR;
> +			ret = -EINTR;
> +			continue;

Why not break?
With continue we'll jump to the condition which is immediately false 
since we just set ret to a negative value.

> +		}
> +
> +		if (need_resched())
> +			schedule();
> +
> +		if (ret > 0)
> +			ret = check_vcpu_requests(vcpu);
> +
> +		vcpu->arch.sae_block.icptr = 0;
> +
> +		arm_vcpu_run(vcpu);
> +
> +		ret = handle_exit(vcpu);
> +
> +	} while (ret > 0);
> +
> +	kvm_sigset_deactivate(vcpu);
> +out:
> +	if (unlikely(vcpu_get_flag(vcpu, INCREMENT_PC)))
> +		adjust_pc(vcpu);
> +
> +	save_vx_regs(vcpu->arch.ctxt.vregs);
> +	kernel_fpu_end(&fpu_save, KERNEL_FPC | KERNEL_VXR);
> +	vcpu_put(vcpu);
> +
> +	return ret;
> +}