Re: [PATCH v2 01/30] KVM: arm64: Extract VMA size resolution in user_mem_abort()

[Anshuman Khandual <anshuman.khandual@arm.com>]

On 27/03/26 5:05 PM, Marc Zyngier wrote:
> From: Fuad Tabba <tabba@google.com>
> 
> As part of an effort to refactor user_mem_abort() into smaller, more
> focused helper functions, extract the logic responsible for determining
> the VMA shift and page size into a new static helper,
> kvm_s2_resolve_vma_size().
> 
> Reviewed-by: Joey Gouly <joey.gouly@arm.com>
> Signed-off-by: Fuad Tabba <tabba@google.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  arch/arm64/kvm/mmu.c | 130 ++++++++++++++++++++++++-------------------
>  1 file changed, 73 insertions(+), 57 deletions(-)
> 
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 17d64a1e11e5c..f8064b2d32045 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1639,6 +1639,77 @@ static int gmem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>  	return ret != -EAGAIN ? ret : 0;
>  }
>  
> +static short kvm_s2_resolve_vma_size(struct vm_area_struct *vma,
> +				     unsigned long hva,
> +				     struct kvm_memory_slot *memslot,
> +				     struct kvm_s2_trans *nested,
> +				     bool *force_pte, phys_addr_t *ipa)
> +{
> +	short vma_shift;
> +	long vma_pagesize;
> +
> +	if (*force_pte)
> +		vma_shift = PAGE_SHIFT;
> +	else
> +		vma_shift = get_vma_page_shift(vma, hva);
> +
> +	switch (vma_shift) {
> +#ifndef __PAGETABLE_PMD_FOLDED
> +	case PUD_SHIFT:
> +		if (fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
> +			break;
> +		fallthrough;
> +#endif
> +	case CONT_PMD_SHIFT:
> +		vma_shift = PMD_SHIFT;
> +		fallthrough;
> +	case PMD_SHIFT:
> +		if (fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE))
> +			break;
> +		fallthrough;
> +	case CONT_PTE_SHIFT:
> +		vma_shift = PAGE_SHIFT;
> +		*force_pte = true;
> +		fallthrough;
> +	case PAGE_SHIFT:
> +		break;
> +	default:
> +		WARN_ONCE(1, "Unknown vma_shift %d", vma_shift);
> +	}
> +
> +	vma_pagesize = 1UL << vma_shift;
> +
> +	if (nested) {
> +		unsigned long max_map_size;
> +
> +		max_map_size = *force_pte ? PAGE_SIZE : PUD_SIZE;
> +
> +		*ipa = kvm_s2_trans_output(nested);
> +
> +		/*
> +		 * If we're about to create a shadow stage 2 entry, then we
> +		 * can only create a block mapping if the guest stage 2 page
> +		 * table uses at least as big a mapping.
> +		 */
> +		max_map_size = min(kvm_s2_trans_size(nested), max_map_size);
> +
> +		/*
> +		 * Be careful that if the mapping size falls between
> +		 * two host sizes, take the smallest of the two.
> +		 */
> +		if (max_map_size >= PMD_SIZE && max_map_size < PUD_SIZE)
> +			max_map_size = PMD_SIZE;
> +		else if (max_map_size >= PAGE_SIZE && max_map_size < PMD_SIZE)
> +			max_map_size = PAGE_SIZE;
> +
> +		*force_pte = (max_map_size == PAGE_SIZE);
> +		vma_pagesize = min_t(long, vma_pagesize, max_map_size);
> +		vma_shift = __ffs(vma_pagesize);
> +	}
> +
> +	return vma_shift;
> +}
> +
>  static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>  			  struct kvm_s2_trans *nested,
>  			  struct kvm_memory_slot *memslot, unsigned long hva,
> @@ -1695,65 +1766,10 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>  		return -EFAULT;
>  	}
>  
> -	if (force_pte)
> -		vma_shift = PAGE_SHIFT;
> -	else
> -		vma_shift = get_vma_page_shift(vma, hva);
> -
> -	switch (vma_shift) {
> -#ifndef __PAGETABLE_PMD_FOLDED
> -	case PUD_SHIFT:
> -		if (fault_supports_stage2_huge_mapping(memslot, hva, PUD_SIZE))
> -			break;
> -		fallthrough;
> -#endif
> -	case CONT_PMD_SHIFT:
> -		vma_shift = PMD_SHIFT;
> -		fallthrough;
> -	case PMD_SHIFT:
> -		if (fault_supports_stage2_huge_mapping(memslot, hva, PMD_SIZE))
> -			break;
> -		fallthrough;
> -	case CONT_PTE_SHIFT:
> -		vma_shift = PAGE_SHIFT;
> -		force_pte = true;
> -		fallthrough;
> -	case PAGE_SHIFT:
> -		break;
> -	default:
> -		WARN_ONCE(1, "Unknown vma_shift %d", vma_shift);
> -	}
> -
> +	vma_shift = kvm_s2_resolve_vma_size(vma, hva, memslot, nested,
> +					    &force_pte, &ipa);
>  	vma_pagesize = 1UL << vma_shift;
>  
> -	if (nested) {
> -		unsigned long max_map_size;
> -
> -		max_map_size = force_pte ? PAGE_SIZE : PUD_SIZE;
> -
> -		ipa = kvm_s2_trans_output(nested);
> -
> -		/*
> -		 * If we're about to create a shadow stage 2 entry, then we
> -		 * can only create a block mapping if the guest stage 2 page
> -		 * table uses at least as big a mapping.
> -		 */
> -		max_map_size = min(kvm_s2_trans_size(nested), max_map_size);
> -
> -		/*
> -		 * Be careful that if the mapping size falls between
> -		 * two host sizes, take the smallest of the two.
> -		 */
> -		if (max_map_size >= PMD_SIZE && max_map_size < PUD_SIZE)
> -			max_map_size = PMD_SIZE;
> -		else if (max_map_size >= PAGE_SIZE && max_map_size < PMD_SIZE)
> -			max_map_size = PAGE_SIZE;
> -
> -		force_pte = (max_map_size == PAGE_SIZE);
> -		vma_pagesize = min_t(long, vma_pagesize, max_map_size);
> -		vma_shift = __ffs(vma_pagesize);
> -	}
> -
>  	/*
>  	 * Both the canonical IPA and fault IPA must be aligned to the
>  	 * mapping size to ensure we find the right PFN and lay down the