Re: [PATCH v3 07/14] KVM: arm64: Size the stage-2 memcache from the fault MMU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] arm64: kvm: spurious allocation in kvm_s2_fault_map during prefault
--

commit 9bf98edb6fb89edb3f2ce746e5754cac7f11bfe0
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>
KVM: arm64: Size the stage-2 memcache from the fault MMU

This commit modifies topup_mmu_memcache() to use the target MMU from s2fd->mmu
rather than the active vCPU MMU. This is a preparatory step for stage-2
pre-faulting, which needs to target the canonical MMU regardless of the vCPU's
context.

> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 5c429065a4feb..221ea069f9bb6 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -2251,7 +2251,7 @@ static int user_mem_abort(const struct kvm_s2_fault_desc *s2fd,
>  	memcache = get_mmu_memcache(s2fd->vcpu);
>  	if (!perm_fault || memslot_is_logging(s2fd->memslot) ||
>  	    is_protected_kvm_enabled()) {
> -		ret = topup_mmu_memcache(s2fd->vcpu, memcache);
> +		ret = topup_mmu_memcache(s2fd->mmu, memcache);
>  		if (ret)
>  			return ret;
>  	}

[Severity: Medium]
Does this decoupling of s2fd->mmu and vcpu->arch.hw_mmu cause a spurious
allocation downstream in kvm_s2_fault_map()?

When pre-faulting is executed on a nested vCPU, s2fd->mmu is set to the
non-nested canonical MMU, while vcpu->arch.hw_mmu remains nested. In
kvm_s2_fault_map(), the allocation of the mapping struct is gated on the active
vCPU MMU:

arch/arm64/kvm/mmu.c:kvm_s2_fault_map() {
	if (kvm_is_nested_s2_mmu(kvm, s2fd->vcpu->arch.hw_mmu)) {
		mapping = kmalloc_obj(struct kvm_guest_s2_mapping,
				      GFP_KERNEL_ACCOUNT);
		if (!mapping) {
			kvm_release_page_unused(s2vi->page);
			return -ENOMEM;
		}
	}
}

However, the consumption and tracking of this mapping relies on pgt->mmu, which
resolves to the target s2fd->mmu:

arch/arm64/kvm/mmu.c:kvm_s2_fault_map() {
		if ((ret == 0 || ret == -EAGAIN) && kvm_is_nested_s2_mmu(kvm, pgt->mmu)) {
			kvm_record_guest_s2_mapping(pgt->mmu, canonical_ipa,
						    gfn_to_gpa(gfn), mapping_size, mapping);
			mapping = NULL;
		}
}

Because kvm_is_nested_s2_mmu() for pgt->mmu evaluates to false during a
pre-fault, the allocation is never used and is subsequently safely freed at the
end of the function.

Could this lead to an unnecessary allocation in the ioctl path that might
spuriously fail and return -ENOMEM to userspace?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922-kvm-arm-prefault-v3-0-787bd3bc7e3f@kernel.org?part=7