Re: [PATCH v2 2/2] KVM: arm64: top up pKVM mapping cache for permission faults

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Permission faults on a PMD during dirty logging skip page table memcache topup but still trigger block-to-page splits, causing an -ENOMEM guest abort.
--

commit 26c4caa3c15677862ff91c344811bed70a7dead2
Author: Bradley Morgan <include@grrlz.net>

KVM: arm64: top up pKVM mapping cache for permission faults

This commit allocates a fresh pkvm_mapping object for pKVM permission
faults, as they may replace page entries with a block entry. It adds a
topup_hyp_memcache_mapping() call in user_mem_abort() for this case.

Link: https://lore.kernel.org/all/20260623161545.EA08E1F000E9@smtp.kernel.org/

> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 445d6cf035c91..bb855f1d6ab47 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c

[ ... ]

> @@ -2073,7 +2082,9 @@ static int user_mem_abort(const struct kvm_s2_fault_desc *s2fd)
>  	 * Permission faults just need to update the existing leaf entry,
>  	 * and so normally don't require allocations from the memcache. The
>  	 * only exception to this is when dirty logging is enabled at runtime
> -	 * and a write fault needs to collapse a block entry into a table.
> +	 * and a write fault needs to collapse a block entry into a table. With
> +	 * pKVM, they may still need a fresh mapping object if the fault turns
> +	 * page entries into a block entry.
>  	 */
>  	memcache = get_mmu_memcache(s2fd->vcpu);
>  	if (!perm_fault || (memslot_is_logging(s2fd->memslot) &&
> -			    kvm_is_write_fault(s2fd->vcpu))) {
> +			    kvm_is_write_fault(s2fd->vcpu))) {

[Severity: High]
This is a pre-existing issue, but can this condition lead to an -ENOMEM
guest abort during dirty logging?

If a guest triggers an instruction permission fault (IABT) on a PMD while
dirty logging is enabled, kvm_is_write_fault() evaluates to false. This
skips topup_mmu_memcache().

Later in kvm_s2_fault_map(), dirty logging forces mapping_size to PAGE_SIZE,
which mismatches the PMD_SIZE perm_fault_granule. This forces a call to
kvm_pgtable_stage2_map() to split the PMD:

arch/arm64/kvm/mmu.c:kvm_s2_fault_map() {
    ...
    } else {
        ret = KVM_PGT_FN(kvm_pgtable_stage2_map)(pgt, gfn_to_gpa(gfn),
                                                 mapping_size,
                                                 __pfn_to_phys(pfn), prot,
                                                 memcache, flags);
    }
}

The split requires a page table allocation from the empty memcache, which
fails with -ENOMEM and fatally aborts the guest.

>  		ret = topup_mmu_memcache(s2fd->vcpu, memcache);
>  		if (ret)
>  			return ret;
> +	} else if (is_protected_kvm_enabled()) {
> +		ret = topup_hyp_memcache_mapping(memcache);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	/*

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623163756.4591-1-include@grrlz.net?part=2