Re: [PATCH v3 14/17] KVM: arm64: Alloc pkvm_hyp_vcpu using pKVM heap allocator

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving pkvm_hyp_vcpu allocation to the global EL2 heap allocator bypasses memory cgroup (memcg) accounting and introduces a charge lifecycle mismatch.
--

--- Patch [14]: [PATCH v3 14/17] KVM: arm64: Alloc pkvm_hyp_vcpu using pKVM heap allocator ---
commit cc29b34f40079b23129d4e94199fd0029bedc7c5
Author: Vincent Donnefort <vdonnefort@google.com>
Subject: KVM: arm64: Alloc pkvm_hyp_vcpu using pKVM heap allocator

This commit transitions the allocation of the hypervisor vCPU state
structure (pkvm_hyp_vcpu) from the host to the hypervisor using the new
pKVM heap allocator (hyp_alloc()). The host now uses pkvm_call_hyp_req()
to invoke __pkvm_create_hyp_vcpu, which automatically handles any top-up
requests if the hypervisor runs out of memory.

> diff --git a/arch/arm64/kvm/pkvm.c b/arch/arm64/kvm/pkvm.c
> --- a/arch/arm64/kvm/pkvm.c
> +++ b/arch/arm64/kvm/pkvm.c
[ ... ]
> @@ -153,28 +153,19 @@ static void __pkvm_destroy_hyp_vm(struct kvm *kvm)
[ ... ]
>  static int __pkvm_create_hyp_vcpu(struct kvm_vcpu *vcpu)
>  {
> -	size_t hyp_vcpu_sz = PAGE_ALIGN(PKVM_HYP_VCPU_SIZE);
>  	pkvm_handle_t handle = vcpu->kvm->arch.pkvm.handle;
> -	void *hyp_vcpu;
>  	int ret;
>  
>  	init_hyp_stage2_memcache(&vcpu->arch.pkvm_memcache);
>  
> -	hyp_vcpu = alloc_pages_exact(hyp_vcpu_sz, GFP_KERNEL_ACCOUNT);
> -	if (!hyp_vcpu)
> -		return -ENOMEM;
> -
> -	ret = kvm_call_hyp_nvhe(__pkvm_init_vcpu, handle, vcpu, hyp_vcpu);
> +	ret = pkvm_call_hyp_req(__pkvm_init_vcpu, handle, vcpu);

[Severity: High]
Does this transition to the EL2 heap allocator bypass memory cgroup
(memcg) accounting?

Previously, pkvm_hyp_vcpu was allocated with GFP_KERNEL_ACCOUNT. By
switching to pkvm_call_hyp_req(), top-up requests are handled by the host
using init_hyp_memcache(), which omits the HYP_MEMCACHE_ACCOUNT_KMEMCG
flag. This falls back to an unaccounted GFP_KERNEL allocation, which could
allow an unprivileged host-userspace process to exhaust host kernel memory
by repeatedly creating VMs and vCPUs.

>  	if (!ret)
>  		vcpu_set_flag(vcpu, VCPU_PKVM_FINALIZED);
> -	else
> -		free_pages_exact(hyp_vcpu, hyp_vcpu_sz);
>  
>  	return ret;
>  }
> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
[ ... ]
> @@ -896,16 +872,15 @@ static int register_hyp_vcpu(struct pkvm_hyp_vm *hyp_vm,
>  	return 0;
>  }
>  
> -int __pkvm_init_vcpu(pkvm_handle_t handle, struct kvm_vcpu *host_vcpu,
> -		     unsigned long vcpu_hva)
> +int __pkvm_init_vcpu(pkvm_handle_t handle, struct kvm_vcpu *host_vcpu)
>  {
>  	struct pkvm_hyp_vcpu *hyp_vcpu;
>  	struct pkvm_hyp_vm *hyp_vm;
>  	int ret;
>  
> -	hyp_vcpu = map_donated_memory(vcpu_hva, sizeof(*hyp_vcpu));
> +	hyp_vcpu = hyp_alloc(sizeof(*hyp_vcpu));

[Severity: High]
Does using the global EL2 heap break the memcg lifecycle?

Even if the top-up were accounted during the host allocation, pages from the
global EL2 heap are not returned to the host upon VM teardown. This means the
memory would remain charged indefinitely to the original container while being
reused for other VMs.

>  	if (!hyp_vcpu)
> -		return -ENOMEM;
> +		return hyp_alloc_errno();

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720171513.1415357-1-vdonnefort@google.com?part=14