Re: [PATCH v15 11/21] KVM: x86/mmu: Allow NULL-able fault in kvm_max_private_mapping_level

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 7/18/2025 12:27 AM, Fuad Tabba wrote:
> From: Ackerley Tng <ackerleytng@google.com>
> 
> Refactor kvm_max_private_mapping_level() to accept a NULL kvm_page_fault
> pointer and rename it to kvm_gmem_max_mapping_level().
> 
> The max_mapping_level x86 operation (previously private_max_mapping_level)
> is designed to potentially be called without an active page fault, for
> instance, when kvm_mmu_max_mapping_level() is determining the maximum
> mapping level for a gfn proactively.
> 
> Allow NULL fault pointer: Modify kvm_max_private_mapping_level() to
> safely handle a NULL fault argument. This aligns its interface with the
> kvm_x86_ops.max_mapping_level operation it wraps, which can also be
> called with NULL.

are you sure of it?

The patch 09 just added the check of fault->is_private for TDX and SEV.

> Rename function to kvm_gmem_max_mapping_level(): This reinforces that
> the function's scope is for guest_memfd-backed memory, which can be
> either private or non-private, removing any remaining "private"
> connotation from its name.
> 
> Optimize max_level checks: Introduce a check in the caller to skip
> querying for max_mapping_level if the current max_level is already
> PG_LEVEL_4K, as no further reduction is possible.
> 
> Acked-by: David Hildenbrand <david@redhat.com>
> Suggested-by: Sean Christoperson <seanjc@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>   arch/x86/kvm/mmu/mmu.c | 16 +++++++---------
>   1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index bb925994cbc5..6bd28fda0fd3 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4467,17 +4467,13 @@ static inline u8 kvm_max_level_for_order(int order)
>   	return PG_LEVEL_4K;
>   }
>   
> -static u8 kvm_max_private_mapping_level(struct kvm *kvm,
> -					struct kvm_page_fault *fault,
> -					int gmem_order)
> +static u8 kvm_gmem_max_mapping_level(struct kvm *kvm, int order,
> +				     struct kvm_page_fault *fault)
>   {
> -	u8 max_level = fault->max_level;
>   	u8 req_max_level;
> +	u8 max_level;
>   
> -	if (max_level == PG_LEVEL_4K)
> -		return PG_LEVEL_4K;
> -
> -	max_level = min(kvm_max_level_for_order(gmem_order), max_level);
> +	max_level = kvm_max_level_for_order(order);
>   	if (max_level == PG_LEVEL_4K)
>   		return PG_LEVEL_4K;
>   
> @@ -4513,7 +4509,9 @@ static int kvm_mmu_faultin_pfn_private(struct kvm_vcpu *vcpu,
>   	}
>   
>   	fault->map_writable = !(fault->slot->flags & KVM_MEM_READONLY);
> -	fault->max_level = kvm_max_private_mapping_level(vcpu->kvm, fault, max_order);
> +	if (fault->max_level >= PG_LEVEL_4K)
> +		fault->max_level = kvm_gmem_max_mapping_level(vcpu->kvm,
> +							      max_order, fault);

I cannot understand why this change is required. In what case will 
fault->max_level < PG_LEVEL_4K?


>   	return RET_PF_CONTINUE;
>   }