Re: [PATCH v17 14/24] KVM: x86/mmu: Enforce guest_memfd's max order when recovering hugepages

[David Hildenbrand]

On 30.07.25 09:33, Xiaoyao Li wrote:
> On 7/30/2025 6:54 AM, Sean Christopherson wrote:
> > Rework kvm_mmu_max_mapping_level() to provide the plumbing to consult
> > guest_memfd (and relevant vendor code) when recovering hugepages, e.g.
> > after disabling live migration.  The flaw has existed since guest_memfd was
> > originally added, but has gone unnoticed due to lack of guest_memfd support
> > for hugepages or dirty logging.
> >
> > Don't actually call into guest_memfd at this time, as it's unclear as to
> > what the API should be.  Ideally, KVM would simply use kvm_gmem_get_pfn(),
> > but invoking kvm_gmem_get_pfn() would lead to sleeping in atomic context
> > if guest_memfd needed to allocate memory (mmu_lock is held).  Luckily,
> > the path isn't actually reachable, so just add a TODO and WARN to ensure
> > the functionality is added alongisde guest_memfd hugepage support, and
> > punt the guest_memfd API design question to the future.
> >
> > Note, calling kvm_mem_is_private() in the non-fault path is safe, so long
> > as mmu_lock is held, as hugepage recovery operates on shadow-present SPTEs,
> > i.e. calling kvm_mmu_max_mapping_level() with @fault=NULL is mutually
> > exclusive with kvm_vm_set_mem_attributes() changing the PRIVATE attribute
> > of the gfn.
> >
> > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > ---
> >    arch/x86/kvm/mmu/mmu.c          | 82 +++++++++++++++++++--------------
> >    arch/x86/kvm/mmu/mmu_internal.h |  2 +-
> >    arch/x86/kvm/mmu/tdp_mmu.c      |  2 +-
> >    3 files changed, 49 insertions(+), 37 deletions(-)
> >
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index 20dd9f64156e..61eb9f723675 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -3302,31 +3302,54 @@ static u8 kvm_max_level_for_order(int order)
> >    	return PG_LEVEL_4K;
> >    }
> >    
> > -static u8 kvm_max_private_mapping_level(struct kvm *kvm, kvm_pfn_t pfn,
> > -					u8 max_level, int gmem_order)
> > +static u8 kvm_max_private_mapping_level(struct kvm *kvm, struct kvm_page_fault *fault,
> > +					const struct kvm_memory_slot *slot, gfn_t gfn)
>
> I don't see why slot and gfn are needed here. Just to keep consistent
> with host_pfn_mapping_level()?

I assume as a preparation to implement the TODO.


Reviewed-by: David Hildenbrand <david@xxxxxxxxxx>

--
Cheers,

David / dhildenb