Re: [PATCH v1 1/4] KVM: arm64: Move pkvm_vcpu_init_traps() to init_pkvm_hyp_vcpu()

[Oliver Upton <oliver.upton@linux.dev>]

On Mon, Oct 14, 2024 at 11:24:09AM +0100, Fuad Tabba wrote:
> Move pkvm_vcpu_init_traps() to the initialization of the
> hypervisor's vcpu state in init_pkvm_hyp_vcpu(), and remove the
> associated hypercall.
> 
> In protected mode, traps need to be initialized whenever a VCPU
> is initialized anyway, and not only for protected VMs. This also
> saves an unnecessary hypercall.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/include/asm/kvm_asm.h               | 1 -
>  arch/arm64/kvm/arm.c                           | 8 --------
>  arch/arm64/kvm/hyp/include/nvhe/trap_handler.h | 2 --
>  arch/arm64/kvm/hyp/nvhe/hyp-main.c             | 8 --------
>  arch/arm64/kvm/hyp/nvhe/pkvm.c                 | 4 +++-
>  5 files changed, 3 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index b36a3b6cc011..7f7866d2bfc2 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -76,7 +76,6 @@ enum __kvm_host_smccc_func {
>  	__KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff,
>  	__KVM_HOST_SMCCC_FUNC___vgic_v3_save_vmcr_aprs,
>  	__KVM_HOST_SMCCC_FUNC___vgic_v3_restore_vmcr_aprs,
> -	__KVM_HOST_SMCCC_FUNC___pkvm_vcpu_init_traps,
>  	__KVM_HOST_SMCCC_FUNC___pkvm_init_vm,
>  	__KVM_HOST_SMCCC_FUNC___pkvm_init_vcpu,
>  	__KVM_HOST_SMCCC_FUNC___pkvm_teardown_vm,
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index a0d01c46e408..ece934bb4531 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -856,14 +856,6 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
>  		static_branch_inc(&userspace_irqchip_in_use);
>  	}
>  
> -	/*
> -	 * Initialize traps for protected VMs.
> -	 * NOTE: Move to run in EL2 directly, rather than via a hypercall, once
> -	 * the code is in place for first run initialization at EL2.
> -	 */
> -	if (kvm_vm_is_protected(kvm))
> -		kvm_call_hyp_nvhe(__pkvm_vcpu_init_traps, vcpu);
> -

Can we instead get rid of this hypercall by implementing the one-time
late initialization as the comment suggests?

It'd be great if the feature configuration for non-protected and
protected VMs is done in a similar way, and that is userspace changes to
the ID registers before KVM_RUN.

So we need this sort of 'late' initialization for that behavior to work.

-- 
Thanks,
Oliver